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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 18 Dec 2024, at 12:34, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> 
> 
> 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.

Ack

>> 
>>> +
>>> +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.

I start to wonder if we should create interface requirements from the guest PoV:

A domain shall have a configurable number of vCPUs (1 to XX).

What do you think ?

>> 
>>> +
>>> +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".

Yes i think this rephrasing is right.

And we also need a market one saying that Xen shall provide multiple schedulers 
(With the details of which and how at product level).

> 
>> and in fact it
>> shall have product level requirements.
> Do you mean this needs to be a product requirement ?

Yes, this is visible to users, the design should say how the time share is done.

>> 
>>> +
>>> +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".

Yes this is what i mean but be careful as this is not really what the NULL 
scheduler is doing.
What you mean is "pinning" here and the NULL scheduler is only about "what runs 
run as long as it is using the 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 .

I am lost in how you achieve such a thing in the configuration.
All you can do is assigned an interrupt to a domain, but the sharing part I do 
not see what Xen has to do with it.

>> 
>>> +
>>> +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.

ack

Bertrand

> 
>> 
>>> +
>>> +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