Re: [Qemu-devel] [PATCH] pci: Fix pci_device_iommu_address_space() bus propagation

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Aug 17, 2015 at 07:39:14PM +1000, Benjamin Herrenschmidt wrote:
> 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.

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

> > 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 <benh@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;
> >  }
> >  
> >