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
