From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40247) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9ReF-0004GU-4A for qemu-devel@nongnu.org; Wed, 23 Nov 2016 02:07:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9ReB-0001p8-RM for qemu-devel@nongnu.org; Wed, 23 Nov 2016 02:07:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46580) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9ReB-0001o0-HD for qemu-devel@nongnu.org; Wed, 23 Nov 2016 02:07:11 -0500 From: Peter Xu Date: Wed, 23 Nov 2016 15:07:04 +0800 Message-Id: <1479884824-26498-1-git-send-email-peterx@redhat.com> Subject: [Qemu-devel] [PATCH v2] vfio: avoid adding same iommu mr for notify List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: alex.williamson@redhat.com, bd.aviv@gmail.com, peterx@redhat.com, david@gibson.dropbear.id.au When one IOMMU memory region is splitted into multiple memory sections, vfio will register multiple same notifiers to a vIOMMU for the same region. That's not sensible. What we need is to register one IOMMU notifier for each IOMMU region, not per section. Solution is simple - we traverse the container->giommu_list, and skip the registration if memory region is already registered. Instead, we use a refcount to note down how many sections are referencing the VFIOGuestIOMMU. To make vfio's region_add() short, vfio_listener_region_add_iommu() is introduced. Signed-off-by: Peter Xu --- v2: - use refcount to make sure the VFIOGuestIOMMU won't be freed until the last referencing memory region section is deleted [David] (David suggested another way to implement - to store section range info in IOMMUNotifier. Will post another patch for it, and we can choose either one we want) Signed-off-by: Peter Xu --- hw/vfio/common.c | 71 ++++++++++++++++++++++++++++--------------- include/hw/vfio/vfio-common.h | 1 + 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 801578b..e75e25c 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -360,6 +360,41 @@ out: rcu_read_unlock(); } +static void vfio_listener_region_add_iommu(VFIOContainer *container, + MemoryRegionSection *section, + hwaddr iova, + hwaddr end) +{ + VFIOGuestIOMMU *giommu; + + QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) { + if (giommu->iommu == section->mr) { + atomic_inc(&giommu->refcount); + return; + } + } + + trace_vfio_listener_region_add_iommu(iova, end); + + /* + * FIXME: For VFIO iommu types which have KVM acceleration to + * avoid bouncing all map/unmaps through qemu this way, this + * would be the right place to wire that up (tell the KVM + * device emulation the VFIO iommu handles to use). + */ + giommu = g_malloc0(sizeof(*giommu)); + giommu->iommu = section->mr; + 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_flags = IOMMU_NOTIFIER_ALL; + atomic_set(&giommu->refcount, 1); + QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); + memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); + memory_region_iommu_replay(giommu->iommu, &giommu->n, false); +} + static void vfio_listener_region_add(MemoryListener *listener, MemoryRegionSection *section) { @@ -439,27 +474,7 @@ static void vfio_listener_region_add(MemoryListener *listener, memory_region_ref(section->mr); if (memory_region_is_iommu(section->mr)) { - VFIOGuestIOMMU *giommu; - - trace_vfio_listener_region_add_iommu(iova, end); - /* - * FIXME: For VFIO iommu types which have KVM acceleration to - * avoid bouncing all map/unmaps through qemu this way, this - * would be the right place to wire that up (tell the KVM - * device emulation the VFIO iommu handles to use). - */ - giommu = g_malloc0(sizeof(*giommu)); - giommu->iommu = section->mr; - 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_flags = IOMMU_NOTIFIER_ALL; - QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); - - memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); - memory_region_iommu_replay(giommu->iommu, &giommu->n, false); - + vfio_listener_region_add_iommu(container, section, iova, end); return; } @@ -526,10 +541,16 @@ static void vfio_listener_region_del(MemoryListener *listener, QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) { if (giommu->iommu == section->mr) { - memory_region_unregister_iommu_notifier(giommu->iommu, - &giommu->n); - QLIST_REMOVE(giommu, giommu_next); - g_free(giommu); + /* + * Only release the object when the last referencing + * memory region section is deleted + */ + if (atomic_fetch_dec(&giommu->refcount) == 1) { + memory_region_unregister_iommu_notifier(giommu->iommu, + &giommu->n); + QLIST_REMOVE(giommu, giommu_next); + g_free(giommu); + } break; } } diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index c582de1..8817f8a 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -97,6 +97,7 @@ typedef struct VFIOGuestIOMMU { MemoryRegion *iommu; hwaddr iommu_offset; IOMMUNotifier n; + unsigned int refcount; QLIST_ENTRY(VFIOGuestIOMMU) giommu_next; } VFIOGuestIOMMU; -- 2.7.4