Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT)

[Peter Xu]

On Thu, Apr 20, 2017 at 04:55:24AM +0000, Liu, Yi L wrote:

[...]

> > > In my previous RFC patchset of fault event reporting, I registered
> > > fault notifier when there is a VFIO group attached to VFIO container
> > > and used the address space to check whether vIOMMU is added. The
> > > address space is returned by vtd_host_dma_iommu(). vtd_find_add_as()
> > > initializes device's IOMMU memory region and put it into device's
> > > address space as root memory region.
> > > Does this make sense?
> > >
> > > @@ -1103,6 +1132,14 @@ static int vfio_connect_container(VFIOGroup
> > > *group, AddressSpace *as,
> > >          goto listener_release_exit;
> > >      }
> > >
> > > +    if (memory_region_is_iommu(container->space->as->root)) {
> > 
> > I would suggest we don't play with as->root any more. After vtd vfio 
> > series, this may
> > not be true if passthrough mode is enabled (then the root may be switched 
> > to an
> > system memory alias). I don't know the best way to check this, one 
> > alternative might
> > be that we check whether
> > container->space->as == system_memory(), it should be workable, but in

Sorry, I was meaning &address_space_memory.

> > a slightly hackish way.
> 
> In my understanding, container->space->as->root cannot work here no matter 
> passthru-mode
> is enabled or not. The code here is aiming to check if vIOMMU exists. After 
> the vfio series,
> the vtd_dev_as->root is not initialized to be a iommu MemoryRegion. Compared 
> with checking
> if it is system_memory(), I think adding a mechanism to get the iommu 
> MemoryRegion may
> be a better choice. Just like the current pci_device_iommu_address_space().

Isn't pci_device_iommu_address_space() used to get that IOMMU memory
region? And, one thing to mention is that container->space->as is
actually derived from pci_device_iommu_address_space() (when calling
vfio_get_group()).

I feel like that playing around with an IOMMU memory region is still
not clear enough in many cases. I still feel like some day we would
like an "IOMMU object". Then, we can register non-iotlb notifiers
against that IOMMU object, rather than memory regions...

Thanks,

-- 
Peter Xu