From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gP7oW-0000hL-9b for qemu-devel@nongnu.org; Tue, 20 Nov 2018 10:19:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gP7oS-0007MM-6Y for qemu-devel@nongnu.org; Tue, 20 Nov 2018 10:19:44 -0500 Date: Tue, 20 Nov 2018 16:19:30 +0100 From: Igor Mammedov Message-ID: <20181120161930.62bc45bf@redhat.com> In-Reply-To: <20181120110427.7321-11-david@redhat.com> References: <20181120110427.7321-1-david@redhat.com> <20181120110427.7321-11-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 10/11] pci/shpc: perform unplug via the hotplug handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-devel@nongnu.org, "Michael S . Tsirkin" , David Gibson , Greg Kurz , Marcel Apfelbaum , Cornelia Huck , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson , Collin Walling , Thomas Huth , qemu-ppc@nongnu.org On Tue, 20 Nov 2018 12:04:26 +0100 David Hildenbrand wrote: > Introduce and use the "unplug" callback. > > This is a preparation for multi-stage hotplug handlers, whereby the bus > hotplug handler is overwritten by the machine hotplug handler. This handler > will then pass control to the bus hotplug handler. So to get this running > cleanly, we also have to make sure to go via the hotplug handler chain when > actually unplugging a device after an unplug request. Lookup the hotplug > handler and call "unplug". > > Reviewed-by: David Gibson > Signed-off-by: David Hildenbrand Reviewed-by: Igor Mammedov > --- > hw/pci-bridge/pci_bridge_dev.c | 10 ++++++++++ > hw/pci-bridge/pcie_pci_bridge.c | 1 + > hw/pci/shpc.c | 11 ++++++++++- > include/hw/pci/pci_bridge.h | 2 ++ > include/hw/pci/shpc.h | 2 ++ > 5 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > index fa0be13ac4..ff6b8323da 100644 > --- a/hw/pci-bridge/pci_bridge_dev.c > +++ b/hw/pci-bridge/pci_bridge_dev.c > @@ -219,6 +219,15 @@ void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > shpc_device_plug_cb(hotplug_dev, dev, errp); > } > > +void pci_bridge_dev_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev); > + > + g_assert(shpc_present(pci_hotplug_dev)); > + shpc_device_unplug_cb(hotplug_dev, dev, errp); > +} > + > void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -251,6 +260,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data) > dc->vmsd = &pci_bridge_dev_vmstate; > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > hc->plug = pci_bridge_dev_plug_cb; > + hc->unplug = pci_bridge_dev_unplug_cb; > hc->unplug_request = pci_bridge_dev_unplug_request_cb; > } > > diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c > index 0ffea680d5..d491b40d04 100644 > --- a/hw/pci-bridge/pcie_pci_bridge.c > +++ b/hw/pci-bridge/pcie_pci_bridge.c > @@ -154,6 +154,7 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data) > dc->reset = &pcie_pci_bridge_reset; > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > hc->plug = pci_bridge_dev_plug_cb; > + hc->unplug = pci_bridge_dev_unplug_cb; > hc->unplug_request = pci_bridge_dev_unplug_request_cb; > } > > diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c > index 098ffaef1d..5de905cb56 100644 > --- a/hw/pci/shpc.c > +++ b/hw/pci/shpc.c > @@ -238,6 +238,7 @@ static void shpc_invalid_command(SHPCDevice *shpc) > > static void shpc_free_devices_in_slot(SHPCDevice *shpc, int slot) > { > + HotplugHandler *hotplug_ctrl; > int devfn; > int pci_slot = SHPC_IDX_TO_PCI(slot); > for (devfn = PCI_DEVFN(pci_slot, 0); > @@ -245,7 +246,9 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc, int slot) > ++devfn) { > PCIDevice *affected_dev = shpc->sec_bus->devices[devfn]; > if (affected_dev) { > - object_unparent(OBJECT(affected_dev)); > + hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(affected_dev)); > + hotplug_handler_unplug(hotplug_ctrl, DEVICE(affected_dev), > + &error_abort); > } > } > } > @@ -540,6 +543,12 @@ void shpc_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > shpc_interrupt_update(pci_hotplug_dev); > } > > +void shpc_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + object_unparent(OBJECT(dev)); > +} > + > void shpc_device_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 6e37c7551a..ba488818d2 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -101,6 +101,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev); > > void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp); > +void pci_bridge_dev_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp); > void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp); > > diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h > index 71293aca58..18f6ec1cd5 100644 > --- a/include/hw/pci/shpc.h > +++ b/include/hw/pci/shpc.h > @@ -47,6 +47,8 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len); > > void shpc_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp); > +void shpc_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp); > void shpc_device_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp); >