On Sun, 2015-07-05 at 09:19 +1000, Benjamin Herrenschmidt wrote: > 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. > 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 <b...@kernel.crashing.org> > --- > > 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(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 29f0b0f..8185bbc 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -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 */ > - 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; > } > >
