Re: [Qemu-devel] [PATCH RFC v4 10/20] memory: add section range info for IOMMU notifier

[Peter Xu <peterx@redhat.com>]

On Mon, Jan 23, 2017 at 12:12:44PM -0700, Alex Williamson wrote:
> On Fri, 20 Jan 2017 21:08:46 +0800
> Peter Xu <peterx@redhat.com> 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 <david@gibson.dropbear.id.au>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > changelog (start from vt-d vfio enablement series v3):
> > v4:
> > - introduce memory_region_iommu_notifier_init() [Jason]
> > ---
> >  hw/vfio/common.c      | 12 +++++++++---
> >  hw/virtio/vhost.c     |  4 ++--
> >  include/exec/memory.h | 19 ++++++++++++++++++-
> >  memory.c              |  5 ++++-
> >  4 files changed, 33 insertions(+), 7 deletions(-)
> 
> 
> Acked-by: Alex Williamson <alex.williamson@redhat.com>

Thanks for the ack!

Sorry that I want to tune this patch a bit - I'll loosen the limit on
the range check. The original patch will notify if iova is inside
range (start, end), while I am tuning it to allow the notification
happen as long as (iova, size) and (start, end) has any overlapping.
The diff against this one would be (for your better reference):

------8<-------

diff --git a/memory.c b/memory.c
index 89104b1..80ab3c1 100644
--- a/memory.c
+++ b/memory.c
@@ -1672,9 +1672,15 @@ void memory_region_notify_iommu(MemoryRegion *mr,
     }

     QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) {
-        if (iommu_notifier->notifier_flags & request_flags &&
-            iommu_notifier->start <= entry.iova &&
-            iommu_notifier->end >= entry.iova) {
+        /*
+         * Skip the notification if the notification does not overlap
+         * with registered range.
+         */
+        if (iommu_notifier->start > entry.iova + entry.addr_mask + 1 ||
+            iommu_notifier->end < entry.iova) {
+            continue;
+        }
+        if (iommu_notifier->notifier_flags & request_flags) {
             iommu_notifier->notify(iommu_notifier, &entry);
         }
     }

------>8-------

I'll post with the complete patch along with the series's next post.

Thanks,

-- peterx