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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 19 Dec 2024, at 11:56, 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.
> 
> v2 - 1. Rephrased the requirements as suggested.
> 
> 2. Split the product requirements into arm64 specific and common.
> 
> 3. The arm64 specific requirements have arm64_ prefixed to their tag names.
> 
> 4. Grant table requirements have been dropped for now.
> 
> 5. Added a market requirement to denote that Xen can support multiple 
> schedulers.
> 
> 6. Updated index.rst as we have a new file ie product-reqs/reqs.rst.
> 
> docs/fusa/reqs/index.rst                   |   1 +
> docs/fusa/reqs/market-reqs/reqs.rst        |  31 ++++
> docs/fusa/reqs/product-reqs/arm64/reqs.rst | 128 +++++++++++++++-
> docs/fusa/reqs/product-reqs/reqs.rst       | 163 +++++++++++++++++++++
> 4 files changed, 321 insertions(+), 2 deletions(-)
> create mode 100644 docs/fusa/reqs/product-reqs/reqs.rst
> 
> diff --git a/docs/fusa/reqs/index.rst b/docs/fusa/reqs/index.rst
> index 8a4dae6fb2..1088a51d52 100644
> --- a/docs/fusa/reqs/index.rst
> +++ b/docs/fusa/reqs/index.rst
> @@ -8,6 +8,7 @@ Requirements documentation
> 
>    intro
>    market-reqs/reqs
> +   product-reqs/reqs
>    product-reqs/arm64/reqs
>    design-reqs/arm64/generic-timer
>    design-reqs/arm64/sbsa-uart
> diff --git a/docs/fusa/reqs/market-reqs/reqs.rst 
> b/docs/fusa/reqs/market-reqs/reqs.rst
> index f456788d96..39b2714237 100644
> --- a/docs/fusa/reqs/market-reqs/reqs.rst
> +++ b/docs/fusa/reqs/market-reqs/reqs.rst
> @@ -47,3 +47,34 @@ 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
> +
> +Multiple schedulers
> +-------------------
> +
> +`XenMkt~multiple_schedulers~1`
> +
> +Description:
> +Xen shall provide different ways of scheduling virtual cpus onto physical 
> cpus.
> +
> +Rationale:
> +
> +Comments:
> +
> +Needs:
> + - XenProd
> diff --git a/docs/fusa/reqs/product-reqs/arm64/reqs.rst 
> b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> index db91c47a02..c8fee0e49f 100644
> --- a/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> +++ b/docs/fusa/reqs/product-reqs/arm64/reqs.rst
> @@ -6,7 +6,7 @@ Domain Creation And Runtime
> Emulated Timer
> --------------
> 
> -`XenProd~emulated_timer~1`
> +`XenProd~arm64_emulated_timer~1`
> 
> Description:
> Xen shall grant access to "Arm Generic Timer" for the domains.
> @@ -25,7 +25,7 @@ Needs:
> Emulated UART
> -------------
> 
> -`XenProd~emulated_uart~1`
> +`XenProd~arm64_emulated_uart~1`
> 
> Description:
> Xen shall provide an "Arm SBSA UART" compliant device to the domains.
> @@ -40,3 +40,127 @@ Covers:
> 
> Needs:
>  - XenSwdgn
> +
> +Linux kernel image
> +------------------
> +
> +`XenProd~arm64_linux_kernel_image~1`
> +
> +Description:
> +Xen shall create a domain with a binary containing header compliant with 
> Arm64
> +Linux kernel image [1].
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Gzip Linux kernel image
> +-----------------------
> +
> +`XenProd~arm64_linux_kernel_gzip_image~1`
> +
> +Description:
> +Xen shall create a domain with a Gzip compressed binary containing header
> +compliant with Arm64 Linux kernel image [1].
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Kernel with uImage header
> +-------------------------
> +
> +`XenProd~arm64_kernel_uimage~1`
> +
> +Description:
> +Xen shall create a domain with a binary containing uImage header [2].
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Gzip kernel with uImage header
> +------------------------------
> +
> +`XenProd~arm64_gzip_kernel_uimage~1`
> +
> +Description:
> +Xen shall create a domain with a Gzip compressed binary containing uImage
> +header [2].
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +SPIs
> +----
> +
> +`XenProd~arm64_spis~1`
> +
> +Description:
> +Xen shall create a domain with a number of shared peripheral interrupts as
> +specified in the device tree.

"a number" is kind of undefined here. If we have a limit then we should specify 
it
here otherwise this becomes hard to test.
I would suggest to rephrase to "assign hardware shared peripheral interrupts
specified in the device tree to a domain"

> +
> +Rationale:
> +
> +Comments:
> +Device tree is a data structure and language for describing hardware which is
> +readable by an operating system [3].
> +A shared peripheral interrupt is a peripheral interrupt that the Arm Generic
> +Interrupt Controller's Distributor interface can route to any combination of
> +processors [4].
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> + - `XenMkt~static_vm_definition~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Virtual PL011
> +-------------
> +
> +`XenProd~arm64_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
> +
> +| [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://docs.kernel.org/devicetree/usage-model.html
> +| [4] 
> https://developer.arm.com/documentation/ihi0048/a/Introduction/Terminology/Interrupt-types?lang=en
> diff --git a/docs/fusa/reqs/product-reqs/reqs.rst 
> b/docs/fusa/reqs/product-reqs/reqs.rst
> new file mode 100644
> index 0000000000..9257fec713
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/reqs.rst
> @@ -0,0 +1,163 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Domain Creation And Runtime
> +===========================
> +
> +Kernel command line arguments
> +-----------------------------
> +
> +`XenProd~kernel_cmd_line_args~1`
> +
> +Description:
> +Xen shall pass kernel command line arguments to a domain via a device tree.

Would it make sense to say where the "command line" to pass is specified ?

> +
> +Rationale:
> +
> +Comments:
> +Device tree is a data structure and language for describing hardware which is
> +readable by an operating system [1].
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Ramdisk
> +-------
> +
> +`XenProd~ramdisk~1`
> +
> +Description:
> +Xen shall provide the address of an initial ramdisk to a domain via a device
> +tree.
> +
> +Rationale:
> +
> +Comments:
> +The initial ramdisk is contained in memory.
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Memory
> +------
> +
> +`XenProd~memory~1`
> +
> +Description:
> +Xen shall create a domain with an amount of memory specified in a device 
> tree.

s/an/the/

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +vCPUs
> +-----
> +
> +`XenProd~vcpus~1`
> +
> +Description:
> +A domain shall have a configurable number of virtual CPUs (1 to XX).

XX should be replaced with "the maximum number specified at compilation
 in the configuration through CONFIG_MAX_CPUS" or something like that.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Credit2 CPU pool scheduler
> +--------------------------
> +
> +`XenProd~credit2_cpu_pool_scheduler~1`
> +
> +Description:
> +Xen shall have a scheduler where a physical cpu can be shared between more 
> than
> +one virtual cpu.

i think you can name it in the req: "a credit2 scheduler"

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> + - `XenMkt~multiple_schedulers~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +NUL CPU pool scheduler
> +----------------------
> +
> +`XenProd~nul_cpu_pool_scheduler~1`
> +
> +Description:
> +Xen shall have a scheduler where the virtual cpu will be always running on 
> its
> +dedicated physical cpu.

name the scheduler and also "domain virtual cpu is always"

> +
> +Rationale:
> +
> +Comments:
> +A NUL CPU pool scheduler maps a virtual cpu to a unique physical cpu.
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> + - `XenMkt~multiple_schedulers~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Assign iomem
> +------------
> +
> +`XenProd~assign_iomem~1`
> +
> +Description:
> +Xen shall support assigning iomem to a domain.

We cannot assign "any iomem" but pages of iomem (address and size aligned to a 
page).

> +
> +Rationale:
> +
> +Comments:
> +
> +Rationale:

2 times rationale

> +
> +Covers:
> + - `XenMkt~static_vm_definition~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Forward interrupts
> +------------------
> +
> +`XenProd~forward_irqs~1`
> +
> +Description:
> +Xen shall support forwarding interrupts to a domain.

I think you need to add "hardware interrupts" here.

> +
> +Rationale:
> +
> +Comments:
> +
> +Rationale:

rationale twice

> +
> +Covers:
> + - `XenMkt~static_vm_definition~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +| [1] https://docs.kernel.org/devicetree/usage-model.html
> -- 
> 2.25.1
> 

Cheers
Bertrand