[Qemu-devel] [PATCH 4/4] vfio: Only use memory listeners when appropriate

[David Gibson]

Currently, vfio registers a MemoryListener for every vfio container we
create, to keep the container's mappings in sync with main system memory.
That's only correct though, if the context the container is attached to
represents a dma address space which actually matches main system memory -
roughly speaking that means that there is no guest side IOMMU above the
vfio device in question.

This patch corrects the code, by only registering the MemoryListener when
the container belongs to a DMAContext which does not include an IOMMU (i.e.
which has no ->translate function).  In other cases we given an error; that
will change when vfio support for guest side IOMMUs is added.

In addition, this generalizes the code slightly, by attaching the
MemoryListener to the DMAContext's underlying AddressSpace, rather than
just assuming that it is main system memory.

Signed-off-by: David Gibson <address@hidden>
---
 hw/misc/vfio.c |   92 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 33 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 707a63c..a4a5310 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -180,6 +180,7 @@ typedef struct VFIOGroup {
 
 typedef struct VFIOIommu {
     QLIST_HEAD(, VFIOContainer) containers;
+    AddressSpace listen_as;
 } VFIOIommu;
 
 #define MSIX_CAP_LENGTH 12
@@ -2648,29 +2649,12 @@ void vfio_iommu_destroy(MemoryRegion *iommu)
     g_free(vfio_iommu);
 }
 
-static int vfio_connect_iommu(VFIOGroup *group, MemoryRegion *iommu)
+static int vfio_create_container_with_listener(VFIOGroup *group,
+                                               MemoryRegion *iommu)
 {
     VFIOIommu *vfio_iommu = iommu_get_vfio(iommu);
     VFIOContainer *container;
-    int ret, fd;
-
-    if (group->container) {
-        if (group->container->iommu == iommu) {
-            return 0;
-        } else {
-            error_report("vfio: group %d used in multiple DMA contexts",
-                         group->groupid);
-            return -EBUSY;
-        }
-    }
-
-    QLIST_FOREACH(container, &vfio_iommu->containers, next) {
-        if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
-            group->container = container;
-            QLIST_INSERT_HEAD(&container->group_list, group, container_next);
-            return 0;
-        }
-    }
+    int fd, ret;
 
     fd = qemu_open("/dev/vfio/vfio", O_RDWR);
     if (fd < 0) {
@@ -2690,15 +2674,15 @@ static int vfio_connect_iommu(VFIOGroup *group, 
MemoryRegion *iommu)
     container->iommu = iommu;
     container->fd = fd;
 
-    if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
-        ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
-        if (ret) {
-            error_report("vfio: failed to set group container: %m");
-            g_free(container);
-            close(fd);
-            return -errno;
-        }
+    ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
+    if (ret) {
+        error_report("vfio: failed to set group container: %m");
+        g_free(container);
+        close(fd);
+        return -errno;
+    }
 
+    if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
         ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU);
         if (ret) {
             error_report("vfio: failed to set iommu for container: %m");
@@ -2706,11 +2690,6 @@ static int vfio_connect_iommu(VFIOGroup *group, 
MemoryRegion *iommu)
             close(fd);
             return -errno;
         }
-
-        container->iommu_data.listener = vfio_memory_listener;
-        container->iommu_data.release = vfio_listener_release;
-
-        memory_listener_register(&container->iommu_data.listener, 
&address_space_memory);
     } else {
         error_report("vfio: No available IOMMU models");
         g_free(container);
@@ -2718,6 +2697,14 @@ static int vfio_connect_iommu(VFIOGroup *group, 
MemoryRegion *iommu)
         return -EINVAL;
     }
 
+    address_space_init(&vfio_iommu->listen_as, iommu);
+
+    container->iommu_data.listener = vfio_memory_listener;
+    container->iommu_data.release = vfio_listener_release;
+
+    memory_listener_register(&container->iommu_data.listener,
+                             &vfio_iommu->listen_as);
+
     QLIST_INIT(&container->group_list);
     QLIST_INSERT_HEAD(&vfio_iommu->containers, container, next);
 
@@ -2727,6 +2714,45 @@ static int vfio_connect_iommu(VFIOGroup *group, 
MemoryRegion *iommu)
     return 0;
 }
 
+static int vfio_connect_iommu(VFIOGroup *group, MemoryRegion *iommu)
+{
+    VFIOContainer *container;
+    VFIOIommu *vfio_iommu;
+    MemoryRegion *mr;
+
+    if (group->container) {
+        if (group->container->iommu == iommu) {
+            return 0;
+        } else {
+            error_report("vfio: group %d used in multiple DMA contexts",
+                         group->groupid);
+            return -EBUSY;
+        }
+    }
+
+    vfio_iommu = iommu_get_vfio(iommu);
+    QLIST_FOREACH(container, &vfio_iommu->containers, next) {
+        if (!ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container->fd)) {
+            group->container = container;
+            QLIST_INSERT_HEAD(&container->group_list, group, container_next);
+            return 0;
+        }
+    }
+
+    mr = iommu;
+    while (!mr->terminates) {
+        mr = mr->parent;
+    }
+
+    if (!memory_region_is_iommu(mr)) {
+        
+        return vfio_create_container_with_listener(group, iommu);
+    }
+
+    error_report("vfio: no support for guest side IOMMU");
+    return -ENODEV;
+}
+
 static void vfio_disconnect_iommu(VFIOGroup *group)
 {
     VFIOContainer *container = group->container;
-- 
1.7.10.4