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

-- 
2.40.1

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,

-- 
Julien Grall

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,

-- 
Julien Grall

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,

-- 
Julien Grall