Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 0/2] intel_iommu: Add support for translation for devices behind bridges

[Alex Williamson]

On Wed, 2015-09-02 at 15:10 +0200, Knut Omang wrote:
> On Wed, 2015-09-02 at 15:30 +0300, Marcel Apfelbaum wrote:
> > On 09/01/2015 09:48 PM, Knut Omang wrote:
> > > This patch set changes the data structure used to handle address 
> > > spaces within
> > > the emulated Intel iommu to support traversal also if bus numbers 
> > > are dynamically
> > > allocated, as is the case for devices that sit behind root ports or 
> > > downstream switches.
> > > This means that we cannot use bus number as index, instead a QLIST 
> > > is used.
> > > 
> > > This requires a change in the API for setup of IOMMUs which is 
> > > taken care of by
> > > the first patch. The second patch implements the fix.
> > > 
> > > The initial patch set had some discussion related to whether this 
> > > fix, applied to the
> > > bridge code, was applicable to all bridges. No clear conclusion 
> > > arised as far as I understood,
> > > in the meantime a number of people have run into the same issue as 
> > > I did which lead me
> > > to implement this, so I gather it might be a useful intermediate 
> > > solution that works until
> > > a better approach can be found? I believe the IOMMU emulation code 
> > > has limited usefulness
> > > if it only supports devices sitting directly on the root complex.
> > Hi,
> > 
> > Thank you for (re)sending the patches!
> > 
> > While I believe you are perfectly right and IOMMU
> > would benefit from this addition, I saw that are some reviews
> > in the  prev thread that are not purely theoretical/philosophical.
> > E.g. PCIDevice *dev field in VTDAddressSpace and maybe a few more.
> 
> Yes, in principle I agree, but the problem is that there is no other
> good reference that would uniquely identify the device as the requester
> IDs (devfn) cannot be used since it changes during enumeration, 
> as argued better for here:
> 
> http://article.gmane.org/gmane.comp.emulators.qemu/317201

There are very specific rules for translating requester IDs across
bridges.  Bus numbers can change during enumeration, devfn cannot.
devfn can however be masked by topology changes from PCIe to PCI.  If we
pretend that the IOMMU can distinguish requester IDs where it can't on
real hardware, we're going to break the guest.  Thanks,

Alex

> > I would suggest to address them so it will be easier to continue the 
> > review process.
> 
> Knut
> 
> > Thank you,
> > Marcel
> > 
> > > 
> > > This is the thread following the initial patch set:
> > > 
> > >    http://thread.gmane.org/gmane.comp.emulators.qemu/302246
> > > 
> > > The patch set was also discussed in this thread:
> > > 
> > >    http://thread.gmane.org/gmane.comp.emulators.qemu/316949
> > > 
> > > Changes from v1:
> > >    - Rebased to current master
> > >    - Fixed minor syntax issues
> > > 
> > > Knut Omang (2):
> > >    iommu: Replace bus+devfn arguments with PCIDevice* in 
> > > PCIIOMMUFunc
> > >    intel_iommu: Add support for translation for devices behind 
> > > bridges.
> > > 
> > >   hw/alpha/typhoon.c            |  2 +-
> > >   hw/i386/intel_iommu.c         | 56 +++++++++++++++++++-----------
> > > -------------
> > >   hw/pci-host/apb.c             |  2 +-
> > >   hw/pci-host/prep.c            |  3 +--
> > >   hw/pci-host/q35.c             | 42 +++++++++++++-----------------
> > > --
> > >   hw/pci/pci.c                  |  7 +++---
> > >   hw/pci/pci_bridge.c           |  6 +++++
> > >   hw/ppc/spapr_pci.c            |  2 +-
> > >   include/hw/i386/intel_iommu.h |  6 +++--
> > >   include/hw/pci/pci.h          |  5 +++-
> > >   10 files changed, 62 insertions(+), 69 deletions(-)
> > > 
> > > --
> > > 2.4.3
> > > 
> >