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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 13/11/2024 09:31, Julien Grall wrote:
> Hi,
Hi Julien,
> On 18/10/2024 16:51, Ayan Kumar Halder 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>
> > ---
> >   .../arm64/dom0less_domain_config.rst          | 267 ++++++++++++++++++
> >   docs/fusa/reqs/market-reqs/reqs.rst           |  15 +
> >   docs/fusa/reqs/product-reqs/arm64/reqs.rst    |  20 ++
> >   3 files changed, 302 insertions(+)
> >   create mode 100644 
> > docs/fusa/reqs/design-reqs/arm64/dom0less_domain_config.rst
> > diff --git 
> > a/docs/fusa/reqs/design-reqs/arm64/dom0less_domain_config.rst 
> > b/docs/fusa/reqs/design-reqs/arm64/dom0less_domain_config.rst
> > new file mode 100644
> > index 0000000000..17b5f8962c
> > --- /dev/null
> > +++ b/docs/fusa/reqs/design-reqs/arm64/dom0less_domain_config.rst
> > @@ -0,0 +1,267 @@
> > +.. SPDX-License-Identifier: CC-BY-4.0
> > +
> > +Dom0less Domain configuration requirements
> > +==========================================
> > +
> > +The following are the requirements related to dom0less domain 
> > configuration for
> > +Arm64 domains.
> > +
> > +Specify Arm64 Linux kernel image
> > +----------------------------------
> > +
> > +`XenSwdgn~arm64_specify_kernel_linux_image~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Arm64 Linux kernel image.
> A link to the specification would be useful when you are referring to 
> an external format.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify Arm64 Gzip Linux kernel image
> > +---------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_kernel_gzip_image~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Arm64 Gzip compressed Linux kernel 
> > image.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify kernel with uImage header
> > +---------------------------------
> > +
> > +`XenSwdgn~arm64_specify_kernel_uimage~1`
> > +
> > +Description:
> > +Xen shall create a domain with a kernel containing uImage header.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify Gzip kernel with uImage header
> > +--------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_gzip_kernel_uimage~1`
> > +
> > +Description:
> > +Xen shall create a domain with a Gzip compressed kernel containing 
> > uImage
> > +header.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify passthrough device tree
> > +-------------------------------
> > +
> > +`XenSwdgn~arm64_specify_passthrough_dt~1`
> > +
> > +Description:
> > +Xen shall support direct assignment of devices to a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify kernel command line arguments
> > +-------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_kernel_cmd_line_args~1`
> > +
> > +Description:
> > +Xen shall pass kernel command line arguments to a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify initial ramdisk
> > +-----------------------
> > +
> > +`XenSwdgn~arm64_specify_initial_ramdisk~1`
> > +
> > +Description:
> > +Xen shall provide initial ramdisk to a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify amount of memory
> > +------------------------
> > +
> > +`XenSwdgn~arm64_specify_memory~1`
> > +
> > +Description:
> > +Xen shall create a domain with specified amount of memory.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Assign a single vCPU
> > +--------------------
> > +
> > +`XenSwdgn~arm64_assign_single_vcpu~1`
> > +
> > +Description:
> > +Xen shall assign a single vCPU to a domain.
> This wording is a bit ambiguous. You don't assign a vCPU to a domain. 
> You create a domain with "N vCPUs". It is also not clear why we are 
> making the distinction between one and ...
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Assign multiple vCPUs
> > +---------------------
> > +
> > +`XenSwdgn~arm64_assign_multiple_vcpus~1`
> > +
> > +Description:
> > +Xen shall assign multiple vCPUs to a domain.
> ... multiple one. From Xen PoV there is no differences.
>
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Assign vCPUs from CPU pool
> > +--------------------------
> > +
> > +`XenSwdgn~arm64_assign_vcpus_cpu_pool~1`
> > +
> > +Description:
> > +Xen shall assign vCPUs to a domain from a CPU pool.
> Same remark about the wording. You create a domain with N vCPUs and 
> *assign* a CPU pool to a domain. 
Ok, so all the previous 3 requirements can be merged into

Xen shall create a domain with N vCPUs and assign a CPU pool to a domain.

Or

Xen shall create a domain with N vCPUs.

(which of the two looks better to you if we keep the next requirement ?)

Comments:

Here N is determined by the device tree configuration provided by the user.

> You also assign pCPU to a CPU pool.
>
> But I am not sure about if this requirement is actually necessary 
> given ...
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify CPU pool scheduler
> > +--------------------------
> > +
> > +`XenSwdgn~arm64_specify_cpu_pool_scheduler~1`
> > +
> > +Description:
> > +Xen shall assign a CPU pool scheduler to a domain.
> ... you have th is one.
So, we can keep it as it is.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Assign virtual UART
> > +-------------------
> > +
> > +`XenSwdgn~arm64_assign_virtual_uart~1`
> > +
> > +Description:
> > +Xen shall assign a virtual UART to a domain.
> Are we talking about the virtual PL011 or the fake emulation of the 
> real UART we do?
virtual PL011.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify number of SPIs
> > +----------------------
> > +
> > +`XenSwdgn~arm64_specify_num_spis~1`
> > +
> > +Description:
> > +Xen shall allocate a specified number of shared peripheral 
> > interrupts for a
> > +domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify grant table version for a domain
> > +----------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_grant_table_version~1`
> > +
> > +Description:
> > +Xen shall create a domain with a specified version of grant table 
> > structure
> Realistically grant table v2 is not supported for Arm and I am not 
> convinced it makes any sense for x86 in embedded system. It is mainly 
> useful when you have a guest with a large amount of address space 
> (IIRC > 4TB).
We can put this detail in comments.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify number of grant table frames for a domain
> > +-------------------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_num_grant_table_frames~1`
> > +
> > +Description:
> > +Xen shall create a domain with a specified number of grant table 
> > frames.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +Specify number of grant maptrack frames for a domain
> > +----------------------------------------------------
> > +
> > +`XenSwdgn~arm64_specify_num_grant_maptrack_frames~1`
> > +
> > +Description:
> > +Xen shall create a domain with a specified number of grant maptrack 
> > frames.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~static_domains_configuration~1`
> > +
> > +| [1] 
> > https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/device-tree/booting.txt
> > +| [2] 
> > https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/device-tree/cpupools.txt
> > diff --git a/docs/fusa/reqs/market-reqs/reqs.rst 
> > b/docs/fusa/reqs/market-reqs/reqs.rst
> > index f456788d96..ca020f9a33 100644
> > --- a/docs/fusa/reqs/market-reqs/reqs.rst
> > +++ b/docs/fusa/reqs/market-reqs/reqs.rst
> > @@ -47,3 +47,18 @@ Comments:
> >     Needs:
> >    - XenProd
> > +
> > +Static VM definition
> > +--------------------
> > +
> > +`XenMkt~static_vm_definition~1`
> > +
> > +Description:
> > +Xen shall support specifying resources for a domain.
> Compare to the other requirements, this is quite a vague. Should we 
> list the resources?
The list of resources depends on what the user has provided in the 
device tree configuration.
But the requirement is correct as it is. Xen allows direct assignment of 
devices to domains (ie passthrough).
How do you want to write it ?

> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Needs:
> > + - XenProd
> > \ No newline at end of file
> > diff --git a/docs/fusa/reqs/product-reqs/arm64/reqs.rst 
> > b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > index db91c47a02..0453dbb862 100644
> > --- a/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > +++ b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> > @@ -40,3 +40,23 @@ Covers:
> >     Needs:
> >    - XenSwdgn
> > +
> > +Configure static domains
> > +------------------------
> > +
> > +`XenProd~static_domains_configuration~1`
> > +
> > +Description:
> > +Xen shall support specifying the resources required for a domain.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Rationale:
> > +
> > +Covers:
> > + - `XenMkt~static_vm_definition~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > \ No newline at end of file
> Missing a newline.
- Ayan