[Qemu-devel] [PATCH 2/3] vfio: free dynamically-allocated data in instance_finalize

[Paolo Bonzini]

In order to enable out-of-BQL address space lookup, destruction of
devices needs to be split in two phases.

Unrealize is the first phase; once it complete no new accesses will
be started, but there may still be pending memory accesses can still
be completed.

The second part is freeing the device, which only happens once all memory
accesses are complete.  At this point the reference count has dropped to
zero, an RCU grace period must have completed (because the RCU-protected
FlatViews hold a reference to the device via memory_region_ref).  This is
when instance_finalize is called.

Freeing data belongs in an instance_finalize callback, because the
dynamically allocated memory can still be used after unrealize by the
pending memory accesses.

This starts the process by creating an instance_finalize callback and
freeing most of the dynamically-allocated data in instance_finalize.
Because instance_finalize is also called on error paths or also when
the device is actually not realized, the common code needs some changes
to be ready for this.  The error path in vfio_initfn can be simplified too.

Cc: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
 hw/vfio/common.c |  5 ++++-
 hw/vfio/pci.c    | 22 +++++++++++++---------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 242b71d..1bf9de9 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -847,7 +847,7 @@ free_group_exit:
 
 void vfio_put_group(VFIOGroup *group)
 {
-    if (!QLIST_EMPTY(&group->device_list)) {
+    if (!group || !QLIST_EMPTY(&group->device_list)) {
         return;
     }
 
@@ -902,6 +902,9 @@ int vfio_get_device(VFIOGroup *group, const char *name,
 
 void vfio_put_base_device(VFIODevice *vbasedev)
 {
+    if (!vbasedev->group) {
+        return;
+    }
     QLIST_REMOVE(vbasedev, next);
     trace_vfio_put_base_device(vbasedev->fd);
     close(vbasedev->fd);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 2f5f718..0e1d229 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3257,7 +3257,7 @@ static int vfio_initfn(PCIDevice *pdev)
     if (ret < (int)MIN(pci_config_size(&vdev->pdev), vdev->config_size)) {
         ret = ret < 0 ? -errno : -EFAULT;
         error_report("vfio: Failed to read device config space");
-        goto out_put;
+        return ret;
     }
 
     /* vfio emulates a lot for us, but some bits need extra love */
@@ -3289,7 +3289,7 @@ static int vfio_initfn(PCIDevice *pdev)
 
     ret = vfio_early_setup_msix(vdev);
     if (ret) {
-        goto out_put;
+        return ret;
     }
 
     vfio_map_bars(vdev);
@@ -3328,17 +3328,24 @@ out_teardown:
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
     vfio_teardown_msi(vdev);
     vfio_unmap_bars(vdev);
-out_put:
+    return ret;
+}
+
+static void vfio_instance_finalize(Object *obj)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(obj);
+    VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pci_dev);
+    VFIOGroup *group = vdev->vbasedev.group;
+
     g_free(vdev->emulated_config_bits);
+    g_free(vdev->rom);
     vfio_put_device(vdev);
     vfio_put_group(group);
-    return ret;
 }
 
 static void vfio_exitfn(PCIDevice *pdev)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
-    VFIOGroup *group = vdev->vbasedev.group;
 
     vfio_unregister_err_notifier(vdev);
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
@@ -3348,10 +3355,6 @@ static void vfio_exitfn(PCIDevice *pdev)
     }
     vfio_teardown_msi(vdev);
     vfio_unmap_bars(vdev);
-    g_free(vdev->emulated_config_bits);
-    g_free(vdev->rom);
-    vfio_put_device(vdev);
-    vfio_put_group(group);
 }
 
 static void vfio_pci_reset(DeviceState *dev)
@@ -3439,6 +3442,7 @@ static const TypeInfo vfio_pci_dev_info = {
     .instance_size = sizeof(VFIOPCIDevice),
     .class_init = vfio_pci_dev_class_init,
     .instance_init = vfio_instance_init,
+    .instance_finalize = vfio_instance_finalize,
 };
 
 static void register_vfio_pci_dev_type(void)
-- 
1.8.3.1