From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMJM5-000230-Cv for qemu-devel@nongnu.org; Fri, 13 Feb 2015 11:44:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMJM1-0007vx-9a for qemu-devel@nongnu.org; Fri, 13 Feb 2015 11:44:37 -0500 Received: from e8.ny.us.ibm.com ([32.97.182.138]:58664) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMJM0-0007vl-RF for qemu-devel@nongnu.org; Fri, 13 Feb 2015 11:44:33 -0500 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 13 Feb 2015 11:44:32 -0500 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id DCCD738C804F for ; Fri, 13 Feb 2015 11:44:28 -0500 (EST) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t1DGiS7E17105082 for ; Fri, 13 Feb 2015 16:44:28 GMT Received: from d01av03.pok.ibm.com (localhost [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t1DGiSbG014219 for ; Fri, 13 Feb 2015 11:44:28 -0500 Message-ID: <54DE29EB.7000709@linux.vnet.ibm.com> Date: Fri, 13 Feb 2015 11:44:27 -0500 From: Matthew Rosato MIME-Version: 1.0 References: <1423839431-3563-1-git-send-email-pbonzini@redhat.com> <1423839431-3563-3-git-send-email-pbonzini@redhat.com> In-Reply-To: <1423839431-3563-3-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] pci: split shpc_cleanup and shpc_free List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: alex.williamson@redhat.com, mst@redhat.com 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 Reviewed-by: Matthew Rosato > --- > 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); > >