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

[Jason Wang]

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.
- 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)

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.

> 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