[Qemu-devel] [PATCH 2/3] pci: split shpc_cleanup and shpc_free

[Paolo Bonzini <pbonzini@redhat.com>]

object_unparent should not be called until the parent device is going to be
destroyed.  Only remove the capability and do memory_region_del_subregion
at unrealize time.  Freeing the data structures is left in shpc_free, to
be called from the instance_finalize callback.

shpc_free follows the same coding style that Alex suggested for VFIO
(i.e. since a test for NULL is requested, clear the field at end).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci-bridge/pci_bridge_dev.c | 14 ++++++++++----
 hw/pci/shpc.c                  | 11 ++++++++++-
 include/hw/pci/shpc.h          |  1 +
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 252ea5e..36f73e1 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -97,6 +97,11 @@ static void pci_bridge_dev_exitfn(PCIDevice *dev)
     pci_bridge_exitfn(dev);
 }
 
+static void pci_bridge_dev_instance_finalize(Object *obj)
+{
+    shpc_free(PCI_DEVICE(obj));
+}
+
 static void pci_bridge_dev_write_config(PCIDevice *d,
                                         uint32_t address, uint32_t val, int len)
 {
@@ -154,10 +159,11 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pci_bridge_dev_info = {
-    .name          = TYPE_PCI_BRIDGE_DEV,
-    .parent        = TYPE_PCI_BRIDGE,
-    .instance_size = sizeof(PCIBridgeDev),
-    .class_init = pci_bridge_dev_class_init,
+    .name              = TYPE_PCI_BRIDGE_DEV,
+    .parent            = TYPE_PCI_BRIDGE,
+    .instance_size     = sizeof(PCIBridgeDev),
+    .class_init        = pci_bridge_dev_class_init,
+    .instance_finalize = pci_bridge_dev_instance_finalize,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
         { }
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 27c496e..5fd7f4b 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -663,13 +663,22 @@ void shpc_cleanup(PCIDevice *d, MemoryRegion *bar)
     SHPCDevice *shpc = d->shpc;
     d->cap_present &= ~QEMU_PCI_CAP_SHPC;
     memory_region_del_subregion(bar, &shpc->mmio);
-    object_unparent(OBJECT(&shpc->mmio));
     /* TODO: cleanup config space changes? */
+}
+
+void shpc_free(PCIDevice *d)
+{
+    SHPCDevice *shpc = d->shpc;
+    if (!shpc) {
+        return;
+    }
+    object_unparent(OBJECT(&shpc->mmio));
     g_free(shpc->config);
     g_free(shpc->cmask);
     g_free(shpc->wmask);
     g_free(shpc->w1cmask);
     g_free(shpc);
+    d->shpc = NULL;
 }
 
 void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
index 025bc5b..9bbea39 100644
--- a/include/hw/pci/shpc.h
+++ b/include/hw/pci/shpc.h
@@ -41,6 +41,7 @@ void shpc_reset(PCIDevice *d);
 int shpc_bar_size(PCIDevice *dev);
 int shpc_init(PCIDevice *dev, PCIBus *sec_bus, MemoryRegion *bar, unsigned off);
 void shpc_cleanup(PCIDevice *dev, MemoryRegion *bar);
+void shpc_free(PCIDevice *dev);
 void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len);
 
 
-- 
1.8.3.1