[Qemu-devel] [PATCH 8/8] spapr: Allow hot plug/unplug of PCI bridges and devices under PCI bridges

From: David Gibson <david@gibson.dropbear.id.au>
To: qemu-ppc@nongnu.org
Cc: mst@redhat.com, qemu-devel@nongnu.org, groug@kaod.org,
	clg@kaod.org, mdroth@linux.ibm.com,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [Qemu-devel] [PATCH 8/8] spapr: Allow hot plug/unplug of PCI bridges and devices under PCI bridges
Date: Thu, 23 May 2019 15:29:18 +1000	[thread overview]
Message-ID: <20190523052918.1129-8-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20190523052918.1129-1-david@gibson.dropbear.id.au>

The pseries machine type already allows PCI hotplug and unplug via the
PAPR mechanism, but only on the root bus of each PHB.  This patch extends
this to allow PCI to PCI bridges to be hotplugged, and devices to be
hotplugged or unplugged under P2P bridges.

For now we disallow hot unplugging P2P bridges.  I tried doing that, but
haven't managed to get it working, I think due to some guest side problems
that need further investigation.

To do this we dynamically construct DRCs when bridges are hot (or cold)
added, which can in turn be used to hotplug devices under the bridge.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_pci.c | 115 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 102 insertions(+), 13 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 94691fcfc2..a4d5e46525 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1257,30 +1257,53 @@ static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
     return drc_from_devfn(phb, chassis, dev->devfn);
 }
 
-static void add_drcs(SpaprPhbState *phb)
+static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
 {
+    Object *owner;
     int i;
+    uint8_t chassis;
+    Error *local_err = NULL;
 
     if (!phb->dr_enabled) {
         return;
     }
 
+    chassis = chassis_from_bus(bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (pci_bus_is_root(bus)) {
+        owner = OBJECT(phb);
+    } else {
+        owner = OBJECT(pci_bridge_get_device(bus));
+    }
+
     for (i = 0; i < PCI_SLOT_MAX * PCI_FUNC_MAX; i++) {
-        spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
-                               drc_id_from_devfn(phb, 0, i));
+        spapr_dr_connector_new(owner, TYPE_SPAPR_DRC_PCI,
+                               drc_id_from_devfn(phb, chassis, i));
     }
 }
 
-static void remove_drcs(SpaprPhbState *phb)
+static void remove_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
 {
     int i;
+    uint8_t chassis;
+    Error *local_err = NULL;
 
     if (!phb->dr_enabled) {
         return;
     }
 
+    chassis = chassis_from_bus(bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     for (i = PCI_SLOT_MAX * PCI_FUNC_MAX - 1; i >= 0; i--) {
-        SpaprDrc *drc = drc_from_devfn(phb, 0, i);
+        SpaprDrc *drc = drc_from_devfn(phb, chassis, i);
 
         if (drc) {
             object_unparent(OBJECT(drc));
@@ -1325,6 +1348,7 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
         .sphb = sphb,
         .err = 0,
     };
+    int ret;
 
     _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
                           RESOURCE_CELLS_ADDRESS));
@@ -1339,6 +1363,12 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
         }
     }
 
+    ret = spapr_dt_drc(fdt, offset, OBJECT(bus->parent_dev),
+                       SPAPR_DR_CONNECTOR_TYPE_PCI);
+    if (ret) {
+        return ret;
+    }
+
     return offset;
 }
 
@@ -1483,11 +1513,26 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
     return 0;
 }
 
+static void spapr_pci_bridge_plug(SpaprPhbState *phb,
+                                  PCIBridge *bridge,
+                                  Error **errp)
+{
+    Error *local_err = NULL;
+    PCIBus *bus = pci_bridge_get_sec_bus(bridge);
+
+    add_drcs(phb, bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
 static void spapr_pci_plug(HotplugHandler *plug_handler,
                            DeviceState *plugged_dev, Error **errp)
 {
     SpaprPhbState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
     PCIDevice *pdev = PCI_DEVICE(plugged_dev);
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(plugged_dev);
     SpaprDrc *drc = drc_from_dev(phb, pdev);
     Error *local_err = NULL;
     PCIBus *bus = PCI_BUS(qdev_get_parent_bus(DEVICE(pdev)));
@@ -1509,6 +1554,14 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
 
     g_assert(drc);
 
+    if (pc->is_bridge) {
+        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev), &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
     /* Following the QEMU convention used for PCIe multifunction
      * hotplug, we do not allow functions to be hotplugged to a
      * slot that already has function 0 present
@@ -1559,9 +1612,26 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void spapr_pci_bridge_unplug(SpaprPhbState *phb,
+                                    PCIBridge *bridge,
+                                    Error **errp)
+{
+    Error *local_err = NULL;
+    PCIBus *bus = pci_bridge_get_sec_bus(bridge);
+
+    remove_drcs(phb, bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
 static void spapr_pci_unplug(HotplugHandler *plug_handler,
                              DeviceState *plugged_dev, Error **errp)
 {
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(plugged_dev);
+    SpaprPhbState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
+
     /* 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
@@ -1573,6 +1643,16 @@ static void spapr_pci_unplug(HotplugHandler *plug_handler,
      * an 'idle' state, as the device cleanup code expects.
      */
     pci_device_reset(PCI_DEVICE(plugged_dev));
+
+    if (pc->is_bridge) {
+        Error *local_err = NULL;
+        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev), &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+        }
+        return;
+    }
+
     object_property_set_bool(OBJECT(plugged_dev), false, "realized", NULL);
 }
 
@@ -1593,6 +1673,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
     g_assert(drc->dev == plugged_dev);
 
     if (!spapr_drc_unplug_requested(drc)) {
+        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(plugged_dev);
         uint32_t slotnr = PCI_SLOT(pdev->devfn);
         SpaprDrc *func_drc;
         SpaprDrcClass *func_drck;
@@ -1606,6 +1687,10 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
             return;
         }
 
+        if (pc->is_bridge) {
+            error_setg(errp, "PCI: Hot unplug of PCI bridges not supported");
+        }
+
         /* ensure any other present functions are pending unplug */
         if (PCI_FUNC(pdev->devfn) == 0) {
             for (i = 1; i < 8; i++) {
@@ -1658,6 +1743,7 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
     SpaprTceTable *tcet;
     int i;
     const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+    Error *local_err = NULL;
 
     spapr_phb_nvgpu_free(sphb);
 
@@ -1678,7 +1764,11 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
         }
     }
 
-    remove_drcs(sphb);
+    remove_drcs(sphb, phb->bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
         if (sphb->lsi_table[i].irq) {
@@ -1743,6 +1833,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     uint64_t msi_window_size = 4096;
     SpaprTceTable *tcet;
     const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+    Error *local_err = NULL;
 
     if (!spapr) {
         error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries machine");
@@ -1879,7 +1970,6 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     /* Initialize the LSI table */
     for (i = 0; i < PCI_NUM_PINS; i++) {
         uint32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i;
-        Error *local_err = NULL;
 
         if (smc->legacy_irq_allocation) {
             irq = spapr_irq_findone(spapr, &local_err);
@@ -1904,7 +1994,11 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     }
 
     /* allocate connectors for child PCI devices */
-    add_drcs(sphb);
+    add_drcs(sphb, phb->bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto unrealize;
+    }
 
     /* DMA setup */
     for (i = 0; i < windows_supported; ++i) {
@@ -2320,11 +2414,6 @@ int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
         return ret;
     }
 
-    ret = spapr_dt_drc(fdt, bus_off, OBJECT(phb), SPAPR_DR_CONNECTOR_TYPE_PCI);
-    if (ret) {
-        return ret;
-    }
-
     spapr_phb_nvgpu_populate_dt(phb, fdt, bus_off, &errp);
     if (errp) {
         error_report_err(errp);
-- 
2.21.0


