[RFC PATCH 05/10] vfio: Add region_map and region_unmap callbacks to VFIODeviceIOOps

Scott J. Goldman posted 10 patches 6 days, 11 hours ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, John Levon <john.levon@nutanix.com>, Thanos Makatos <thanos.makatos@nutanix.com>, "Cédric Le Goater" <clg@redhat.com>, Alex Williamson <alex@shazbot.org>, Tony Krowiak <akrowiak@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, "Scott J. Goldman" <scottjgo@gmail.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[RFC PATCH 05/10] vfio: Add region_map and region_unmap callbacks to VFIODeviceIOOps
Posted by Scott J. Goldman 6 days, 11 hours ago
Rename vfio_region_mmap() and vfio_region_unmap() to
vfio_region_mmap_fd() and vfio_region_unmap_fd() respectively, and
introduce new region_map and region_unmap callbacks in
VFIODeviceIOOps.

The new vfio_region_mmap() and vfio_region_unmap() functions now
dispatch through these io_ops callbacks, allowing different backends
to provide their own region mapping implementations. Both the ioctl
and vfio-user backends implement the callbacks by calling the renamed
fd-based variants.

This refactor enables future backends that may require alternate
region mapping strategies.

Signed-off-by: Scott J. Goldman <scottjgo@gmail.com>
---
 hw/vfio-user/device.c         |  16 ++++-
 hw/vfio/device.c              |  14 +++++
 hw/vfio/region.c              | 108 +++++++++++++++++++++++-----------
 hw/vfio/vfio-region.h         |   4 ++
 include/hw/vfio/vfio-device.h |  25 ++++++++
 5 files changed, 131 insertions(+), 36 deletions(-)

diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 64ef35b320..957d19217b 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -12,6 +12,7 @@
 #include "qemu/lockable.h"
 #include "qemu/thread.h"
 
+#include "hw/vfio/vfio-region.h"
 #include "hw/vfio-user/device.h"
 #include "hw/vfio-user/trace.h"
 
@@ -428,6 +429,18 @@ static int vfio_user_device_io_region_write(VFIODevice *vbasedev, uint8_t index,
     return ret;
 }
 
+static int vfio_user_device_io_region_map(VFIODevice *vbasedev,
+                                          VFIORegion *region)
+{
+    return vfio_region_mmap_fd(region);
+}
+
+static void vfio_user_device_io_region_unmap(VFIODevice *vbasedev,
+                                             VFIORegion *region)
+{
+    vfio_region_unmap_fd(region);
+}
+
 /*
  * Socket-based io_ops
  */
@@ -437,5 +450,6 @@ VFIODeviceIOOps vfio_user_device_io_ops_sock = {
     .set_irqs = vfio_user_device_io_set_irqs,
     .region_read = vfio_user_device_io_region_read,
     .region_write = vfio_user_device_io_region_write,
-
+    .region_map = vfio_user_device_io_region_map,
+    .region_unmap = vfio_user_device_io_region_unmap,
 };
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 338becffa7..1b703dcbec 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -23,6 +23,7 @@
 
 #include "hw/vfio/vfio-device.h"
 #include "hw/vfio/pci.h"
+#include "hw/vfio/vfio-region.h"
 #include "hw/core/iommu.h"
 #include "hw/core/hw-error.h"
 #include "trace.h"
@@ -656,6 +657,17 @@ static int vfio_device_io_region_write(VFIODevice *vbasedev, uint8_t index,
     return ret < 0 ? -errno : ret;
 }
 
+static int vfio_device_io_region_map(VFIODevice *vbasedev, VFIORegion *region)
+{
+    return vfio_region_mmap_fd(region);
+}
+
+static void vfio_device_io_region_unmap(VFIODevice *vbasedev,
+                                        VFIORegion *region)
+{
+    vfio_region_unmap_fd(region);
+}
+
 static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
     .device_feature = vfio_device_io_device_feature,
     .get_region_info = vfio_device_io_get_region_info,
@@ -663,4 +675,6 @@ static VFIODeviceIOOps vfio_device_io_ops_ioctl = {
     .set_irqs = vfio_device_io_set_irqs,
     .region_read = vfio_device_io_region_read,
     .region_write = vfio_device_io_region_write,
+    .region_map = vfio_device_io_region_map,
+    .region_unmap = vfio_device_io_region_unmap,
 };
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index 47fdc2df34..9f7780e06c 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -273,15 +273,48 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
     return 0;
 }
 
-static void vfio_subregion_unmap(VFIORegion *region, int index)
+void vfio_region_register_mmap(VFIORegion *region, int index)
 {
+    char *name;
+
+    if (!region->mmaps[index].mmap) {
+        return;
+    }
+
+    name = g_strdup_printf("%s mmaps[%d]",
+                           memory_region_name(region->mem), index);
+    memory_region_init_ram_device_ptr(&region->mmaps[index].mem,
+                                      memory_region_owner(region->mem),
+                                      name, region->mmaps[index].size,
+                                      region->mmaps[index].mmap);
+    g_free(name);
+    memory_region_add_subregion(region->mem, region->mmaps[index].offset,
+                                &region->mmaps[index].mem);
+
+    trace_vfio_region_mmap(memory_region_name(&region->mmaps[index].mem),
+                           region->mmaps[index].offset,
+                           region->mmaps[index].offset +
+                           region->mmaps[index].size - 1);
+}
+
+void vfio_region_unregister_mmap(VFIORegion *region, int index)
+{
+    if (!region->mmaps[index].mmap) {
+        return;
+    }
+
     trace_vfio_region_unmap(memory_region_name(&region->mmaps[index].mem),
                             region->mmaps[index].offset,
                             region->mmaps[index].offset +
                             region->mmaps[index].size - 1);
     memory_region_del_subregion(region->mem, &region->mmaps[index].mem);
-    munmap(region->mmaps[index].mmap, region->mmaps[index].size);
     object_unparent(OBJECT(&region->mmaps[index].mem));
+}
+
+static void vfio_region_unmap_fd_one(VFIORegion *region, int index)
+{
+    vfio_region_unregister_mmap(region, index);
+    munmap(region->mmaps[index].mmap, region->mmaps[index].size);
     region->mmaps[index].mmap = NULL;
 }
 
@@ -342,14 +375,13 @@ static bool vfio_region_create_dma_buf(VFIORegion *region, Error **errp)
     return true;
 }
 
-int vfio_region_mmap(VFIORegion *region)
+int vfio_region_mmap_fd(VFIORegion *region)
 {
     void *map_base, *map_align;
     Error *local_err = NULL;
     int i, ret, prot = 0;
     off_t map_offset = 0;
     size_t align;
-    char *name;
     int fd;
 
     if (!region->mem || !region->nr_mmaps) {
@@ -417,21 +449,7 @@ int vfio_region_mmap(VFIORegion *region)
             goto no_mmap;
         }
 
-        name = g_strdup_printf("%s mmaps[%d]",
-                               memory_region_name(region->mem), i);
-        memory_region_init_ram_device_ptr(&region->mmaps[i].mem,
-                                          memory_region_owner(region->mem),
-                                          name, region->mmaps[i].size,
-                                          region->mmaps[i].mmap);
-        g_free(name);
-        memory_region_add_subregion(region->mem, region->mmaps[i].offset,
-                                    &region->mmaps[i].mem);
-
-        trace_vfio_region_mmap(memory_region_name(&region->mmaps[i].mem),
-                               region->mmaps[i].offset,
-                               region->mmaps[i].offset +
-                               region->mmaps[i].size - 1);
-
+        vfio_region_register_mmap(region, i);
         map_offset = region->mmaps[i].offset + region->mmaps[i].size;
     }
 
@@ -457,13 +475,13 @@ no_mmap:
     region->mmaps[i].mmap = NULL;
 
     for (i--; i >= 0; i--) {
-        vfio_subregion_unmap(region, i);
+        vfio_region_unmap_fd_one(region, i);
     }
 
     return ret;
 }
 
-void vfio_region_unmap(VFIORegion *region)
+void vfio_region_unmap_fd(VFIORegion *region)
 {
     int i;
 
@@ -473,41 +491,61 @@ void vfio_region_unmap(VFIORegion *region)
 
     for (i = 0; i < region->nr_mmaps; i++) {
         if (region->mmaps[i].mmap) {
-            vfio_subregion_unmap(region, i);
+            vfio_region_unmap_fd_one(region, i);
         }
     }
 }
 
-void vfio_region_exit(VFIORegion *region)
+int vfio_region_mmap(VFIORegion *region)
 {
-    int i;
+    VFIODevice *vbasedev;
+
+    if (!region->mem) {
+        return 0;
+    }
+
+    vbasedev = region->vbasedev;
+    if (!vbasedev->io_ops || !vbasedev->io_ops->region_map) {
+        return -EINVAL;
+    }
+
+    return vbasedev->io_ops->region_map(vbasedev, region);
+}
+
+void vfio_region_unmap(VFIORegion *region)
+{
+    VFIODevice *vbasedev;
 
     if (!region->mem) {
         return;
     }
 
-    for (i = 0; i < region->nr_mmaps; i++) {
-        if (region->mmaps[i].mmap) {
-            memory_region_del_subregion(region->mem, &region->mmaps[i].mem);
-        }
+    vbasedev = region->vbasedev;
+    if (!vbasedev->io_ops || !vbasedev->io_ops->region_unmap) {
+        return;
+    }
+
+    vbasedev->io_ops->region_unmap(vbasedev, region);
+}
+
+void vfio_region_exit(VFIORegion *region)
+{
+    if (!region->mem) {
+        return;
     }
 
+    vfio_region_unmap(region);
+
     trace_vfio_region_exit(region->vbasedev->name, region->nr);
 }
 
 void vfio_region_finalize(VFIORegion *region)
 {
-    int i;
-
     if (!region->mem) {
         return;
     }
 
-    for (i = 0; i < region->nr_mmaps; i++) {
-        if (region->mmaps[i].mmap) {
-            munmap(region->mmaps[i].mmap, region->mmaps[i].size);
-        }
-    }
+    vfio_region_unmap(region);
 
     g_free(region->mem);
     g_free(region->mmaps);
diff --git a/hw/vfio/vfio-region.h b/hw/vfio/vfio-region.h
index 9b21d4ee5b..afdce466b1 100644
--- a/hw/vfio/vfio-region.h
+++ b/hw/vfio/vfio-region.h
@@ -39,6 +39,10 @@ uint64_t vfio_region_read(void *opaque,
                           hwaddr addr, unsigned size);
 int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
                       int index, const char *name, Error **errp);
+void vfio_region_register_mmap(VFIORegion *region, int index);
+void vfio_region_unregister_mmap(VFIORegion *region, int index);
+int vfio_region_mmap_fd(VFIORegion *region);
+void vfio_region_unmap_fd(VFIORegion *region);
 int vfio_region_mmap(VFIORegion *region);
 void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
 void vfio_region_unmap(VFIORegion *region);
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 17c5db369c..1a3b42bcaf 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -44,6 +44,7 @@ enum {
 typedef struct VFIODeviceOps VFIODeviceOps;
 typedef struct VFIODeviceIOOps VFIODeviceIOOps;
 typedef struct VFIOMigration VFIOMigration;
+typedef struct VFIORegion VFIORegion;
 
 typedef struct IOMMUFDBackend IOMMUFDBackend;
 typedef struct VFIOIOASHwpt VFIOIOASHwpt;
@@ -260,6 +261,30 @@ struct VFIODeviceIOOps {
      */
     int (*region_write)(VFIODevice *vdev, uint8_t nr, off_t off, uint32_t size,
                         void *data, bool post);
+
+    /**
+     * @region_map
+     *
+     * Map a region's directly accessible subranges and register any mmap-backed
+     * subregions with QEMU.
+     *
+     * @vdev: #VFIODevice to use
+     * @region: #VFIORegion to map
+     *
+     * Returns 0 on success or -errno.
+     */
+    int (*region_map)(VFIODevice *vdev, VFIORegion *region);
+
+    /**
+     * @region_unmap
+     *
+     * Unregister any mmap-backed subregions for a region and release their
+     * backend mappings.
+     *
+     * @vdev: #VFIODevice to use
+     * @region: #VFIORegion to unmap
+     */
+    void (*region_unmap)(VFIODevice *vdev, VFIORegion *region);
 };
 
 void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
-- 
2.50.1 (Apple Git-155)