Fix hotplug/hotunplug issue about virtio 1.0 devices

Hi Michael and Gerd,

I found an hotplug/hotunplug issue about virtio 1.0 devices and trying to fix it.
The bug description is:
(qemu) device_add virtio-gpu-pci,id=gpu0
(qemu) device_del gpu0

(qemu) device_add virtio-gpu-pci,id=gpu0
Duplicate ID 'gpu0' for device
Try "help device_add" for more information

My fix looks like this:

hw/virtio/virtio-pci.c +6 -0

index 283401a..098fc83 100644
@@ -1413,6 +1413,12 @@ static void
 virtio_pci_modern_region_map(VirtIOPCIProxy *proxy,
     virtio_pci_add_mem_cap(proxy, cap);
 }
 
+static void virtio_pci_modern_region_unmap(VirtIOPCIProxy *proxy,
+                                           VirtIOPCIRegion *region)
+{
+    memory_region_del_subregion(&proxy->modern_bar, &region->mr);
+}
+
 /* This is called by virtio-bus just after the device is plugged. */
 static void virtio_pci_device_plugged(DeviceState *d, Error **errp)

{

hw/virtio/virtio-pci.c +10 -0

@@ -1584,6 +1590,16 @@ static void virtio_pci_exit(PCIDevice *pci_dev)
 
     msix_uninit_exclusive_bar(pci_dev);
     address_space_destroy(&proxy->modern_as);
+    object_unparent(OBJECT(&proxy->modern_cfg));
+
+    bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    if (modern) {
+        virtio_pci_modern_region_unmap(proxy, &proxy->common);
+        virtio_pci_modern_region_unmap(proxy, &proxy->isr);
+        virtio_pci_modern_region_unmap(proxy, &proxy->device);
+        virtio_pci_modern_region_unmap(proxy, &proxy->notify);
+    }
+    memory_region_unref(&proxy->modern_bar);
 }
 
 static void virtio_pci_reset(DeviceState *qdev)

But after applying the fix, I got the following errors:
(qemu) device_add virtio-gpu-pci,id=gpu0
(qemu) device_del gpu0
(qemu) **
ERROR:qom/object.c:825:object_unref: assertion failed: (obj->ref > 0)
......

I think because of the grace period of rcu, the function memory_region_unref(as->root) in do_address_space_destroy isn't performed immediately,
The do_address_space_destroy is always performed after virtio_pci_exit, That caused 'assertion failed: (obj->ref > 0)'.
I have no idea whether my guess is correct or in correct, and don't know how to avoid this assertion failure in this situation.
Does my fix make sense ? May I have your ideas or suggestions?

Thanks!
Lin

Quoted 7 lines · Lin Ma

Hi Michael and Gerd,

I found an hotplug/hotunplug issue about virtio 1.0 devices and trying to fix it.
The bug description is:
(qemu) device_add virtio-gpu-pci,id=gpu0
(qemu) device_del gpu0

At this point, you must wait for guest to ack device removal.

Quoted 64 lines

(qemu) device_add virtio-gpu-pci,id=gpu0
Duplicate ID 'gpu0' for device
Try "help device_add" for more information

My fix looks like this:

hw/virtio/virtio-pci.c +6 -0

index 283401a..098fc83 100644
@@ -1413,6 +1413,12 @@ static void
 virtio_pci_modern_region_map(VirtIOPCIProxy *proxy,
      virtio_pci_add_mem_cap(proxy, cap);
  }
 
+static void virtio_pci_modern_region_unmap(VirtIOPCIProxy *proxy,
+                                           VirtIOPCIRegion *region)
+{
+    memory_region_del_subregion(&proxy->modern_bar, &region->mr);
+}
+
  /* This is called by virtio-bus just after the device is plugged. */
  static void virtio_pci_device_plugged(DeviceState *d, Error **errp)

  {

hw/virtio/virtio-pci.c +10 -0

@@ -1584,6 +1590,16 @@ static void virtio_pci_exit(PCIDevice *pci_dev)
 
      msix_uninit_exclusive_bar(pci_dev);
      address_space_destroy(&proxy->modern_as);
+    object_unparent(OBJECT(&proxy->modern_cfg));
+
+    bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    if (modern) {
+        virtio_pci_modern_region_unmap(proxy, &proxy->common);
+        virtio_pci_modern_region_unmap(proxy, &proxy->isr);
+        virtio_pci_modern_region_unmap(proxy, &proxy->device);
+        virtio_pci_modern_region_unmap(proxy, &proxy->notify);
+    }
+    memory_region_unref(&proxy->modern_bar);
  }
 
  static void virtio_pci_reset(DeviceState *qdev)

But after applying the fix, I got the following errors:
(qemu) device_add virtio-gpu-pci,id=gpu0
(qemu) device_del gpu0
(qemu) **
ERROR:qom/object.c:825:object_unref: assertion failed: (obj->ref > 0)
......

I think because of the grace period of rcu, the function memory_region_unref(as->root) in do_address_space_destroy isn't performed immediately,
The do_address_space_destroy is always performed after virtio_pci_exit, That caused 'assertion failed: (obj->ref > 0)'.
I have no idea whether my guess is correct or in correct, and don't know how to avoid this assertion failure in this situation.
Does my fix make sense ? May I have your ideas or suggestions?

Thanks!
Lin