Re: [Qemu-devel] [PATCH for 2.8 10/11] Revert "intel_iommu: Throw hw_error on notify_started"

[David Gibson]

On Thu, 1 Sep 2016 11:58:48 +0800
Peter Xu <address@hidden> wrote:

> On Wed, Aug 31, 2016 at 08:43:42PM -0600, Alex Williamson wrote:
> > > > >>This reverts commit 3cb3b1549f5401dc3a5e1d073e34063dc274136f. Vhost
> > > > >>device IOTLB API will get notified and send invalidation request to
> > > > >>vhost through this notifier.    
> > > > >AFAICT this series does not address the original problem for which
> > > > >commit 3cb3b1549f54 was added.  We've only addressed the very narrow
> > > > >use case of a device iotlb firing the iommu notifier therefore this
> > > > >change is a regression versus 2.7 since it allows invalid
> > > > >configurations with a physical iommu which will never receive the
> > > > >necessary notifies from intel-iommu emulation to work properly.  
> > > > >Thanks,
> > > > >
> > > > >Alex    
> > > > 
> > > > Looking at vfio, it cares about map but vhost only cares about IOTLB
> > > > invalidation. Then I think we probably need another kind of notifier in 
> > > > this
> > > > case to avoid this.    
> > > 
> > > Shall we leverage IOMMUTLBEntry.perm == IOMMU_NONE as a sign for
> > > invalidation? If so, we can use the same IOTLB interface as before.
> > > IMHO these two interfaces are not conflicting?
> > > 
> > > Alex,
> > > 
> > > Do you mean we should still disallow user from passing through devices
> > > while Intel IOMMU enabled? If so, not sure whether patch below can
> > > solve the issue.
> > > 
> > > It seems that we need a "name" for either IOMMU notifier
> > > provider/consumer, and we should not allow (provider==Intel &&
> > > consumer==VFIO) happen. In the following case, I added a name for
> > > provider, and VFIO checks it.  
> > 
> > Absolutely not, intel-iommu emulation is simply incomplete, the IOMMU
> > notifier is never called for mappings.  There's a whole aspect of
> > iommu notifiers that intel-iommu simply hasn't bothered to implement.
> > Don't punish vfio for actually making use of the interface as it was
> > intended to be used.  AFAICT you're implementing the unmap/invalidation
> > half, without the actual mapping half of the interface.  It's broken
> > and incompatible with any iommu notifiers that expect to see both
> > sides.  Thanks,  
> 
> Yeah I think I got your point. Thanks for the explanation.
> 
> Now I agree with Jason that we may need another notifier mechanism.

What!?  I see no reason you need a different notifier, just fix the
implementation of the current one.  As a bonus this will also give you
working VFIO passthrough with vIOMMU on x86, something which should
work already, but doesn't.


-- 
David Gibson <address@hidden>
Senior Software Engineer, Virtualization, Red Hat

pgpyioYD3d5gL.pgp
Description: OpenPGP digital signature