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".
Signed-off-by: David Hildenbrand <david@redhat.com>
---
hw/ppc/spapr_pci.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 58afa46204..64b8591023 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1370,18 +1370,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
/* Callback to be called during DRC release. */
void spapr_phb_remove_pci_device_cb(DeviceState *dev)
{
- /* some version guests do not wait for completion of a device
- * cleanup (generally done asynchronously by the kernel) before
- * signaling to QEMU that the device is safe, but instead sleep
- * for some 'safe' period of time. unfortunately on a busy host
- * this sleep isn't guaranteed to be long enough, resulting in
- * bad things like IRQ lines being left asserted during final
- * device removal. to deal with this we call reset just prior
- * to finalizing the device, which will put the device back into
- * an 'idle' state, as the device cleanup code expects.
- */
- pci_device_reset(PCI_DEVICE(dev));
- object_unparent(OBJECT(dev));
+ HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
+
+ hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
}
static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
@@ -1490,6 +1481,23 @@ out:
}
}
+static void spapr_pci_unplug(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev, Error **errp)
+{
+ /* some version guests do not wait for completion of a device
+ * cleanup (generally done asynchronously by the kernel) before
+ * signaling to QEMU that the device is safe, but instead sleep
+ * for some 'safe' period of time. unfortunately on a busy host
+ * this sleep isn't guaranteed to be long enough, resulting in
+ * bad things like IRQ lines being left asserted during final
+ * device removal. to deal with this we call reset just prior
+ * to finalizing the device, which will put the device back into
+ * an 'idle' state, as the device cleanup code expects.
+ */
+ pci_device_reset(PCI_DEVICE(plugged_dev));
+ object_unparent(OBJECT(plugged_dev));
+}
+
static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
DeviceState *plugged_dev, Error **errp)
{
@@ -1965,6 +1973,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
dc->user_creatable = true;
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
hp->plug = spapr_pci_plug;
+ hp->unplug = spapr_pci_unplug;
hp->unplug_request = spapr_pci_unplug_request;
}
--
2.17.2
On Mon, 5 Nov 2018 11:20:44 +0100
David Hildenbrand <david@redhat.com> 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".
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/spapr_pci.c | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58afa46204..64b8591023 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1370,18 +1370,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
> /* Callback to be called during DRC release. */
> void spapr_phb_remove_pci_device_cb(DeviceState *dev)
> {
> - /* some version guests do not wait for completion of a device
> - * cleanup (generally done asynchronously by the kernel) before
> - * signaling to QEMU that the device is safe, but instead sleep
> - * for some 'safe' period of time. unfortunately on a busy host
> - * this sleep isn't guaranteed to be long enough, resulting in
> - * bad things like IRQ lines being left asserted during final
> - * device removal. to deal with this we call reset just prior
> - * to finalizing the device, which will put the device back into
> - * an 'idle' state, as the device cleanup code expects.
> - */
> - pci_device_reset(PCI_DEVICE(dev));
> - object_unparent(OBJECT(dev));
> + HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +
> + hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
> }
>
> static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
> @@ -1490,6 +1481,23 @@ out:
> }
> }
>
> +static void spapr_pci_unplug(HotplugHandler *plug_handler,
> + DeviceState *plugged_dev, Error **errp)
> +{
> + /* some version guests do not wait for completion of a device
> + * cleanup (generally done asynchronously by the kernel) before
> + * signaling to QEMU that the device is safe, but instead sleep
> + * for some 'safe' period of time. unfortunately on a busy host
> + * this sleep isn't guaranteed to be long enough, resulting in
> + * bad things like IRQ lines being left asserted during final
> + * device removal. to deal with this we call reset just prior
> + * to finalizing the device, which will put the device back into
> + * an 'idle' state, as the device cleanup code expects.
> + */
> + pci_device_reset(PCI_DEVICE(plugged_dev));
> + object_unparent(OBJECT(plugged_dev));
> +}
> +
> static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> DeviceState *plugged_dev, Error **errp)
> {
> @@ -1965,6 +1973,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> dc->user_creatable = true;
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> hp->plug = spapr_pci_plug;
> + hp->unplug = spapr_pci_unplug;
> hp->unplug_request = spapr_pci_unplug_request;
> }
>
On 05.11.18 11:31, Greg Kurz wrote: > On Mon, 5 Nov 2018 11:20:44 +0100 > David Hildenbrand <david@redhat.com> 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". >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- > > Reviewed-by: Greg Kurz <groug@kaod.org> BTW, sorry for dropping you RB! (realized it just now) -- Thanks, David / dhildenb
On Mon, Nov 05, 2018 at 11:20:44AM +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".
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr_pci.c | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58afa46204..64b8591023 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1370,18 +1370,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
> /* Callback to be called during DRC release. */
> void spapr_phb_remove_pci_device_cb(DeviceState *dev)
> {
> - /* some version guests do not wait for completion of a device
> - * cleanup (generally done asynchronously by the kernel) before
> - * signaling to QEMU that the device is safe, but instead sleep
> - * for some 'safe' period of time. unfortunately on a busy host
> - * this sleep isn't guaranteed to be long enough, resulting in
> - * bad things like IRQ lines being left asserted during final
> - * device removal. to deal with this we call reset just prior
> - * to finalizing the device, which will put the device back into
> - * an 'idle' state, as the device cleanup code expects.
> - */
> - pci_device_reset(PCI_DEVICE(dev));
> - object_unparent(OBJECT(dev));
> + HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +
> + hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
> }
>
> static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
> @@ -1490,6 +1481,23 @@ out:
> }
> }
>
> +static void spapr_pci_unplug(HotplugHandler *plug_handler,
> + DeviceState *plugged_dev, Error **errp)
> +{
> + /* some version guests do not wait for completion of a device
> + * cleanup (generally done asynchronously by the kernel) before
> + * signaling to QEMU that the device is safe, but instead sleep
> + * for some 'safe' period of time. unfortunately on a busy host
> + * this sleep isn't guaranteed to be long enough, resulting in
> + * bad things like IRQ lines being left asserted during final
> + * device removal. to deal with this we call reset just prior
> + * to finalizing the device, which will put the device back into
> + * an 'idle' state, as the device cleanup code expects.
> + */
> + pci_device_reset(PCI_DEVICE(plugged_dev));
> + object_unparent(OBJECT(plugged_dev));
> +}
> +
> static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> DeviceState *plugged_dev, Error **errp)
> {
> @@ -1965,6 +1973,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> dc->user_creatable = true;
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> hp->plug = spapr_pci_plug;
> + hp->unplug = spapr_pci_unplug;
> hp->unplug_request = spapr_pci_unplug_request;
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Hi All using mainline QEMU code and below boot up commands, the QEMU will be hang when do bootup, if I add the "-enable-kvm" parameter, it can boot up successfully, do you know the reason about it? I also try the "remotes/origin/stable-2.12" branch code using below same boot up commands, it does not have this hang issue. it seems the mainline code has some issue. ./qemu-system-aarch64 -m 1024 -cpu cortex-a57 -machine virt,gic-version=3 \ -smp 4 -nographic -kernel Image -append "root=/dev/sda1 console=ttyAMA0 sched_debug ignore_loglevel" \ -device virtio-scsi-device,id=scsi -drive file=./linaro.img,id=rootimg,cache=unsafe,if=none -device scsi-hd,drive=rootimg
On Mon, 5 Nov 2018 11:20:44 +0100
David Hildenbrand <david@redhat.com> 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".
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/ppc/spapr_pci.c | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58afa46204..64b8591023 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1370,18 +1370,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
> /* Callback to be called during DRC release. */
> void spapr_phb_remove_pci_device_cb(DeviceState *dev)
> {
> - /* some version guests do not wait for completion of a device
> - * cleanup (generally done asynchronously by the kernel) before
> - * signaling to QEMU that the device is safe, but instead sleep
> - * for some 'safe' period of time. unfortunately on a busy host
> - * this sleep isn't guaranteed to be long enough, resulting in
> - * bad things like IRQ lines being left asserted during final
> - * device removal. to deal with this we call reset just prior
> - * to finalizing the device, which will put the device back into
> - * an 'idle' state, as the device cleanup code expects.
> - */
> - pci_device_reset(PCI_DEVICE(dev));
> - object_unparent(OBJECT(dev));
> + HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +
> + hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
> }
>
> static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
> @@ -1490,6 +1481,23 @@ out:
> }
> }
>
> +static void spapr_pci_unplug(HotplugHandler *plug_handler,
> + DeviceState *plugged_dev, Error **errp)
> +{
> + /* some version guests do not wait for completion of a device
> + * cleanup (generally done asynchronously by the kernel) before
> + * signaling to QEMU that the device is safe, but instead sleep
> + * for some 'safe' period of time. unfortunately on a busy host
> + * this sleep isn't guaranteed to be long enough, resulting in
> + * bad things like IRQ lines being left asserted during final
> + * device removal. to deal with this we call reset just prior
> + * to finalizing the device, which will put the device back into
> + * an 'idle' state, as the device cleanup code expects.
> + */
> + pci_device_reset(PCI_DEVICE(plugged_dev));
> + object_unparent(OBJECT(plugged_dev));
> +}
> +
> static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> DeviceState *plugged_dev, Error **errp)
> {
> @@ -1965,6 +1973,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> dc->user_creatable = true;
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> hp->plug = spapr_pci_plug;
> + hp->unplug = spapr_pci_unplug;
> hp->unplug_request = spapr_pci_unplug_request;
> }
>
© 2016 - 2025 Red Hat, Inc.