Re: [PATCH v2] docs: fusa: Add dom0less domain configuration requirements

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 18/12/2024 08:27, Bertrand Marquis wrote:
> Hi Ayan,
Hi Bertrand,
> > On 12 Dec 2024, at 20:03, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> >
> > From: Michal Orzel <michal.orzel@xxxxxxx>
> >
> > Add requirements for dom0less domain creation.
> >
> > Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > ---
> > Changes from v1 :-
> >
> > 1. As the dom0less domain creation requirements specifies the dt properties
> > for creating domains, it has been moved to product requirements. Product
> > requirements define the interface Xen exposes to other domains.
> >
> > 2. For the requirements which introduces new terms (like grant table, etc), I
> > have provided the definition as part of the comments.
> >
> > 3. Introduced new market requirements to specify that Xen can assign iomem and
> > irqs to domains.
> >
> > 4. The design requirements will be added later.
> >
> > docs/fusa/reqs/market-reqs/reqs.rst        |  16 ++
> > docs/fusa/reqs/product-reqs/arm64/reqs.rst | 306 +++++++++++++++++++++
> > 2 files changed, 322 insertions(+)
> >
> > diff --git a/docs/fusa/reqs/market-reqs/reqs.rst 
> > b/docs/fusa/reqs/market-reqs/reqs.rst
> > index f456788d96..47e1b6ad61 100644
> > --- a/docs/fusa/reqs/market-reqs/reqs.rst
> > +++ b/docs/fusa/reqs/market-reqs/reqs.rst
> > @@ -47,3 +47,19 @@ Comments:
> >
> > Needs:
> >   - XenProd
> > +
> > +Static VM definition
> > +--------------------
> > +
> > +`XenMkt~static_vm_definition~1`
> > +
> > +Description:
> > +Xen shall support assigning peripherals to various domains.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +Peripheral implies an iomem (input output memory) and/or interrupts.
> > +
> > +Needs:
> > + - XenProd
> > diff --git a/docs/fusa/reqs/product-reqs/arm64/reqs.rst 
> > b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > index db91c47a02..66f2978733 100644
> > --- a/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > +++ b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > @@ -40,3 +40,309 @@ Covers:
> >
> > Needs:
> >   - XenSwdgn
> > +
> > +Linux kernel image
> > +------------------
> > +
> > +`XenProd~linux_kernel_image~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Arm64 Linux kernel image [1].
> This shall be rephrased to mention that it shall be a binary with a header 
> compliant with the Linux kernel image format.
> We do not want to say that we can only boot Linux.
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Gzip Linux kernel image
> > +-----------------------
> > +
> > +`XenProd~linux_kernel_gzip_image~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Arm64 Gzip compressed Linux kernel image.
> Ditto.
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Kernel with uImage header
> > +-------------------------
> > +
> > +`XenProd~kernel_uimage~1`
> > +
> > +Description:
> > +Xen shall create a domain with a kernel containing uImage header [2].
> I would remove kernel and say binary executable and add compatible or something 
> like that.
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Gzip kernel with uImage header
> > +------------------------------
> > +
> > +`XenSwdgn~arm64_gzip_kernel_uimage~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Gzip compressed kernel containing uImage
> > +header [2].
> Same
Agreed with all the above.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Kernel command line arguments
> > +-----------------------------
> > +
> > +`XenSwdgn~kernel_cmd_line_args~1`
> > +
> > +Description:
> > +Xen shall pass kernel command line arguments to a domain.
> I am a bit wondering if this one and the following are not a bit to generic.
> Should we say through DT or ACPI header for example ?
Yes, I can say through device tree. And then I can explain device tree 
in the comments.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Ramdisk
> > +-------
> > +
> > +`XenSwdgn~ramdisk~1`
> > +
> > +Description:
> > +Xen shall provide initial ramdisk to a domain.
> This should be mentioning that it is provided in memory and the address is 
> provided through DT.
Ack.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Memory
> > +------
> > +
> > +`XenSwdgn~memory~1`
> > +
> > +Description:
> > +Xen shall create a domain with specified amount of memory.
> I am missing the where this is specified here ? i guess this is also DT
Yes, this is also DT.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +vCPUs
> > +-----
> > +
> > +`XenSwdgn~vcpus~1`
> > +
> > +Description:
> > +Xen shall create a domain with a number of virtual CPUs.
> number here is unprecise
Can I say with one or more number of virtual CPUS ? How would you want 
me to define.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Credit2 CPU pool scheduler
> > +--------------------------
> > +
> > +`XenSwdgn~credit2_cpu_pool_scheduler~1`
> > +
> > +Description:
> > +Xen shall assign a Credit2 CPU pool scheduler [3] to a domain.
> What is Credit2 ? this needs to be defined somewhere
I have provided a link to the credit2 documentation.

https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/features/sched_credit2.pandoc

Do I still need to define it as
"Credit2 is a scheduling mechanism where more than one virtual cpus shares a 
physical cpus based on a time sharing mechanism."

or should the requirement be rephrased as

"Xen shall have a scheduler where a physical cpu can be shared between more than one 
virtual cpu".

> and in fact it
> shall have product level requirements.
Do you mean this needs to be a product requirement ?
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +NUL CPU pool scheduler
> > +----------------------
> > +
> > +`XenSwdgn~nul_cpu_pool_scheduler~1`
> > +
> > +Description:
> > +Xen shall assign a NUL CPU pool scheduler to a domain.
> Same
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +A NUL CPU pool scheduler maps a virtual cpu to a unique physical cpu.
> This is a product requirement saying that Xen shall have a scheduler with such 
> characteristics
> and I think this is not enough to define it.
I don't understand this bit. Do you mean this should be product 
requirement written as "Xen shall have a scheduler where a virtual cpu 
is always assigned to a unique physical cpu".
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +SPIs
> > +----
> > +
> > +`XenSwdgn~spis~1`
> > +
> > +Description:
> > +Xen shall allocate a specified number of shared peripheral interrupts for a
> > +domain.
> This is very ambiguous. What do you mean here ?
Xen shall provide a way to specify the number of shared peripheral 
interrupts for a domain via the device tree .
> > +
> > +Rationale:
> > +
> > +Comments:
> > +A shared peripheral interrupt is an interrupt generated by a peripheral that is
> > +accessible across all the cpu cores.
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > + - `XenMkt~static_vm_definition~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Grant table frames
> > +------------------
> > +
> > +`XenSwdgn~grant_table_frames~1`
> > +
> > +Description:
> > +Xen shall create a domain with a specified number of grant table frames.
> It is really weird to say that Xen shall create something specific without this 
> being
> linked to an higher level definition of the goal.
ok, I will drop this and the following requirement for now.

When we have market requirement to specify that "Xen shall allow sharing 
of buffer with a domain", then we can add this and the following 
requirement.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +Grant tables are a mechanism for sharing and transferring frames (memory 
> > buffers)
> > +between domains.
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Grant maptrack frames
> > +---------------------
> > +
> > +`XenSwdgn~grant_maptrack_frames~1`
> > +
> > +Description:
> > +Xen shall create a domain with a specified number of grant maptrack frames.
> Why is this needed ? what is the high level req for this ?
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +Maptrack frame is the metadata for tracking the memory mapped into a domain.
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Virtual PL011
> > +-------------
> > +
> > +`XenProd~virtual_pl011~1`
> > +
> > +Description:
> > +Xen shall provide an "Arm PL011 UART" compliant device to the domains.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~run_arm64_domains~1`
> > + - `XenMkt~provide_console_domains~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Assign iomem
> > +------------
> > +
> > +`XenProd~assign_iomem~1`
> > +
> > +Description:
> > +Xen shall support assigning iomem to a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Rationale:
> > +
> > +Covers:
> > + - `XenMkt~static_vm_definition~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Forward interrupts
> > +------------------
> > +
> > +`XenProd~forward_irqs~1`
> > +
> > +Description:
> > +Xen shall support forwarding interrupts to a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Rationale:
> > +
> > +Covers:
> > + - `XenMkt~static_vm_definition~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +| [1] 
> > https://github.com/torvalds/linux/blob/master/Documentation/arch/arm64/booting.rst
> > +| [2] https://source.denx.de/u-boot/u-boot/-/blob/master/include/image.h#L315
> > +| [3] 
> > https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/features/sched_credit2.pandoc
> > --
> > 2.25.1
- Ayan