diff mbox series

docs: fusa: Add requirements for Device Passthrough

Message ID 20241007185508.3044115-1-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series docs: fusa: Add requirements for Device Passthrough | expand

Commit Message

Oleksandr Tyshchenko Oct. 7, 2024, 6:55 p.m. UTC
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Add common requirements for a physical device assignment to Arm64
and AMD64 PVH domains.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
Based on:
[PATCH] docs: fusa: Replace VM with domain
https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
---
---
 .../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

Comments

Bertrand Marquis Oct. 8, 2024, 6:17 a.m. UTC | #1
Hi,

> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> 
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Add common requirements for a physical device assignment to Arm64
> and AMD64 PVH domains.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> Based on:
> [PATCH] docs: fusa: Replace VM with domain
> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
> ---
> ---
> .../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.

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.

Shouldn't this requirement in fact be an assumption of use ?

> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Discover PCI devices from hardware domain
> +-----------------------------------------
> +
> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
> +
> +Description:
> +The hardware domain shall enumerate and discover PCI devices and inform Xen
> +about their appearance and disappearance.

Again this is a design requirement telling what should be done by a domain.
This is an interface or an assumption of use but not a Xen design req.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Discover PCI devices from Xen
> +-----------------------------
> +
> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
> +
> +Description:
> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
> +boot if the hardware domain is not present.

I am a bit wondering here why we would not want Xen to always do it if we have
the code to do it in Xen anyway.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign PCI device to domain (with IOMMU)
> +----------------------------------------
> +
> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
> +
> +Description:
> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
> +a domain during its creation using passthrough (partial) device tree on Arm64
> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
> +protected by the IOMMU.

This is a very long and complex requirement.
I would suggest to split it in 3:
- generic: Xen shall support assign PCI devices to domains.
- arm64 one: Xen shall assign PCI devices based on device tree (explain how this is configured in dts)
- amd: xxxx based on hyperlaunch
- Xen shall use the IOMMU to enforce DMA operations done by a PCI device assigned to a domain to be restricted to the memory of the given domain.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign PCI device from domain (with IOMMU)
> +--------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified PCI device from a domain during its destruction.
> +The physical device to be deassigned is protected by the IOMMU.

Remove second sentence or turn it into a req to say that the PCI device shall not be allowed to do DMA anymore somehow.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Forbid the same PCI device assignment to multiple domains
> +---------------------------------------------------------
> +
> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
> +
> +Description:
> +Xen shall not assign the same PCI device to multiple domains by failing to
> +create a new domain if the device to be passed through is already assigned
> +to the existing domain. Also different PCI devices which share some resources
> +(interrupts, IOMMU connections) can be assigned only to the same domain.

Please split and simplify
- Xen shall assign a single device to a single domain
- Xen shall assign PCI devices sharing resources to the same domain.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Requirements for Arm64 only
> +===========================
> +
> +Assign interrupt-less platform device to domain
> +-----------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
> +
> +Description:
> +Xen shall assign a specified platform device that has only a MMIO region
> +(does not have any interrupts) to a domain during its creation using passthrough
> +device tree.
> +The example of interrupt-less device is PWM or clock controller.

I am a bit puzzled by this req. Why making a specific case for interrupt less ?

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign interrupt-less platform device from domain
> +---------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
> +
> +Description:
> +Xen shall deassign a specified platform device that has only a MMIO region
> +(does not have any interrupts) from a domain during its destruction.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign non-DMA-capable platform device to domain
> +------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
> +
> +Description:
> +Xen shall assign a specified non-DMA-capable platform device to a domain during
> +its creation using passthrough device tree.
> +The example of non-DMA-capable device is Timer.

Again why making a specific case here ?

Wouldn't it me more logic to describe device passthrough and then what Xen should do for interrupts and for DMA ?

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign non-DMA-capable platform device from domain
> +----------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
> +
> +Description:
> +Xen shall deassign a specified non-DMA-capable platform device from a domain
> +during its destruction.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign DMA-capable platform device to domain (with IOMMU)
> +---------------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
> +
> +Description:
> +Xen shall assign a specified DMA-capable platform device to a domain during
> +its creation using passthrough device tree. The physical device to be assigned
> +is protected by the IOMMU.

This requirement is not a design but an higher level as it does not tell anything about implementation.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign DMA-capable platform device from domain (with IOMMU)
> +-------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified DMA-capable platform device from a domain during
> +its destruction. The physical device to be deassigned is protected by the IOMMU.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign DMA-capable platform device to domain (without IOMMU)
> +------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
> +
> +Description:
> +Xen shall assign a specified DMA-capable platform device to a domain during
> +its creation using passthrough device tree. The physical device to be assigned
> +is not protected by the IOMMU.
> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
> +the direct mapped domains only. The direct mapped domain must be either safe or
> +additional security mechanisms for protecting against possible malicious or
> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
> +and Xilinx peripheral protection unit (XPPU).


Please split in several reqs.


Stopping here my review for now.

Cheers
Bertrand

> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign DMA-capable platform device from domain (without IOMMU)
> +----------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_dma_platform_device_without_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified DMA-capable platform device from a domain during
> +its destruction. The physical device to be deassigned is not protected
> +by the IOMMU.
> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Map platform device MMIO region identity
> +----------------------------------------
> +
> +`XenSwdgn~passthrough_map_platform_device_mmio_region_ident~1`
> +
> +Description:
> +Xen shall map platform device memory region identity (guest address ==
> +physical address) when assigning a specified platform device to a domain.
> +The device can be either non-DMA-capable or DMA-capable.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Map platform device MMIO region non-identity
> +--------------------------------------------
> +
> +`XenSwdgn~passthrough_map_platform_device_mmio_region_non_ident~1`
> +
> +Description:
> +Xen shall map platform device memory region non-identity (guest address !=
> +physical address) when assigning a specified platform device to a domain.
> +The device can be either non-DMA-capable or DMA-capable.
> +
> +Rationale:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign PCI device to domain (without IOMMU)
> +-------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_pci_device_without_iommu~1`
> +
> +Description:
> +Xen shall assign a specified PCI device to a domain during its creation using
> +passthrough device tree. The physical device to be assigned is not protected
> +by the IOMMU.
> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
> +the direct mapped domains only. The direct mapped domain must be either safe or
> +additional security mechanisms for protecting against possible malicious or
> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
> +and Xilinx peripheral protection unit (XPPU).
> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign PCI device from domain (without IOMMU)
> +-----------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_pci_device_without_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified PCI device from a domain during its destruction.
> +The physical device to be deassigned is not protected by the IOMMU.
> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Forbid the same platform device assignment to multiple domains
> +--------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_forbid_same_platform_device_assignment~1`
> +
> +Description:
> +Xen shall not assign the same platform device to multiple domains by failing
> +to create a new domain if the device to be passed through is already assigned
> +to the existing domain. Also, different platform devices which share some
> +resources (interrupts, IOMMU connections) can be assigned only to the same
> +domain.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Notes
> +=====
> +
> +The AMD64 PVH-specific requirements are written under the assumption that once
> +the Hyperlaunch feature is completed, Xen shall assign a PCI device to boot
> +time domains. This is not the case today, where the PCI device can be passed
> +through only to domains launched by a control (toolstack) domain.
> +
> +The Arm64-specific requirements are written under the assumption that once
> +the dom0less PCI Passthrough feature is completed, Xen shall assign a PCI device
> +to boot time domains. This is not the case today, where only the platform device
> +Passthrough is supported.
> +
> +[1] https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/passthrough.txt;hb=HEAD
> +[2] https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/passthrough-noiommu.txt;hb=HEAD
> diff --git a/docs/fusa/reqs/index.rst b/docs/fusa/reqs/index.rst
> index 183f183b1f..19c2f26b2b 100644
> --- a/docs/fusa/reqs/index.rst
> +++ b/docs/fusa/reqs/index.rst
> @@ -10,3 +10,4 @@ Requirements documentation
>    market-reqs
>    product-reqs
>    design-reqs/arm64
> +   design-reqs/common
> diff --git a/docs/fusa/reqs/market-reqs/reqs.rst b/docs/fusa/reqs/market-reqs/reqs.rst
> index f456788d96..37a443395b 100644
> --- a/docs/fusa/reqs/market-reqs/reqs.rst
> +++ b/docs/fusa/reqs/market-reqs/reqs.rst
> @@ -47,3 +47,36 @@ Comments:
> 
> Needs:
>  - XenProd
> +
> +Run AMD-x86 domains
> +-------------------
> +
> +`XenMkt~run_x86_domains~1`
> +
> +Description:
> +Xen shall run AMD-x86 domains.
> +
> +Rationale:
> +
> +Comments:
> +
> +Needs:
> + - XenProd
> +
> +Domain device assignment
> +------------------------
> +
> +`XenMkt~domain_device_assignment~1`
> +
> +Description:
> +Xen shall assign device to each domain.
> +
> +For example, it shall assign GPU to domain A, MMC to domain B. Only the domain
> +assigned to a device, shall have exclusive access to the device.
> +
> +Rationale:
> +
> +Comments:
> +
> +Needs:
> + - XenProd
> diff --git a/docs/fusa/reqs/product-reqs/common/reqs.rst b/docs/fusa/reqs/product-reqs/common/reqs.rst
> new file mode 100644
> index 0000000000..9304399e4d
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/common/reqs.rst
> @@ -0,0 +1,29 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Domain Creation And Runtime
> +===========================
> +
> +Device Passthrough
> +------------------
> +
> +`XenProd~device_passthrough~1`
> +
> +Description:
> +Xen shall provide mechanism for assigning a physical device to the domains.
> +
> +For example:
> +
> +- PCI passthrough
> +- MMC passthrough
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~run_arm64_domains~1`
> + - `XenMkt~run_x86_domains~1`
> + - `XenMkt~domain_device_assignment~1`
> +
> +Needs:
> + - XenSwdgn
> -- 
> 2.34.1
>
Oleksandr Tyshchenko Oct. 8, 2024, 6:53 p.m. UTC | #2
On 08.10.24 09:17, Bertrand Marquis wrote:
> Hi,

Hello Bertrand


> 
>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> Add common requirements for a physical device assignment to Arm64
>> and AMD64 PVH domains.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> ---
>> Based on:
>> [PATCH] docs: fusa: Replace VM with domain
>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>> ---
>> ---
>> .../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.

> 
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Discover PCI devices from hardware domain
>> +-----------------------------------------
>> +
>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>> +
>> +Description:
>> +The hardware domain shall enumerate and discover PCI devices and inform Xen
>> +about their appearance and disappearance.
> 
> Again this is a design requirement telling what should be done by a domain.
> This is an interface or an assumption of use but not a Xen design req.

I agree, will convert to Assumption of use on domain.


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Discover PCI devices from Xen
>> +-----------------------------
>> +
>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>> +
>> +Description:
>> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
>> +boot if the hardware domain is not present.
> 
> I am a bit wondering here why we would not want Xen to always do it if we have
> the code to do it in Xen anyway.

Makes sense, will drop "if the hardware domain is not present".


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign PCI device to domain (with IOMMU)
>> +----------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
>> +a domain during its creation using passthrough (partial) device tree on Arm64
>> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
>> +protected by the IOMMU.
> 
> This is a very long and complex requirement.
> I would suggest to split it in 3:
> - generic: Xen shall support assign PCI devices to domains.
> - arm64 one: Xen shall assign PCI devices based on device tree (explain how this is configured in dts)
> - amd: xxxx based on hyperlaunch

I agree, will split, but ...

> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device assigned to a domain to be restricted to the memory of the given domain.


  ... does this need to be a separate 4th requirement here (and for the 
similar requirement for the platform device down the document) or this 
sentence is meant to be added to all resulting generic/arm64/amd 
requirements?

I would like to clarify, there are two groups of requirements to cover 
DMA-capable devices in this document:
- for devices that are behind the IOMMU and IOMMU can be used for them, 
those requirements description explicitly mention "device xxx is 
protected by the IOMMU" in addition to "(with IOMMU)" in the subject.
- for devices that are not behind the IOMMU or IOMMU cannot be used for 
them, those requirements description explicitly mention "device xxx is 
not protected by the IOMMU" in addition to "(without IOMMU)" in the subject.

> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign PCI device from domain (with IOMMU)
>> +--------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall deassign a specified PCI device from a domain during its destruction.
>> +The physical device to be deassigned is protected by the IOMMU.
> 
> Remove second sentence or turn it into a req to say that the PCI device shall not be allowed to do DMA anymore somehow.


I would like to clarify, the second sentence here is just to indicate 
what type of device (in the context of IOMMU involvement) the 
requirement is talking about, not about special care for denying any DMA 
from it after deassigning.

If you still think that we need a new requirement to explicitly 
highlight that, I will be ok to create, in that case, I assume, the 
platform device will want to gain the similar requirement. Please let me 
know your preference.


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Forbid the same PCI device assignment to multiple domains
>> +---------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
>> +
>> +Description:
>> +Xen shall not assign the same PCI device to multiple domains by failing to
>> +create a new domain if the device to be passed through is already assigned
>> +to the existing domain. Also different PCI devices which share some resources
>> +(interrupts, IOMMU connections) can be assigned only to the same domain.
> 
> Please split and simplify
> - Xen shall assign a single device to a single domain
> - Xen shall assign PCI devices sharing resources to the same domain.

Good point, will split.


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Requirements for Arm64 only
>> +===========================
>> +
>> +Assign interrupt-less platform device to domain
>> +-----------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
>> +
>> +Description:
>> +Xen shall assign a specified platform device that has only a MMIO region
>> +(does not have any interrupts) to a domain during its creation using passthrough
>> +device tree.
>> +The example of interrupt-less device is PWM or clock controller.
> 
> I am a bit puzzled by this req. Why making a specific case for interrupt less ?


Those devices exist and can be assigned to a domain, they are configured 
slightly differently in comparison with devices with interrupts 
("xen,path" is not needed for the former), other code paths are executed 
in Xen.

More technical details:
The allowance of the platform device assignment which is not behind an 
IOMMU (for both non-DMA-capable and DMA-capable devices) is specified 
using device tree property ("xen,force-assign-without-iommu") in the 
device node described in the passthrough device tree. The said property 
also allows the interrupt-less platform device assignment (a device that 
has only a MMIO region) without specifying the corresponding node in the 
host device via device tree property ("xen,path").


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign interrupt-less platform device from domain
>> +---------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
>> +
>> +Description:
>> +Xen shall deassign a specified platform device that has only a MMIO region
>> +(does not have any interrupts) from a domain during its destruction.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign non-DMA-capable platform device to domain
>> +------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
>> +
>> +Description:
>> +Xen shall assign a specified non-DMA-capable platform device to a domain during
>> +its creation using passthrough device tree.
>> +The example of non-DMA-capable device is Timer.
> 
> Again why making a specific case here ?

Almost the same answer as for interrupt-less device. Here "xen,path" is 
needed.


> 
> Wouldn't it me more logic to describe device passthrough and then what Xen should do for interrupts and for DMA ?

I can add more details on how this is configured/what properties are 
used, etc in rationale for each requirement mentioning device tree. Or 
do you mean something else?


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign non-DMA-capable platform device from domain
>> +----------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
>> +
>> +Description:
>> +Xen shall deassign a specified non-DMA-capable platform device from a domain
>> +during its destruction.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign DMA-capable platform device to domain (with IOMMU)
>> +---------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified DMA-capable platform device to a domain during
>> +its creation using passthrough device tree. The physical device to be assigned
>> +is protected by the IOMMU.
> 
> This requirement is not a design but an higher level as it does not tell anything about implementation.

Ok, will add details regarding passthrough/host device trees.


> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign DMA-capable platform device from domain (with IOMMU)
>> +-------------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall deassign a specified DMA-capable platform device from a domain during
>> +its destruction. The physical device to be deassigned is protected by the IOMMU.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign DMA-capable platform device to domain (without IOMMU)
>> +------------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified DMA-capable platform device to a domain during
>> +its creation using passthrough device tree. The physical device to be assigned
>> +is not protected by the IOMMU.
>> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
>> +the direct mapped domains only. The direct mapped domain must be either safe or
>> +additional security mechanisms for protecting against possible malicious or
>> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
>> +and Xilinx peripheral protection unit (XPPU).
> 
> 
> Please split in several reqs.


I agree, will do. I feel it should be split into the following requirements:
- Assign DMA-capable platform device to domain (without IOMMU)
- Create direct mapped domain
- Enable additional security mechanisms in direct mapped domain

To be honest, I'm not quite sure whether it is worth creating the last 
requirement ...


> 
> 
> Stopping here my review for now

Thanks for the review.


> 
> Cheers
> Bertrand
> 

[snip]
Stefano Stabellini Oct. 8, 2024, 10:46 p.m. UTC | #3
On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
> > > On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> > > 
> > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > > 
> > > Add common requirements for a physical device assignment to Arm64
> > > and AMD64 PVH domains.
> > > 
> > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > > ---
> > > Based on:
> > > [PATCH] docs: fusa: Replace VM with domain
> > > https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
> > > ---
> > > ---
> > > .../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.


> > > +
> > > +Comments:
> > > +
> > > +Covers:
> > > + - `XenProd~device_passthrough~1`
> > > +
> > > +Discover PCI devices from hardware domain
> > > +-----------------------------------------
> > > +
> > > +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
> > > +
> > > +Description:
> > > +The hardware domain shall enumerate and discover PCI devices and inform
> > > Xen
> > > +about their appearance and disappearance.
> > 
> > Again this is a design requirement telling what should be done by a domain.
> > This is an interface or an assumption of use but not a Xen design req.
> 
> I agree, will convert to Assumption of use on domain.

This example better aligns with our definition of Assumption of Use so
far: we expect the hardware domain to enumerate and discover PCI
devices, then notify Xen about their appearance or removal. This is an
expectation placed on the hardware domain, not on Xen itself. I agree
with Bertrand that, as written, it is more of an Assumption of Use than
a Requirement.

However, rather than converting it into an Assumption of Use, I think we
should rewrite it as a requirement focused on Xen's interfaces for
enumeration. For instance:

"Xen shall provide hypercalls to allow the hardware domain to inform Xen
about the presence of PCI devices."


> > > +
> > > +Rationale:
> > > +
> > > +Comments:
> > > +
> > > +Covers:
> > > + - `XenProd~device_passthrough~1`
> > > +
> > > +Discover PCI devices from Xen
> > > +-----------------------------
> > > +
> > > +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
> > > +
> > > +Description:
> > > +Xen shall discover PCI devices (enumerated by the firmware beforehand)
> > > during
> > > +boot if the hardware domain is not present.
> > 
> > I am a bit wondering here why we would not want Xen to always do it if we
> > have
> > the code to do it in Xen anyway.
> 
> Makes sense, will drop "if the hardware domain is not present".

+1

 
> > 
> > > +
> > > +Rationale:
> > > +
> > > +Comments:
> > > +
> > > +Covers:
> > > + - `XenProd~device_passthrough~1`
> > > +
> > > +Assign PCI device to domain (with IOMMU)
> > > +----------------------------------------
> > > +
> > > +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
> > > +
> > > +Description:
> > > +Xen shall assign a specified PCI device (always implied as DMA-capable)
> > > to
> > > +a domain during its creation using passthrough (partial) device tree on
> > > Arm64
> > > +and Hyperlaunch device tree on AMD-x86. The physical device to be
> > > assigned is
> > > +protected by the IOMMU.
> > 
> > This is a very long and complex requirement.
> > I would suggest to split it in 3:
> > - generic: Xen shall support assign PCI devices to domains.
> > - arm64 one: Xen shall assign PCI devices based on device tree (explain how
> > this is configured in dts)
> > - amd: xxxx based on hyperlaunch
> 
> I agree, will split, but ...
> 
> > - Xen shall use the IOMMU to enforce DMA operations done by a PCI device
> > assigned to a domain to be restricted to the memory of the given domain.
> 
> 
>  ... does this need to be a separate 4th requirement here (and for the similar
> requirement for the platform device down the document) or this sentence is
> meant to be added to all resulting generic/arm64/amd requirements?

This is not specific to PCI, though? The generic requirement is "Xen
shall use the IOMMU to enforce DMA operations done by a DMA-capable
device assigned to a domain to be restricted to the memory of the given
domain".

I think it is also OK to both have a PCI-specific and a
non-PCI-specific requirement for that, I just wanted to mention that it
doesn't look like something to PCI-specific.
Bertrand Marquis Oct. 9, 2024, 6:30 a.m. UTC | #4
Hi Stefano,

> On 9 Oct 2024, at 00:46, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>> 
>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>> 
>>>> Add common requirements for a physical device assignment to Arm64
>>>> and AMD64 PVH domains.
>>>> 
>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>> ---
>>>> Based on:
>>>> [PATCH] docs: fusa: Replace VM with domain
>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>> ---
>>>> ---
>>>> .../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
- "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)
- interface and what we expect a domain will do with it

> 
> 
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from hardware domain
>>>> +-----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>>>> +
>>>> +Description:
>>>> +The hardware domain shall enumerate and discover PCI devices and inform
>>>> Xen
>>>> +about their appearance and disappearance.
>>> 
>>> Again this is a design requirement telling what should be done by a domain.
>>> This is an interface or an assumption of use but not a Xen design req.
>> 
>> I agree, will convert to Assumption of use on domain.
> 
> This example better aligns with our definition of Assumption of Use so
> far: we expect the hardware domain to enumerate and discover PCI
> devices, then notify Xen about their appearance or removal. This is an
> expectation placed on the hardware domain, not on Xen itself. I agree
> with Bertrand that, as written, it is more of an Assumption of Use than
> a Requirement.
> 
> However, rather than converting it into an Assumption of Use, I think we
> should rewrite it as a requirement focused on Xen's interfaces for
> enumeration. For instance:
> 
> "Xen shall provide hypercalls to allow the hardware domain to inform Xen
> about the presence of PCI devices."
> 
> 
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from Xen
>>>> +-----------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>>>> +
>>>> +Description:
>>>> +Xen shall discover PCI devices (enumerated by the firmware beforehand)
>>>> during
>>>> +boot if the hardware domain is not present.
>>> 
>>> I am a bit wondering here why we would not want Xen to always do it if we
>>> have
>>> the code to do it in Xen anyway.
>> 
>> Makes sense, will drop "if the hardware domain is not present".
> 
> +1
> 
> 
>>> 
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Assign PCI device to domain (with IOMMU)
>>>> +----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>>> +
>>>> +Description:
>>>> +Xen shall assign a specified PCI device (always implied as DMA-capable)
>>>> to
>>>> +a domain during its creation using passthrough (partial) device tree on
>>>> Arm64
>>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be
>>>> assigned is
>>>> +protected by the IOMMU.
>>> 
>>> This is a very long and complex requirement.
>>> I would suggest to split it in 3:
>>> - generic: Xen shall support assign PCI devices to domains.
>>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how
>>> this is configured in dts)
>>> - amd: xxxx based on hyperlaunch
>> 
>> I agree, will split, but ...
>> 
>>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device
>>> assigned to a domain to be restricted to the memory of the given domain.
>> 
>> 
>> ... does this need to be a separate 4th requirement here (and for the similar
>> requirement for the platform device down the document) or this sentence is
>> meant to be added to all resulting generic/arm64/amd requirements?
> 
> This is not specific to PCI, though? The generic requirement is "Xen
> shall use the IOMMU to enforce DMA operations done by a DMA-capable
> device assigned to a domain to be restricted to the memory of the given
> domain".
> 
> I think it is also OK to both have a PCI-specific and a
> non-PCI-specific requirement for that, I just wanted to mention that it
> doesn't look like something to PCI-specific.

I think we should completely abstract out the DMA engine problem as it
is not specific to a PCI or not device but to any DMA capable stuff.

Maybe the same also goes for interrupts and how those are assigned or
forwarded or not to a domain (attached or not to a device).

Cheers
Bertrand
Bertrand Marquis Oct. 9, 2024, 6:36 a.m. UTC | #5
Hi Oleksandr,

> On 8 Oct 2024, at 20:53, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> 
> 
> 
> On 08.10.24 09:17, Bertrand Marquis wrote:
>> Hi,
> 
> Hello Bertrand
> 
> 
>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>> 
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> 
>>> Add common requirements for a physical device assignment to Arm64
>>> and AMD64 PVH domains.
>>> 
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> ---
>>> Based on:
>>> [PATCH] docs: fusa: Replace VM with domain
>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>> ---
>>> ---
>>> .../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.
remove arm64 or amd64, this is always true

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

I would turn this in positive: restrict DMA devices to only have access to the memory of the Domain there are assigned to or no memory at all if not assigned (maybe 2 reqs here).

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

As was suggested by stefano, i agree with him on turning it differently. Please see his answer.

> 
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Discover PCI devices from hardware domain
>>> +-----------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>>> +
>>> +Description:
>>> +The hardware domain shall enumerate and discover PCI devices and inform Xen
>>> +about their appearance and disappearance.
>> Again this is a design requirement telling what should be done by a domain.
>> This is an interface or an assumption of use but not a Xen design req.
> 
> I agree, will convert to Assumption of use on domain.
> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Discover PCI devices from Xen
>>> +-----------------------------
>>> +
>>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>>> +
>>> +Description:
>>> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
>>> +boot if the hardware domain is not present.
>> I am a bit wondering here why we would not want Xen to always do it if we have
>> the code to do it in Xen anyway.
> 
> Makes sense, will drop "if the hardware domain is not present".
> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Assign PCI device to domain (with IOMMU)
>>> +----------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>> +
>>> +Description:
>>> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
>>> +a domain during its creation using passthrough (partial) device tree on Arm64
>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
>>> +protected by the IOMMU.
>> This is a very long and complex requirement.
>> I would suggest to split it in 3:
>> - generic: Xen shall support assign PCI devices to domains.
>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how this is configured in dts)
>> - amd: xxxx based on hyperlaunch
> 
> I agree, will split, but ...
> 
>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device assigned to a domain to be restricted to the memory of the given domain.
> 
> 
> ... does this need to be a separate 4th requirement here (and for the similar requirement for the platform device down the document) or this sentence is meant to be added to all resulting generic/arm64/amd requirements?
> 
> I would like to clarify, there are two groups of requirements to cover DMA-capable devices in this document:
> - for devices that are behind the IOMMU and IOMMU can be used for them, those requirements description explicitly mention "device xxx is protected by the IOMMU" in addition to "(with IOMMU)" in the subject.
> - for devices that are not behind the IOMMU or IOMMU cannot be used for them, those requirements description explicitly mention "device xxx is not protected by the IOMMU" in addition to "(without IOMMU)" in the subject.

I think you need to be more generic and any DMA engine that is not protected by an IOMMU shall not be assigned to a non trusted domain.
This is in fact a requirement on the integrator, Xen cannot do much about this.

> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Deassign PCI device from domain (with IOMMU)
>>> +--------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
>>> +
>>> +Description:
>>> +Xen shall deassign a specified PCI device from a domain during its destruction.
>>> +The physical device to be deassigned is protected by the IOMMU.
>> Remove second sentence or turn it into a req to say that the PCI device shall not be allowed to do DMA anymore somehow.
> 
> 
> I would like to clarify, the second sentence here is just to indicate what type of device (in the context of IOMMU involvement) the requirement is talking about, not about special care for denying any DMA from it after deassigning.
> 
> If you still think that we need a new requirement to explicitly highlight that, I will be ok to create, in that case, I assume, the platform device will want to gain the similar requirement. Please let me know your preference.

As said in the mail to stefano, i think we should try to generalise more.
So i would say we should handle:
- register assignments
- DMA engine handling
- interrupt handling

A device is a just a logical construct which may or may not contain or use several of those elements.

> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Forbid the same PCI device assignment to multiple domains
>>> +---------------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
>>> +
>>> +Description:
>>> +Xen shall not assign the same PCI device to multiple domains by failing to
>>> +create a new domain if the device to be passed through is already assigned
>>> +to the existing domain. Also different PCI devices which share some resources
>>> +(interrupts, IOMMU connections) can be assigned only to the same domain.
>> Please split and simplify
>> - Xen shall assign a single device to a single domain
>> - Xen shall assign PCI devices sharing resources to the same domain.
> 
> Good point, will split.
> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Requirements for Arm64 only
>>> +===========================
>>> +
>>> +Assign interrupt-less platform device to domain
>>> +-----------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
>>> +
>>> +Description:
>>> +Xen shall assign a specified platform device that has only a MMIO region
>>> +(does not have any interrupts) to a domain during its creation using passthrough
>>> +device tree.
>>> +The example of interrupt-less device is PWM or clock controller.
>> I am a bit puzzled by this req. Why making a specific case for interrupt less ?
> 
> 
> Those devices exist and can be assigned to a domain, they are configured slightly differently in comparison with devices with interrupts ("xen,path" is not needed for the former), other code paths are executed in Xen.
> 
> More technical details:
> The allowance of the platform device assignment which is not behind an IOMMU (for both non-DMA-capable and DMA-capable devices) is specified using device tree property ("xen,force-assign-without-iommu") in the device node described in the passthrough device tree. The said property also allows the interrupt-less platform device assignment (a device that has only a MMIO region) without specifying the corresponding node in the host device via device tree property ("xen,path").

Please see upper.

> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Deassign interrupt-less platform device from domain
>>> +---------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
>>> +
>>> +Description:
>>> +Xen shall deassign a specified platform device that has only a MMIO region
>>> +(does not have any interrupts) from a domain during its destruction.
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Assign non-DMA-capable platform device to domain
>>> +------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
>>> +
>>> +Description:
>>> +Xen shall assign a specified non-DMA-capable platform device to a domain during
>>> +its creation using passthrough device tree.
>>> +The example of non-DMA-capable device is Timer.
>> Again why making a specific case here ?
> 
> Almost the same answer as for interrupt-less device. Here "xen,path" is needed.
> 
> 
>> Wouldn't it me more logic to describe device passthrough and then what Xen should do for interrupts and for DMA ?
> 
> I can add more details on how this is configured/what properties are used, etc in rationale for each requirement mentioning device tree. Or do you mean something else?
> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Deassign non-DMA-capable platform device from domain
>>> +----------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
>>> +
>>> +Description:
>>> +Xen shall deassign a specified non-DMA-capable platform device from a domain
>>> +during its destruction.
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Assign DMA-capable platform device to domain (with IOMMU)
>>> +---------------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
>>> +
>>> +Description:
>>> +Xen shall assign a specified DMA-capable platform device to a domain during
>>> +its creation using passthrough device tree. The physical device to be assigned
>>> +is protected by the IOMMU.
>> This requirement is not a design but an higher level as it does not tell anything about implementation.
> 
> Ok, will add details regarding passthrough/host device trees.
> 
> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Deassign DMA-capable platform device from domain (with IOMMU)
>>> +-------------------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
>>> +
>>> +Description:
>>> +Xen shall deassign a specified DMA-capable platform device from a domain during
>>> +its destruction. The physical device to be deassigned is protected by the IOMMU.
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~device_passthrough~1`
>>> +
>>> +Assign DMA-capable platform device to domain (without IOMMU)
>>> +------------------------------------------------------------
>>> +
>>> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
>>> +
>>> +Description:
>>> +Xen shall assign a specified DMA-capable platform device to a domain during
>>> +its creation using passthrough device tree. The physical device to be assigned
>>> +is not protected by the IOMMU.
>>> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
>>> +the direct mapped domains only. The direct mapped domain must be either safe or
>>> +additional security mechanisms for protecting against possible malicious or
>>> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
>>> +and Xilinx peripheral protection unit (XPPU).
>> Please split in several reqs.
> 
> 
> I agree, will do. I feel it should be split into the following requirements:
> - Assign DMA-capable platform device to domain (without IOMMU)
> - Create direct mapped domain
> - Enable additional security mechanisms in direct mapped domain
> 
> To be honest, I'm not quite sure whether it is worth creating the last requirement ...

I do not think the last one is needed here.
It could be an integrator guidance at best.

Cheers
Bertrand

> 
> 
>> Stopping here my review for now
> 
> Thanks for the review.
> 
> 
>> Cheers
>> Bertrand
> 
> [snip]
Julien Grall Oct. 9, 2024, 7:26 a.m. UTC | #6
Hi,

On 07/10/2024 19:55, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Add common requirements for a physical device assignment to Arm64
> and AMD64 PVH domains.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> Based on:
> [PATCH] docs: fusa: Replace VM with domain
> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
> ---
> ---
>   .../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
This requirement would prevent us to expose a virtual SMMU to the guest. 
I think the requirement should only be Xen configures the stage-2 IOMMU.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Discover PCI devices from hardware domain
> +-----------------------------------------
> +
> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
> +
> +Description:
> +The hardware domain shall enumerate and discover PCI devices and inform Xen
> +about their appearance and disappearance.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Discover PCI devices from Xen
> +-----------------------------
> +
> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
> +
> +Description:
> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
> +boot if the hardware domain is not present.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign PCI device to domain (with IOMMU)
> +----------------------------------------
> +
> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
> +
> +Description:
> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
> +a domain during its creation using passthrough (partial) device tree on Arm64
> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
> +protected by the IOMMU.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign PCI device from domain (with IOMMU)
> +--------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified PCI device from a domain during its destruction.
> +The physical device to be deassigned is protected by the IOMMU.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Forbid the same PCI device assignment to multiple domains
> +---------------------------------------------------------
> +
> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
> +
> +Description:
> +Xen shall not assign the same PCI device to multiple domains by failing to
> +create a new domain if the device to be passed through is already assigned
> +to the existing domain. Also different PCI devices which share some resources
> +(interrupts, IOMMU connections) can be assigned only to the same domain.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Requirements for Arm64 only
> +===========================
> +
> +Assign interrupt-less platform device to domain
> +-----------------------------------------------

Why does it need to be "interrupt-less"?

> +
> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
> +
> +Description:
> +Xen shall assign a specified platform device that has only a MMIO region
> +(does not have any interrupts) to a domain during its creation using passthrough
> +device tree.

Is this requirement meant to be written from a dom0less point of view? 
Asking because platform device are assigned using an xl configuration 
for non-dom0less.


> +The example of interrupt-less device is PWM or clock controller.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign interrupt-less platform device from domain
> +---------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
> +
> +Description:
> +Xen shall deassign a specified platform device that has only a MMIO region
> +(does not have any interrupts) from a domain during its destruction.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign non-DMA-capable platform device to domain
> +------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
> +
> +Description:
> +Xen shall assign a specified non-DMA-capable platform device to a domain during
> +its creation using passthrough device tree.
> +The example of non-DMA-capable device is Timer.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign non-DMA-capable platform device from domain
> +----------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
> +
> +Description:
> +Xen shall deassign a specified non-DMA-capable platform device from a domain
> +during its destruction.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign DMA-capable platform device to domain (with IOMMU)
> +---------------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
> +
> +Description:
> +Xen shall assign a specified DMA-capable platform device to a domain during
> +its creation using passthrough device tree. The physical device to be assigned
> +is protected by the IOMMU.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign DMA-capable platform device from domain (with IOMMU)
> +-------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified DMA-capable platform device from a domain during
> +its destruction. The physical device to be deassigned is protected by the IOMMU.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Assign DMA-capable platform device to domain (without IOMMU)
> +------------------------------------------------------------
> +
> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
> +
> +Description:
> +Xen shall assign a specified DMA-capable platform device to a domain during
> +its creation using passthrough device tree. The physical device to be assigned
> +is not protected by the IOMMU.
> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
> +the direct mapped domains only. The direct mapped domain must be either safe or

What do you mean by "safe" in the context? Did you intend to say "trusted"?

> +additional security mechanisms for protecting against possible malicious or
> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
> +and Xilinx peripheral protection unit (XPPU).
> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Deassign DMA-capable platform device from domain (without IOMMU)
> +----------------------------------------------------------------

Do we actually need a separate section for deassign assign without the 
IOMMU? IOW, can this be combined with the deassign with IOMMU?

> +
> +`XenSwdgn~passthrough_deassign_dma_platform_device_without_iommu~1`
> +
> +Description:
> +Xen shall deassign a specified DMA-capable platform device from a domain during
> +its destruction. The physical device to be deassigned is not protected
> +by the IOMMU.
> +
> +Rationale:
> +The IOMMU is absent from the system or it is disabled (status = "disabled"
> +in the host device tree).
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Map platform device MMIO region identity
> +----------------------------------------

Can you explain why we need to make the distinction between identity 
mapping and ...

> +
> +`XenSwdgn~passthrough_map_platform_device_mmio_region_ident~1`
> +
> +Description:
> +Xen shall map platform device memory region identity (guest address ==
> +physical address) when assigning a specified platform device to a domain.
> +The device can be either non-DMA-capable or DMA-capable.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~device_passthrough~1`
> +
> +Map platform device MMIO region non-identity
> +--------------------------------------------

... non-identity one?

Cheers,
Julien Grall Oct. 9, 2024, 7:30 a.m. UTC | #7
Hi Bertrand,

On 09/10/2024 07:30, Bertrand Marquis wrote:
> Hi Stefano,
> 
>> On 9 Oct 2024, at 00:46, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>
>> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Assign PCI device to domain (with IOMMU)
>>>>> +----------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall assign a specified PCI device (always implied as DMA-capable)
>>>>> to
>>>>> +a domain during its creation using passthrough (partial) device tree on
>>>>> Arm64
>>>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be
>>>>> assigned is
>>>>> +protected by the IOMMU.
>>>>
>>>> This is a very long and complex requirement.
>>>> I would suggest to split it in 3:
>>>> - generic: Xen shall support assign PCI devices to domains.
>>>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how
>>>> this is configured in dts)
>>>> - amd: xxxx based on hyperlaunch
>>>
>>> I agree, will split, but ...
>>>
>>>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device
>>>> assigned to a domain to be restricted to the memory of the given domain.
>>>
>>>
>>> ... does this need to be a separate 4th requirement here (and for the similar
>>> requirement for the platform device down the document) or this sentence is
>>> meant to be added to all resulting generic/arm64/amd requirements?
>>
>> This is not specific to PCI, though? The generic requirement is "Xen
>> shall use the IOMMU to enforce DMA operations done by a DMA-capable
>> device assigned to a domain to be restricted to the memory of the given
>> domain".
>>
>> I think it is also OK to both have a PCI-specific and a
>> non-PCI-specific requirement for that, I just wanted to mention that it
>> doesn't look like something to PCI-specific.
> 
> I think we should completely abstract out the DMA engine problem as it
> is not specific to a PCI or not device but to any DMA capable stuff.

+1

> 
> Maybe the same also goes for interrupts and how those are assigned or
> forwarded or not to a domain (attached or not to a device).

I am not sure we can abstract this one because:
  * For platform devices, we should be able to support SPIs and MSI(-x) 
(only the former is so far).
  * For PCI devices, at least on Arm, AFAIR, we only intend to support 
MSI(-x).

Cheers,
Ayan Kumar Halder Oct. 9, 2024, 11:24 a.m. UTC | #8
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@kernel.org> wrote:
>>
>> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>>
>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>>
>>>>> Add common requirements for a physical device assignment to Arm64
>>>>> and AMD64 PVH domains.
>>>>>
>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>> ---
>>>>> Based on:
>>>>> [PATCH] docs: fusa: Replace VM with domain
>>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>>> ---
>>>>> ---
>>>>> .../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.

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

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

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

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

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.

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

Kind regards,

Ayan
Bertrand Marquis Oct. 9, 2024, 12:36 p.m. UTC | #9
Hi Ayan,

> On 9 Oct 2024, at 13:24, Ayan Kumar Halder <ayankuma@amd.com> 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@kernel.org> wrote:
>>> 
>>> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>>> 
>>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>>> 
>>>>>> Add common requirements for a physical device assignment to Arm64
>>>>>> and AMD64 PVH domains.
>>>>>> 
>>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>>> ---
>>>>>> Based on:
>>>>>> [PATCH] docs: fusa: Replace VM with domain
>>>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>>>> ---
>>>>>> ---
>>>>>> .../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
Oleksandr Tyshchenko Oct. 9, 2024, 2:42 p.m. UTC | #10
On 09.10.24 01:46, Stefano Stabellini wrote:


Hello Stefano

> On Tue, 8 Oct 2024, Oleksandr Tyshchenko wrote:
>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>
>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>
>>>> Add common requirements for a physical device assignment to Arm64
>>>> and AMD64 PVH domains.
>>>>
>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>> ---
>>>> Based on:
>>>> [PATCH] docs: fusa: Replace VM with domain
>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>> ---
>>>> ---
>>>> .../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.

Right, what I meant was new Assumption of use *on Xen* that would 
require updating docs/fusa/reqs/intro.rst.


> 
> 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, good explanation! Just maybe "Xen shall
configure the IOMMU *at domain creation time* according to the stage 2 
translation tables" since the IOMMU context is created at that time. 
Although, if we are going to deal with boot time domains only, *at boot* 
also sounds ok, I think.


> 
> 
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from hardware domain
>>>> +-----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>>>> +
>>>> +Description:
>>>> +The hardware domain shall enumerate and discover PCI devices and inform
>>>> Xen
>>>> +about their appearance and disappearance.
>>>
>>> Again this is a design requirement telling what should be done by a domain.
>>> This is an interface or an assumption of use but not a Xen design req.
>>
>> I agree, will convert to Assumption of use on domain.
> 
> This example better aligns with our definition of Assumption of Use so
> far: we expect the hardware domain to enumerate and discover PCI
> devices, then notify Xen about their appearance or removal. This is an
> expectation placed on the hardware domain, not on Xen itself. I agree
> with Bertrand that, as written, it is more of an Assumption of Use than
> a Requirement.
> 
> However, rather than converting it into an Assumption of Use, I think we
> should rewrite it as a requirement focused on Xen's interfaces for
> enumeration. For instance:
> 
> "Xen shall provide hypercalls to allow the hardware domain to inform Xen
> about the presence of PCI devices."

I agree, will rewrite.


> 
> 
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from Xen
>>>> +-----------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>>>> +
>>>> +Description:
>>>> +Xen shall discover PCI devices (enumerated by the firmware beforehand)
>>>> during
>>>> +boot if the hardware domain is not present.
>>>
>>> I am a bit wondering here why we would not want Xen to always do it if we
>>> have
>>> the code to do it in Xen anyway.
>>
>> Makes sense, will drop "if the hardware domain is not present".
> 
> +1
> 
>   
>>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Assign PCI device to domain (with IOMMU)
>>>> +----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>>> +
>>>> +Description:
>>>> +Xen shall assign a specified PCI device (always implied as DMA-capable)
>>>> to
>>>> +a domain during its creation using passthrough (partial) device tree on
>>>> Arm64
>>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be
>>>> assigned is
>>>> +protected by the IOMMU.
>>>
>>> This is a very long and complex requirement.
>>> I would suggest to split it in 3:
>>> - generic: Xen shall support assign PCI devices to domains.
>>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how
>>> this is configured in dts)
>>> - amd: xxxx based on hyperlaunch
>>
>> I agree, will split, but ...
>>
>>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device
>>> assigned to a domain to be restricted to the memory of the given domain.
>>
>>
>>   ... does this need to be a separate 4th requirement here (and for the similar
>> requirement for the platform device down the document) or this sentence is
>> meant to be added to all resulting generic/arm64/amd requirements?
> 
> This is not specific to PCI, though? The generic requirement is "Xen
> shall use the IOMMU to enforce DMA operations done by a DMA-capable
> device assigned to a domain to be restricted to the memory of the given
> domain".

Makes sense.


> 
> I think it is also OK to both have a PCI-specific and a
> non-PCI-specific requirement for that, I just wanted to mention that it
> doesn't look like something to PCI-specific.


ok, let's have a single requirement with generic subject, in description 
I can add the following:

The DMA-capable device can be either PCI or platform device.
Oleksandr Tyshchenko Oct. 9, 2024, 5:20 p.m. UTC | #11
On 09.10.24 09:36, Bertrand Marquis wrote:
> Hi Oleksandr,

Hello Bertrand


> 
>> On 8 Oct 2024, at 20:53, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>
>>
>>
>> On 08.10.24 09:17, Bertrand Marquis wrote:
>>> Hi,
>>
>> Hello Bertrand
>>
>>
>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>
>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>
>>>> Add common requirements for a physical device assignment to Arm64
>>>> and AMD64 PVH domains.
>>>>
>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>> ---
>>>> Based on:
>>>> [PATCH] docs: fusa: Replace VM with domain
>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>> ---
>>>> ---
>>>> .../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.
> remove arm64 or amd64, this is always true >
>> - 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.
> 
> I would turn this in positive: restrict DMA devices to only have access to the memory of the Domain there are assigned to or no memory at all if not assigned (maybe 2 reqs here).


ok to both. However, I am a bit lost ...



> 
>>
>>
>>> 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.
> 
> As was suggested by stefano, i agree with him on turning it differently. Please see his answer.


  ... yes, I saw his answer and completely agree with him as well, my 
question is what I should do with the rationale?

The rationale I proposed above explains why we do not want the IOMMU
in particular to be assigned to a domain. But, it belongs to the initial 
"Hide IOMMU from a domain" requirement. Now, with turning it into "Xen 
shall configure the IOMMU at boot according to the stage 2 translation
tables" requirement should the rationale still be present?


> 
>>
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from hardware domain
>>>> +-----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>>>> +
>>>> +Description:
>>>> +The hardware domain shall enumerate and discover PCI devices and inform Xen
>>>> +about their appearance and disappearance.
>>> Again this is a design requirement telling what should be done by a domain.
>>> This is an interface or an assumption of use but not a Xen design req.
>>
>> I agree, will convert to Assumption of use on domain.
>>
>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Discover PCI devices from Xen
>>>> +-----------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>>>> +
>>>> +Description:
>>>> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
>>>> +boot if the hardware domain is not present.
>>> I am a bit wondering here why we would not want Xen to always do it if we have
>>> the code to do it in Xen anyway.
>>
>> Makes sense, will drop "if the hardware domain is not present".
>>
>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Assign PCI device to domain (with IOMMU)
>>>> +----------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>>> +
>>>> +Description:
>>>> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
>>>> +a domain during its creation using passthrough (partial) device tree on Arm64
>>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
>>>> +protected by the IOMMU.
>>> This is a very long and complex requirement.
>>> I would suggest to split it in 3:
>>> - generic: Xen shall support assign PCI devices to domains.
>>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how this is configured in dts)
>>> - amd: xxxx based on hyperlaunch
>>
>> I agree, will split, but ...
>>
>>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device assigned to a domain to be restricted to the memory of the given domain.
>>
>>
>> ... does this need to be a separate 4th requirement here (and for the similar requirement for the platform device down the document) or this sentence is meant to be added to all resulting generic/arm64/amd requirements?
>>
>> I would like to clarify, there are two groups of requirements to cover DMA-capable devices in this document:
>> - for devices that are behind the IOMMU and IOMMU can be used for them, those requirements description explicitly mention "device xxx is protected by the IOMMU" in addition to "(with IOMMU)" in the subject.
>> - for devices that are not behind the IOMMU or IOMMU cannot be used for them, those requirements description explicitly mention "device xxx is not protected by the IOMMU" in addition to "(without IOMMU)" in the subject.
> 
> I think you need to be more generic and any DMA engine that is not protected by an IOMMU shall not be assigned to a non trusted domain.
> This is in fact a requirement on the integrator, Xen cannot do much about this.

yes, I agree


> 
>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Deassign PCI device from domain (with IOMMU)
>>>> +--------------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
>>>> +
>>>> +Description:
>>>> +Xen shall deassign a specified PCI device from a domain during its destruction.
>>>> +The physical device to be deassigned is protected by the IOMMU.
>>> Remove second sentence or turn it into a req to say that the PCI device shall not be allowed to do DMA anymore somehow.
>>
>>
>> I would like to clarify, the second sentence here is just to indicate what type of device (in the context of IOMMU involvement) the requirement is talking about, not about special care for denying any DMA from it after deassigning.
>>
>> If you still think that we need a new requirement to explicitly highlight that, I will be ok to create, in that case, I assume, the platform device will want to gain the similar requirement. Please let me know your preference.
> 
> As said in the mail to stefano, i think we should try to generalise more.
> So i would say we should handle:
> - register assignments
> - DMA engine handling
> - interrupt handling
> 
> A device is a just a logical construct which may or may not contain or use several of those elements.


I agree regarding DMA engine handling. As for register assignments and 
interrupt handling I am not quite sure. I am afraid we will need to 
differentiate between platform and PCI devices.


> 
>>
>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Forbid the same PCI device assignment to multiple domains
>>>> +---------------------------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
>>>> +
>>>> +Description:
>>>> +Xen shall not assign the same PCI device to multiple domains by failing to
>>>> +create a new domain if the device to be passed through is already assigned
>>>> +to the existing domain. Also different PCI devices which share some resources
>>>> +(interrupts, IOMMU connections) can be assigned only to the same domain.
>>> Please split and simplify
>>> - Xen shall assign a single device to a single domain
>>> - Xen shall assign PCI devices sharing resources to the same domain.
>>
>> Good point, will split.
>>
>>
>>>> +
>>>> +Rationale:
>>>> +
>>>> +Comments:
>>>> +
>>>> +Covers:
>>>> + - `XenProd~device_passthrough~1`
>>>> +
>>>> +Requirements for Arm64 only
>>>> +===========================
>>>> +
>>>> +Assign interrupt-less platform device to domain
>>>> +-----------------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
>>>> +
>>>> +Description:
>>>> +Xen shall assign a specified platform device that has only a MMIO region
>>>> +(does not have any interrupts) to a domain during its creation using passthrough
>>>> +device tree.
>>>> +The example of interrupt-less device is PWM or clock controller.
>>> I am a bit puzzled by this req. Why making a specific case for interrupt less ?
>>
>>
>> Those devices exist and can be assigned to a domain, they are configured slightly differently in comparison with devices with interrupts ("xen,path" is not needed for the former), other code paths are executed in Xen.
>>
>> More technical details:
>> The allowance of the platform device assignment which is not behind an IOMMU (for both non-DMA-capable and DMA-capable devices) is specified using device tree property ("xen,force-assign-without-iommu") in the device node described in the passthrough device tree. The said property also allows the interrupt-less platform device assignment (a device that has only a MMIO region) without specifying the corresponding node in the host device via device tree property ("xen,path").
> 
> Please see upper.

Yes, what I got from the text above is that we won't need a separate set 
of requirements for interrupt-less device, I will drop them. The same 
goes for "Assign/Deassign *non-DMA-capable* platform device to domain". 
Please let me know if I got this wrong.


[snip]


>>>> +
>>>> +Assign DMA-capable platform device to domain (without IOMMU)
>>>> +------------------------------------------------------------
>>>> +
>>>> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
>>>> +
>>>> +Description:
>>>> +Xen shall assign a specified DMA-capable platform device to a domain during
>>>> +its creation using passthrough device tree. The physical device to be assigned
>>>> +is not protected by the IOMMU.
>>>> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
>>>> +the direct mapped domains only. The direct mapped domain must be either safe or
>>>> +additional security mechanisms for protecting against possible malicious or
>>>> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
>>>> +and Xilinx peripheral protection unit (XPPU).
>>> Please split in several reqs.
>>
>>
>> I agree, will do. I feel it should be split into the following requirements:
>> - Assign DMA-capable platform device to domain (without IOMMU)
>> - Create direct mapped domain
>> - Enable additional security mechanisms in direct mapped domain
>>
>> To be honest, I'm not quite sure whether it is worth creating the last requirement ...
> 
> I do not think the last one is needed here.
> It could be an integrator guidance at best.

ok, thanks for the clarification.


> 
> Cheers
> Bertrand
> 
>>
>>
>>> Stopping here my review for now
>>
>> Thanks for the review.
>>
>>
>>> Cheers
>>> Bertrand
>>
>> [snip]
> 
>
Oleksandr Tyshchenko Oct. 9, 2024, 7:22 p.m. UTC | #12
On 09.10.24 10:26, Julien Grall wrote:
> Hi,

Hello Julien


> 
> On 07/10/2024 19:55, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> Add common requirements for a physical device assignment to Arm64
>> and AMD64 PVH domains.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> ---
>> Based on:
>> [PATCH] docs: fusa: Replace VM with domain
>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>> ---
>> ---
>>   .../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
> This requirement would prevent us to expose a virtual SMMU to the guest.


Are you talking about assigning stage-1 SMMU to the guest? Yes, that is 
a valid point...


> I think the requirement should only be Xen configures the stage-2 IOMMU.


    ... you are right, as was discussed in separate emails, the 
requirement would be turned into "Xen shall configure the IOMMU at boot 
according to the stage 2 translation tables."

> 
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Discover PCI devices from hardware domain
>> +-----------------------------------------
>> +
>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>> +
>> +Description:
>> +The hardware domain shall enumerate and discover PCI devices and 
>> inform Xen
>> +about their appearance and disappearance.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Discover PCI devices from Xen
>> +-----------------------------
>> +
>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>> +
>> +Description:
>> +Xen shall discover PCI devices (enumerated by the firmware 
>> beforehand) during
>> +boot if the hardware domain is not present.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign PCI device to domain (with IOMMU)
>> +----------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified PCI device (always implied as 
>> DMA-capable) to
>> +a domain during its creation using passthrough (partial) device tree 
>> on Arm64
>> +and Hyperlaunch device tree on AMD-x86. The physical device to be 
>> assigned is
>> +protected by the IOMMU.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign PCI device from domain (with IOMMU)
>> +--------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall deassign a specified PCI device from a domain during its 
>> destruction.
>> +The physical device to be deassigned is protected by the IOMMU.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Forbid the same PCI device assignment to multiple domains
>> +---------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
>> +
>> +Description:
>> +Xen shall not assign the same PCI device to multiple domains by 
>> failing to
>> +create a new domain if the device to be passed through is already 
>> assigned
>> +to the existing domain. Also different PCI devices which share some 
>> resources
>> +(interrupts, IOMMU connections) can be assigned only to the same domain.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Requirements for Arm64 only
>> +===========================
>> +
>> +Assign interrupt-less platform device to domain
>> +-----------------------------------------------
> 
> Why does it need to be "interrupt-less"?


This requirement describes one of the possible cases of platform device.

My answer from another email:

Those devices exist and can be assigned to a domain, they are configured
slightly differently in comparison with devices with interrupts
("xen,path" is not needed for the former), other code paths are executed
in Xen.

**********

There was an intention to keep in separate requirements all relevant 
possible cases that can be done using what the passthrough feature 
provides (let's say all possible combinations of dom0less passthrough 
properties).

This is why the document has, for example, the following special cases:
- Assign interrupt-less platform device to domain
- Assign non-DMA-capable platform device to domain
- Assign DMA-capable platform device to domain (with IOMMU)
- Assign DMA-capable platform device to domain (without IOMMU)
...

Now, as I got from other emails, this scheme is not suited well and 
needs to be reworked.


> 
>> +
>> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
>> +
>> +Description:
>> +Xen shall assign a specified platform device that has only a MMIO region
>> +(does not have any interrupts) to a domain during its creation using 
>> passthrough
>> +device tree.
> 
> Is this requirement meant to be written from a dom0less point of view? 
> Asking because platform device are assigned using an xl configuration 
> for non-dom0less.

Yes. The more, all requirements in this document imply boot time 
domains. Likely, it should be explicitly stated at the beginning of this 
document.

> 
> 
>> +The example of interrupt-less device is PWM or clock controller.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign interrupt-less platform device from domain
>> +---------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
>> +
>> +Description:
>> +Xen shall deassign a specified platform device that has only a MMIO 
>> region
>> +(does not have any interrupts) from a domain during its destruction.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign non-DMA-capable platform device to domain
>> +------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
>> +
>> +Description:
>> +Xen shall assign a specified non-DMA-capable platform device to a 
>> domain during
>> +its creation using passthrough device tree.
>> +The example of non-DMA-capable device is Timer.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign non-DMA-capable platform device from domain
>> +----------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
>> +
>> +Description:
>> +Xen shall deassign a specified non-DMA-capable platform device from a 
>> domain
>> +during its destruction.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign DMA-capable platform device to domain (with IOMMU)
>> +---------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified DMA-capable platform device to a domain 
>> during
>> +its creation using passthrough device tree. The physical device to be 
>> assigned
>> +is protected by the IOMMU.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign DMA-capable platform device from domain (with IOMMU)
>> +-------------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
>> +
>> +Description:
>> +Xen shall deassign a specified DMA-capable platform device from a 
>> domain during
>> +its destruction. The physical device to be deassigned is protected by 
>> the IOMMU.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Assign DMA-capable platform device to domain (without IOMMU)
>> +------------------------------------------------------------
>> +
>> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
>> +
>> +Description:
>> +Xen shall assign a specified DMA-capable platform device to a domain 
>> during
>> +its creation using passthrough device tree. The physical device to be 
>> assigned
>> +is not protected by the IOMMU.
>> +The DMA-capable device assignment which is not behind an IOMMU is 
>> allowed for
>> +the direct mapped domains only. The direct mapped domain must be 
>> either safe or
> 
> What do you mean by "safe" in the context? Did you intend to say "trusted"?

Yes, trusted, thanks.


> 
>> +additional security mechanisms for protecting against possible 
>> malicious or
>> +buggy DMA devices must be in place, e.g. Xilinx memory protection 
>> unit (XMPU)
>> +and Xilinx peripheral protection unit (XPPU).
>> +
>> +Rationale:
>> +The IOMMU is absent from the system or it is disabled (status = 
>> "disabled"
>> +in the host device tree).
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Deassign DMA-capable platform device from domain (without IOMMU)
>> +----------------------------------------------------------------
> 
> Do we actually need a separate section for deassign assign without the 
> IOMMU? IOW, can this be combined with the deassign with IOMMU?


The point was that on deassigment, Xen additionally detaches the device 
from the IOMMU (if the device is protected by the IOMMU) in comparison 
with the device not protected by the IOMMU, and this needs to be covered 
and tested somehow. Therefore, two separate requirement exist here.

Or do you, perhaps, mean to combine with "Deassign non-DMA-capable 
platform device from domain"? Which could be combined, I think.

> 
>> +
>> +`XenSwdgn~passthrough_deassign_dma_platform_device_without_iommu~1`
>> +
>> +Description:
>> +Xen shall deassign a specified DMA-capable platform device from a 
>> domain during
>> +its destruction. The physical device to be deassigned is not protected
>> +by the IOMMU.
>> +
>> +Rationale:
>> +The IOMMU is absent from the system or it is disabled (status = 
>> "disabled"
>> +in the host device tree).
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Map platform device MMIO region identity
>> +----------------------------------------
> 
> Can you explain why we need to make the distinction between identity 
> mapping and ... >
>> +
>> +`XenSwdgn~passthrough_map_platform_device_mmio_region_ident~1`
>> +
>> +Description:
>> +Xen shall map platform device memory region identity (guest address ==
>> +physical address) when assigning a specified platform device to a 
>> domain.
>> +The device can be either non-DMA-capable or DMA-capable.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~device_passthrough~1`
>> +
>> +Map platform device MMIO region non-identity
>> +--------------------------------------------
> 
> ... non-identity one?

MMIO remapping is also what user can do using passthrough feature, the 
point again was how could we check that it worked if there was no 
requirement covering it.


> 
> Cheers,
>
Bertrand Marquis Oct. 10, 2024, 6:22 a.m. UTC | #13
Hi Oleksandr,

> On 9 Oct 2024, at 19:20, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> 
> 
> 
> On 09.10.24 09:36, Bertrand Marquis wrote:
>> Hi Oleksandr,
> 
> Hello Bertrand
> 
> 
>>> On 8 Oct 2024, at 20:53, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On 08.10.24 09:17, Bertrand Marquis wrote:
>>>> Hi,
>>> 
>>> Hello Bertrand
>>> 
>>> 
>>>>> On 7 Oct 2024, at 20:55, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>>>>> 
>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>> 
>>>>> Add common requirements for a physical device assignment to Arm64
>>>>> and AMD64 PVH domains.
>>>>> 
>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>>> ---
>>>>> Based on:
>>>>> [PATCH] docs: fusa: Replace VM with domain
>>>>> https://patchew.org/Xen/20241007182603.826807-1-ayan.kumar.halder@amd.com/
>>>>> ---
>>>>> ---
>>>>> .../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.
>> remove arm64 or amd64, this is always true >
>>> - 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.
>> I would turn this in positive: restrict DMA devices to only have access to the memory of the Domain there are assigned to or no memory at all if not assigned (maybe 2 reqs here).
> 
> 
> ok to both. However, I am a bit lost ...
> 
> 
> 
>>> 
>>> 
>>>> 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.
>> As was suggested by stefano, i agree with him on turning it differently. Please see his answer.
> 
> 
> ... yes, I saw his answer and completely agree with him as well, my question is what I should do with the rationale?
> 
> The rationale I proposed above explains why we do not want the IOMMU
> in particular to be assigned to a domain. But, it belongs to the initial "Hide IOMMU from a domain" requirement. Now, with turning it into "Xen shall configure the IOMMU at boot according to the stage 2 translation
> tables" requirement should the rationale still be present?

No this would now be part of integrator role or something like that but should not be a rationale here anymore.

> 
> 
>>> 
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Discover PCI devices from hardware domain
>>>>> +-----------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
>>>>> +
>>>>> +Description:
>>>>> +The hardware domain shall enumerate and discover PCI devices and inform Xen
>>>>> +about their appearance and disappearance.
>>>> Again this is a design requirement telling what should be done by a domain.
>>>> This is an interface or an assumption of use but not a Xen design req.
>>> 
>>> I agree, will convert to Assumption of use on domain.
>>> 
>>> 
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Discover PCI devices from Xen
>>>>> +-----------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall discover PCI devices (enumerated by the firmware beforehand) during
>>>>> +boot if the hardware domain is not present.
>>>> I am a bit wondering here why we would not want Xen to always do it if we have
>>>> the code to do it in Xen anyway.
>>> 
>>> Makes sense, will drop "if the hardware domain is not present".
>>> 
>>> 
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Assign PCI device to domain (with IOMMU)
>>>>> +----------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall assign a specified PCI device (always implied as DMA-capable) to
>>>>> +a domain during its creation using passthrough (partial) device tree on Arm64
>>>>> +and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
>>>>> +protected by the IOMMU.
>>>> This is a very long and complex requirement.
>>>> I would suggest to split it in 3:
>>>> - generic: Xen shall support assign PCI devices to domains.
>>>> - arm64 one: Xen shall assign PCI devices based on device tree (explain how this is configured in dts)
>>>> - amd: xxxx based on hyperlaunch
>>> 
>>> I agree, will split, but ...
>>> 
>>>> - Xen shall use the IOMMU to enforce DMA operations done by a PCI device assigned to a domain to be restricted to the memory of the given domain.
>>> 
>>> 
>>> ... does this need to be a separate 4th requirement here (and for the similar requirement for the platform device down the document) or this sentence is meant to be added to all resulting generic/arm64/amd requirements?
>>> 
>>> I would like to clarify, there are two groups of requirements to cover DMA-capable devices in this document:
>>> - for devices that are behind the IOMMU and IOMMU can be used for them, those requirements description explicitly mention "device xxx is protected by the IOMMU" in addition to "(with IOMMU)" in the subject.
>>> - for devices that are not behind the IOMMU or IOMMU cannot be used for them, those requirements description explicitly mention "device xxx is not protected by the IOMMU" in addition to "(without IOMMU)" in the subject.
>> I think you need to be more generic and any DMA engine that is not protected by an IOMMU shall not be assigned to a non trusted domain.
>> This is in fact a requirement on the integrator, Xen cannot do much about this.
> 
> yes, I agree
> 
> 
>>> 
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Deassign PCI device from domain (with IOMMU)
>>>>> +--------------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall deassign a specified PCI device from a domain during its destruction.
>>>>> +The physical device to be deassigned is protected by the IOMMU.
>>>> Remove second sentence or turn it into a req to say that the PCI device shall not be allowed to do DMA anymore somehow.
>>> 
>>> 
>>> I would like to clarify, the second sentence here is just to indicate what type of device (in the context of IOMMU involvement) the requirement is talking about, not about special care for denying any DMA from it after deassigning.
>>> 
>>> If you still think that we need a new requirement to explicitly highlight that, I will be ok to create, in that case, I assume, the platform device will want to gain the similar requirement. Please let me know your preference.
>> As said in the mail to stefano, i think we should try to generalise more.
>> So i would say we should handle:
>> - register assignments
>> - DMA engine handling
>> - interrupt handling
>> A device is a just a logical construct which may or may not contain or use several of those elements.
> 
> 
> I agree regarding DMA engine handling. As for register assignments and interrupt handling I am not quite sure. I am afraid we will need to differentiate between platform and PCI devices.

Once a PCI device has been detected it becomes some registers and interrupts.
There might be some PCI ways to configure some stuff but appart from that i do not see any difference with a platform device.
Do i miss something ?

> 
> 
>>> 
>>> 
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Forbid the same PCI device assignment to multiple domains
>>>>> +---------------------------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall not assign the same PCI device to multiple domains by failing to
>>>>> +create a new domain if the device to be passed through is already assigned
>>>>> +to the existing domain. Also different PCI devices which share some resources
>>>>> +(interrupts, IOMMU connections) can be assigned only to the same domain.
>>>> Please split and simplify
>>>> - Xen shall assign a single device to a single domain
>>>> - Xen shall assign PCI devices sharing resources to the same domain.
>>> 
>>> Good point, will split.
>>> 
>>> 
>>>>> +
>>>>> +Rationale:
>>>>> +
>>>>> +Comments:
>>>>> +
>>>>> +Covers:
>>>>> + - `XenProd~device_passthrough~1`
>>>>> +
>>>>> +Requirements for Arm64 only
>>>>> +===========================
>>>>> +
>>>>> +Assign interrupt-less platform device to domain
>>>>> +-----------------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall assign a specified platform device that has only a MMIO region
>>>>> +(does not have any interrupts) to a domain during its creation using passthrough
>>>>> +device tree.
>>>>> +The example of interrupt-less device is PWM or clock controller.
>>>> I am a bit puzzled by this req. Why making a specific case for interrupt less ?
>>> 
>>> 
>>> Those devices exist and can be assigned to a domain, they are configured slightly differently in comparison with devices with interrupts ("xen,path" is not needed for the former), other code paths are executed in Xen.
>>> 
>>> More technical details:
>>> The allowance of the platform device assignment which is not behind an IOMMU (for both non-DMA-capable and DMA-capable devices) is specified using device tree property ("xen,force-assign-without-iommu") in the device node described in the passthrough device tree. The said property also allows the interrupt-less platform device assignment (a device that has only a MMIO region) without specifying the corresponding node in the host device via device tree property ("xen,path").
>> Please see upper.
> 
> Yes, what I got from the text above is that we won't need a separate set of requirements for interrupt-less device, I will drop them. The same goes for "Assign/Deassign *non-DMA-capable* platform device to domain". Please let me know if I got this wrong.


No sounds good.

> 
> 
> [snip]
> 
> 
>>>>> +
>>>>> +Assign DMA-capable platform device to domain (without IOMMU)
>>>>> +------------------------------------------------------------
>>>>> +
>>>>> +`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
>>>>> +
>>>>> +Description:
>>>>> +Xen shall assign a specified DMA-capable platform device to a domain during
>>>>> +its creation using passthrough device tree. The physical device to be assigned
>>>>> +is not protected by the IOMMU.
>>>>> +The DMA-capable device assignment which is not behind an IOMMU is allowed for
>>>>> +the direct mapped domains only. The direct mapped domain must be either safe or
>>>>> +additional security mechanisms for protecting against possible malicious or
>>>>> +buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
>>>>> +and Xilinx peripheral protection unit (XPPU).
>>>> Please split in several reqs.
>>> 
>>> 
>>> I agree, will do. I feel it should be split into the following requirements:
>>> - Assign DMA-capable platform device to domain (without IOMMU)
>>> - Create direct mapped domain
>>> - Enable additional security mechanisms in direct mapped domain
>>> 
>>> To be honest, I'm not quite sure whether it is worth creating the last requirement ...
>> I do not think the last one is needed here.
>> It could be an integrator guidance at best.
> 
> ok, thanks for the clarification.

Cheers
Bertrand

> 
> 
>> Cheers
>> Bertrand
>>> 
>>> 
>>>> Stopping here my review for now
>>> 
>>> Thanks for the review.
>>> 
>>> 
>>>> Cheers
>>>> Bertrand
>>> 
>>> [snip]
Ayan Kumar Halder Oct. 10, 2024, 9:18 a.m. UTC | #14
Hi Bertrand,

< snip>

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

Agreed, we need to document somewhere that Domains shall not use 
registers like HVC_DCC, etc which are not properly emulated by Xen.

Yes, it should not be a part of AoU as Xen's behaviour is not dependent 
on it.

>
> Xen behaviour if used should be specified.
Agreed, there should be a document stating the behavior of Xen if non 
emulated registers are accessed by domains (as an example).
>
>> 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.

I agree that this is an AoU on firmware or platform. But we can agree 
that this AoU cannot be tested by us (within the scope of Xen's safety 
certification) as we do not know on which hardware platform Xen is 
deployed. The system integrator (or hardware manufacturer) should know 
the correct value of system counter frequency. Thus, they should be able 
to test this.

Our intention is to keep the scope of Xen's safety certification 
decoupled from a specific hardware platform. Is this making sense ?

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

Yes, we will be having requirements for the hypercalls. Do you mean this ?

- Ayan
Bertrand Marquis Oct. 10, 2024, 10:24 a.m. UTC | #15
HI Ayan,

> On 10 Oct 2024, at 11:18, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> Hi Bertrand,
> 
> < snip>
> 
>>> 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.
> 
> Agreed, we need to document somewhere that Domains shall not use registers like HVC_DCC, etc which are not properly emulated by Xen.
> 
> Yes, it should not be a part of AoU as Xen's behaviour is not dependent on it.
> 
>> 
>> Xen behaviour if used should be specified.
> Agreed, there should be a document stating the behavior of Xen if non emulated registers are accessed by domains (as an example).
>> 
>>> 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.
> 
> I agree that this is an AoU on firmware or platform. But we can agree that this AoU cannot be tested by us (within the scope of Xen's safety certification) as we do not know on which hardware platform Xen is deployed. The system integrator (or hardware manufacturer) should know the correct value of system counter frequency. Thus, they should be able to test this.
> 
> Our intention is to keep the scope of Xen's safety certification decoupled from a specific hardware platform. Is this making sense ?

It does as long as the expectations on the hardware are well documented.

> 
>> 
>>>> - 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.
> 
> Yes, we will be having requirements for the hypercalls. Do you mean this ?

Yes

Cheers
Bertrand

> 
> - Ayan
>
diff mbox series

Patch

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:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Discover PCI devices from hardware domain
+-----------------------------------------
+
+`XenSwdgn~passthrough_discover_pci_devices_from_hwdom~1`
+
+Description:
+The hardware domain shall enumerate and discover PCI devices and inform Xen
+about their appearance and disappearance.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Discover PCI devices from Xen
+-----------------------------
+
+`XenSwdgn~passthrough_discover_pci_devices_from_xen~1`
+
+Description:
+Xen shall discover PCI devices (enumerated by the firmware beforehand) during
+boot if the hardware domain is not present.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Assign PCI device to domain (with IOMMU)
+----------------------------------------
+
+`XenSwdgn~passthrough_assign_pci_device_with_iommu~1`
+
+Description:
+Xen shall assign a specified PCI device (always implied as DMA-capable) to
+a domain during its creation using passthrough (partial) device tree on Arm64
+and Hyperlaunch device tree on AMD-x86. The physical device to be assigned is
+protected by the IOMMU.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign PCI device from domain (with IOMMU)
+--------------------------------------------
+
+`XenSwdgn~passthrough_deassign_pci_device_with_iommu~1`
+
+Description:
+Xen shall deassign a specified PCI device from a domain during its destruction.
+The physical device to be deassigned is protected by the IOMMU.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Forbid the same PCI device assignment to multiple domains
+---------------------------------------------------------
+
+`XenSwdgn~passthrough_forbid_same_pci_device_assignment~1`
+
+Description:
+Xen shall not assign the same PCI device to multiple domains by failing to
+create a new domain if the device to be passed through is already assigned
+to the existing domain. Also different PCI devices which share some resources
+(interrupts, IOMMU connections) can be assigned only to the same domain.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Requirements for Arm64 only
+===========================
+
+Assign interrupt-less platform device to domain
+-----------------------------------------------
+
+`XenSwdgn~passthrough_assign_interrupt_less_platform_device~1`
+
+Description:
+Xen shall assign a specified platform device that has only a MMIO region
+(does not have any interrupts) to a domain during its creation using passthrough
+device tree.
+The example of interrupt-less device is PWM or clock controller.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign interrupt-less platform device from domain
+---------------------------------------------------
+
+`XenSwdgn~passthrough_deassign_interrupt_less_platform_device~1`
+
+Description:
+Xen shall deassign a specified platform device that has only a MMIO region
+(does not have any interrupts) from a domain during its destruction.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Assign non-DMA-capable platform device to domain
+------------------------------------------------
+
+`XenSwdgn~passthrough_assign_non_dma_platform_device~1`
+
+Description:
+Xen shall assign a specified non-DMA-capable platform device to a domain during
+its creation using passthrough device tree.
+The example of non-DMA-capable device is Timer.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign non-DMA-capable platform device from domain
+----------------------------------------------------
+
+`XenSwdgn~passthrough_deassign_non_dma_platform_device~1`
+
+Description:
+Xen shall deassign a specified non-DMA-capable platform device from a domain
+during its destruction.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Assign DMA-capable platform device to domain (with IOMMU)
+---------------------------------------------------------
+
+`XenSwdgn~passthrough_assign_dma_platform_device_with_iommu~1`
+
+Description:
+Xen shall assign a specified DMA-capable platform device to a domain during
+its creation using passthrough device tree. The physical device to be assigned
+is protected by the IOMMU.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign DMA-capable platform device from domain (with IOMMU)
+-------------------------------------------------------------
+
+`XenSwdgn~passthrough_deassign_dma_platform_device_with_iommu~1`
+
+Description:
+Xen shall deassign a specified DMA-capable platform device from a domain during
+its destruction. The physical device to be deassigned is protected by the IOMMU.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Assign DMA-capable platform device to domain (without IOMMU)
+------------------------------------------------------------
+
+`XenSwdgn~passthrough_assign_dma_platform_device_without_iommu~1`
+
+Description:
+Xen shall assign a specified DMA-capable platform device to a domain during
+its creation using passthrough device tree. The physical device to be assigned
+is not protected by the IOMMU.
+The DMA-capable device assignment which is not behind an IOMMU is allowed for
+the direct mapped domains only. The direct mapped domain must be either safe or
+additional security mechanisms for protecting against possible malicious or
+buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
+and Xilinx peripheral protection unit (XPPU).
+
+Rationale:
+The IOMMU is absent from the system or it is disabled (status = "disabled"
+in the host device tree).
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign DMA-capable platform device from domain (without IOMMU)
+----------------------------------------------------------------
+
+`XenSwdgn~passthrough_deassign_dma_platform_device_without_iommu~1`
+
+Description:
+Xen shall deassign a specified DMA-capable platform device from a domain during
+its destruction. The physical device to be deassigned is not protected
+by the IOMMU.
+
+Rationale:
+The IOMMU is absent from the system or it is disabled (status = "disabled"
+in the host device tree).
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Map platform device MMIO region identity
+----------------------------------------
+
+`XenSwdgn~passthrough_map_platform_device_mmio_region_ident~1`
+
+Description:
+Xen shall map platform device memory region identity (guest address ==
+physical address) when assigning a specified platform device to a domain.
+The device can be either non-DMA-capable or DMA-capable.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Map platform device MMIO region non-identity
+--------------------------------------------
+
+`XenSwdgn~passthrough_map_platform_device_mmio_region_non_ident~1`
+
+Description:
+Xen shall map platform device memory region non-identity (guest address !=
+physical address) when assigning a specified platform device to a domain.
+The device can be either non-DMA-capable or DMA-capable.
+
+Rationale:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Assign PCI device to domain (without IOMMU)
+-------------------------------------------
+
+`XenSwdgn~passthrough_assign_pci_device_without_iommu~1`
+
+Description:
+Xen shall assign a specified PCI device to a domain during its creation using
+passthrough device tree. The physical device to be assigned is not protected
+by the IOMMU.
+The DMA-capable device assignment which is not behind an IOMMU is allowed for
+the direct mapped domains only. The direct mapped domain must be either safe or
+additional security mechanisms for protecting against possible malicious or
+buggy DMA devices must be in place, e.g. Xilinx memory protection unit (XMPU)
+and Xilinx peripheral protection unit (XPPU).
+
+Rationale:
+The IOMMU is absent from the system or it is disabled (status = "disabled"
+in the host device tree).
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Deassign PCI device from domain (without IOMMU)
+-----------------------------------------------
+
+`XenSwdgn~passthrough_deassign_pci_device_without_iommu~1`
+
+Description:
+Xen shall deassign a specified PCI device from a domain during its destruction.
+The physical device to be deassigned is not protected by the IOMMU.
+
+Rationale:
+The IOMMU is absent from the system or it is disabled (status = "disabled"
+in the host device tree).
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Forbid the same platform device assignment to multiple domains
+--------------------------------------------------------------
+
+`XenSwdgn~passthrough_forbid_same_platform_device_assignment~1`
+
+Description:
+Xen shall not assign the same platform device to multiple domains by failing
+to create a new domain if the device to be passed through is already assigned
+to the existing domain. Also, different platform devices which share some
+resources (interrupts, IOMMU connections) can be assigned only to the same
+domain.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~device_passthrough~1`
+
+Notes
+=====
+
+The AMD64 PVH-specific requirements are written under the assumption that once
+the Hyperlaunch feature is completed, Xen shall assign a PCI device to boot
+time domains. This is not the case today, where the PCI device can be passed
+through only to domains launched by a control (toolstack) domain.
+
+The Arm64-specific requirements are written under the assumption that once
+the dom0less PCI Passthrough feature is completed, Xen shall assign a PCI device
+to boot time domains. This is not the case today, where only the platform device
+Passthrough is supported.
+
+[1] https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/passthrough.txt;hb=HEAD
+[2] https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=docs/misc/arm/passthrough-noiommu.txt;hb=HEAD
diff --git a/docs/fusa/reqs/index.rst b/docs/fusa/reqs/index.rst
index 183f183b1f..19c2f26b2b 100644
--- a/docs/fusa/reqs/index.rst
+++ b/docs/fusa/reqs/index.rst
@@ -10,3 +10,4 @@  Requirements documentation
    market-reqs
    product-reqs
    design-reqs/arm64
+   design-reqs/common
diff --git a/docs/fusa/reqs/market-reqs/reqs.rst b/docs/fusa/reqs/market-reqs/reqs.rst
index f456788d96..37a443395b 100644
--- a/docs/fusa/reqs/market-reqs/reqs.rst
+++ b/docs/fusa/reqs/market-reqs/reqs.rst
@@ -47,3 +47,36 @@  Comments:
 
 Needs:
  - XenProd
+
+Run AMD-x86 domains
+-------------------
+
+`XenMkt~run_x86_domains~1`
+
+Description:
+Xen shall run AMD-x86 domains.
+
+Rationale:
+
+Comments:
+
+Needs:
+ - XenProd
+
+Domain device assignment
+------------------------
+
+`XenMkt~domain_device_assignment~1`
+
+Description:
+Xen shall assign device to each domain.
+
+For example, it shall assign GPU to domain A, MMC to domain B. Only the domain
+assigned to a device, shall have exclusive access to the device.
+
+Rationale:
+
+Comments:
+
+Needs:
+ - XenProd
diff --git a/docs/fusa/reqs/product-reqs/common/reqs.rst b/docs/fusa/reqs/product-reqs/common/reqs.rst
new file mode 100644
index 0000000000..9304399e4d
--- /dev/null
+++ b/docs/fusa/reqs/product-reqs/common/reqs.rst
@@ -0,0 +1,29 @@ 
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Domain Creation And Runtime
+===========================
+
+Device Passthrough
+------------------
+
+`XenProd~device_passthrough~1`
+
+Description:
+Xen shall provide mechanism for assigning a physical device to the domains.
+
+For example:
+
+- PCI passthrough
+- MMC passthrough
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenMkt~run_arm64_domains~1`
+ - `XenMkt~run_x86_domains~1`
+ - `XenMkt~domain_device_assignment~1`
+
+Needs:
+ - XenSwdgn