Re: [Qemu-devel] [PATCH v5 1/3] memory: introduce IOMMUNotifier and its caps

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

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

On Tue, Sep 20, 2016 at 03:16:33PM +0800, Peter Xu wrote:
> On Tue, Sep 20, 2016 at 04:12:05PM +1000, David Gibson wrote:
> > On Wed, Sep 14, 2016 at 04:25:46PM +0800, Peter Xu wrote:
> > > IOMMU Notifier list is used for notifying IO address mapping changes.
> > > Currently VFIO is the only user.
> > > 
> > > However it is possible that future consumer like vhost would like to
> > > only listen to part of its notifications (e.g., cache invalidations).
> > > 
> > > This patch introduced IOMMUNotifier and IOMMUNotfierFlag bits for a
> > > finer grained control of it.
> > > 
> > > IOMMUNotifier contains a bitfield for the notify consumer describing
> > > what kind of notification it is interested in. Currently two kinds of
> > > notifications are defined:
> > > 
> > > - IOMMU_NOTIFIER_MAP:    for newly mapped entries (additions)
> > > - IOMMU_NOTIFIER_UNMAP:  for entries to be removed (cache invalidates)
> > > 
> > > When registering the IOMMU notifier, we need to specify one or multiple
> > > types of messages to listen to.
> > > 
> > > When notifications are triggered, its type will be checked against the
> > > notifier's type bits, and only notifiers with registered bits will be
> > > notified.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > 
> > I still don't see the big fat comment saying that in-place changes to
> > an IOMMU mapping aren't permitted.
> 
> IMHO if we are using MAP and UNMAP here then it's fairly clear even we
> will support in-place change in the future. I can add one more
> paragraph for in-place change like:
> 
>   For any IOMMU implementation, an in-place mapping change should be
>   notified with an UNMAP following a MAP.
> 
> Do you think this works?

I guess it will do.

> 
> [...]
> 
> > > +struct IOMMUNotifier {
> > > +    void (*notify)(struct IOMMUNotifier *notifier, void *data);
> > 
> > Given that we now have a special notifier type for this purpose, we
> > could actually type this to take an IOMMUTLBEntry instead of a void *.
> 
> Yep. I can fix that.

Great.

-- 
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: 819 bytes --]