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

[Knut Omang]

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

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