* [Qemu-devel] [PATCH v2 0/3] Introduce IOMMUNotifier struct
@ 2016-09-06 13:24 Peter Xu
2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps Peter Xu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Xu @ 2016-09-06 13:24 UTC (permalink / raw)
To: qemu-devel
Cc: dgibson, jasowang, mst, pbonzini, cornelia.huck, wexu, vkaplans,
alex.williamson, peterx
This is merely a re-written for v1 (so no change log provided)... But
anyway let's call it v2.
The idea originates from one of Alex's reply:
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00254.html
But after further discussions, it seems that only adding a simple type
for notifier is not enough. This series introduced IOMMUNotifier
struct to replace the old Notifier interface. Along with it, we can
provide registration for one (or multiple) of the IOMMU notifications:
- cache invalidations
- entry changes
This is a support material for Jason's vhost dmar patchset.
Please read commit messages for detailed information. Thanks,
Peter Xu (3):
memory: introduce IOMMUNotifier and its caps
memory: generalize iommu_ops.notify_started to notifier_add
intel_iommu: allow invalidation typed notifiers
hw/i386/intel_iommu.c | 16 +++++++++------
hw/ppc/spapr_iommu.c | 18 +++++++++++------
hw/vfio/common.c | 5 +++--
include/exec/memory.h | 45 +++++++++++++++++++++++++++++++++----------
include/hw/ppc/spapr.h | 1 +
include/hw/vfio/vfio-common.h | 2 +-
memory.c | 45 ++++++++++++++++++++++++++++++-------------
7 files changed, 94 insertions(+), 38 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps 2016-09-06 13:24 [Qemu-devel] [PATCH v2 0/3] Introduce IOMMUNotifier struct Peter Xu @ 2016-09-06 13:24 ` Peter Xu 2016-09-06 14:29 ` Paolo Bonzini 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 2/3] memory: generalize iommu_ops.notify_started to notifier_add Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 3/3] intel_iommu: allow invalidation typed notifiers Peter Xu 2 siblings, 1 reply; 6+ messages in thread From: Peter Xu @ 2016-09-06 13:24 UTC (permalink / raw) To: qemu-devel Cc: dgibson, jasowang, mst, pbonzini, cornelia.huck, wexu, vkaplans, alex.williamson, peterx 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 IOMMUNotfierCap 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_CHANGE: for entry changes (additions) - IOMMU_NOTIFIER_INVALIDATION: for entry removals (cache invalidates) When registering the IOMMU notifier, we need to specify one or multiple capability bit(s) to listen to. When notifications are triggered, it will be checked against the notifier's capability bits, and only notifiers with registered bits will be notified. Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/vfio/common.c | 5 +++-- include/exec/memory.h | 37 +++++++++++++++++++++++++++++++------ include/hw/vfio/vfio-common.h | 2 +- memory.c | 35 ++++++++++++++++++++++++++++------- 4 files changed, 63 insertions(+), 16 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index b313e7c..04e1cb4 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -295,7 +295,7 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section) static void vfio_iommu_map_notify(Notifier *n, void *data) { - VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); + VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n.notifier); VFIOContainer *container = giommu->container; IOMMUTLBEntry *iotlb = data; hwaddr iova = iotlb->iova + giommu->iommu_offset; @@ -453,7 +453,8 @@ static void vfio_listener_region_add(MemoryListener *listener, giommu->iommu_offset = section->offset_within_address_space - section->offset_within_region; giommu->container = container; - giommu->n.notify = vfio_iommu_map_notify; + giommu->n.notifier.notify = vfio_iommu_map_notify; + giommu->n.notifier_caps = 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..52914c1 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_INVALIDATION = 0x1, + /* Notify entry changes (newly created entries) */ + IOMMU_NOTIFIER_CHANGE = 0x2, +} IOMMUNotifierCap; + +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_INVALIDATION | \ + IOMMU_NOTIFIER_CHANGE) + +struct IOMMUNotifier { + Notifier notifier; + IOMMUNotifierCap notifier_caps; +}; +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 @@ -620,11 +641,13 @@ 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 Notifier within the + * IOMMUNotifier 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 +659,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 +670,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 0eb6895..f513c5a 100644 --- a/memory.c +++ b/memory.c @@ -1513,13 +1513,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_caps != IOMMU_NOTIFIER_NONE); if (mr->iommu_ops->notify_started && QLIST_EMPTY(&mr->iommu_notify.notifiers)) { mr->iommu_ops->notify_started(mr); } - notifier_list_add(&mr->iommu_notify, n); + notifier_list_add(&mr->iommu_notify, &n->notifier); } uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) @@ -1531,7 +1534,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; @@ -1541,7 +1545,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) for (addr = 0; addr < memory_region_size(mr); addr += granularity) { iotlb = mr->iommu_ops->translate(mr, addr, is_write); if (iotlb.perm != IOMMU_NONE) { - n->notify(n, &iotlb); + n->notifier.notify(&n->notifier, &iotlb); } /* if (2^64 - MR size) < granularity, it's possible to get an @@ -1552,9 +1556,10 @@ 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); + notifier_remove(&n->notifier); if (mr->iommu_ops->notify_stopped && QLIST_EMPTY(&mr->iommu_notify.notifiers)) { mr->iommu_ops->notify_stopped(mr); @@ -1564,8 +1569,24 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) void memory_region_notify_iommu(MemoryRegion *mr, IOMMUTLBEntry entry) { + IOMMUNotifier *iommu_notify; + IOMMUNotifierCap request_cap; + Notifier *notifier, *next; + assert(memory_region_is_iommu(mr)); - notifier_list_notify(&mr->iommu_notify, &entry); + + if (entry.perm & IOMMU_RW) { + request_cap = IOMMU_NOTIFIER_CHANGE; + } else { + request_cap = IOMMU_NOTIFIER_INVALIDATION; + } + + QLIST_FOREACH_SAFE(notifier, &mr->iommu_notify.notifiers, node, next) { + iommu_notify = container_of(notifier, IOMMUNotifier, notifier); + if (iommu_notify->notifier_caps & request_cap) { + notifier->notify(notifier, &entry); + } + } } void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps Peter Xu @ 2016-09-06 14:29 ` Paolo Bonzini 2016-09-07 4:55 ` Peter Xu 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2016-09-06 14:29 UTC (permalink / raw) To: Peter Xu, qemu-devel Cc: mst, jasowang, vkaplans, alex.williamson, wexu, cornelia.huck, dgibson On 06/09/2016 15:24, 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 IOMMUNotfierCap 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_CHANGE: for entry changes (additions) > - IOMMU_NOTIFIER_INVALIDATION: for entry removals (cache invalidates) > > When registering the IOMMU notifier, we need to specify one or multiple > capability bit(s) to listen to. > > When notifications are triggered, it will be checked against the > notifier's capability bits, and only notifiers with registered bits will > be notified. Since you're walking the notifier list manually anyway, I think it's simpler to get rid of Notifier completely. Otherwise, this looks pretty good! Paolo > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/vfio/common.c | 5 +++-- > include/exec/memory.h | 37 +++++++++++++++++++++++++++++++------ > include/hw/vfio/vfio-common.h | 2 +- > memory.c | 35 ++++++++++++++++++++++++++++------- > 4 files changed, 63 insertions(+), 16 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index b313e7c..04e1cb4 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -295,7 +295,7 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section) > > static void vfio_iommu_map_notify(Notifier *n, void *data) > { > - VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); > + VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n.notifier); > VFIOContainer *container = giommu->container; > IOMMUTLBEntry *iotlb = data; > hwaddr iova = iotlb->iova + giommu->iommu_offset; > @@ -453,7 +453,8 @@ static void vfio_listener_region_add(MemoryListener *listener, > giommu->iommu_offset = section->offset_within_address_space - > section->offset_within_region; > giommu->container = container; > - giommu->n.notify = vfio_iommu_map_notify; > + giommu->n.notifier.notify = vfio_iommu_map_notify; > + giommu->n.notifier_caps = 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..52914c1 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_INVALIDATION = 0x1, > + /* Notify entry changes (newly created entries) */ > + IOMMU_NOTIFIER_CHANGE = 0x2, > +} IOMMUNotifierCap; > + > +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_INVALIDATION | \ > + IOMMU_NOTIFIER_CHANGE) > + > +struct IOMMUNotifier { > + Notifier notifier; > + IOMMUNotifierCap notifier_caps; > +}; > +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 > @@ -620,11 +641,13 @@ 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 Notifier within the > + * IOMMUNotifier 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 +659,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 +670,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 0eb6895..f513c5a 100644 > --- a/memory.c > +++ b/memory.c > @@ -1513,13 +1513,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_caps != IOMMU_NOTIFIER_NONE); > if (mr->iommu_ops->notify_started && > QLIST_EMPTY(&mr->iommu_notify.notifiers)) { > mr->iommu_ops->notify_started(mr); > } > - notifier_list_add(&mr->iommu_notify, n); > + notifier_list_add(&mr->iommu_notify, &n->notifier); > } > > uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) > @@ -1531,7 +1534,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; > @@ -1541,7 +1545,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool is_write) > for (addr = 0; addr < memory_region_size(mr); addr += granularity) { > iotlb = mr->iommu_ops->translate(mr, addr, is_write); > if (iotlb.perm != IOMMU_NONE) { > - n->notify(n, &iotlb); > + n->notifier.notify(&n->notifier, &iotlb); > } > > /* if (2^64 - MR size) < granularity, it's possible to get an > @@ -1552,9 +1556,10 @@ 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); > + notifier_remove(&n->notifier); > if (mr->iommu_ops->notify_stopped && > QLIST_EMPTY(&mr->iommu_notify.notifiers)) { > mr->iommu_ops->notify_stopped(mr); > @@ -1564,8 +1569,24 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, Notifier *n) > void memory_region_notify_iommu(MemoryRegion *mr, > IOMMUTLBEntry entry) > { > + IOMMUNotifier *iommu_notify; > + IOMMUNotifierCap request_cap; > + Notifier *notifier, *next; > + > assert(memory_region_is_iommu(mr)); > - notifier_list_notify(&mr->iommu_notify, &entry); > + > + if (entry.perm & IOMMU_RW) { > + request_cap = IOMMU_NOTIFIER_CHANGE; > + } else { > + request_cap = IOMMU_NOTIFIER_INVALIDATION; > + } > + > + QLIST_FOREACH_SAFE(notifier, &mr->iommu_notify.notifiers, node, next) { > + iommu_notify = container_of(notifier, IOMMUNotifier, notifier); > + if (iommu_notify->notifier_caps & request_cap) { > + notifier->notify(notifier, &entry); > + } > + } > } > > void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps 2016-09-06 14:29 ` Paolo Bonzini @ 2016-09-07 4:55 ` Peter Xu 0 siblings, 0 replies; 6+ messages in thread From: Peter Xu @ 2016-09-07 4:55 UTC (permalink / raw) To: Paolo Bonzini Cc: qemu-devel, mst, jasowang, vkaplans, alex.williamson, wexu, cornelia.huck, dgibson On Tue, Sep 06, 2016 at 04:29:52PM +0200, Paolo Bonzini wrote: > Since you're walking the notifier list manually anyway, I think it's > simpler to get rid of Notifier completely. Otherwise, this looks pretty > good! Yes that should be cleaner. Will fix. Thanks! -- peterx ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] memory: generalize iommu_ops.notify_started to notifier_add 2016-09-06 13:24 [Qemu-devel] [PATCH v2 0/3] Introduce IOMMUNotifier struct Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps Peter Xu @ 2016-09-06 13:24 ` Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 3/3] intel_iommu: allow invalidation typed notifiers Peter Xu 2 siblings, 0 replies; 6+ messages in thread From: Peter Xu @ 2016-09-06 13:24 UTC (permalink / raw) To: qemu-devel Cc: dgibson, jasowang, mst, pbonzini, cornelia.huck, wexu, vkaplans, alex.williamson, peterx 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. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/i386/intel_iommu.c | 4 ++-- hw/ppc/spapr_iommu.c | 18 ++++++++++++------ include/exec/memory.h | 8 ++++---- include/hw/ppc/spapr.h | 1 + memory.c | 10 ++++------ 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 28c31a2..c6bd8f6 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1974,7 +1974,7 @@ static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion *iommu, hwaddr addr, return ret; } -static void vtd_iommu_notify_started(MemoryRegion *iommu) +static void vtd_iommu_notifier_add(MemoryRegion *iommu, IOMMUNotifier *n) { VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu); @@ -2348,7 +2348,7 @@ static void vtd_init(IntelIOMMUState *s) memset(s->womask, 0, DMAR_REG_SIZE); s->iommu_ops.translate = vtd_iommu_translate; - s->iommu_ops.notify_started = vtd_iommu_notify_started; + s->iommu_ops.notifier_add = vtd_iommu_notifier_add; s->root = 0; s->root_extended = false; s->dmar_enabled = false; diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 6bc4d4d..84421e0 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -156,14 +156,20 @@ static uint64_t spapr_tce_get_min_page_size(MemoryRegion *iommu) return 1ULL << tcet->page_shift; } -static void spapr_tce_notify_started(MemoryRegion *iommu) +static void spapr_tce_notifier_add(MemoryRegion *iommu, IOMMUNotifier *n) { - spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), true); + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu); + if (tcet->iommu_users++ == 0) { + spapr_tce_set_need_vfio(tcet, true); + } } -static void spapr_tce_notify_stopped(MemoryRegion *iommu) +static void spapr_tce_notifier_del(MemoryRegion *iommu) { - spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), false); + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu); + if (--tcet->iommu_users == 0) { + spapr_tce_set_need_vfio(tcet, false); + } } static int spapr_tce_table_post_load(void *opaque, int version_id) @@ -246,8 +252,8 @@ static const VMStateDescription vmstate_spapr_tce_table = { static MemoryRegionIOMMUOps spapr_iommu_ops = { .translate = spapr_tce_translate_iommu, .get_min_page_size = spapr_tce_get_min_page_size, - .notify_started = spapr_tce_notify_started, - .notify_stopped = spapr_tce_notify_stopped, + .notifier_add = spapr_tce_notifier_add, + .notifier_del = spapr_tce_notifier_del, }; static int spapr_tce_table_realize(DeviceState *dev) diff --git a/include/exec/memory.h b/include/exec/memory.h index 52914c1..3a2e7d8 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -174,10 +174,10 @@ struct MemoryRegionIOMMUOps { IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bool is_write); /* Returns minimum supported page size */ uint64_t (*get_min_page_size)(MemoryRegion *iommu); - /* Called when the first notifier is set */ - void (*notify_started)(MemoryRegion *iommu); - /* Called when the last notifier is removed */ - void (*notify_stopped)(MemoryRegion *iommu); + /* Called when someone registers to the notify list */ + void (*notifier_add)(MemoryRegion *iommu, IOMMUNotifier *n); + /* Called when someone unregisters from the notify list */ + void (*notifier_del)(MemoryRegion *iommu, IOMMUNotifier *n); }; typedef struct CoalescedMemoryRange CoalescedMemoryRange; diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index caf7be9..08627ec 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -564,6 +564,7 @@ struct sPAPRTCETable { MemoryRegion root, iommu; struct VIOsPAPRDevice *vdev; /* for @bypass migration compatibility only */ QLIST_ENTRY(sPAPRTCETable) list; + int iommu_users; }; sPAPRTCETable *spapr_tce_find_by_liobn(target_ulong liobn); diff --git a/memory.c b/memory.c index f513c5a..a50e627 100644 --- a/memory.c +++ b/memory.c @@ -1518,9 +1518,8 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, { /* We need to register for at least one bitfield */ assert(n->notifier_caps != IOMMU_NOTIFIER_NONE); - if (mr->iommu_ops->notify_started && - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { - mr->iommu_ops->notify_started(mr); + if (mr->iommu_ops->notifier_add) { + mr->iommu_ops->notifier_add(mr, n); } notifier_list_add(&mr->iommu_notify, &n->notifier); } @@ -1560,9 +1559,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, IOMMUNotifier *n) { notifier_remove(&n->notifier); - if (mr->iommu_ops->notify_stopped && - QLIST_EMPTY(&mr->iommu_notify.notifiers)) { - mr->iommu_ops->notify_stopped(mr); + if (mr->iommu_ops->notifier_del) { + mr->iommu_ops->notifier_del(mr, n); } } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] intel_iommu: allow invalidation typed notifiers 2016-09-06 13:24 [Qemu-devel] [PATCH v2 0/3] Introduce IOMMUNotifier struct Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 2/3] memory: generalize iommu_ops.notify_started to notifier_add Peter Xu @ 2016-09-06 13:24 ` Peter Xu 2 siblings, 0 replies; 6+ messages in thread From: Peter Xu @ 2016-09-06 13:24 UTC (permalink / raw) To: qemu-devel Cc: dgibson, jasowang, mst, pbonzini, cornelia.huck, wexu, vkaplans, alex.williamson, peterx Intel vIOMMU is still lacking of a complete IOMMU notifier mechanism. Before that is achieved, let's open a door for vhost DMAR support, which only requires device-IOTLB based cache invalidations. Meanwhile, converting hw_error() to error_report() and exit(), to make the error messages clean and obvious (so no CPU registers will be dumped). Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/i386/intel_iommu.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index c6bd8f6..227c931 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1978,10 +1978,14 @@ static void vtd_iommu_notifier_add(MemoryRegion *iommu, IOMMUNotifier *n) { VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu); - hw_error("Device at bus %s addr %02x.%d requires iommu notifier which " - "is currently not supported by intel-iommu emulation", - vtd_as->bus->qbus.name, PCI_SLOT(vtd_as->devfn), - PCI_FUNC(vtd_as->devfn)); + if (n->notifier_caps & IOMMU_NOTIFIER_CHANGE) { + error_report("Device at bus %s addr %02x.%d requires iommu " + "notifier which is currently not supported by " + "intel-iommu emulation", + vtd_as->bus->qbus.name, PCI_SLOT(vtd_as->devfn), + PCI_FUNC(vtd_as->devfn)); + exit(1); + } } static const VMStateDescription vtd_vmstate = { -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-07 4:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-06 13:24 [Qemu-devel] [PATCH v2 0/3] Introduce IOMMUNotifier struct Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 1/3] memory: introduce IOMMUNotifier and its caps Peter Xu 2016-09-06 14:29 ` Paolo Bonzini 2016-09-07 4:55 ` Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 2/3] memory: generalize iommu_ops.notify_started to notifier_add Peter Xu 2016-09-06 13:24 ` [Qemu-devel] [PATCH v2 3/3] intel_iommu: allow invalidation typed notifiers Peter Xu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.