[Qemu-devel] [PATCH v5 6/9] acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API

[Igor Mammedov <imammedo@redhat.com>]

Split piix4_device_hotplug() into hotplug/unplug callbacks
and register them as "hotplug-handler" interface implementation of
PIIX4_PM device.

Replace pci_bus_hotplug() wiring with setting link on
PCI BUS "hotplug-handler" property to PIIX4_PM device.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* use error_abort to make error handling less verbose
---
 hw/acpi/pcihp.c         |   43 +++++++++++++++++++++++++++++++------------
 hw/acpi/piix4.c         |   36 ++++++++++++++++++++++--------------
 include/hw/acpi/acpi.h  |    1 +
 include/hw/acpi/pcihp.h |   10 ++++++----
 4 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 3bd5a06..e4ff732 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -46,6 +46,7 @@
 # define ACPI_PCIHP_DPRINTF(format, ...)     do { } while (0)
 #endif
 
+#define ACPI_PCI_HOTPLUG_STATUS 2
 #define ACPI_PCIHP_ADDR 0xae00
 #define ACPI_PCIHP_SIZE 0x0014
 #define ACPI_PCIHP_LEGACY_SIZE 0x000f
@@ -179,29 +180,47 @@ void acpi_pcihp_reset(AcpiPciHpState *s)
     acpi_pcihp_update(s);
 }
 
-int acpi_pcihp_device_hotplug(AcpiPciHpState *s, PCIDevice *dev,
-                              PCIHotplugState state)
+void acpi_pcihp_device_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+                               DeviceState *dev, Error **errp)
 {
-    int slot = PCI_SLOT(dev->devfn);
-    int bsel = acpi_pcihp_get_bsel(dev->bus);
+    PCIDevice *pdev = PCI_DEVICE(dev);
+    int slot = PCI_SLOT(pdev->devfn);
+    int bsel = acpi_pcihp_get_bsel(pdev->bus);
     if (bsel < 0) {
-        return -1;
+        error_setg(errp, "Not supported bus. Bus doesn't have propery '"
+                   ACPI_PCIHP_PROP_BSEL "' set.");
+        return;
     }
 
     /* Don't send event when device is enabled during qemu machine creation:
      * it is present on boot, no hotplug event is necessary. We do send an
      * event when the device is disabled later. */
-    if (state == PCI_COLDPLUG_ENABLED) {
-        return 0;
+    if (!dev->hotplugged) {
+        return;
     }
 
-    if (state == PCI_HOTPLUG_ENABLED) {
-        s->acpi_pcihp_pci_status[bsel].up |= (1U << slot);
-    } else {
-        s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
+    s->acpi_pcihp_pci_status[bsel].up |= (1U << slot);
+
+    ar->gpe.sts[0] |= ACPI_PCI_HOTPLUG_STATUS;
+    acpi_update_sci(ar, irq);
+}
+
+void acpi_pcihp_device_unplug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+                                 DeviceState *dev, Error **errp)
+{
+    PCIDevice *pdev = PCI_DEVICE(dev);
+    int slot = PCI_SLOT(pdev->devfn);
+    int bsel = acpi_pcihp_get_bsel(pdev->bus);
+    if (bsel < 0) {
+        error_setg(errp, "Not supported bus. Bus doesn't have propery '"
+                   ACPI_PCIHP_PROP_BSEL "' set.");
+        return;
     }
 
-    return 0;
+    s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
+
+    ar->gpe.sts[0] |= ACPI_PCI_HOTPLUG_STATUS;
+    acpi_update_sci(ar, irq);
 }
 
 static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 077776a..9f21653 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -32,6 +32,7 @@
 #include "hw/acpi/piix4.h"
 #include "hw/acpi/pcihp.h"
 #include "hw/acpi/cpu_hotplug.h"
+#include "hw/hotplug.h"
 
 //#define DEBUG
 
@@ -44,8 +45,6 @@
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
 
-#define PIIX4_PCI_HOTPLUG_STATUS 2
-
 struct pci_status {
     uint32_t up; /* deprecated, maintained for migration compatibility */
     uint32_t down;
@@ -311,24 +310,26 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque)
     acpi_pm1_evt_power_down(&s->ar);
 }
 
-static int piix4_acpi_pci_hotplug(DeviceState *qdev, PCIDevice *dev,
-                                  PCIHotplugState state)
+static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
 {
-    PIIX4PMState *s = PIIX4_PM(qdev);
-    int ret = acpi_pcihp_device_hotplug(&s->acpi_pci_hotplug, dev, state);
-    if (ret < 0) {
-        return ret;
-    }
-    s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
+    PIIX4PMState *s = PIIX4_PM(hotplug_dev);
+    acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev, errp);
+}
 
-    acpi_update_sci(&s->ar, s->irq);
-    return 0;
+static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    PIIX4PMState *s = PIIX4_PM(hotplug_dev);
+    acpi_pcihp_device_unplug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
+                                errp);
 }
 
-static void piix4_update_bus_hotplug(PCIBus *bus, void *opaque)
+static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque)
 {
     PIIX4PMState *s = opaque;
-    pci_bus_hotplug(bus, piix4_acpi_pci_hotplug, DEVICE(s));
+
+    qbus_set_hotplug_handler(BUS(pci_bus), DEVICE(s), &error_abort);
 }
 
 static void piix4_pm_machine_ready(Notifier *n, void *opaque)
@@ -535,6 +536,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
     k->init = piix4_pm_initfn;
     k->config_write = pm_write_config;
@@ -551,6 +553,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
      */
     dc->cannot_instantiate_with_device_add_yet = true;
     dc->hotpluggable = false;
+    hc->plug = piix4_pci_device_plug_cb;
+    hc->unplug = piix4_pci_device_unplug_cb;
 }
 
 static const TypeInfo piix4_pm_info = {
@@ -558,6 +562,10 @@ static const TypeInfo piix4_pm_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIX4PMState),
     .class_init    = piix4_pm_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void piix4_pm_register_types(void)
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 3e53297..a9fae9d 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -24,6 +24,7 @@
 #include "qemu/notify.h"
 #include "qemu/option.h"
 #include "exec/memory.h"
+#include "hw/irq.h"
 
 /* from linux include/acpi/actype.h */
 /* Default ACPI register widths */
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index 0a90e4a..9323838 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -29,7 +29,8 @@
 
 #include <inttypes.h>
 #include <qemu/typedefs.h>
-#include "hw/pci/pci.h" /* for PCIHotplugState */
+#include "hw/acpi/acpi.h"
+#include "migration/vmstate.h"
 
 typedef struct AcpiPciHpPciStatus {
     uint32_t up;
@@ -52,9 +53,10 @@ typedef struct AcpiPciHpState {
 void acpi_pcihp_init(AcpiPciHpState *, PCIBus *root,
                      MemoryRegion *address_space_io, bool bridges_enabled);
 
-/* Invoke on device hotplug */
-int acpi_pcihp_device_hotplug(AcpiPciHpState *, PCIDevice *,
-                              PCIHotplugState state);
+void acpi_pcihp_device_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+                               DeviceState *dev, Error **errp);
+void acpi_pcihp_device_unplug_cb(ACPIREGS *ar, qemu_irq irq, AcpiPciHpState *s,
+                                 DeviceState *dev, Error **errp);
 
 /* Called on reset */
 void acpi_pcihp_reset(AcpiPciHpState *s);
-- 
1.7.1