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

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

On Sun, Oct 26, 2014 at 02:15:24PM +0100, Knut Omang wrote:
> On Sun, 2014-10-26 at 13:06 +0100, Jan Kiszka wrote:
> > On 2014-10-21 00:34, Knut Omang wrote:
> > > @@ -65,11 +66,12 @@ struct VTDContextCacheEntry {
> > >  };
> > >  
> > >  struct VTDAddressSpace {
> > > -    uint8_t bus_num;
> > > +    PCIDevice *dev;
> > 
> > This change is not helpful for clean handling of non-PCI devices (i.e.
> > platform device interrupt remapping => you had to pull
> > Q35_PSEUDO_BUS_PLATFORM into intel_iommu, which is violating the
> > layering). Please leave bus_num in place - or convert to a 16-bit SID.
> 
> Hmm - I see..
> - the problem I tried to solve is that the bus number of devices below a
> root port or downstream switch has not been initialized when
> q35_host_dma_iommu is called, so what happens is that the device in the
> root port gets indexed as if it were on bus 0.
> I am not that familiar with what type of non-pci devices that exists but
> I suppose moving this up to the most generic device type that has a bus
> associated with it is one way to go.
> 
> An alternative implementation that would work in the intel case would be
> to keep the list in intel_iommu but provide a callback that iommus can
> subscribe to to get notifications when bus numbers change?
> 
> Knut

I dislike callbacks.

IMO the right thing as usual is to do what real hardware does.

After all it's devices put the requester id in transactions.

How about we add "source id" in AddressSpace structure,
or add a wrapper structure including AddressSpace and
source id.
Update in pci core on config writes into bus number.

Paolo, would that be ok with you?


> > >      uint8_t devfn;
> > >      AddressSpace as;
> > >      MemoryRegion iommu;
> > >      IntelIOMMUState *iommu_state;
> > > +    QLIST_ENTRY(VTDAddressSpace) iommu_next; /* For traversal by the iommu */
> > >      VTDContextCacheEntry context_cache_entry;
> > >  };
> > >  
> > 
> > Jan
> > 
> > 
>