Re: [Qemu-devel] [PATCH v3 2/3] memory: generalize iommu_ops.notify_started to notifier_add

[David Gibson]

On Thu, Sep 08, 2016 at 06:22:40PM +0800, Peter Xu wrote:
> On Wed, Sep 07, 2016 at 12:54:23PM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 07/09/2016 08:05, David Gibson wrote:
> > > On Wed, Sep 07, 2016 at 01:32:23PM +0800, Peter Xu wrote:
> > >> Considering that we may have multiple IOMMU notifier consumers in the
> > >> future, converting iommu_ops.notify_{started|stopped} into some more
> > >> general form. Now we can trap all notifier registerations and
> > >> deregistrations, rather than only the first ones.
> > >>
> > >> Power was leveraging the notifier_{started|stopped}, adding iommu_user
> > >> field for counting on Power guests to achieve the same goal.
> > > 
> > > Requiring each vIOMMU frontend to reference count or whatever seems
> > > like a pain.  The semantics of notify_started() were designed to avoid
> > > that.
> > > 
> > > Instead I'd suggest a callback which gets tripped any time the logical
> > > OR of the requested notifications for all current notifiers changes.
> > 
> > I like the suggestion.  Alternatively you could call notify_stopped if
> > old & ~new is nonzero (and you pass old & ~new), and notify_started if
> > new & ~old is nonzero (and you pass new & ~old).
> 
> I think now I understand the point... Then I'd prefer to use David's
> suggestion. A single notify_changed() looks cleaner. To be more
> explicit, I would prefer to rename it to notifier_flag_changed(),
> since notify_changed() looks like to be called every time notifier
> list changed, but actually it is for monitoring the flags.

That sounds reasonable.  I think notifier_flag_changed() should be
passed both the old and new flags, to save the backend having to keep
track of the old ones - which flags have changed might affect what the
callback needs to do.

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

signature.asc
Description: PGP signature