]> git.ipfire.org Git - thirdparty/qemu.git/blobdiff - hw/ppc/spapr_pci.c
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-4.1-20190612' into staging
[thirdparty/qemu.git] / hw / ppc / spapr_pci.c
index e594b781f93936138e809f2072651eb313913f7d..957ae88bbdf6b8ad8a15141b3b3c7953fddb3e35 100644 (file)
@@ -1174,8 +1174,8 @@ static const PCIClass pci_classes[] = {
     { "data-processing-controller", spc_subclass },
 };
 
-static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
-                                        uint8_t iface)
+static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
+                                      uint8_t iface)
 {
     const PCIClass *pclass;
     const PCISubClass *psubclass;
@@ -1217,77 +1217,223 @@ static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
     return name;
 }
 
-static gchar *pci_get_node_name(PCIDevice *dev)
+/*
+ * DRC helper functions
+ */
+
+static uint32_t drc_id_from_devfn(SpaprPhbState *phb,
+                                  uint8_t chassis, int32_t devfn)
 {
-    int slot = PCI_SLOT(dev->devfn);
-    int func = PCI_FUNC(dev->devfn);
-    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
-    const char *name;
+    return (phb->index << 16) | (chassis << 8) | devfn;
+}
 
-    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
-                                ccode & 0xff);
+static SpaprDrc *drc_from_devfn(SpaprPhbState *phb,
+                                uint8_t chassis, int32_t devfn)
+{
+    return spapr_drc_by_id(TYPE_SPAPR_DRC_PCI,
+                           drc_id_from_devfn(phb, chassis, devfn));
+}
 
-    if (func != 0) {
-        return g_strdup_printf("%s@%x,%x", name, slot, func);
+static uint8_t chassis_from_bus(PCIBus *bus, Error **errp)
+{
+    if (pci_bus_is_root(bus)) {
+        return 0;
+    } else {
+        PCIDevice *bridge = pci_bridge_get_device(bus);
+
+        return object_property_get_uint(OBJECT(bridge), "chassis_nr", errp);
+    }
+}
+
+static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
+{
+    Error *local_err = NULL;
+    uint8_t chassis = chassis_from_bus(pci_get_bus(dev), &local_err);
+
+    if (local_err) {
+        error_report_err(local_err);
+        return NULL;
+    }
+
+    return drc_from_devfn(phb, chassis, dev->devfn);
+}
+
+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 {
-        return g_strdup_printf("%s@%x", name, slot);
+        owner = OBJECT(pci_bridge_get_device(bus));
+    }
+
+    for (i = 0; i < PCI_SLOT_MAX * PCI_FUNC_MAX; i++) {
+        spapr_dr_connector_new(owner, TYPE_SPAPR_DRC_PCI,
+                               drc_id_from_devfn(phb, chassis, i));
+    }
+}
+
+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, chassis, i);
+
+        if (drc) {
+            object_unparent(OBJECT(drc));
+        }
     }
 }
 
-static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
-                                            PCIDevice *pdev);
+typedef struct PciWalkFdt {
+    void *fdt;
+    int offset;
+    SpaprPhbState *sphb;
+    int err;
+} PciWalkFdt;
+
+static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev,
+                               void *fdt, int parent_offset);
+
+static void spapr_dt_pci_device_cb(PCIBus *bus, PCIDevice *pdev,
+                                   void *opaque)
+{
+    PciWalkFdt *p = opaque;
+    int err;
+
+    if (p->err) {
+        /* Something's already broken, don't keep going */
+        return;
+    }
+
+    err = spapr_dt_pci_device(p->sphb, pdev, p->fdt, p->offset);
+    if (err < 0) {
+        p->err = err;
+    }
+}
+
+/* Augment PCI device node with bridge specific information */
+static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus *bus,
+                               void *fdt, int offset)
+{
+    PciWalkFdt cbinfo = {
+        .fdt = fdt,
+        .offset = offset,
+        .sphb = sphb,
+        .err = 0,
+    };
+    int ret;
+
+    _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
+                          RESOURCE_CELLS_ADDRESS));
+    _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
+                          RESOURCE_CELLS_SIZE));
+
+    if (bus) {
+        pci_for_each_device_reverse(bus, pci_bus_num(bus),
+                                    spapr_dt_pci_device_cb, &cbinfo);
+        if (cbinfo.err) {
+            return cbinfo.err;
+        }
+    }
+
+    ret = spapr_dt_drc(fdt, offset, OBJECT(bus->parent_dev),
+                       SPAPR_DR_CONNECTOR_TYPE_PCI);
+    if (ret) {
+        return ret;
+    }
+
+    return offset;
+}
 
-static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
-                                       SpaprPhbState *sphb)
+/* create OF node for pci device and required OF DT properties */
+static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev,
+                               void *fdt, int parent_offset)
 {
+    int offset;
+    const gchar *basename;
+    gchar *nodename;
+    int slot = PCI_SLOT(dev->devfn);
+    int func = PCI_FUNC(dev->devfn);
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     ResourceProps rp;
-    bool is_bridge = false;
-    int pci_status;
-    char *buf = NULL;
-    uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
+    SpaprDrc *drc = drc_from_dev(sphb, dev);
+    uint32_t vendor_id = pci_default_read_config(dev, PCI_VENDOR_ID, 2);
+    uint32_t device_id = pci_default_read_config(dev, PCI_DEVICE_ID, 2);
+    uint32_t revision_id = pci_default_read_config(dev, PCI_REVISION_ID, 1);
     uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
-    uint32_t max_msi, max_msix;
+    uint32_t irq_pin = pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1);
+    uint32_t subsystem_id = pci_default_read_config(dev, PCI_SUBSYSTEM_ID, 2);
+    uint32_t subsystem_vendor_id =
+        pci_default_read_config(dev, PCI_SUBSYSTEM_VENDOR_ID, 2);
+    uint32_t cache_line_size =
+        pci_default_read_config(dev, PCI_CACHE_LINE_SIZE, 1);
+    uint32_t pci_status = pci_default_read_config(dev, PCI_STATUS, 2);
+    gchar *loc_code;
+
+    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
+                                  ccode & 0xff);
 
-    if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
-        PCI_HEADER_TYPE_BRIDGE) {
-        is_bridge = true;
+    if (func != 0) {
+        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
+    } else {
+        nodename = g_strdup_printf("%s@%x", basename, slot);
     }
 
+    _FDT(offset = fdt_add_subnode(fdt, parent_offset, nodename));
+
+    g_free(nodename);
+
     /* in accordance with PAPR+ v2.7 13.6.3, Table 181 */
-    _FDT(fdt_setprop_cell(fdt, offset, "vendor-id",
-                          pci_default_read_config(dev, PCI_VENDOR_ID, 2)));
-    _FDT(fdt_setprop_cell(fdt, offset, "device-id",
-                          pci_default_read_config(dev, PCI_DEVICE_ID, 2)));
-    _FDT(fdt_setprop_cell(fdt, offset, "revision-id",
-                          pci_default_read_config(dev, PCI_REVISION_ID, 1)));
-    _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode));
-    if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) {
-        _FDT(fdt_setprop_cell(fdt, offset, "interrupts",
-                 pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)));
-    }
+    _FDT(fdt_setprop_cell(fdt, offset, "vendor-id", vendor_id));
+    _FDT(fdt_setprop_cell(fdt, offset, "device-id", device_id));
+    _FDT(fdt_setprop_cell(fdt, offset, "revision-id", revision_id));
 
-    if (!is_bridge) {
-        _FDT(fdt_setprop_cell(fdt, offset, "min-grant",
-            pci_default_read_config(dev, PCI_MIN_GNT, 1)));
-        _FDT(fdt_setprop_cell(fdt, offset, "max-latency",
-            pci_default_read_config(dev, PCI_MAX_LAT, 1)));
+    _FDT(fdt_setprop_cell(fdt, offset, "class-code", ccode));
+    if (irq_pin) {
+        _FDT(fdt_setprop_cell(fdt, offset, "interrupts", irq_pin));
     }
 
-    if (pci_default_read_config(dev, PCI_SUBSYSTEM_ID, 2)) {
-        _FDT(fdt_setprop_cell(fdt, offset, "subsystem-id",
-                 pci_default_read_config(dev, PCI_SUBSYSTEM_ID, 2)));
+    if (subsystem_id) {
+        _FDT(fdt_setprop_cell(fdt, offset, "subsystem-id", subsystem_id));
     }
 
-    if (pci_default_read_config(dev, PCI_SUBSYSTEM_VENDOR_ID, 2)) {
+    if (subsystem_vendor_id) {
         _FDT(fdt_setprop_cell(fdt, offset, "subsystem-vendor-id",
-                 pci_default_read_config(dev, PCI_SUBSYSTEM_VENDOR_ID, 2)));
+                              subsystem_vendor_id));
     }
 
-    _FDT(fdt_setprop_cell(fdt, offset, "cache-line-size",
-        pci_default_read_config(dev, PCI_CACHE_LINE_SIZE, 1)));
+    _FDT(fdt_setprop_cell(fdt, offset, "cache-line-size", cache_line_size));
+
 
     /* the following fdt cells are masked off the pci status register */
-    pci_status = pci_default_read_config(dev, PCI_STATUS, 2);
     _FDT(fdt_setprop_cell(fdt, offset, "devsel-speed",
                           PCI_STATUS_DEVSEL_MASK & pci_status));
 
@@ -1301,32 +1447,23 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
         _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
     }
 
-    _FDT(fdt_setprop_string(fdt, offset, "name",
-                            pci_find_device_name((ccode >> 16) & 0xff,
-                                                 (ccode >> 8) & 0xff,
-                                                 ccode & 0xff)));
-
-    buf = spapr_phb_get_loc_code(sphb, dev);
-    _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
-    g_free(buf);
+    loc_code = spapr_phb_get_loc_code(sphb, dev);
+    _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", loc_code));
+    g_free(loc_code);
 
-    if (drc_index) {
-        _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
+    if (drc) {
+        _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index",
+                              spapr_drc_index(drc)));
     }
 
-    _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
-                          RESOURCE_CELLS_ADDRESS));
-    _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
-                          RESOURCE_CELLS_SIZE));
-
     if (msi_present(dev)) {
-        max_msi = msi_nr_vectors_allocated(dev);
+        uint32_t max_msi = msi_nr_vectors_allocated(dev);
         if (max_msi) {
             _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
         }
     }
     if (msix_present(dev)) {
-        max_msix = dev->msix_entries_nr;
+        uint32_t max_msix = dev->msix_entries_nr;
         if (max_msix) {
             _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
         }
@@ -1342,22 +1479,19 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
     }
 
     spapr_phb_nvgpu_populate_pcidev_dt(dev, fdt, offset, sphb);
-}
-
-/* create OF node for pci device and required OF DT properties */
-static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
-                                     void *fdt, int node_offset)
-{
-    int offset;
-    gchar *nodename;
 
-    nodename = pci_get_node_name(dev);
-    _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
-    g_free(nodename);
-
-    spapr_populate_pci_child_dt(dev, fdt, offset, phb);
+    if (!pc->is_bridge) {
+        /* Properties only for non-bridges */
+        uint32_t min_grant = pci_default_read_config(dev, PCI_MIN_GNT, 1);
+        uint32_t max_latency = pci_default_read_config(dev, PCI_MAX_LAT, 1);
+        _FDT(fdt_setprop_cell(fdt, offset, "min-grant", min_grant));
+        _FDT(fdt_setprop_cell(fdt, offset, "max-latency", max_latency));
+        return offset;
+    } else {
+        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
 
-    return offset;
+        return spapr_dt_pci_bus(sphb, sec_bus, fdt, offset);
+    }
 }
 
 /* Callback to be called during DRC release. */
@@ -1369,33 +1503,6 @@ void spapr_phb_remove_pci_device_cb(DeviceState *dev)
     object_unparent(OBJECT(dev));
 }
 
-static SpaprDrc *spapr_phb_get_pci_func_drc(SpaprPhbState *phb,
-                                                    uint32_t busnr,
-                                                    int32_t devfn)
-{
-    return spapr_drc_by_id(TYPE_SPAPR_DRC_PCI,
-                           (phb->index << 16) | (busnr << 8) | devfn);
-}
-
-static SpaprDrc *spapr_phb_get_pci_drc(SpaprPhbState *phb,
-                                               PCIDevice *pdev)
-{
-    uint32_t busnr = pci_bus_num(PCI_BUS(qdev_get_parent_bus(DEVICE(pdev))));
-    return spapr_phb_get_pci_func_drc(phb, busnr, pdev->devfn);
-}
-
-static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
-                                            PCIDevice *pdev)
-{
-    SpaprDrc *drc = spapr_phb_get_pci_drc(phb, pdev);
-
-    if (!drc) {
-        return 0;
-    }
-
-    return spapr_drc_index(drc);
-}
-
 int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
                           void *fdt, int *fdt_start_offset, Error **errp)
 {
@@ -1403,16 +1510,31 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
     SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(plug_handler);
     PCIDevice *pdev = PCI_DEVICE(drc->dev);
 
-    *fdt_start_offset = spapr_create_pci_child_dt(sphb, pdev, fdt, 0);
+    *fdt_start_offset = spapr_dt_pci_device(sphb, pdev, fdt, 0);
     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);
-    SpaprDrc *drc = spapr_phb_get_pci_drc(phb, pdev);
+    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)));
     uint32_t slotnr = PCI_SLOT(pdev->devfn);
@@ -1433,6 +1555,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
@@ -1457,14 +1587,19 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
         spapr_drc_reset(drc);
     } else if (PCI_FUNC(pdev->devfn) == 0) {
         int i;
+        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
+
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
 
         for (i = 0; i < 8; i++) {
             SpaprDrc *func_drc;
             SpaprDrcClass *func_drck;
             SpaprDREntitySense state;
 
-            func_drc = spapr_phb_get_pci_func_drc(phb, pci_bus_num(bus),
-                                                  PCI_DEVFN(slotnr, i));
+            func_drc = drc_from_devfn(phb, chassis, PCI_DEVFN(slotnr, i));
             func_drck = SPAPR_DR_CONNECTOR_GET_CLASS(func_drc);
             state = func_drck->dr_entity_sense(func_drc);
 
@@ -1478,9 +1613,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
@@ -1492,6 +1644,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);
 }
 
@@ -1500,7 +1662,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
 {
     SpaprPhbState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
     PCIDevice *pdev = PCI_DEVICE(plugged_dev);
-    SpaprDrc *drc = spapr_phb_get_pci_drc(phb, pdev);
+    SpaprDrc *drc = drc_from_dev(phb, pdev);
 
     if (!phb->dr_enabled) {
         error_setg(errp, QERR_BUS_NO_HOTPLUG,
@@ -1512,18 +1674,28 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
     g_assert(drc->dev == plugged_dev);
 
     if (!spapr_drc_unplug_requested(drc)) {
-        PCIBus *bus = PCI_BUS(qdev_get_parent_bus(DEVICE(pdev)));
+        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(plugged_dev);
         uint32_t slotnr = PCI_SLOT(pdev->devfn);
         SpaprDrc *func_drc;
         SpaprDrcClass *func_drck;
         SpaprDREntitySense state;
         int i;
+        Error *local_err = NULL;
+        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
+
+        if (local_err) {
+            error_propagate(errp, local_err);
+            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++) {
-                func_drc = spapr_phb_get_pci_func_drc(phb, pci_bus_num(bus),
-                                                      PCI_DEVFN(slotnr, i));
+                func_drc = drc_from_devfn(phb, chassis, PCI_DEVFN(slotnr, i));
                 func_drck = SPAPR_DR_CONNECTOR_GET_CLASS(func_drc);
                 state = func_drck->dr_entity_sense(func_drc);
                 if (state == SPAPR_DR_ENTITY_SENSE_PRESENT
@@ -1544,8 +1716,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
          */
         if (PCI_FUNC(pdev->devfn) == 0) {
             for (i = 7; i >= 0; i--) {
-                func_drc = spapr_phb_get_pci_func_drc(phb, pci_bus_num(bus),
-                                                      PCI_DEVFN(slotnr, i));
+                func_drc = drc_from_devfn(phb, chassis, PCI_DEVFN(slotnr, i));
                 func_drck = SPAPR_DR_CONNECTOR_GET_CLASS(func_drc);
                 state = func_drck->dr_entity_sense(func_drc);
                 if (state == SPAPR_DR_ENTITY_SENSE_PRESENT) {
@@ -1573,6 +1744,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);
 
@@ -1593,15 +1765,10 @@ static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
         }
     }
 
-    if (sphb->dr_enabled) {
-        for (i = PCI_SLOT_MAX * 8 - 1; i >= 0; i--) {
-            SpaprDrc *drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PCI,
-                                                    (sphb->index << 16) | i);
-
-            if (drc) {
-                object_unparent(OBJECT(drc));
-            }
-        }
+    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--) {
@@ -1645,6 +1812,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");
@@ -1678,7 +1846,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     }
 
     if (spapr_pci_find_phb(spapr, sphb->buid)) {
-        error_setg(errp, "PCI host bridges must have unique BUIDs");
+        SpaprPhbState *s;
+
+        error_setg(errp, "PCI host bridges must have unique indexes");
+        error_append_hint(errp, "The following indexes are already in use:");
+        QLIST_FOREACH(s, &spapr->phbs, list) {
+            error_append_hint(errp, " %d", s->index);
+        }
+        error_append_hint(errp, "\nTry another value for the index property\n");
         return;
     }
 
@@ -1790,7 +1965,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);
@@ -1815,11 +1989,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     }
 
     /* allocate connectors for child PCI devices */
-    if (sphb->dr_enabled) {
-        for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
-            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
-                                   (sphb->index << 16) | i);
-        }
+    add_drcs(sphb, phb->bus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto unrealize;
     }
 
     /* DMA setup */
@@ -2069,44 +2242,6 @@ static const TypeInfo spapr_phb_info = {
     }
 };
 
-typedef struct SpaprFdt {
-    void *fdt;
-    int node_off;
-    SpaprPhbState *sphb;
-} SpaprFdt;
-
-static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
-                                          void *opaque)
-{
-    PCIBus *sec_bus;
-    SpaprFdt *p = opaque;
-    int offset;
-    SpaprFdt s_fdt;
-
-    offset = spapr_create_pci_child_dt(p->sphb, pdev, p->fdt, p->node_off);
-    if (!offset) {
-        error_report("Failed to create pci child device tree node");
-        return;
-    }
-
-    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=
-         PCI_HEADER_TYPE_BRIDGE)) {
-        return;
-    }
-
-    sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
-    if (!sec_bus) {
-        return;
-    }
-
-    s_fdt.fdt = p->fdt;
-    s_fdt.node_off = offset;
-    s_fdt.sphb = p->sphb;
-    pci_for_each_device_reverse(sec_bus, pci_bus_num(sec_bus),
-                                spapr_populate_pci_devices_dt,
-                                &s_fdt);
-}
-
 static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev,
                                            void *opaque)
 {
@@ -2144,8 +2279,8 @@ static void spapr_phb_pci_enumerate(SpaprPhbState *phb)
 
 }
 
-int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
-                          uint32_t nr_msis, int *node_offset)
+int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
+                 uint32_t nr_msis, int *node_offset)
 {
     int bus_off, i, j, ret;
     uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
@@ -2192,8 +2327,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
                                 cpu_to_be32(0x0),
                                 cpu_to_be32(phb->numa_node)};
     SpaprTceTable *tcet;
-    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
-    SpaprFdt s_fdt;
     SpaprDrc *drc;
     Error *errp = NULL;
 
@@ -2206,8 +2339,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     /* Write PHB properties */
     _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
     _FDT(fdt_setprop_string(fdt, bus_off, "compatible", "IBM,Logical_PHB"));
-    _FDT(fdt_setprop_cell(fdt, bus_off, "#address-cells", 0x3));
-    _FDT(fdt_setprop_cell(fdt, bus_off, "#size-cells", 0x2));
     _FDT(fdt_setprop_cell(fdt, bus_off, "#interrupt-cells", 0x1));
     _FDT(fdt_setprop(fdt, bus_off, "used-by-rtas", NULL, 0));
     _FDT(fdt_setprop(fdt, bus_off, "bus-range", &bus_range, sizeof(bus_range)));
@@ -2272,17 +2403,9 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     spapr_phb_pci_enumerate(phb);
     _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
 
-    /* Populate tree nodes with PCI devices attached */
-    s_fdt.fdt = fdt;
-    s_fdt.node_off = bus_off;
-    s_fdt.sphb = phb;
-    pci_for_each_device_reverse(bus, pci_bus_num(bus),
-                                spapr_populate_pci_devices_dt,
-                                &s_fdt);
-
-    ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
-                                SPAPR_DR_CONNECTOR_TYPE_PCI);
-    if (ret) {
+    /* Walk the bridge and subordinate buses */
+    ret = spapr_dt_pci_bus(phb, PCI_HOST_BRIDGE(phb)->bus, fdt, bus_off);
+    if (ret < 0) {
         return ret;
     }