Re: [Qemu-devel] [PATCH 0/3] memory: add IOMMU notifier type

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3504 bytes --]

On Tue, Sep 06, 2016 at 02:26:44PM +0800, Jason Wang wrote:
> 
> 
> On 2016年09月06日 13:49, Peter Xu wrote:
> > On Tue, Sep 06, 2016 at 03:06:17PM +1000, David Gibson wrote:
> > > On Mon, Sep 05, 2016 at 03:21:18PM +0800, Peter Xu wrote:
> > > > In the thread:
> > > > 
> > > >    https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00254.html
> > > > 
> > > > Alex proposed a way for vhost DMAR to be enabled without breaking
> > > > existing protections on vIOMMU and device assignments. This series
> > > > tried to implement the idea, by introducing a IOMMU notifier type for
> > > > each IOMMU memory region.
> > > Hrm, I'm pretty dubious about this concept, since it's basically just
> > > an interim hack for an incomplete notifier implementation on x86.
> > > What makes just fixing the notifier so difficult?
> > Aviv is working on the full notifier support for that. It's been
> > months since his last post though. If he cannot continue it (due to
> > any reason), I can take it over. But for now, we may still need to
> > wait for his patches to fully enable a complete notifier mechanism.
> 
> Yes, and the issue is:
> 
> - There's no way for current intel IOMMU code to be notified when guest map
> a page. So it's impossible for intel IOMMU to co-work with vfio now.
> - A solution is caching mode (CM) support which requires a TLB invalidation
> even if it was a non-present to present changing, but this is still under
> development.

Right.  AIUI the whole point of CM is to allow this sort of
virtualization.

What I hadn't reaalized before was that even with CM==0, invalidations
were still notified, otherwise I didn't see how anything was possible.

> - VT-d spec requires: "Hardware implementations of this architecture must
> support operation corresponding to CM=0." So even if we have CM mode which
> can work with vfio, we still must support intel IOMMU with CM disabled. It
> was not only a spec requirement but also a performance consideration. (CM is
> usually slower)

Well.. this isn't a hardware implementation, so I don't think it's
really a spec requirement, although I can see why you'd want it so
that you can do something as near as possible indistinguishable from
hardware.

> So in conclusion, there's a mode for intel IOMMU that can't work with vfio
> at all. Fixing the notifier does not solve this since the notifier won't be
> able to be triggered.

Ok, I understand.  CM==1 will be able to have the full notifier, but
CM==0 won't and will never work with VFIO, so we need a way to detect
that cleanly.

> 
> > 
> > I don't know how POWER works to provide a complete notifier, and
> > whether POWER can selectively enable the notified items... But for
> > Intel VT-d, it provided two choices: by default, only cache
> > invalidations are notified (even, we can disalbe cache invaliations),
> > but if one want to have a complete notifier, just set the CM bit to 1.
> > So I just think it'll be cool if we can support both cases. E.g., for
> > vhost, it does not need to be notified with newly added entries, but
> > only cache invalidations. IMHO we can't just force vhost to use a
> > complete notifier while actually it only needs part of it.
> > 
> > Thanks,
> > 
> > -- peterx
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]