Re: [PATCH] docs: fusa: Add requirements for Device Passthrough

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 9 Oct 2024, at 13:24, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> 
> Hi Bertrand/Stefano/all,
> 
> Let me know if the explanation makes sense.
> 
> On 09/10/2024 07:30, Bertrand Marquis wrote:
>> Hi Stefano,
>> 
>>> On 9 Oct 2024, at 00:46, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
>>> 
>>> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@xxxxxxxxx> 
>>>>>> wrote:
>>>>>> 
>>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>>>>> 
>>>>>> Add common requirements for a physical device assignment to Arm64
>>>>>> and AMD64 PVH domains.
>>>>>> 
>>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>>>>> ---
>>>>>> Based on:
>>>>>> [PATCH] docs: fusa: Replace VM with domain
>>>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@xxxxxxx/
>>>>>> ---
>>>>>> ---
>>>>>> .../reqs/design-reqs/common/passthrough.rst   | 365 ++++++++++++++++++
>>>>>> docs/fusa/reqs/index.rst                      |   1 +
>>>>>> docs/fusa/reqs/market-reqs/reqs.rst           |  33 ++
>>>>>> docs/fusa/reqs/product-reqs/common/reqs.rst   |  29 ++
>>>>>> 4 files changed, 428 insertions(+)
>>>>>> create mode 100644 docs/fusa/reqs/design-reqs/common/passthrough.rst
>>>>>> create mode 100644 docs/fusa/reqs/product-reqs/common/reqs.rst
>>>>>> 
>>>>>> diff --git a/docs/fusa/reqs/design-reqs/common/passthrough.rst
>>>>>> b/docs/fusa/reqs/design-reqs/common/passthrough.rst
>>>>>> new file mode 100644
>>>>>> index 0000000000..a1d6676f65
>>>>>> --- /dev/null
>>>>>> +++ b/docs/fusa/reqs/design-reqs/common/passthrough.rst
>>>>>> @@ -0,0 +1,365 @@
>>>>>> +.. SPDX-License-Identifier: CC-BY-4.0
>>>>>> +
>>>>>> +Device Passthrough
>>>>>> +==================
>>>>>> +
>>>>>> +The following are the requirements related to a physical device
>>>>>> assignment
>>>>>> +[1], [2] to Arm64 and AMD64 PVH domains.
>>>>>> +
>>>>>> +Requirements for both Arm64 and AMD64 PVH
>>>>>> +=========================================
>>>>>> +
>>>>>> +Hide IOMMU from a domain
>>>>>> +------------------------
>>>>>> +
>>>>>> +`XenSwdgn~passthrough_hide_iommu_from_domain~1`
>>>>>> +
>>>>>> +Description:
>>>>>> +Xen shall not expose the IOMMU device to the domain even if I/O
>>>>>> virtualization
>>>>>> +is disabled. The IOMMU shall be under hypervisor control only.
>>>>>> +
>>>>>> +Rationale:
>>>>> I think there should be a rationale here to explain why we do not want the
>>>>> IOMMU
>>>>> in particular to be assigned to a domain.
>>>> 
>>>> ok, will add. I propose the following text:
>>>> 
>>>> Xen having the whole picture of the host resources and device assignment
>>>> unlike the individual domain makes use of the IOMMU to:
>>>> - perform DMA Remapping on Arm64 and AMD64 platforms, which is also known 
>>>> as
>>>> stage-2 (or 2nd stage) address translations for DMA devices passed through 
>>>> to
>>>> domains and Interrupt Remapping on AMD64 platforms.
>>>> - provide access protection functionalities to prevent malicious or buggy 
>>>> DMA
>>>> devices from accessing arbitrary memory ranges (e.g. memory allocated to 
>>>> other
>>>> domains) or from generating interrupts that could affect other domains.
>>>> 
>>>> 
>>>>> Added to that, I am not completely sure that there is a clear way to test
>>>>> this one
>>>>> as for example one could assign registers not known by Xen.
>>>> I am afraid you are right, valid point. For example, on Arm64, if there is 
>>>> no
>>>> corresponding driver in use, we will end up exposing IOMMU dt node to Dom0.
>>>> 
>>>> 
>>>>> Shouldn't this requirement in fact be an assumption of use ?
>>>> Assumption of use on Xen? From my PoV sounds reasonable, will do.
>>> In my view, this does not qualify as an Assumption of Use, as it does
>>> not align with the definition we have used so far. If we were to
>>> categorize this as an Assumption of Use, we would need to change the
>>> definition.
>>> 
>>> We have defined an Assumption of Use as something Xen expects from the
>>> rest of the system for it to function correctly, such as being loaded at
>>> EL2. On the other hand, 'Requirements' refer to behaviors we expect Xen
>>> to exhibit.
>>> 
>>> Our goal is for Xen to configure the IOMMU at boot using the stage 2
>>> translation, and to ensure that Xen prevents domains from altering the
>>> IOMMU configuration. These are specific expectations of Xen's behavior,
>>> so I believe they fall under Requirements and should be validated in
>>> some way.
>>> 
>>> However, we could adjust the wording. For example, we might replace the
>>> negative phrasing with a positive requirement, such as: 'Xen shall
>>> configure the IOMMU at boot according to the stage 2 translation
>>> tables.' There is no need to explicitly state that the IOMMU is not
>>> exposed to guests, as nothing is exposed unless explicitly allowed or
>>> mentioned. We could, however, include a brief note about it for clarity.
>> I agree that this is the right way to turn the requirement into something
>> that Xen shall do.
>> 
>> Now i think we will need to have a discussion to clear up what to do with:
>> - assumption of use
> 
> Assumption of use is something that other software/hardware components need 
> to do to ensure the correct behavior of Xen.
> 
> For eg 1) AoU on hardware :- The hardware needs to support NS-EL2. The 
> hardware needs to have GICv3, SMMUv3 as these are in the scope of safety 
> certification. The hardware needs to have Cortex-A53  r0p4 as these have some 
> errata fixes which affect Xen.

Ack

> 
> 2) AoU on bootloader/firmware - 1) Bootloader/Firmware needs to load Xen in 
> NS-EL2 mode. 2) Bootloader/Firmware needs to initialize DDR

Ack

> 
> 3) AoU on compiler - 1) The GCC version used should be 5.1 or later.

Ack

> 
> 4) AoU on domain - 1) Domains should not use HVC DCC registers as Xen does 
> not emulate them.

Xen does not depend on that, the domain does so this is only a Xen expected 
behaviour and we should document that Domains shall not use it.

Xen behaviour if used should be specified.

> 
> The AoUs can either be tested or need to be stated explicitly in the safety 
> manual.
> 
>> - "integrator" (word always problematic in Fusa as usually use to bail out
>> and give responsibility to someone else) shall and shall not do (for example
>> giving access to IOMMU registers to a domain)
> 
> The responsibility with the integrator lies for things which cannot be 
> tested. For eg Xen has to be built with a particular configuration (eg 
> SMMUv3) or a specific CPU errata. Integrator should provide at the most X 
> amount of memory for each domain. SMMU (or any specific device) should not be 
> assigned to a domain (which should be under Xen's control).

Ack

> 
> For some of the AoUs which cannot be tested (eg Bootloader/Firmare needs to 
> initialize the DDR, CNTFRQ_EL0 needs to contain the correct system counter 
> frequency), the responsibility will lie with the integrator.

This is an AoU on the firmware or the platform not on the integrator.

> 
>> - interface and what we expect a domain will do with it
> 
> This should be covered as part of AoU on domain. We can have more examples of 
> this in near future.

In my mind interface are for example hypercall numbers and behaviours.
I would not expect to find this kind of stuff as AoU.

Cheers
Bertrand

> 
> Kind regards,
> 
> Ayan