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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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