[Qemu-devel] [RFC PATCH 0/2] spapr: move pci device creation to Qemu

[Qemu-devel] [RFC PATCH 0/2] spapr: move pci device creation to Qemu

[Nikunj A Dadhania]

The patch series creates PCI DT nodes in QEMU. The new
hotplug code needs the device node creation in Qemu. While during
boot, nodes were created in SLOF. It makes more sense to consolidate
the code to one place for better maintainability. 

Also, patches for populating ibm,loc-code was getting very complicated
with use of RTAS/HCALL

Nikunj A Dadhania (2):
  spapr: enumerate and add PCI device tree
  spapr: populate ibm,loc-code

 hw/ppc/spapr_pci.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 153 insertions(+), 13 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree

[Nikunj A Dadhania]

All the PCI enumeration and device node creation was off-loaded to
SLOF. With PCI hotplug support, code needed to be added to add device
node. This creates multiple copy of the code one in SLOF and other in
hotplug code. To unify this, the patch adds the pci device node
creation in Qemu. For backward compatibility, a flag
"qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not
do device node creation.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 84 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 3796d54..abf71f7 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -23,6 +23,7 @@
  * THE SOFTWARE.
  */
 #include "hw/hw.h"
+#include "hw/sysbus.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
@@ -35,6 +36,7 @@
 #include "qemu/error-report.h"
 #include "qapi/qmp/qerror.h"
 
+#include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/ppc/spapr_drc.h"
 #include "sysemu/device_tree.h"
@@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
      * processed by OF beforehand
      */
     _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
-    _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
+    if (drc_name) {
+        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
+    }
     _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
 
     _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
@@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
     void *fdt = NULL;
     int fdt_start_offset = 0;
 
-    /* boot-time devices get their device tree node created by SLOF, but for
-     * hotplugged devices we need QEMU to generate it so the guest can fetch
-     * it via RTAS
-     */
     if (dev->hotplugged) {
         fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name,
                                         &fdt_start_offset);
@@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
     return PCI_HOST_BRIDGE(dev);
 }
 
-typedef struct sPAPRTCEDT {
+typedef struct sPAPRFDT {
     void *fdt;
     int node_off;
-} sPAPRTCEDT;
+    uint32_t index;
+} sPAPRFDT;
 
 static int spapr_phb_children_dt(Object *child, void *opaque)
 {
-    sPAPRTCEDT *p = opaque;
+    sPAPRFDT *p = opaque;
     sPAPRTCETable *tcet;
 
     tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
@@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque)
     return 1;
 }
 
+static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque)
+{
+    sPAPRFDT *p = opaque;
+    int ret, offset;
+    int slot = PCI_SLOT(pdev->devfn);
+    int func = PCI_FUNC(pdev->devfn);
+    char nodename[512];
+
+    if (func) {
+        sprintf(nodename, "pci@%d,%d", slot, func);
+    } else {
+        sprintf(nodename, "pci@%d", slot);
+    }
+    offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
+    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
+    g_assert(!ret);
+
+    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
+         PCI_HEADER_TYPE_BRIDGE)) {
+        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
+        if(sec_bus) {
+            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
+                                spapr_populate_pci_devices_dt,
+                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
+        }
+    }
+    return;
+}
+
+static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque)
+{
+    unsigned short *bus_no = (unsigned short *) opaque;
+    unsigned short primary = *bus_no;
+    unsigned short secondary;
+    unsigned short subordinate = 0xff;
+
+    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
+         PCI_HEADER_TYPE_BRIDGE)) {
+        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
+        secondary = *bus_no + 1;
+        pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
+        pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
+        pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1);
+        *bus_no = *bus_no + 1;
+        if (sec_bus) {
+            pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
+            pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
+            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
+            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
+                                spapr_phb_pci_enumerate_bridge,
+                                bus_no);
+            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
+        }
+    }
+}
+
+static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
+{
+    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
+    unsigned short bus_no = 0;
+
+    pci_for_each_device(bus, pci_bus_num(bus),
+                        spapr_phb_pci_enumerate_bridge,
+                        &bus_no);
+
+}
+
 int spapr_populate_pci_dt(sPAPRPHBState *phb,
                           uint32_t xics_phandle,
                           void *fdt)
@@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     uint32_t interrupt_map_mask[] = {
         cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
     uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
+    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
 
     /* Start populating the FDT */
     sprintf(nodename, "pci@%" PRIx64, phb->buid);
@@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
                      sizeof(interrupt_map)));
 
     object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
-                         &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
+                         &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
+
+    spapr_phb_pci_enumerate(phb);
+    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
+    pci_for_each_device(bus, pci_bus_num(bus),
+                        spapr_populate_pci_devices_dt,
+                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
 
     ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
                                 SPAPR_DR_CONNECTOR_TYPE_PCI);
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 2/2] spapr: populate ibm,loc-code

[Nikunj A Dadhania]

Each hardware instance has a platform unique location code.  The OF
device tree that describes a part of a hardware entity must include
the “ibm,loc-code” property with a value that represents the location
code for that hardware entity.

Populate ibm,loc-code.
1) PCI passthru devices need to identify with its own ibm,loc-code
   available on the host.
2) Emulated devices encode as following:
   qemu_<name>:<phb-index>:<slot>.<fn>

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 75 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index abf71f7..c69e732 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -744,6 +744,70 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     return &phb->iommu_as;
 }
 
+static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value)
+{
+    char *host;
+    char path[PATH_MAX];
+
+    host = object_property_get_str(OBJECT(pdev), "host", NULL);
+    if (!host) {
+        return false;
+    }
+
+    snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
+    g_free(host);
+
+    return g_file_get_contents(path, value, NULL, NULL);
+}
+
+static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
+{
+    char path[PATH_MAX], *buf = NULL;
+
+    /* We have a vfio host bridge lets get the path. */
+    if (!spapr_phb_vfio_get_devspec_value(pdev, &buf)) {
+        return NULL;
+    }
+
+    snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
+    g_free(buf);
+
+    if (g_file_get_contents(path, &buf, NULL, NULL)) {
+        return buf;
+    } else {
+        return NULL;
+    }
+}
+
+static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
+{
+    char *path = g_malloc(PATH_MAX);
+
+    if (!path) {
+        return NULL;
+    }
+
+    /*
+     * For non-vfio devices and failures make up the location code out
+     * of the name, slot and function.
+     *
+     *       qemu_<name>:<phb-index>:<slot>.<fn>
+     */
+    snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
+             sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+    return path;
+}
+
+
+static char *spapr_ibm_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev)
+{
+    if (object_dynamic_cast(OBJECT(pdev), "vfio-pci") != NULL) {
+        return spapr_phb_vfio_get_loc_code(sphb, pdev);
+    } else {
+        return spapr_phb_get_loc_code(sphb, pdev);
+    }
+}
+
 /* Macros to operate with address in OF binding to PCI */
 #define b_x(x, p, l)    (((x) & ((1<<(l))-1)) << (p))
 #define b_n(x)          b_x((x), 31, 1) /* 0 if relocatable */
@@ -873,12 +937,12 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp)
 }
 
 static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
-                                       int phb_index, int drc_index,
-                                       const char *drc_name)
+                                       sPAPRPHBState *phb, int drc_index)
 {
     ResourceProps rp;
     bool is_bridge = false;
     int pci_status;
+    char *buf = NULL;
 
     if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
         PCI_HEADER_TYPE_BRIDGE) {
@@ -940,8 +1004,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
      * processed by OF beforehand
      */
     _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
-    if (drc_name) {
-        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
+    buf = spapr_ibm_get_loc_code(phb, dev);
+    if (buf) {
+        _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
+        g_free(buf);
     }
     _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
 
@@ -978,8 +1044,7 @@ static void *spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
         sprintf(nodename, "pci@%d", slot);
     }
     offset = fdt_add_subnode(fdt, 0, nodename);
-    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb->index, drc_index,
-                                      drc_name);
+    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb, drc_index);
     g_assert(!ret);
 
     *dt_offset = offset;
@@ -1476,7 +1541,7 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
 typedef struct sPAPRFDT {
     void *fdt;
     int node_off;
-    uint32_t index;
+    sPAPRPHBState *sphb;
 } sPAPRFDT;
 
 static int spapr_phb_children_dt(Object *child, void *opaque)
@@ -1511,7 +1576,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
         sprintf(nodename, "pci@%d", slot);
     }
     offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
-    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
+    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->sphb, 0);
     g_assert(!ret);
 
     if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
@@ -1520,7 +1585,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
         if(sec_bus) {
             pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
                                 spapr_populate_pci_devices_dt,
-                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
+                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .sphb = p->sphb }));
         }
     }
     return;
@@ -1654,7 +1719,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
     pci_for_each_device(bus, pci_bus_num(bus),
                         spapr_populate_pci_devices_dt,
-                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
+                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .sphb = phb }));
 
     ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
                                 SPAPR_DR_CONNECTOR_TYPE_PCI);
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 0/2] spapr: move pci device creation to Qemu

[Nikunj A Dadhania]

Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:

> The patch series creates PCI DT nodes in QEMU. The new
> hotplug code needs the device node creation in Qemu. While during
> boot, nodes were created in SLOF. It makes more sense to consolidate
> the code to one place for better maintainability. 

Based on sPAPR PCI Hotplug patches by Michael Roth

https://github.com/mdroth/qemu/commits/spapr-hotplug-pci

>
> Also, patches for populating ibm,loc-code was getting very complicated
> with use of RTAS/HCALL
>
> Nikunj A Dadhania (2):
>   spapr: enumerate and add PCI device tree
>   spapr: populate ibm,loc-code
>
>  hw/ppc/spapr_pci.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 153 insertions(+), 13 deletions(-)
>
> -- 
> 1.8.3.1

Re: [Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7586 bytes --]

On Wed, Apr 22, 2015 at 04:05:33PM +0530, Nikunj A Dadhania wrote:
> All the PCI enumeration and device node creation was off-loaded to
> SLOF. With PCI hotplug support, code needed to be added to add device
> node. This creates multiple copy of the code one in SLOF and other in
> hotplug code. To unify this, the patch adds the pci device node
> creation in Qemu. For backward compatibility, a flag
> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not
> do device node creation.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 84 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 3796d54..abf71f7 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -23,6 +23,7 @@
>   * THE SOFTWARE.
>   */
>  #include "hw/hw.h"
> +#include "hw/sysbus.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
> @@ -35,6 +36,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/qmp/qerror.h"
>  
> +#include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/ppc/spapr_drc.h"
>  #include "sysemu/device_tree.h"
> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>       * processed by OF beforehand
>       */
>      _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
> -    _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
> +    if (drc_name) {
> +        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
> +    }
>      _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>  
>      _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
>      void *fdt = NULL;
>      int fdt_start_offset = 0;
>  
> -    /* boot-time devices get their device tree node created by SLOF, but for
> -     * hotplugged devices we need QEMU to generate it so the guest can fetch
> -     * it via RTAS
> -     */
>      if (dev->hotplugged) {
>          fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name,
>                                          &fdt_start_offset);
> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>      return PCI_HOST_BRIDGE(dev);
>  }
>  
> -typedef struct sPAPRTCEDT {
> +typedef struct sPAPRFDT {
>      void *fdt;
>      int node_off;
> -} sPAPRTCEDT;
> +    uint32_t index;
> +} sPAPRFDT;
>  
>  static int spapr_phb_children_dt(Object *child, void *opaque)
>  {
> -    sPAPRTCEDT *p = opaque;
> +    sPAPRFDT *p = opaque;
>      sPAPRTCETable *tcet;
>  
>      tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque)
>      return 1;
>  }
>  
> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque)
> +{
> +    sPAPRFDT *p = opaque;
> +    int ret, offset;
> +    int slot = PCI_SLOT(pdev->devfn);
> +    int func = PCI_FUNC(pdev->devfn);
> +    char nodename[512];
> +
> +    if (func) {
> +        sprintf(nodename, "pci@%d,%d", slot, func);
> +    } else {
> +        sprintf(nodename, "pci@%d", slot);
> +    }
> +    offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
> +    g_assert(!ret);
> +
> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> +         PCI_HEADER_TYPE_BRIDGE)) {
> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> +        if(sec_bus) {

You have several minor indent style problems - please run through
scripts/checkpatch.pl and correct accordingly.

> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +                                spapr_populate_pci_devices_dt,
> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));

The inline structure definition is pretty hard to read.  I'd prefer to
see that declared as a local.

Otherwise, this looks ok.

> +        }
> +    }
> +    return;
> +}
> +
> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque)
> +{
> +    unsigned short *bus_no = (unsigned short *) opaque;
> +    unsigned short primary = *bus_no;
> +    unsigned short secondary;
> +    unsigned short subordinate = 0xff;
> +
> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> +         PCI_HEADER_TYPE_BRIDGE)) {
> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> +        secondary = *bus_no + 1;
> +        pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
> +        pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
> +        pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1);
> +        *bus_no = *bus_no + 1;
> +        if (sec_bus) {
> +            pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
> +            pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +                                spapr_phb_pci_enumerate_bridge,
> +                                bus_no);
> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
> +        }
> +    }
> +}
> +
> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
> +{
> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> +    unsigned short bus_no = 0;
> +
> +    pci_for_each_device(bus, pci_bus_num(bus),
> +                        spapr_phb_pci_enumerate_bridge,
> +                        &bus_no);
> +
> +}
> +
>  int spapr_populate_pci_dt(sPAPRPHBState *phb,
>                            uint32_t xics_phandle,
>                            void *fdt)
> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>      uint32_t interrupt_map_mask[] = {
>          cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
>      uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>  
>      /* Start populating the FDT */
>      sprintf(nodename, "pci@%" PRIx64, phb->buid);
> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>                       sizeof(interrupt_map)));
>  
>      object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
> -                         &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
> +                         &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
> +
> +    spapr_phb_pci_enumerate(phb);
> +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
> +    pci_for_each_device(bus, pci_bus_num(bus),
> +                        spapr_populate_pci_devices_dt,
> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
>  
>      ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>                                  SPAPR_DR_CONNECTOR_TYPE_PCI);

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC PATCH 2/2] spapr: populate ibm,loc-code

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7034 bytes --]

On Wed, Apr 22, 2015 at 04:05:34PM +0530, Nikunj A Dadhania wrote:
> Each hardware instance has a platform unique location code.  The OF
> device tree that describes a part of a hardware entity must include
> the “ibm,loc-code” property with a value that represents the location
> code for that hardware entity.
> 
> Populate ibm,loc-code.
> 1) PCI passthru devices need to identify with its own ibm,loc-code
>    available on the host.
> 2) Emulated devices encode as following:
>    qemu_<name>:<phb-index>:<slot>.<fn>
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

This is going to conflict with Alexey's patches which merge the plan
and vfio PAPR PHB types, so we'll need to work out which will go in first.

> ---
>  hw/ppc/spapr_pci.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 75 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index abf71f7..c69e732 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -744,6 +744,70 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>      return &phb->iommu_as;
>  }
>  
> +static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value)
> +{
> +    char *host;
> +    char path[PATH_MAX];
> +
> +    host = object_property_get_str(OBJECT(pdev), "host", NULL);
> +    if (!host) {
> +        return false;
> +    }
> +
> +    snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
> +    g_free(host);
> +
> +    return g_file_get_contents(path, value, NULL, NULL);
> +}
> +
> +static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
> +{
> +    char path[PATH_MAX], *buf = NULL;
> +
> +    /* We have a vfio host bridge lets get the path. */
> +    if (!spapr_phb_vfio_get_devspec_value(pdev, &buf)) {
> +        return NULL;
> +    }
> +
> +    snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
> +    g_free(buf);
> +
> +    if (g_file_get_contents(path, &buf, NULL, NULL)) {
> +        return buf;
> +    } else {
> +        return NULL;
> +    }
> +}
> +
> +static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
> +{
> +    char *path = g_malloc(PATH_MAX);
> +
> +    if (!path) {
> +        return NULL;
> +    }
> +
> +    /*
> +     * For non-vfio devices and failures make up the location code out
> +     * of the name, slot and function.
> +     *
> +     *       qemu_<name>:<phb-index>:<slot>.<fn>
> +     */
> +    snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
> +             sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +    return path;
> +}
> +
> +
> +static char *spapr_ibm_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev)
> +{
> +    if (object_dynamic_cast(OBJECT(pdev), "vfio-pci") != NULL) {
> +        return spapr_phb_vfio_get_loc_code(sphb, pdev);
> +    } else {
> +        return spapr_phb_get_loc_code(sphb, pdev);
> +    }
> +}
> +
>  /* Macros to operate with address in OF binding to PCI */
>  #define b_x(x, p, l)    (((x) & ((1<<(l))-1)) << (p))
>  #define b_n(x)          b_x((x), 31, 1) /* 0 if relocatable */
> @@ -873,12 +937,12 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp)
>  }
>  
>  static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
> -                                       int phb_index, int drc_index,
> -                                       const char *drc_name)
> +                                       sPAPRPHBState *phb, int drc_index)
>  {
>      ResourceProps rp;
>      bool is_bridge = false;
>      int pci_status;
> +    char *buf = NULL;
>  
>      if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
>          PCI_HEADER_TYPE_BRIDGE) {
> @@ -940,8 +1004,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>       * processed by OF beforehand
>       */
>      _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
> -    if (drc_name) {
> -        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
> +    buf = spapr_ibm_get_loc_code(phb, dev);
> +    if (buf) {
> +        _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
> +        g_free(buf);
>      }
>      _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>  
> @@ -978,8 +1044,7 @@ static void *spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
>          sprintf(nodename, "pci@%d", slot);
>      }
>      offset = fdt_add_subnode(fdt, 0, nodename);
> -    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb->index, drc_index,
> -                                      drc_name);
> +    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb, drc_index);
>      g_assert(!ret);
>  
>      *dt_offset = offset;
> @@ -1476,7 +1541,7 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>  typedef struct sPAPRFDT {
>      void *fdt;
>      int node_off;
> -    uint32_t index;
> +    sPAPRPHBState *sphb;
>  } sPAPRFDT;
>  
>  static int spapr_phb_children_dt(Object *child, void *opaque)
> @@ -1511,7 +1576,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
>          sprintf(nodename, "pci@%d", slot);
>      }
>      offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
> -    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->sphb, 0);
>      g_assert(!ret);
>  
>      if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> @@ -1520,7 +1585,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
>          if(sec_bus) {
>              pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>                                  spapr_populate_pci_devices_dt,
> -                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .sphb = p->sphb }));
>          }
>      }
>      return;
> @@ -1654,7 +1719,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>      _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>      pci_for_each_device(bus, pci_bus_num(bus),
>                          spapr_populate_pci_devices_dt,
> -                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .sphb = phb }));
>  
>      ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>                                  SPAPR_DR_CONNECTOR_TYPE_PCI);

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [RFC PATCH 2/2] spapr: populate ibm,loc-code

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Wed, Apr 22, 2015 at 04:05:34PM +0530, Nikunj A Dadhania wrote:
>> Each hardware instance has a platform unique location code.  The OF
>> device tree that describes a part of a hardware entity must include
>> the “ibm,loc-code” property with a value that represents the location
>> code for that hardware entity.
>> 
>> Populate ibm,loc-code.
>> 1) PCI passthru devices need to identify with its own ibm,loc-code
>>    available on the host.
>> 2) Emulated devices encode as following:
>>    qemu_<name>:<phb-index>:<slot>.<fn>
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>
> This is going to conflict with Alexey's patches which merge the plan
> and vfio PAPR PHB types, so we'll need to work out which will go in first.

Sure, let me know, I can rebase if needed.

>
>> ---
>>  hw/ppc/spapr_pci.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 75 insertions(+), 10 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index abf71f7..c69e732 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -744,6 +744,70 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>>      return &phb->iommu_as;
>>  }
>>  
>> +static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value)
>> +{
>> +    char *host;
>> +    char path[PATH_MAX];
>> +
>> +    host = object_property_get_str(OBJECT(pdev), "host", NULL);
>> +    if (!host) {
>> +        return false;
>> +    }
>> +
>> +    snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
>> +    g_free(host);
>> +
>> +    return g_file_get_contents(path, value, NULL, NULL);
>> +}
>> +
>> +static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
>> +{
>> +    char path[PATH_MAX], *buf = NULL;
>> +
>> +    /* We have a vfio host bridge lets get the path. */
>> +    if (!spapr_phb_vfio_get_devspec_value(pdev, &buf)) {
>> +        return NULL;
>> +    }
>> +
>> +    snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
>> +    g_free(buf);
>> +
>> +    if (g_file_get_contents(path, &buf, NULL, NULL)) {
>> +        return buf;
>> +    } else {
>> +        return NULL;
>> +    }
>> +}
>> +
>> +static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
>> +{
>> +    char *path = g_malloc(PATH_MAX);
>> +
>> +    if (!path) {
>> +        return NULL;
>> +    }
>> +
>> +    /*
>> +     * For non-vfio devices and failures make up the location code out
>> +     * of the name, slot and function.
>> +     *
>> +     *       qemu_<name>:<phb-index>:<slot>.<fn>
>> +     */
>> +    snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
>> +             sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>> +    return path;
>> +}
>> +
>> +
>> +static char *spapr_ibm_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev)
>> +{
>> +    if (object_dynamic_cast(OBJECT(pdev), "vfio-pci") != NULL) {
>> +        return spapr_phb_vfio_get_loc_code(sphb, pdev);
>> +    } else {
>> +        return spapr_phb_get_loc_code(sphb, pdev);
>> +    }
>> +}
>> +
>>  /* Macros to operate with address in OF binding to PCI */
>>  #define b_x(x, p, l)    (((x) & ((1<<(l))-1)) << (p))
>>  #define b_n(x)          b_x((x), 31, 1) /* 0 if relocatable */
>> @@ -873,12 +937,12 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp)
>>  }
>>  
>>  static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>> -                                       int phb_index, int drc_index,
>> -                                       const char *drc_name)
>> +                                       sPAPRPHBState *phb, int drc_index)
>>  {
>>      ResourceProps rp;
>>      bool is_bridge = false;
>>      int pci_status;
>> +    char *buf = NULL;
>>  
>>      if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
>>          PCI_HEADER_TYPE_BRIDGE) {
>> @@ -940,8 +1004,10 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>>       * processed by OF beforehand
>>       */
>>      _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
>> -    if (drc_name) {
>> -        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
>> +    buf = spapr_ibm_get_loc_code(phb, dev);
>> +    if (buf) {
>> +        _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
>> +        g_free(buf);
>>      }
>>      _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>>  
>> @@ -978,8 +1044,7 @@ static void *spapr_create_pci_child_dt(sPAPRPHBState *phb, PCIDevice *dev,
>>          sprintf(nodename, "pci@%d", slot);
>>      }
>>      offset = fdt_add_subnode(fdt, 0, nodename);
>> -    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb->index, drc_index,
>> -                                      drc_name);
>> +    ret = spapr_populate_pci_child_dt(dev, fdt, offset, phb, drc_index);
>>      g_assert(!ret);
>>  
>>      *dt_offset = offset;
>> @@ -1476,7 +1541,7 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>>  typedef struct sPAPRFDT {
>>      void *fdt;
>>      int node_off;
>> -    uint32_t index;
>> +    sPAPRPHBState *sphb;
>>  } sPAPRFDT;
>>  
>>  static int spapr_phb_children_dt(Object *child, void *opaque)
>> @@ -1511,7 +1576,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
>>          sprintf(nodename, "pci@%d", slot);
>>      }
>>      offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
>> -    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
>> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->sphb, 0);
>>      g_assert(!ret);
>>  
>>      if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
>> @@ -1520,7 +1585,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *op
>>          if(sec_bus) {
>>              pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>>                                  spapr_populate_pci_devices_dt,
>> -                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
>> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .sphb = p->sphb }));
>>          }
>>      }
>>      return;
>> @@ -1654,7 +1719,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>      _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>>      pci_for_each_device(bus, pci_bus_num(bus),
>>                          spapr_populate_pci_devices_dt,
>> -                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
>> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .sphb = phb }));
>>  
>>      ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>>                                  SPAPR_DR_CONNECTOR_TYPE_PCI);
>
> -- 
> 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

Re: [Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Wed, Apr 22, 2015 at 04:05:33PM +0530, Nikunj A Dadhania wrote:
>> All the PCI enumeration and device node creation was off-loaded to
>> SLOF. With PCI hotplug support, code needed to be added to add device
>> node. This creates multiple copy of the code one in SLOF and other in
>> hotplug code. To unify this, the patch adds the pci device node
>> creation in Qemu. For backward compatibility, a flag
>> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not
>> do device node creation.
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 84 insertions(+), 9 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 3796d54..abf71f7 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -23,6 +23,7 @@
>>   * THE SOFTWARE.
>>   */
>>  #include "hw/hw.h"
>> +#include "hw/sysbus.h"
>>  #include "hw/pci/pci.h"
>>  #include "hw/pci/msi.h"
>>  #include "hw/pci/msix.h"
>> @@ -35,6 +36,7 @@
>>  #include "qemu/error-report.h"
>>  #include "qapi/qmp/qerror.h"
>>  
>> +#include "hw/pci/pci_bridge.h"
>>  #include "hw/pci/pci_bus.h"
>>  #include "hw/ppc/spapr_drc.h"
>>  #include "sysemu/device_tree.h"
>> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>>       * processed by OF beforehand
>>       */
>>      _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
>> -    _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
>> +    if (drc_name) {
>> +        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
>> +    }
>>      _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>>  
>>      _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
>> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
>>      void *fdt = NULL;
>>      int fdt_start_offset = 0;
>>  
>> -    /* boot-time devices get their device tree node created by SLOF, but for
>> -     * hotplugged devices we need QEMU to generate it so the guest can fetch
>> -     * it via RTAS
>> -     */
>>      if (dev->hotplugged) {
>>          fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name,
>>                                          &fdt_start_offset);
>> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>>      return PCI_HOST_BRIDGE(dev);
>>  }
>>  
>> -typedef struct sPAPRTCEDT {
>> +typedef struct sPAPRFDT {
>>      void *fdt;
>>      int node_off;
>> -} sPAPRTCEDT;
>> +    uint32_t index;
>> +} sPAPRFDT;
>>  
>>  static int spapr_phb_children_dt(Object *child, void *opaque)
>>  {
>> -    sPAPRTCEDT *p = opaque;
>> +    sPAPRFDT *p = opaque;
>>      sPAPRTCETable *tcet;
>>  
>>      tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
>> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque)
>>      return 1;
>>  }
>>  
>> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque)
>> +{
>> +    sPAPRFDT *p = opaque;
>> +    int ret, offset;
>> +    int slot = PCI_SLOT(pdev->devfn);
>> +    int func = PCI_FUNC(pdev->devfn);
>> +    char nodename[512];
>> +
>> +    if (func) {
>> +        sprintf(nodename, "pci@%d,%d", slot, func);
>> +    } else {
>> +        sprintf(nodename, "pci@%d", slot);
>> +    }
>> +    offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
>> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
>> +    g_assert(!ret);
>> +
>> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
>> +         PCI_HEADER_TYPE_BRIDGE)) {
>> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>> +        if(sec_bus) {
>
> You have several minor indent style problems - please run through
> scripts/checkpatch.pl and correct accordingly.

Sure, will do.

>
>> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>> +                                spapr_populate_pci_devices_dt,
>> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
>
> The inline structure definition is pretty hard to read.  I'd prefer to
> see that declared as a local.

Will update it.

>
> Otherwise, this looks ok.
>
>> +        }
>> +    }
>> +    return;
>> +}
>> +
>> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque)
>> +{
>> +    unsigned short *bus_no = (unsigned short *) opaque;
>> +    unsigned short primary = *bus_no;
>> +    unsigned short secondary;
>> +    unsigned short subordinate = 0xff;
>> +
>> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
>> +         PCI_HEADER_TYPE_BRIDGE)) {
>> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>> +        secondary = *bus_no + 1;
>> +        pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
>> +        pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
>> +        pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1);
>> +        *bus_no = *bus_no + 1;
>> +        if (sec_bus) {
>> +            pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
>> +            pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
>> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
>> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>> +                                spapr_phb_pci_enumerate_bridge,
>> +                                bus_no);
>> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
>> +        }
>> +    }
>> +}
>> +
>> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
>> +{
>> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>> +    unsigned short bus_no = 0;
>> +
>> +    pci_for_each_device(bus, pci_bus_num(bus),
>> +                        spapr_phb_pci_enumerate_bridge,
>> +                        &bus_no);
>> +
>> +}
>> +
>>  int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>                            uint32_t xics_phandle,
>>                            void *fdt)
>> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>      uint32_t interrupt_map_mask[] = {
>>          cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
>>      uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
>> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>>  
>>      /* Start populating the FDT */
>>      sprintf(nodename, "pci@%" PRIx64, phb->buid);
>> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>                       sizeof(interrupt_map)));
>>  
>>      object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
>> -                         &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
>> +                         &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
>> +
>> +    spapr_phb_pci_enumerate(phb);
>> +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>> +    pci_for_each_device(bus, pci_bus_num(bus),
>> +                        spapr_populate_pci_devices_dt,
>> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
>>  
>>      ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>>                                  SPAPR_DR_CONNECTOR_TYPE_PCI);
>
> -- 
> 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

Re: [Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree

[Alexey Kardashevskiy]

On 04/22/2015 08:35 PM, Nikunj A Dadhania wrote:
> All the PCI enumeration and device node creation was off-loaded to
> SLOF. With PCI hotplug support, code needed to be added to add device
> node. This creates multiple copy of the code one in SLOF and other in
> hotplug code. To unify this, the patch adds the pci device node
> creation in Qemu. For backward compatibility, a flag
> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not
> do device node creation.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>   hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 84 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 3796d54..abf71f7 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -23,6 +23,7 @@
>    * THE SOFTWARE.
>    */
>   #include "hw/hw.h"
> +#include "hw/sysbus.h"
>   #include "hw/pci/pci.h"
>   #include "hw/pci/msi.h"
>   #include "hw/pci/msix.h"
> @@ -35,6 +36,7 @@
>   #include "qemu/error-report.h"
>   #include "qapi/qmp/qerror.h"
>
> +#include "hw/pci/pci_bridge.h"
>   #include "hw/pci/pci_bus.h"
>   #include "hw/ppc/spapr_drc.h"
>   #include "sysemu/device_tree.h"
> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>        * processed by OF beforehand
>        */
>       _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
> -    _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
> +    if (drc_name) {


Where did drc_name come from? By now, there is no "loc-code" patch in 
spapr-next, or there is?


> +        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
> +    }
>       _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>
>       _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
>       void *fdt = NULL;
>       int fdt_start_offset = 0;
>
> -    /* boot-time devices get their device tree node created by SLOF, but for
> -     * hotplugged devices we need QEMU to generate it so the guest can fetch
> -     * it via RTAS
> -     */
>       if (dev->hotplugged) {
>           fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name,
>                                           &fdt_start_offset);
> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>       return PCI_HOST_BRIDGE(dev);
>   }
>
> -typedef struct sPAPRTCEDT {
> +typedef struct sPAPRFDT {
>       void *fdt;
>       int node_off;
> -} sPAPRTCEDT;
> +    uint32_t index;
> +} sPAPRFDT;

You definitely will have to rebase it - the sPAPRTCEDT is gone in what I 
posted last time (and what I hope will get to  spapr-next first :) ).



>   static int spapr_phb_children_dt(Object *child, void *opaque)
>   {
> -    sPAPRTCEDT *p = opaque;
> +    sPAPRFDT *p = opaque;
>       sPAPRTCETable *tcet;
>
>       tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque)
>       return 1;
>   }
>
> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque)
> +{
> +    sPAPRFDT *p = opaque;
> +    int ret, offset;
> +    int slot = PCI_SLOT(pdev->devfn);
> +    int func = PCI_FUNC(pdev->devfn);
> +    char nodename[512];
> +
> +    if (func) {
> +        sprintf(nodename, "pci@%d,%d", slot, func);
> +    } else {
> +        sprintf(nodename, "pci@%d", slot);
> +    }
> +    offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
> +    g_assert(!ret);
> +
> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> +         PCI_HEADER_TYPE_BRIDGE)) {


Since you'll be doing respin anyway, I suggest changing "==" to "!=", add 
return here and reduce indents. And below, and further down.



> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> +        if(sec_bus) {
> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +                                spapr_populate_pci_devices_dt,
> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
> +        }
> +    }
> +    return;
> +}
> +
> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque)
> +{
> +    unsigned short *bus_no = (unsigned short *) opaque;
> +    unsigned short primary = *bus_no;
> +    unsigned short secondary;
> +    unsigned short subordinate = 0xff;
> +
> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
> +         PCI_HEADER_TYPE_BRIDGE)) {
> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> +        secondary = *bus_no + 1;
> +        pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
> +        pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
> +        pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1);
> +        *bus_no = *bus_no + 1;
> +        if (sec_bus) {
> +            pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
> +            pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +                                spapr_phb_pci_enumerate_bridge,
> +                                bus_no);
> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
> +        }
> +    }
> +}
> +
> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
> +{
> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> +    unsigned short bus_no = 0;
> +
> +    pci_for_each_device(bus, pci_bus_num(bus),
> +                        spapr_phb_pci_enumerate_bridge,
> +                        &bus_no);
> +
> +}
> +
>   int spapr_populate_pci_dt(sPAPRPHBState *phb,
>                             uint32_t xics_phandle,
>                             void *fdt)
> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>       uint32_t interrupt_map_mask[] = {
>           cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
>       uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>
>       /* Start populating the FDT */
>       sprintf(nodename, "pci@%" PRIx64, phb->buid);
> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>                        sizeof(interrupt_map)));
>
>       object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
> -                         &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
> +                         &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
> +
> +    spapr_phb_pci_enumerate(phb);
> +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));


Cannot SLOF see that there are nodes already and just not touch them?


> +    pci_for_each_device(bus, pci_bus_num(bus),
> +                        spapr_populate_pci_devices_dt,
> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
>
>       ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>                                   SPAPR_DR_CONNECTOR_TYPE_PCI);
>


-- 
Alexey

Re: [Qemu-devel] [RFC PATCH 1/2] spapr: enumerate and add PCI device tree

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/22/2015 08:35 PM, Nikunj A Dadhania wrote:
>> All the PCI enumeration and device node creation was off-loaded to
>> SLOF. With PCI hotplug support, code needed to be added to add device
>> node. This creates multiple copy of the code one in SLOF and other in
>> hotplug code. To unify this, the patch adds the pci device node
>> creation in Qemu. For backward compatibility, a flag
>> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not
>> do device node creation.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>   hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 84 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 3796d54..abf71f7 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -23,6 +23,7 @@
>>    * THE SOFTWARE.
>>    */
>>   #include "hw/hw.h"
>> +#include "hw/sysbus.h"
>>   #include "hw/pci/pci.h"
>>   #include "hw/pci/msi.h"
>>   #include "hw/pci/msix.h"
>> @@ -35,6 +36,7 @@
>>   #include "qemu/error-report.h"
>>   #include "qapi/qmp/qerror.h"
>>
>> +#include "hw/pci/pci_bridge.h"
>>   #include "hw/pci/pci_bus.h"
>>   #include "hw/ppc/spapr_drc.h"
>>   #include "sysemu/device_tree.h"
>> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>>        * processed by OF beforehand
>>        */
>>       _FDT(fdt_setprop_string(fdt, offset, "name", "pci"));
>> -    _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
>> +    if (drc_name) {
>
>
> Where did drc_name come from? By now, there is no "loc-code" patch in 
> spapr-next, or there is?

loc-code is not there yet, but the hotplug code generates its own name
and sends as drc_name. I am getting rid of this in the next patch.

>
>
>> +        _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name)));
>> +    }
>>       _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>>
>>       _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
>> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
>>       void *fdt = NULL;
>>       int fdt_start_offset = 0;
>>
>> -    /* boot-time devices get their device tree node created by SLOF, but for
>> -     * hotplugged devices we need QEMU to generate it so the guest can fetch
>> -     * it via RTAS
>> -     */
>>       if (dev->hotplugged) {
>>           fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name,
>>                                           &fdt_start_offset);
>> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
>>       return PCI_HOST_BRIDGE(dev);
>>   }
>>
>> -typedef struct sPAPRTCEDT {
>> +typedef struct sPAPRFDT {
>>       void *fdt;
>>       int node_off;
>> -} sPAPRTCEDT;
>> +    uint32_t index;
>> +} sPAPRFDT;
>
> You definitely will have to rebase it - the sPAPRTCEDT is gone in what I 
> posted last time (and what I hope will get to  spapr-next first :) ).

Sure, that should be fine.

>
>
>
>>   static int spapr_phb_children_dt(Object *child, void *opaque)
>>   {
>> -    sPAPRTCEDT *p = opaque;
>> +    sPAPRFDT *p = opaque;
>>       sPAPRTCETable *tcet;
>>
>>       tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE);
>> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque)
>>       return 1;
>>   }
>>
>> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque)
>> +{
>> +    sPAPRFDT *p = opaque;
>> +    int ret, offset;
>> +    int slot = PCI_SLOT(pdev->devfn);
>> +    int func = PCI_FUNC(pdev->devfn);
>> +    char nodename[512];
>> +
>> +    if (func) {
>> +        sprintf(nodename, "pci@%d,%d", slot, func);
>> +    } else {
>> +        sprintf(nodename, "pci@%d", slot);
>> +    }
>> +    offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
>> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, NULL);
>> +    g_assert(!ret);
>> +
>> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
>> +         PCI_HEADER_TYPE_BRIDGE)) {
>
>
> Since you'll be doing respin anyway, I suggest changing "==" to "!=", add 
> return here and reduce indents. And below, and further down.

Yes, will be better.

>
>
>
>> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>> +        if(sec_bus) {
>> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>> +                                spapr_populate_pci_devices_dt,
>> +                                &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index }));
>> +        }
>> +    }
>> +    return;
>> +}
>> +
>> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque)
>> +{
>> +    unsigned short *bus_no = (unsigned short *) opaque;
>> +    unsigned short primary = *bus_no;
>> +    unsigned short secondary;
>> +    unsigned short subordinate = 0xff;
>> +
>> +    if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) ==
>> +         PCI_HEADER_TYPE_BRIDGE)) {
>> +        PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>> +        secondary = *bus_no + 1;
>> +        pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
>> +        pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
>> +        pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1);
>> +        *bus_no = *bus_no + 1;
>> +        if (sec_bus) {
>> +            pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
>> +            pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1);
>> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
>> +            pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>> +                                spapr_phb_pci_enumerate_bridge,
>> +                                bus_no);
>> +            pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
>> +        }
>> +    }
>> +}
>> +
>> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
>> +{
>> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>> +    unsigned short bus_no = 0;
>> +
>> +    pci_for_each_device(bus, pci_bus_num(bus),
>> +                        spapr_phb_pci_enumerate_bridge,
>> +                        &bus_no);
>> +
>> +}
>> +
>>   int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>                             uint32_t xics_phandle,
>>                             void *fdt)
>> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>       uint32_t interrupt_map_mask[] = {
>>           cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
>>       uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
>> +    PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>>
>>       /* Start populating the FDT */
>>       sprintf(nodename, "pci@%" PRIx64, phb->buid);
>> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>>                        sizeof(interrupt_map)));
>>
>>       object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
>> -                         &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off }));
>> +                         &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off }));
>> +
>> +    spapr_phb_pci_enumerate(phb);
>> +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>
>
> Cannot SLOF see that there are nodes already and just not touch them?

This is more explicit. So that in absence of this, I can kick in old
SLOF enumeration code and in presence, start reading the nodes.

>
>
>> +    pci_for_each_device(bus, pci_bus_num(bus),
>> +                        spapr_populate_pci_devices_dt,
>> +                        &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index }));
>>
>>       ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>>                                   SPAPR_DR_CONNECTOR_TYPE_PCI);
>>
>
>
> -- 
> Alexey

Regards
Nikunj