Re: [PATCH 06/17] pnv_phb4.c: attach default root port in phb4 realize()

[Cédric Le Goater]

On 12/28/21 20:37, Daniel Henrique Barboza wrote:

We're adding the default pnv_phb4_root_port in
pnv_chip_power9_pec_realize() by going into each stack, from eack pec,

each

accessing the stack PHB and adding the port.

This will be an annoyance when trying to implement user creatable PHB4
devices because, when that happens, stack->phb is not guaranteed to be
valid at that time (we'll assign a PHB to its stack in phb4_realize(),
after stk_realize()).

Let's move the attachment of the default root port to pnv_phb4_realize()
instead. This will create all the default root ports we already create
today, and it'll be one less thing to worry about when implementing user
creatable PHB4s.

Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>

This makes sense. I would simply modify and resend the initial patch.

Thanks,

C.

---
  hw/pci-host/pnv_phb4.c |  7 +++++++
  hw/ppc/pnv.c           | 20 +++-----------------
  include/hw/ppc/pnv.h   |  1 +
  3 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 4554490e51..daa468b812 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -22,6 +22,7 @@
  #include "hw/irq.h"
  #include "hw/qdev-properties.h"
  #include "qom/object.h"
+#include "sysemu/sysemu.h"
  #include "trace.h"

#define phb_error(phb, fmt, ...) \

@@ -1224,6 +1225,12 @@ static void pnv_phb4_realize(DeviceState *dev, Error 
**errp)
      pnv_phb4_update_xsrc(phb);

phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);

+
+    /* Add the default pnv-phb4-root-port */
+    if (defaults_enabled()) {
+        pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb),
+                                 TYPE_PNV_PHB4_ROOT_PORT);
+    }
  }

static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 605296fab5..c88fef26cf 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1222,7 +1222,7 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error 
**errp)
  }

/* Attach a root port */

-static void pnv_phb_attach_root_port(PCIHostState *pci, const char *name)
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name)
  {
      PCIDevice *root = pci_new(PCI_DEVFN(0, 0), name);

@@ -1478,7 +1478,7 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)

  static void pnv_chip_power9_pec_realize(PnvChip *chip, Error **errp)
  {
      Pnv9Chip *chip9 = PNV9_CHIP(chip);
-    int i, j;
+    int i;

for (i = 0; i < chip->num_pecs; i++) {

          PnvPhb4PecState *pec = &chip9->pecs[i];
@@ -1500,13 +1500,6 @@ static void pnv_chip_power9_pec_realize(PnvChip *chip, 
Error **errp)

pnv_xscom_add_subregion(chip, pec_nest_base, &pec->nest_regs_mr);

          pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
-
-        for (j = 0; j < pec->num_stacks; j++) {
-            PnvPHB4 *phb = &pec->stacks[j].phb;
-
-            pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb),
-                                     TYPE_PNV_PHB4_ROOT_PORT);
-        }
      }
  }

@@ -1726,7 +1719,7 @@ static void pnv_chip_power10_quad_realize(Pnv10Chip *chip10, Error **errp)

  static void pnv_chip_power10_phb_realize(PnvChip *chip, Error **errp)
  {
      Pnv10Chip *chip10 = PNV10_CHIP(chip);
-    int i, j;
+    int i;

for (i = 0; i < chip->num_pecs; i++) {

          PnvPhb4PecState *pec = &chip10->pecs[i];
@@ -1748,13 +1741,6 @@ static void pnv_chip_power10_phb_realize(PnvChip *chip, 
Error **errp)

pnv_xscom_add_subregion(chip, pec_nest_base, &pec->nest_regs_mr);

          pnv_xscom_add_subregion(chip, pec_pci_base, &pec->pci_regs_mr);
-
-        for (j = 0; j < pec->num_stacks; j++) {
-            PnvPHB4 *phb = &pec->stacks[j].phb;
-
-            pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb),
-                                     TYPE_PNV_PHB5_ROOT_PORT);
-        }
      }
  }

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h

index 6c48fc62ff..7f2197dcc0 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -196,6 +196,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER10,

PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir);

  void pnv_chip_parent_fixup(PnvChip *chip, Object *obj, int index);
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name);

#define TYPE_PNV_MACHINE MACHINE_TYPE_NAME("powernv")

  typedef struct PnvMachineClass PnvMachineClass;