[Qemu-devel] [PATCH 10/11] pci/pcie: convert PCIE hotplug to use hotplug-handler API

[Igor Mammedov]

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

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

Signed-off-by: Igor Mammedov <address@hidden>
---
 hw/pci/pcie.c         | 73 +++++++++++++++++++++++++++++++++------------------
 hw/pci/pcie_port.c    |  8 ++++++
 include/hw/pci/pcie.h |  5 ++++
 3 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index ca60cf2..d4e978c 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -26,6 +26,7 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pcie_regs.h"
 #include "qemu/range.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_PCIE
 #ifdef DEBUG_PCIE
@@ -216,28 +217,20 @@ static void pcie_cap_slot_event(PCIDevice *dev, 
PCIExpressHotPlugEvent event)
     hotplug_event_notify(dev);
 }
 
-static int pcie_cap_slot_hotplug(DeviceState *qdev,
-                                 PCIDevice *pci_dev, PCIHotplugState state)
+static void pcie_cap_slot_hotplug_common(PCIDevice *hotplug_dev,
+                                         DeviceState *dev,
+                                         uint8_t **exp_cap, Error **errp)
 {
-    PCIDevice *d = PCI_DEVICE(qdev);
-    uint8_t *exp_cap = d->config + d->exp.exp_cap;
-    uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
-
-    /* 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) {
-        pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
-                                   PCI_EXP_SLTSTA_PDS);
-        return 0;
-    }
+    PCIDevice *pci_dev = PCI_DEVICE(dev);
+    *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
+    uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
 
     PCIE_DEV_PRINTF(pci_dev, "hotplug state: %d\n", state);
     if (sltsta & PCI_EXP_SLTSTA_EIS) {
         /* the slot is electromechanically locked.
          * This error is propagated up to qdev and then to HMP/QMP.
          */
-        return -EBUSY;
+        error_setg_errno(errp, -EBUSY, "slot is electromechanically locked");
     }
 
     /* TODO: multifunction hot-plug.
@@ -245,18 +238,40 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
      * hot plugged/unplugged.
      */
     assert(PCI_FUNC(pci_dev->devfn) == 0);
+}
 
-    if (state == PCI_HOTPLUG_ENABLED) {
+void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                              Error **errp)
+{
+    uint8_t *exp_cap;
+
+    pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
+
+    /* 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 (!dev->hotplugged) {
         pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
                                    PCI_EXP_SLTSTA_PDS);
-        pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
-    } else {
-        object_unparent(OBJECT(pci_dev));
-        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
-                                     PCI_EXP_SLTSTA_PDS);
-        pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
+        return;
     }
-    return 0;
+
+    pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+                               PCI_EXP_SLTSTA_PDS);
+    pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
+}
+
+void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                                 Error **errp)
+{
+    uint8_t *exp_cap;
+
+    pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
+
+    object_unparent(OBJECT(dev));
+    pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
+                                 PCI_EXP_SLTSTA_PDS);
+    pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
 }
 
 /* pci express slot for pci express root/downstream port
@@ -264,6 +279,8 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
 void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
 {
     uint32_t pos = dev->exp.exp_cap;
+    BusState *bus = BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev)));
+    Error *local_error = NULL;
 
     pci_word_test_and_set_mask(dev->config + pos + PCI_EXP_FLAGS,
                                PCI_EXP_FLAGS_SLOT);
@@ -305,8 +322,14 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
 
     dev->exp.hpev_notified = false;
 
-    pci_bus_hotplug(pci_bridge_get_sec_bus(PCI_BRIDGE(dev)),
-                    pcie_cap_slot_hotplug, &dev->qdev);
+    object_property_set_link(OBJECT(bus), OBJECT(dev),
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, &local_error);
+    if (error_is_set(&local_error)) {
+        qerror_report_err(local_error);
+        error_free(local_error);
+        abort();
+    }
+    bus->allow_hotplug = 1;
 }
 
 void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index 2adb030..fa24877 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -19,6 +19,7 @@
  */
 
 #include "hw/pci/pcie_port.h"
+#include "hw/hotplug.h"
 
 void pcie_port_init_reg(PCIDevice *d)
 {
@@ -149,8 +150,11 @@ static Property pcie_slot_props[] = {
 static void pcie_slot_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     dc->props = pcie_slot_props;
+    hc->plug = pcie_cap_slot_hotplug_cb;
+    hc->unplug = pcie_cap_slot_hot_unplug_cb;
 }
 
 static const TypeInfo pcie_slot_type_info = {
@@ -159,6 +163,10 @@ static const TypeInfo pcie_slot_type_info = {
     .instance_size = sizeof(PCIESlot),
     .abstract = true,
     .class_init = pcie_slot_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
 };
 
 static void pcie_port_register_types(void)
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 1966169..b0bf7e3 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -25,6 +25,7 @@
 #include "hw/pci/pci_regs.h"
 #include "hw/pci/pcie_regs.h"
 #include "hw/pci/pcie_aer.h"
+#include "hw/hotplug.h"
 
 typedef enum {
     /* for attention and power indicator */
@@ -122,4 +123,8 @@ extern const VMStateDescription vmstate_pcie_device;
     .offset     = vmstate_offset_value(_state, _field, PCIDevice),   \
 }
 
+void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                              Error **errp);
+void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                                 Error **errp);
 #endif /* QEMU_PCIE_H */
-- 
1.8.3.1