On 02/13/2015 09:57 AM, Paolo Bonzini wrote: > 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 <pbonz...@redhat.com>
Reviewed-by: Matthew Rosato <mjros...@linux.vnet.ibm.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); > >