On Fri, Jan 13, 2017 at 03:55:22PM +0800, Jason Wang wrote: > > > On 2017年01月13日 11:06, Peter Xu wrote: > >In this patch, IOMMUNotifier.{start|end} are introduced to store section > >information for a specific notifier. When notification occurs, we not > >only check the notification type (MAP|UNMAP), but also check whether the > >notified iova is in the range of specific IOMMU notifier, and skip those > >notifiers if not in the listened range. > > > >When removing an region, we need to make sure we removed the correct > >VFIOGuestIOMMU by checking the IOMMUNotifier.start address as well. > > > >Suggested-by: David Gibson <da...@gibson.dropbear.id.au> > >Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > >Acked-by: Paolo Bonzini <pbonz...@redhat.com> > >Signed-off-by: Peter Xu <pet...@redhat.com> > >--- > > hw/vfio/common.c | 7 ++++++- > > include/exec/memory.h | 3 +++ > > memory.c | 4 +++- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > >diff --git a/hw/vfio/common.c b/hw/vfio/common.c > >index 801578b..6f648da 100644 > >--- a/hw/vfio/common.c > >+++ b/hw/vfio/common.c > >@@ -455,6 +455,10 @@ static void vfio_listener_region_add(MemoryListener > >*listener, > > giommu->container = container; > > giommu->n.notify = vfio_iommu_map_notify; > > giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL; > >+ giommu->n.start = section->offset_within_region; > >+ llend = int128_add(int128_make64(giommu->n.start), section->size); > >+ llend = int128_sub(llend, int128_one()); > >+ giommu->n.end = int128_get64(llend); > > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); > > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); > >@@ -525,7 +529,8 @@ static void vfio_listener_region_del(MemoryListener > >*listener, > > VFIOGuestIOMMU *giommu; > > QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) { > >- if (giommu->iommu == section->mr) { > >+ if (giommu->iommu == section->mr && > >+ giommu->n.start == section->offset_within_region) { > > memory_region_unregister_iommu_notifier(giommu->iommu, > > &giommu->n); > > QLIST_REMOVE(giommu, giommu_next); > >diff --git a/include/exec/memory.h b/include/exec/memory.h > >index bec9756..7649e74 100644 > >--- a/include/exec/memory.h > >+++ b/include/exec/memory.h > >@@ -84,6 +84,9 @@ typedef enum { > > struct IOMMUNotifier { > > void (*notify)(struct IOMMUNotifier *notifier, IOMMUTLBEntry *data); > > IOMMUNotifierFlag notifier_flags; > >+ /* Notify for address space range start <= addr <= end */ > >+ hwaddr start; > >+ hwaddr end; > > QLIST_ENTRY(IOMMUNotifier) node; > > }; > > typedef struct IOMMUNotifier IOMMUNotifier; > >diff --git a/memory.c b/memory.c > >index 2bfc37f..e88bb54 100644 > >--- a/memory.c > >+++ b/memory.c > >@@ -1671,7 +1671,9 @@ void memory_region_notify_iommu(MemoryRegion *mr, > > } > > QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { > >- if (iommu_notifier->notifier_flags & request_flags) { > >+ if (iommu_notifier->notifier_flags & request_flags && > >+ iommu_notifier->start <= entry.iova && > >+ iommu_notifier->end >= entry.iova) { > > iommu_notifier->notify(iommu_notifier, &entry); > > } > > } > > This seems breaks vhost device IOTLB. How about keep the the behavior > somehow?
Thanks to point out. How about I squash this into this patch? --------8<-------- diff --git a/memory.c b/memory.c index e88bb54..6de02dd 100644 --- a/memory.c +++ b/memory.c @@ -1608,8 +1608,14 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, return; } + if (n->start == 0 && n->end == 0) { + /* If these are not specified, we listen to the whole range */ + n->end = (hwaddr)(-1); + } + /* We need to register for at least one bitfield */ assert(n->notifier_flags != IOMMU_NOTIFIER_NONE); + assert(n->start <= n->end); QLIST_INSERT_HEAD(&mr->iommu_notify, n, node); memory_region_update_iommu_notify_flags(mr); } -------->8-------- -- peterx