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

[Oleksandr Tyshchenko <olekstysh@gmail.com>]

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