| Message ID | 1474433936-19617-2-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 21, 2016 at 12:58:54PM +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. > > (For any IOMMU implementation, an in-place mapping change should be > notified with an UNMAP followed by a MAP.) Ok, I wasn't clear. I meant a big fat comment in the *code*, not just in the commit message. It should not be necessary to look at the commit history to figure out how to use an interface correctly Even a comment in the code is barely adequate, compared to designing the interface signatures such that it's obvious. Please bear in mind: http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html and http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html Couple of other tiny nits that I wouldn't bother with it weren't for the above. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/vfio/common.c | 3 ++- > include/exec/memory.h | 38 +++++++++++++++++++++++++++++++------- > include/hw/vfio/vfio-common.h | 2 +- > memory.c | 37 ++++++++++++++++++++++++++++--------- > 4 files changed, 62 insertions(+), 18 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index b313e7c..89a993b 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -293,7 +293,7 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section) > section->offset_within_address_space & (1ULL << 63); > } > > -static void vfio_iommu_map_notify(Notifier *n, void *data) > +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *data) This change leaves a now pointless IOMMUTLBEntry *iotlb = data a few lines below. > { > VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); > VFIOContainer *container = giommu->container; > @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener *listener, > section->offset_within_region; > giommu->container = container; > giommu->n.notify = vfio_iommu_map_notify; > + giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL; > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); > > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 3e4d416..a3ec7aa 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -67,6 +67,27 @@ struct IOMMUTLBEntry { > IOMMUAccessFlags perm; > }; > > +/* > + * Bitmap for differnet IOMMUNotifier capabilities. Each notifier can s/differnet/different/ > + * register with one or multiple IOMMU Notifier capability bit(s). > + */ > +typedef enum { > + IOMMU_NOTIFIER_NONE = 0, > + /* Notify cache invalidations */ > + IOMMU_NOTIFIER_UNMAP = 0x1, > + /* Notify entry changes (newly created entries) */ > + IOMMU_NOTIFIER_MAP = 0x2, > +} IOMMUNotifierFlag; > + > +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP) > + > +struct IOMMUNotifier { > + void (*notify)(struct IOMMUNotifier *notifier, IOMMUTLBEntry *data); > + IOMMUNotifierFlag notifier_flags; > + QLIST_ENTRY(IOMMUNotifier) node; > +}; > +typedef struct IOMMUNotifier IOMMUNotifier; > + > /* New-style MMIO accessors can indicate that the transaction failed. > * A zero (MEMTX_OK) response means success; anything else is a failure > * of some kind. The memory subsystem will bitwise-OR together results > @@ -201,7 +222,7 @@ struct MemoryRegion { > const char *name; > unsigned ioeventfd_nb; > MemoryRegionIoeventfd *ioeventfds; > - NotifierList iommu_notify; > + QLIST_HEAD(, IOMMUNotifier) iommu_notify; > }; > > /** > @@ -620,11 +641,12 @@ void memory_region_notify_iommu(MemoryRegion *mr, > * IOMMU translation entries. > * > * @mr: the memory region to observe > - * @n: the notifier to be added; the notifier receives a pointer to an > - * #IOMMUTLBEntry as the opaque value; the pointer ceases to be > - * valid on exit from the notifier. > + * @n: the IOMMUNotifier to be added; the notify callback receives a > + * pointer to an #IOMMUTLBEntry as the opaque value; the pointer > + * ceases to be valid on exit from the notifier. > */ > -void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n); > +void memory_region_register_iommu_notifier(MemoryRegion *mr, > + IOMMUNotifier *n); > > /** > * memory_region_iommu_replay: replay existing IOMMU translations to > @@ -636,7 +658,8 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n); > * @is_write: Whether to treat the replay as a translate "write" > * through the iommu > */ > -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write); > +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, > + bool is_write); > > /** > * memory_region_unregister_iommu_notifier: unregister a notifier for > @@ -646,7 +669,8 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write); > * needs to be called > * @n: the notifier to be removed. > */ > -void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n); > +void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > + IOMMUNotifier *n); > > /** > * memory_region_name: get a memory region's name > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 94dfae3..c17602e 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -93,7 +93,7 @@ typedef struct VFIOGuestIOMMU { > VFIOContainer *container; > MemoryRegion *iommu; > hwaddr iommu_offset; > - Notifier n; > + IOMMUNotifier n; > QLIST_ENTRY(VFIOGuestIOMMU) giommu_next; > } VFIOGuestIOMMU; > > diff --git a/memory.c b/memory.c > index 1a1baf5..69d9d9a 100644 > --- a/memory.c > +++ b/memory.c > @@ -1413,7 +1413,7 @@ void memory_region_init_iommu(MemoryRegion *mr, > memory_region_init(mr, owner, name, size); > mr->iommu_ops = ops, > mr->terminates = true; /* then re-forwards */ > - notifier_list_init(&mr->iommu_notify); > + QLIST_INIT(&mr->iommu_notify); > } > > static void memory_region_finalize(Object *obj) > @@ -1508,13 +1508,16 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client) > return memory_region_get_dirty_log_mask(mr) & (1 << client); > } > > -void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n) > +void memory_region_register_iommu_notifier(MemoryRegion *mr, > + IOMMUNotifier *n) > { > + /* We need to register for at least one bitfield */ > + assert(n->notifier_flags != IOMMU_NOTIFIER_NONE); > if (mr->iommu_ops->notify_started && > - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { > + QLIST_EMPTY(&mr->iommu_notify)) { > mr->iommu_ops->notify_started(mr); > } > - notifier_list_add(&mr->iommu_notify, n); > + QLIST_INSERT_HEAD(&mr->iommu_notify, n, node); > } > > uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) > @@ -1526,7 +1529,8 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) > return TARGET_PAGE_SIZE; > } > > -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) > +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, > + bool is_write) > { > hwaddr addr, granularity; > IOMMUTLBEntry iotlb; > @@ -1547,11 +1551,12 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) > } > } > > -void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) > +void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > + IOMMUNotifier *n) > { > - notifier_remove(n); > + QLIST_REMOVE(n, node); > if (mr->iommu_ops->notify_stopped && > - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { > + QLIST_EMPTY(&mr->iommu_notify)) { > mr->iommu_ops->notify_stopped(mr); > } > } > @@ -1559,8 +1564,22 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) > void memory_region_notify_iommu(MemoryRegion *mr, > IOMMUTLBEntry entry) > { > + IOMMUNotifier *iommu_notifier; > + IOMMUNotifierFlag request_flags; > + > assert(memory_region_is_iommu(mr)); > - notifier_list_notify(&mr->iommu_notify, &entry); > + > + if (entry.perm & IOMMU_RW) { > + request_flags = IOMMU_NOTIFIER_MAP; > + } else { > + request_flags = IOMMU_NOTIFIER_UNMAP; > + } > + > + QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { > + if (iommu_notifier->notifier_flags & request_flags) { > + iommu_notifier->notify(iommu_notifier, &entry); > + } > + } > } > > void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
On Thu, Sep 22, 2016 at 03:20:48PM +1000, David Gibson wrote: > On Wed, Sep 21, 2016 at 12:58:54PM +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. > > > > (For any IOMMU implementation, an in-place mapping change should be > > notified with an UNMAP followed by a MAP.) > > Ok, I wasn't clear. I meant a big fat comment in the *code*, not just > in the commit message. It should not be necessary to look at the > commit history to figure out how to use an interface correctly > > Even a comment in the code is barely adequate, compared to designing > the interface signatures such that it's obvious. > > Please bear in mind: > http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html > and http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html > Thanks for the links. Maybe a better solution is to re-design the IOTLB interface. However that's out of the scope of this series, and another patchset can be opened for the refactoring work IMHO if there is a strong willingness. For now I can add this into comments: ---------8<----------- @@ -607,6 +628,15 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr); /** * memory_region_notify_iommu: notify a change in an IOMMU translation entry. * + * The notification type will be decided by entry.perm bits: + * + * - For UNMAP (cache invalidation) notifies: set entry.perm to IOMMU_NONE. + * - For MAP (newly added entry) notifies: set entry.perm to the + * permission of the page (which is definitely !IOMMU_NONE). + * + * Note: for any IOMMU implementation, an in-place mapping change + * should be notified with an UNMAP followed by a MAP. + * * @mr: the memory region that was changed * @entry: the new entry in the IOMMU translation table. The entry * replaces all old entries for the same virtual I/O address range. --------->8----------- [...] > > -static void vfio_iommu_map_notify(Notifier *n, void *data) > > +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *data) > > This change leaves a now pointless IOMMUTLBEntry *iotlb = data a few > lines below. Yes, will fix. > > > { > > VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); > > VFIOContainer *container = giommu->container; > > @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener *listener, > > section->offset_within_region; > > giommu->container = container; > > giommu->n.notify = vfio_iommu_map_notify; > > + giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL; > > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); > > > > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > index 3e4d416..a3ec7aa 100644 > > --- a/include/exec/memory.h > > +++ b/include/exec/memory.h > > @@ -67,6 +67,27 @@ struct IOMMUTLBEntry { > > IOMMUAccessFlags perm; > > }; > > > > +/* > > + * Bitmap for differnet IOMMUNotifier capabilities. Each notifier can > > s/differnet/different/ Will fix. Thanks. -- peterx
On Thu, Sep 22, 2016 at 03:17:46PM +0800, Peter Xu wrote: > On Thu, Sep 22, 2016 at 03:20:48PM +1000, David Gibson wrote: > > On Wed, Sep 21, 2016 at 12:58:54PM +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. > > > > > > (For any IOMMU implementation, an in-place mapping change should be > > > notified with an UNMAP followed by a MAP.) > > > > Ok, I wasn't clear. I meant a big fat comment in the *code*, not just > > in the commit message. It should not be necessary to look at the > > commit history to figure out how to use an interface correctly > > > > Even a comment in the code is barely adequate, compared to designing > > the interface signatures such that it's obvious. > > > > Please bear in mind: > > http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html > > and http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html > > > > Thanks for the links. > > Maybe a better solution is to re-design the IOTLB interface. However > that's out of the scope of this series, and another patchset can be > opened for the refactoring work IMHO if there is a strong willingness. > > For now I can add this into comments: > > ---------8<----------- > > @@ -607,6 +628,15 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr); > /** > * memory_region_notify_iommu: notify a change in an IOMMU translation entry. > * > + * The notification type will be decided by entry.perm bits: > + * > + * - For UNMAP (cache invalidation) notifies: set entry.perm to IOMMU_NONE. > + * - For MAP (newly added entry) notifies: set entry.perm to the > + * permission of the page (which is definitely !IOMMU_NONE). > + * > + * Note: for any IOMMU implementation, an in-place mapping change > + * should be notified with an UNMAP followed by a MAP. > + * > * @mr: the memory region that was changed > * @entry: the new entry in the IOMMU translation table. The entry > * replaces all old entries for the same virtual I/O address range. > > --------->8----------- Thanks, that looks about as good as we can get with a comment. > > [...] > > > > -static void vfio_iommu_map_notify(Notifier *n, void *data) > > > +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *data) > > > > This change leaves a now pointless IOMMUTLBEntry *iotlb = data a few > > lines below. > > Yes, will fix. > > > > > > { > > > VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); > > > VFIOContainer *container = giommu->container; > > > @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener *listener, > > > section->offset_within_region; > > > giommu->container = container; > > > giommu->n.notify = vfio_iommu_map_notify; > > > + giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL; > > > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); > > > > > > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); > > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > > index 3e4d416..a3ec7aa 100644 > > > --- a/include/exec/memory.h > > > +++ b/include/exec/memory.h > > > @@ -67,6 +67,27 @@ struct IOMMUTLBEntry { > > > IOMMUAccessFlags perm; > > > }; > > > > > > +/* > > > + * Bitmap for differnet IOMMUNotifier capabilities. Each notifier can > > > > s/differnet/different/ > > Will fix. Thanks. > > -- peterx >
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index b313e7c..89a993b 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -293,7 +293,7 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section) section->offset_within_address_space & (1ULL << 63); } -static void vfio_iommu_map_notify(Notifier *n, void *data) +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *data) { VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); VFIOContainer *container = giommu->container; @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener *listener, section->offset_within_region; giommu->container = container; giommu->n.notify = vfio_iommu_map_notify; + giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL; QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); diff --git a/include/exec/memory.h b/include/exec/memory.h index 3e4d416..a3ec7aa 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -67,6 +67,27 @@ struct IOMMUTLBEntry { IOMMUAccessFlags perm; }; +/* + * Bitmap for differnet IOMMUNotifier capabilities. Each notifier can + * register with one or multiple IOMMU Notifier capability bit(s). + */ +typedef enum { + IOMMU_NOTIFIER_NONE = 0, + /* Notify cache invalidations */ + IOMMU_NOTIFIER_UNMAP = 0x1, + /* Notify entry changes (newly created entries) */ + IOMMU_NOTIFIER_MAP = 0x2, +} IOMMUNotifierFlag; + +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP) + +struct IOMMUNotifier { + void (*notify)(struct IOMMUNotifier *notifier, IOMMUTLBEntry *data); + IOMMUNotifierFlag notifier_flags; + QLIST_ENTRY(IOMMUNotifier) node; +}; +typedef struct IOMMUNotifier IOMMUNotifier; + /* New-style MMIO accessors can indicate that the transaction failed. * A zero (MEMTX_OK) response means success; anything else is a failure * of some kind. The memory subsystem will bitwise-OR together results @@ -201,7 +222,7 @@ struct MemoryRegion { const char *name; unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; - NotifierList iommu_notify; + QLIST_HEAD(, IOMMUNotifier) iommu_notify; }; /** @@ -620,11 +641,12 @@ void memory_region_notify_iommu(MemoryRegion *mr, * IOMMU translation entries. * * @mr: the memory region to observe - * @n: the notifier to be added; the notifier receives a pointer to an - * #IOMMUTLBEntry as the opaque value; the pointer ceases to be - * valid on exit from the notifier. + * @n: the IOMMUNotifier to be added; the notify callback receives a + * pointer to an #IOMMUTLBEntry as the opaque value; the pointer + * ceases to be valid on exit from the notifier. */ -void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n); +void memory_region_register_iommu_notifier(MemoryRegion *mr, + IOMMUNotifier *n); /** * memory_region_iommu_replay: replay existing IOMMU translations to @@ -636,7 +658,8 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n); * @is_write: Whether to treat the replay as a translate "write" * through the iommu */ -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write); +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, + bool is_write); /** * memory_region_unregister_iommu_notifier: unregister a notifier for @@ -646,7 +669,8 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write); * needs to be called * @n: the notifier to be removed. */ -void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n); +void memory_region_unregister_iommu_notifier(MemoryRegion *mr, + IOMMUNotifier *n); /** * memory_region_name: get a memory region's name diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index 94dfae3..c17602e 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -93,7 +93,7 @@ typedef struct VFIOGuestIOMMU { VFIOContainer *container; MemoryRegion *iommu; hwaddr iommu_offset; - Notifier n; + IOMMUNotifier n; QLIST_ENTRY(VFIOGuestIOMMU) giommu_next; } VFIOGuestIOMMU; diff --git a/memory.c b/memory.c index 1a1baf5..69d9d9a 100644 --- a/memory.c +++ b/memory.c @@ -1413,7 +1413,7 @@ void memory_region_init_iommu(MemoryRegion *mr, memory_region_init(mr, owner, name, size); mr->iommu_ops = ops, mr->terminates = true; /* then re-forwards */ - notifier_list_init(&mr->iommu_notify); + QLIST_INIT(&mr->iommu_notify); } static void memory_region_finalize(Object *obj) @@ -1508,13 +1508,16 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client) return memory_region_get_dirty_log_mask(mr) & (1 << client); } -void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n) +void memory_region_register_iommu_notifier(MemoryRegion *mr, + IOMMUNotifier *n) { + /* We need to register for at least one bitfield */ + assert(n->notifier_flags != IOMMU_NOTIFIER_NONE); if (mr->iommu_ops->notify_started && - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { + QLIST_EMPTY(&mr->iommu_notify)) { mr->iommu_ops->notify_started(mr); } - notifier_list_add(&mr->iommu_notify, n); + QLIST_INSERT_HEAD(&mr->iommu_notify, n, node); } uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) @@ -1526,7 +1529,8 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) return TARGET_PAGE_SIZE; } -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, + bool is_write) { hwaddr addr, granularity; IOMMUTLBEntry iotlb; @@ -1547,11 +1551,12 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) } } -void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) +void memory_region_unregister_iommu_notifier(MemoryRegion *mr, + IOMMUNotifier *n) { - notifier_remove(n); + QLIST_REMOVE(n, node); if (mr->iommu_ops->notify_stopped && - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { + QLIST_EMPTY(&mr->iommu_notify)) { mr->iommu_ops->notify_stopped(mr); } } @@ -1559,8 +1564,22 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) void memory_region_notify_iommu(MemoryRegion *mr, IOMMUTLBEntry entry) { + IOMMUNotifier *iommu_notifier; + IOMMUNotifierFlag request_flags; + assert(memory_region_is_iommu(mr)); - notifier_list_notify(&mr->iommu_notify, &entry); + + if (entry.perm & IOMMU_RW) { + request_flags = IOMMU_NOTIFIER_MAP; + } else { + request_flags = IOMMU_NOTIFIER_UNMAP; + } + + QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { + if (iommu_notifier->notifier_flags & request_flags) { + iommu_notifier->notify(iommu_notifier, &entry); + } + } } void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
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. (For any IOMMU implementation, an in-place mapping change should be notified with an UNMAP followed by a MAP.) Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/vfio/common.c | 3 ++- include/exec/memory.h | 38 +++++++++++++++++++++++++++++++------- include/hw/vfio/vfio-common.h | 2 +- memory.c | 37 ++++++++++++++++++++++++++++--------- 4 files changed, 62 insertions(+), 18 deletions(-)