| Message ID | 20230607030220.22698-1-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | SMMU handling for PCIe Passthrough on ARM | expand |
Hi Stewart, On 07/06/2023 04:02, Stewart Hildebrand wrote: > This series introduces SMMU handling for PCIe passthrough on ARM. These patches > are independent from (and don't depend on) the vPCI reference counting/locking > work in progress, and should be able to be upstreamed independently. Can you clarify how this code was tested? Does this require code not yet upstreamed? Cheers,
On 6/7/23 03:19, Julien Grall wrote: > Hi Stewart, > > On 07/06/2023 04:02, Stewart Hildebrand wrote: >> This series introduces SMMU handling for PCIe passthrough on ARM. These patches >> are independent from (and don't depend on) the vPCI reference counting/locking >> work in progress, and should be able to be upstreamed independently. > > Can you clarify how this code was tested? Does this require code not yet > upstreamed? I'm testing the series standalone (+ config changes) by using a PCI device in dom0, and also in combination with the vPCI series [3] [4] for passthrough to a domU. Here are some more details on the test cases I'm using. 1. Using the PCI device in dom0 with the pci-passthrough=yes arg. In this case a couple of additional config changes [1] [2] are needed to select CONFIG_HAS_PCI=y, CONFIG_HAS_VPCI=y, and make has_vpci() return true. Aside from this series itself and the config changes, nothing else not-yet-upstreamed is required for this test case. It is on my TODO list to upstream these config changes, which I think will be useful on their own, not necessarily as part of any other series. Actually, your question prompted me to look at my test cases a bit more carefully, and I discovered a case that v4 of this series doesn't handle right. In order for the PCI device to be usable in dom0, it should be assigned by default to dom0/hardware domain during PHYSDEVOP_pci_device_add. But v4 of this series doesn't assign it by default to dom0/hardware domain. I initially missed this because I happened to assign the stream ID of the PCI device to dom0 by the iommus property. In other words, I initially tested with this: &pcie { iommus = <&smmu 0x4d0>; iommu-map = <0x0 &smmu 0x4d0 0x10000>; iommu-map-mask = <0x0>; }; But I should be testing with this (i.e. omitting the iommus property): &pcie { iommu-map = <0x0 &smmu 0x4d0 0x10000>; iommu-map-mask = <0x0>; }; Omitting iommus currently breaks using a PCI device in dom0 in v4. I'll fix this in v5. 2. To test the phantom bits, since I don't have a phantom device readily available, I use the pci-phantom arg and a carefully constructed iommu-map property. The PCI device's stream ID is actually 0x4d0, but I put 0x4ce in the iommu-map to make it appear as if it's one of the phantom functions generating the DMA traffic. pci-phantom=01:00,1 &pcie { /* phantom test */ iommu-map = <0x0 &smmu 0x4ce 0x10000>; iommu-map-mask = <0x7>; }; 3. Passthrough to a domU. In this case the vPCI series is needed [3], and an MSI series not yet submitted to the list [4] (or another way to hack/assign the IRQ to the domU), in addition to the 2 config changes [1] [2]. The procedure is described at [5]. [1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/9a08f1f7ce28ec619640ba9ce11018bf443e9a0e [2] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382 [3] https://lists.xenproject.org/archives/html/xen-devel/2023-06/msg00863.html [4] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commits/poc/pci-passthrough [5] https://lists.xenproject.org/archives/html/xen-devel/2022-12/msg00682.html
Hi, On 15/06/2023 22:05, Stewart Hildebrand wrote: > On 6/7/23 03:19, Julien Grall wrote: >> On 07/06/2023 04:02, Stewart Hildebrand wrote: >>> This series introduces SMMU handling for PCIe passthrough on ARM. These patches >>> are independent from (and don't depend on) the vPCI reference counting/locking >>> work in progress, and should be able to be upstreamed independently. >> >> Can you clarify how this code was tested? Does this require code not yet >> upstreamed? > > I'm testing the series standalone (+ config changes) by using a PCI device in dom0, and also in combination with the vPCI series [3] [4] for passthrough to a domU. > > > Here are some more details on the test cases I'm using. Thanks that's helpful! One comment about the first test case. > > > 1. Using the PCI device in dom0 with the pci-passthrough=yes arg. In this case a couple of additional config changes [1] [2] are needed to select CONFIG_HAS_PCI=y, CONFIG_HAS_VPCI=y, and make has_vpci() return true. Aside from this series itself and the config changes, nothing else not-yet-upstreamed is required for this test case. It is on my TODO list to upstream these config changes, which I think will be useful on their own, not necessarily as part of any other series. I find a bit confusing that the IOMMU support for dom0 is gated behind 'pci-passthrough'. I was expecting that the iommu would also be properly configured for PCI if we using 'iommu=yes'. Cheers,
Hi Julien, > On 25 Jun 2023, at 1:56 pm, Julien Grall <julien@xen.org> wrote: > > Hi, > > On 15/06/2023 22:05, Stewart Hildebrand wrote: >> On 6/7/23 03:19, Julien Grall wrote: >>> On 07/06/2023 04:02, Stewart Hildebrand wrote: >>>> This series introduces SMMU handling for PCIe passthrough on ARM. These patches >>>> are independent from (and don't depend on) the vPCI reference counting/locking >>>> work in progress, and should be able to be upstreamed independently. >>> >>> Can you clarify how this code was tested? Does this require code not yet >>> upstreamed? >> I'm testing the series standalone (+ config changes) by using a PCI device in dom0, and also in combination with the vPCI series [3] [4] for passthrough to a domU. >> Here are some more details on the test cases I'm using. > > Thanks that's helpful! One comment about the first test case. > >> 1. Using the PCI device in dom0 with the pci-passthrough=yes arg. In this case a couple of additional config changes [1] [2] are needed to select CONFIG_HAS_PCI=y, CONFIG_HAS_VPCI=y, and make has_vpci() return true. Aside from this series itself and the config changes, nothing else not-yet-upstreamed is required for this test case. It is on my TODO list to upstream these config changes, which I think will be useful on their own, not necessarily as part of any other series. > > I find a bit confusing that the IOMMU support for dom0 is gated behind 'pci-passthrough'. I was expecting that the iommu would also be properly configured for PCI if we using 'iommu=yes'. As per my understanding Xen can configure the iommus for PCI device without "pci-passthrough” enabled if we follow below path: 1) PCI host bridge is already enumerated and powered on in firmware before Xen boot 2) PCI devices are scanned in Xen. (https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/bce463e1588a45e1bfdf59fc0d5f88b16604e439) 3) After scanning the PCI devices add PCI devices to iommu ( iommu_add_device() ) If PCI host bridge is not enumerated then we need "pci-passthrough” enabled to allow Linux to do enumeration and to inform Xen via PHYSDEVOP_pci_device_add hyper call to add PCI devices in Xen This is implemented as part of PCI passthrough feature. Regards, Rahul
Hi Rahul, On 25/06/2023 15:28, Rahul Singh wrote: >> On 25 Jun 2023, at 1:56 pm, Julien Grall <julien@xen.org> wrote: >> On 15/06/2023 22:05, Stewart Hildebrand wrote: >>> On 6/7/23 03:19, Julien Grall wrote: >>>> On 07/06/2023 04:02, Stewart Hildebrand wrote: >>>>> This series introduces SMMU handling for PCIe passthrough on ARM. These patches >>>>> are independent from (and don't depend on) the vPCI reference counting/locking >>>>> work in progress, and should be able to be upstreamed independently. >>>> >>>> Can you clarify how this code was tested? Does this require code not yet >>>> upstreamed? >>> I'm testing the series standalone (+ config changes) by using a PCI device in dom0, and also in combination with the vPCI series [3] [4] for passthrough to a domU. >>> Here are some more details on the test cases I'm using. >> >> Thanks that's helpful! One comment about the first test case. >> >>> 1. Using the PCI device in dom0 with the pci-passthrough=yes arg. In this case a couple of additional config changes [1] [2] are needed to select CONFIG_HAS_PCI=y, CONFIG_HAS_VPCI=y, and make has_vpci() return true. Aside from this series itself and the config changes, nothing else not-yet-upstreamed is required for this test case. It is on my TODO list to upstream these config changes, which I think will be useful on their own, not necessarily as part of any other series. >> >> I find a bit confusing that the IOMMU support for dom0 is gated behind 'pci-passthrough'. I was expecting that the iommu would also be properly configured for PCI if we using 'iommu=yes'. > > As per my understanding Xen can configure the iommus for PCI device without "pci-passthrough” enabled > if we follow below path: > > 1) PCI host bridge is already enumerated and powered on in firmware before Xen boot > 2) PCI devices are scanned in Xen. > (https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/bce463e1588a45e1bfdf59fc0d5f88b16604e439) > 3) After scanning the PCI devices add PCI devices to iommu ( iommu_add_device() ) > > If PCI host bridge is not enumerated then we need "pci-passthrough” enabled to allow Linux to do > enumeration and to inform Xen via PHYSDEVOP_pci_device_add hyper call to add PCI devices in Xen > This is implemented as part of PCI passthrough feature. Right, but selecting "pci-passthrough" to be able to use the IOMMU in dom0 is confusing. We already support PCI in dom0 and adding the support (IOMMU + PCI) has no relation with assigning a device to the guest. IOW one may want to use IOMMU in dom0 without assigning devices to the guest. I think part of the code gated by "pci-passthrough" should also be available when the IOMMU is enabled. This would allow users to use IOMMU + PCI in dom0 without any extra patches and/or command line option. Cheers,
This series introduces SMMU handling for PCIe passthrough on ARM. These patches are independent from (and don't depend on) the vPCI reference counting/locking work in progress, and should be able to be upstreamed independently. v3->v4: * split a change from ("xen/arm: Move is_protected flag to struct device") into a new separate patch * see individual patches for further details v2->v3: * drop "pci/arm: Use iommu_add_dt_pci_device()" * drop "RFC: pci/arm: don't do iommu call for phantom functions" * move invocation of sideband ID mapping function to add_device() platform_ops/iommu_ops hook v1->v2: * phantom device handling * shuffle around iommu_add_dt_pci_device() Oleksandr Andrushchenko (1): xen/arm: smmuv2: Add PCI devices support for SMMUv2 Oleksandr Tyshchenko (4): xen/arm: Improve readability of check for registered devices xen/arm: Move is_protected flag to struct device iommu/arm: Add iommu_dt_xlate() iommu/arm: Introduce iommu_add_dt_pci_sideband_ids API Rahul Singh (1): xen/arm: smmuv3: Add PCI devices support for SMMUv3 Stewart Hildebrand (1): iommu/arm: iommu_add_dt_pci_sideband_ids phantom handling xen/arch/arm/domain_build.c | 4 +- xen/arch/arm/include/asm/device.h | 14 ++ xen/common/device_tree.c | 2 +- xen/drivers/passthrough/arm/ipmmu-vmsa.c | 4 +- xen/drivers/passthrough/arm/smmu-v3.c | 81 ++++++++- xen/drivers/passthrough/arm/smmu.c | 122 +++++++++++--- xen/drivers/passthrough/device_tree.c | 205 ++++++++++++++++++++--- xen/include/xen/device_tree.h | 38 +++-- xen/include/xen/iommu.h | 22 ++- 9 files changed, 423 insertions(+), 69 deletions(-)