pci: Fix pci_device_iommu_address_space() bus propagation

he current code walks up the bus tree for an iommu, however it passes to the iommu_fn() callback the bus/devfn of the immediate child of the level where the callback was found, rather than the original bus/devfn where the search started from.

This prevents iommu's like POWER8 (and in fact also Q35) to properly provide an address space for a subset of devices that aren't immediate
children of the iommu.

PCIe carries the originator bdfn acccross to the iommu on all DMA transactions, so we must be able to properly identify devices at all levels.

This changes the function pci_device_iommu_address_space() to pass the original pointers to the iommu_fn() callback instead.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

---

With this, I can implement PHB3's (POWER8) iommu properly, I haven't submitted the P8 native patch series yet but if you are curious, you can look there:

https://github.com/ozbenh/qemu

And more specifically:

https://github.com/ozbenh/qemu/commit/67fe0460c75417908a2b54426cb54fe
5a1299a13

For the PHB3 code.

 hw/pci/pci.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

hw/pci/pci.c +3 -7

index 29f0b0f..8185bbc 100644
@@ -2403,17 +2403,14 @@ static void pci_device_class_init(ObjectClass 
 *klass, void *data)
 AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
 {
     PCIBus *bus = PCI_BUS(dev->bus);
+    PCIBus *iommu_bus = bus;
 
-    if (bus->iommu_fn) {
-        return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn);
+    while(iommu_bus && !iommu_bus->iommu_fn && iommu_bus
->parent_dev) {
+        iommu_bus = PCI_BUS(iommu_bus->parent_dev->bus);
     }
-
-    if (bus->parent_dev) {
-        /** We are ignoring the bus master DMA bit of the bridge
-         *  as it would complicate things such as VFIO for no good 

reason */

hw/pci/pci.c +2 -3

-        return pci_device_iommu_address_space(bus->parent_dev);
+    if (iommu_bus && iommu_bus->iommu_fn) {
+        return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, dev
->devfn);
     }
-
     return &address_space_memory;
 }

Quoted 4 lines · Benjamin Herrenschmidt

he current code walks up the bus tree for an iommu, however it passes to the iommu_fn() callback the bus/devfn of the immediate child of the level where the callback was found, rather than the original bus/devfn where the search started from.

Hi Michael ! Any comment on this ? I'd like to post my series for "bare metal" power8 support and the iommu implementation relies on this to work.

Quoted 63 lines

This prevents iommu's like POWER8 (and in fact also Q35) to properly provide an address space for a subset of devices that aren't immediate
children of the iommu.

PCIe carries the originator bdfn acccross to the iommu on all DMA transactions, so we must be able to properly identify devices at all levels.

This changes the function pci_device_iommu_address_space() to pass the original pointers to the iommu_fn() callback instead.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

---

With this, I can implement PHB3's (POWER8) iommu properly, I haven't submitted the P8 native patch series yet but if you are curious, you can look there:

https://github.com/ozbenh/qemu

And more specifically:

https://github.com/ozbenh/qemu/commit/67fe0460c75417908a2b54426cb54fe
5a1299a13

For the PHB3 code.

 hw/pci/pci.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

hw/pci/pci.c +3 -7

index 29f0b0f..8185bbc 100644
@@ -2403,17 +2403,14 @@ static void pci_device_class_init(ObjectClass 
 *klass, void *data)
 AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
 {
     PCIBus *bus = PCI_BUS(dev->bus);
+    PCIBus *iommu_bus = bus;
 
-    if (bus->iommu_fn) {
-        return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn);
+    while(iommu_bus && !iommu_bus->iommu_fn && iommu_bus
->parent_dev) {
+        iommu_bus = PCI_BUS(iommu_bus->parent_dev->bus);
     }
-
-    if (bus->parent_dev) {
-        /** We are ignoring the bus master DMA bit of the bridge
-         *  as it would complicate things such as VFIO for no good 

reason */

hw/pci/pci.c +2 -3

-        return pci_device_iommu_address_space(bus->parent_dev);
+    if (iommu_bus && iommu_bus->iommu_fn) {
+        return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, dev
->devfn);
     }
-
     return &address_space_memory;
 }

Quoted 8 lines · Benjamin Herrenschmidt

Quoted 6 lines · Benjamin Herrenschmidt

he current code walks up the bus tree for an iommu, however it passes
to the iommu_fn() callback the bus/devfn of the immediate child of
the level where the callback was found, rather than the original bus/devfn where the search started from.

Quoted 5 lines

Hi Michael ! Any comment on this ? I'd like to post my series for "bare
metal" power8 support and the iommu implementation relies on this to
work.

I've picked this up for my next pull request.
Thanks, and sorry about the delay.