[PATCH v2 0/9] pnv-phb related cleanups

[PATCH v2 0/9] pnv-phb related cleanups

[Daniel Henrique Barboza]

This second version contains changes and fixes suggested by Frederic and Cedric
in the v1 review.

As I've mentioned in the v1, I cropped patches that are more related with
the user created device logic. Patches 8-11 will be resend later in another
series.

I ended up sliding in a couple of new patches (8 and 9) to fix an issue I found
while spinning this new version.

No functional changes, aside from the QOM change in patch 2, were made
intentionally.

changes from v2:
- patch 3:
  * removed 'if (dev)' check in pnv_phb3_root_port_realize()
- patch 5:
  * opencoded pnv_ics_get_phb_ics() into pnv_ics_get()
  * removed pnv_ics_get_phb_ics()
  * removed ForeachPhb3Args struct
- patch 6:
  * restore the 'ics_resend()' call back in pnv_ics_resend()
- patch 8 (new):
  * remove INTERFACE_PCIE_DEVICE from pnv-phb3-root-bus
- patch 9 (new):
  * remove INTERFACE_PCIE_DEVICE from pnv-phb4-root-bus
- v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg02333.html 


Daniel Henrique Barboza (9):
  ppc/pnv: move root port attach to pnv_phb4_realize()
  ppc/pnv: attach phb3/phb4 root ports in QOM tree
  ppc/pnv: use dev->parent_bus->parent to get the PHB
  ppc/pnv: use dev instead of pci->qdev in root_port_realize()
  ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array
  ppc/pnv: make pnv_ics_resend() use chip8->phbs[]
  ppc/pnv: make pnv_chip_power8_pic_print_info() use chip8->phbs[]
  ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb3 root bus
  ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb4 root bus

 hw/pci-host/pnv_phb3.c     | 20 ++++-----
 hw/pci-host/pnv_phb4.c     | 23 +++++-----
 hw/pci-host/pnv_phb4_pec.c |  3 --
 hw/ppc/pnv.c               | 89 ++++++++++++++------------------------
 include/hw/ppc/pnv.h       |  2 +-
 5 files changed, 57 insertions(+), 80 deletions(-)

-- 
2.36.1

[PATCH v2 1/9] ppc/pnv: move root port attach to pnv_phb4_realize()

[Daniel Henrique Barboza]

Creating a root port is something related to the PHB, not the PEC. It
also makes the logic more in line with what pnv-phb3 does.

Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4.c     | 4 ++++
 hw/pci-host/pnv_phb4_pec.c | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 6594016121..23ad8de7ee 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
 static void pnv_phb4_realize(DeviceState *dev, Error **errp)
 {
     PnvPHB4 *phb = PNV_PHB4(dev);
+    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     XiveSource *xsrc = &phb->xsrc;
     int nr_irqs;
@@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
     pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
     pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
 
+    /* Add a single Root port if running with defaults */
+    pnv_phb_attach_root_port(pci, pecc->rp_model);
+
     /* Setup XIVE Source */
     if (phb->big_phb) {
         nr_irqs = PNV_PHB4_MAX_INTs;
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 8b7e823fa5..c9aaf1c28e 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
     if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
         return;
     }
-
-    /* Add a single Root port if running with defaults */
-    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
 }
 
 static void pnv_pec_realize(DeviceState *dev, Error **errp)
-- 
2.36.1

[PATCH v2 2/9] ppc/pnv: attach phb3/phb4 root ports in QOM tree

[Daniel Henrique Barboza]

At this moment we leave the pnv-phb3(4)-root-port unattached in QOM:

  /unattached (container)
(...)
    /device[2] (pnv-phb3-root-port)
      /bus master container[0] (memory-region)
      /bus master[0] (memory-region)
      /pci_bridge_io[0] (memory-region)
      /pci_bridge_io[1] (memory-region)
      /pci_bridge_mem[0] (memory-region)
      /pci_bridge_pci[0] (memory-region)
      /pci_bridge_pref_mem[0] (memory-region)
      /pci_bridge_vga_io_hi[0] (memory-region)
      /pci_bridge_vga_io_lo[0] (memory-region)
      /pci_bridge_vga_mem[0] (memory-region)
      /pcie.0 (PCIE)

Let's make changes in pnv_phb_attach_root_port() to attach the created
root ports to its corresponding PHB.

This is the result afterwards:

    /pnv-phb3[0] (pnv-phb3)
      /lsi (ics)
      /msi (phb3-msi)
      /msi32[0] (memory-region)
      /msi64[0] (memory-region)
      /pbcq (pnv-pbcq)
    (...)
      /phb3_iommu[0] (pnv-phb3-iommu-memory-region)
      /pnv-phb3-root.0 (pnv-phb3-root)
        /pnv-phb3-root-port[0] (pnv-phb3-root-port)
          /bus master container[0] (memory-region)
          /bus master[0] (memory-region)
          /pci_bridge_io[0] (memory-region)
          /pci_bridge_io[1] (memory-region)
          /pci_bridge_mem[0] (memory-region)
          /pci_bridge_pci[0] (memory-region)
          /pci_bridge_pref_mem[0] (memory-region)
          /pci_bridge_vga_io_hi[0] (memory-region)
          /pci_bridge_vga_io_lo[0] (memory-region)
          /pci_bridge_vga_mem[0] (memory-region)
          /pcie.0 (PCIE)

Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c | 2 +-
 hw/pci-host/pnv_phb4.c | 2 +-
 hw/ppc/pnv.c           | 7 ++++++-
 include/hw/ppc/pnv.h   | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 26ac9b7123..4ba660f8b9 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1052,7 +1052,7 @@ static void pnv_phb3_realize(DeviceState *dev, Error **errp)
 
     pci_setup_iommu(pci->bus, pnv_phb3_dma_iommu, phb);
 
-    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB3_ROOT_PORT);
+    pnv_phb_attach_root_port(pci, TYPE_PNV_PHB3_ROOT_PORT, phb->phb_id);
 }
 
 void pnv_phb3_update_regions(PnvPHB3 *phb)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 23ad8de7ee..ffd9d8a947 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1585,7 +1585,7 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
     pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
 
     /* Add a single Root port if running with defaults */
-    pnv_phb_attach_root_port(pci, pecc->rp_model);
+    pnv_phb_attach_root_port(pci, pecc->rp_model, phb->phb_id);
 
     /* Setup XIVE Source */
     if (phb->big_phb) {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7c08a78d6c..40e0cbd84d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1190,9 +1190,14 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
 }
 
 /* Attach a root port device */
-void pnv_phb_attach_root_port(PCIHostState *pci, const char *name)
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name, int index)
 {
     PCIDevice *root = pci_new(PCI_DEVFN(0, 0), name);
+    g_autofree char *default_id = g_strdup_printf("%s[%d]", name, index);
+    const char *dev_id = DEVICE(root)->id;
+
+    object_property_add_child(OBJECT(pci->bus), dev_id ? dev_id : default_id,
+                              OBJECT(root));
 
     pci_realize_and_unref(root, pci->bus, &error_fatal);
 }
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 86cb7d7f97..033890a23f 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -189,7 +189,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER10,
                          TYPE_PNV_CHIP_POWER10)
 
 PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir);
-void pnv_phb_attach_root_port(PCIHostState *pci, const char *name);
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name, int index);
 
 #define TYPE_PNV_MACHINE       MACHINE_TYPE_NAME("powernv")
 typedef struct PnvMachineClass PnvMachineClass;
-- 
2.36.1

[PATCH v2 3/9] ppc/pnv: use dev->parent_bus->parent to get the PHB

[Daniel Henrique Barboza]

It is not advisable to execute an object_dynamic_cast() to poke into
bus->qbus.parent and follow it up with a C cast into the PnvPHB type we
think we got.

A better way is to access the PnvPHB object via a QOM macro accessing
the existing parent links of the DeviceState. For a given
pnv-phb3/4-root-port 'dev', dev->parent_bus will give us the PHB bus,
and dev->parent_bus->parent is the PHB. Use the adequate QOM macro to
assert the type, and keep the NULL check in case we didn't get the
object we were expecting.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c | 10 +++++++---
 hw/pci-host/pnv_phb4.c | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 4ba660f8b9..5e7f827415 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1139,12 +1139,16 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
 {
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
     PCIDevice *pci = PCI_DEVICE(dev);
-    PCIBus *bus = pci_get_bus(pci);
     PnvPHB3 *phb = NULL;
     Error *local_err = NULL;
 
-    phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                          TYPE_PNV_PHB3);
+    /*
+     * dev->parent_bus gives access to the pnv-phb-root bus.
+     * The PnvPHB3 is the owner (parent) of the bus.
+     */
+    if (dev->parent_bus) {
+        phb = PNV_PHB3(dev->parent_bus->parent);
+    }
 
     if (!phb) {
         error_setg(errp,
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index ffd9d8a947..a0ee52e820 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1782,12 +1782,16 @@ static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
 {
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
     PCIDevice *pci = PCI_DEVICE(dev);
-    PCIBus *bus = pci_get_bus(pci);
     PnvPHB4 *phb = NULL;
     Error *local_err = NULL;
 
-    phb = (PnvPHB4 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                          TYPE_PNV_PHB4);
+    /*
+     * dev->parent_bus gives access to the pnv-phb-root bus.
+     * The PnvPHB4 is the owner (parent) of the bus.
+     */
+    if (dev->parent_bus) {
+        phb = PNV_PHB4(dev->parent_bus->parent);
+    }
 
     if (!phb) {
         error_setg(errp, "%s must be connected to pnv-phb4 buses", dev->id);
-- 
2.36.1

[PATCH v2 4/9] ppc/pnv: use dev instead of pci->qdev in root_port_realize()

[Daniel Henrique Barboza]

We already have access to the 'dev' object.

Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c | 4 ++--
 hw/pci-host/pnv_phb4.c | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 5e7f827415..8c03cc94f2 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1157,8 +1157,8 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
     }
 
     /* Set unique chassis/slot values for the root port */
-    qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id);
-    qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id);
+    qdev_prop_set_uint8(dev, "chassis", phb->chip_id);
+    qdev_prop_set_uint16(dev, "slot", phb->phb_id);
 
     rpc->parent_realize(dev, &local_err);
     if (local_err) {
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index a0ee52e820..61b45fe33c 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1781,7 +1781,6 @@ static void pnv_phb4_root_port_reset(DeviceState *dev)
 static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
 {
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
-    PCIDevice *pci = PCI_DEVICE(dev);
     PnvPHB4 *phb = NULL;
     Error *local_err = NULL;
 
@@ -1799,8 +1798,8 @@ static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
     }
 
     /* Set unique chassis/slot values for the root port */
-    qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id);
-    qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id);
+    qdev_prop_set_uint8(dev, "chassis", phb->chip_id);
+    qdev_prop_set_uint16(dev, "slot", phb->phb_id);
 
     rpc->parent_realize(dev, &local_err);
     if (local_err) {
-- 
2.36.1

[PATCH v2 5/9] ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array

[Daniel Henrique Barboza]

The function is working today by getting all the child objects of the
chip, interacting with each of them to check whether the child is a PHB,
and then doing what needs to be done.

We have all the chip PHBs in the phbs[] array so interacting with all
child objects is unneeded. Open code pnv_ics_get_phb_ics() into
pnv_ics_get() and remove both pnv_ics_get_phb_ics() and the
ForeachPhb3Args struct.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pnv.c | 38 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 40e0cbd84d..ff7f803662 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1939,44 +1939,28 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir)
     return NULL;
 }
 
-typedef struct ForeachPhb3Args {
-    int irq;
-    ICSState *ics;
-} ForeachPhb3Args;
-
-static int pnv_ics_get_child(Object *child, void *opaque)
-{
-    ForeachPhb3Args *args = opaque;
-    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-    if (phb3) {
-        if (ics_valid_irq(&phb3->lsis, args->irq)) {
-            args->ics = &phb3->lsis;
-        }
-        if (ics_valid_irq(ICS(&phb3->msis), args->irq)) {
-            args->ics = ICS(&phb3->msis);
-        }
-    }
-    return args->ics ? 1 : 0;
-}
-
 static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
 {
     PnvMachineState *pnv = PNV_MACHINE(xi);
-    ForeachPhb3Args args = { irq, NULL };
-    int i;
+    int i, j;
 
     for (i = 0; i < pnv->num_chips; i++) {
-        PnvChip *chip = pnv->chips[i];
         Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
 
         if (ics_valid_irq(&chip8->psi.ics, irq)) {
             return &chip8->psi.ics;
         }
 
-        object_child_foreach(OBJECT(chip), pnv_ics_get_child, &args);
-        if (args.ics) {
-            return args.ics;
+        for (j = 0; j < chip8->num_phbs; j++) {
+            PnvPHB3 *phb3 = &chip8->phbs[j];
+
+            if (ics_valid_irq(&phb3->lsis, irq)) {
+                return &phb3->lsis;
+            }
+
+            if (ics_valid_irq(ICS(&phb3->msis), irq)) {
+                return ICS(&phb3->msis);
+            }
         }
     }
     return NULL;
-- 
2.36.1

[PATCH v2 6/9] ppc/pnv: make pnv_ics_resend() use chip8->phbs[]

[Daniel Henrique Barboza]

pnv_ics_resend() is scrolling through all the child objects of the chip
to search for the PHBs. It's faster and simpler to just use the phbs[]
array.

pnv_ics_resend_child() was folded into pnv_ics_resend() since it's too
simple to justify its own function.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pnv.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index ff7f803662..08136def8e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1979,28 +1979,22 @@ PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
     return NULL;
 }
 
-static int pnv_ics_resend_child(Object *child, void *opaque)
-{
-    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-    if (phb3) {
-        ics_resend(&phb3->lsis);
-        ics_resend(ICS(&phb3->msis));
-    }
-    return 0;
-}
-
 static void pnv_ics_resend(XICSFabric *xi)
 {
     PnvMachineState *pnv = PNV_MACHINE(xi);
-    int i;
+    int i, j;
 
     for (i = 0; i < pnv->num_chips; i++) {
-        PnvChip *chip = pnv->chips[i];
         Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
 
         ics_resend(&chip8->psi.ics);
-        object_child_foreach(OBJECT(chip), pnv_ics_resend_child, NULL);
+
+        for (j = 0; j < chip8->num_phbs; j++) {
+            PnvPHB3 *phb3 = &chip8->phbs[j];
+
+            ics_resend(&phb3->lsis);
+            ics_resend(ICS(&phb3->msis));
+        }
     }
 }
 
-- 
2.36.1

[PATCH v2 7/9] ppc/pnv: make pnv_chip_power8_pic_print_info() use chip8->phbs[]

[Daniel Henrique Barboza]

It's inneficient to scroll all child objects when we have all PHBs
available in chip8->phbs[].

pnv_chip_power8_pic_print_info_child() ended up folded into
pic_print_info() for simplicity.

Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pnv.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 08136def8e..2a9067687b 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -652,25 +652,19 @@ static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
     return PNV_CHIP_GET_CLASS(chip)->isa_create(chip, errp);
 }
 
-static int pnv_chip_power8_pic_print_info_child(Object *child, void *opaque)
-{
-    Monitor *mon = opaque;
-    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-    if (phb3) {
-        pnv_phb3_msi_pic_print_info(&phb3->msis, mon);
-        ics_pic_print_info(&phb3->lsis, mon);
-    }
-    return 0;
-}
-
 static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon)
 {
     Pnv8Chip *chip8 = PNV8_CHIP(chip);
+    int i;
 
     ics_pic_print_info(&chip8->psi.ics, mon);
-    object_child_foreach(OBJECT(chip),
-                         pnv_chip_power8_pic_print_info_child, mon);
+
+    for (i = 0; i < chip8->num_phbs; i++) {
+        PnvPHB3 *phb3 = &chip8->phbs[i];
+
+        pnv_phb3_msi_pic_print_info(&phb3->msis, mon);
+        ics_pic_print_info(&phb3->lsis, mon);
+    }
 }
 
 static int pnv_chip_power9_pic_print_info_child(Object *child, void *opaque)
-- 
2.36.1

[PATCH v2 8/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb3 root bus

[Daniel Henrique Barboza]

It's unneeded. No other PCIE_BUS implements this interface.

Fixes: 9ae1329ee2fe ("ppc/pnv: Add models for POWER8 PHB3 PCIe Host bridge")
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 8c03cc94f2..696c9bef26 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1129,10 +1129,6 @@ static const TypeInfo pnv_phb3_root_bus_info = {
     .name = TYPE_PNV_PHB3_ROOT_BUS,
     .parent = TYPE_PCIE_BUS,
     .class_init = pnv_phb3_root_bus_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_PCIE_DEVICE },
-        { }
-    },
 };
 
 static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
-- 
2.36.1

[PATCH v2 9/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb4 root bus

[Daniel Henrique Barboza]

It's unneeded. No other PCIE_BUS implements this interface.

Fixes: 4f9924c4d4cf ("ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge")
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 61b45fe33c..81f7c1fe8f 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1751,10 +1751,6 @@ static const TypeInfo pnv_phb4_root_bus_info = {
     .name = TYPE_PNV_PHB4_ROOT_BUS,
     .parent = TYPE_PCIE_BUS,
     .class_init = pnv_phb4_root_bus_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_PCIE_DEVICE },
-        { }
-    },
 };
 
 static void pnv_phb4_root_port_reset(DeviceState *dev)
-- 
2.36.1

Re: [PATCH v2 5/9] ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array

[Cédric Le Goater]

On 6/18/22 13:01, Daniel Henrique Barboza wrote:
> The function is working today by getting all the child objects of the
> chip, interacting with each of them to check whether the child is a PHB,
> and then doing what needs to be done.
> 
> We have all the chip PHBs in the phbs[] array so interacting with all
> child objects is unneeded. Open code pnv_ics_get_phb_ics() into
> pnv_ics_get() and remove both pnv_ics_get_phb_ics() and the
> ForeachPhb3Args struct.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/ppc/pnv.c | 38 +++++++++++---------------------------
>   1 file changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 40e0cbd84d..ff7f803662 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1939,44 +1939,28 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir)
>       return NULL;
>   }
>   
> -typedef struct ForeachPhb3Args {
> -    int irq;
> -    ICSState *ics;
> -} ForeachPhb3Args;
> -
> -static int pnv_ics_get_child(Object *child, void *opaque)
> -{
> -    ForeachPhb3Args *args = opaque;
> -    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
> -
> -    if (phb3) {
> -        if (ics_valid_irq(&phb3->lsis, args->irq)) {
> -            args->ics = &phb3->lsis;
> -        }
> -        if (ics_valid_irq(ICS(&phb3->msis), args->irq)) {
> -            args->ics = ICS(&phb3->msis);
> -        }
> -    }
> -    return args->ics ? 1 : 0;
> -}
> -
>   static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
>   {
>       PnvMachineState *pnv = PNV_MACHINE(xi);
> -    ForeachPhb3Args args = { irq, NULL };
> -    int i;
> +    int i, j;
>   
>       for (i = 0; i < pnv->num_chips; i++) {
> -        PnvChip *chip = pnv->chips[i];
>           Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
>   
>           if (ics_valid_irq(&chip8->psi.ics, irq)) {
>               return &chip8->psi.ics;
>           }
>   
> -        object_child_foreach(OBJECT(chip), pnv_ics_get_child, &args);
> -        if (args.ics) {
> -            return args.ics;
> +        for (j = 0; j < chip8->num_phbs; j++) {
> +            PnvPHB3 *phb3 = &chip8->phbs[j];
> +
> +            if (ics_valid_irq(&phb3->lsis, irq)) {
> +                return &phb3->lsis;
> +            }
> +
> +            if (ics_valid_irq(ICS(&phb3->msis), irq)) {
> +                return ICS(&phb3->msis);
> +            }
>           }
>       }
>       return NULL;

Re: [PATCH v2 6/9] ppc/pnv: make pnv_ics_resend() use chip8->phbs[]

[Cédric Le Goater]

On 6/18/22 13:01, Daniel Henrique Barboza wrote:
> pnv_ics_resend() is scrolling through all the child objects of the chip
> to search for the PHBs. It's faster and simpler to just use the phbs[]
> array.
> 
> pnv_ics_resend_child() was folded into pnv_ics_resend() since it's too
> simple to justify its own function.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/ppc/pnv.c | 22 ++++++++--------------
>   1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index ff7f803662..08136def8e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1979,28 +1979,22 @@ PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
>       return NULL;
>   }
>   
> -static int pnv_ics_resend_child(Object *child, void *opaque)
> -{
> -    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
> -
> -    if (phb3) {
> -        ics_resend(&phb3->lsis);
> -        ics_resend(ICS(&phb3->msis));
> -    }
> -    return 0;
> -}
> -
>   static void pnv_ics_resend(XICSFabric *xi)
>   {
>       PnvMachineState *pnv = PNV_MACHINE(xi);
> -    int i;
> +    int i, j;
>   
>       for (i = 0; i < pnv->num_chips; i++) {
> -        PnvChip *chip = pnv->chips[i];
>           Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
>   
>           ics_resend(&chip8->psi.ics);
> -        object_child_foreach(OBJECT(chip), pnv_ics_resend_child, NULL);
> +
> +        for (j = 0; j < chip8->num_phbs; j++) {
> +            PnvPHB3 *phb3 = &chip8->phbs[j];
> +
> +            ics_resend(&phb3->lsis);
> +            ics_resend(ICS(&phb3->msis));
> +        }
>       }
>   }
>   

Re: [PATCH v2 7/9] ppc/pnv: make pnv_chip_power8_pic_print_info() use chip8->phbs[]

[Cédric Le Goater]

On 6/18/22 13:02, Daniel Henrique Barboza wrote:
> It's inneficient to scroll all child objects when we have all PHBs
> available in chip8->phbs[].
> 
> pnv_chip_power8_pic_print_info_child() ended up folded into
> pic_print_info() for simplicity.
> 
> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/ppc/pnv.c | 22 ++++++++--------------
>   1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 08136def8e..2a9067687b 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -652,25 +652,19 @@ static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
>       return PNV_CHIP_GET_CLASS(chip)->isa_create(chip, errp);
>   }
>   
> -static int pnv_chip_power8_pic_print_info_child(Object *child, void *opaque)
> -{
> -    Monitor *mon = opaque;
> -    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
> -
> -    if (phb3) {
> -        pnv_phb3_msi_pic_print_info(&phb3->msis, mon);
> -        ics_pic_print_info(&phb3->lsis, mon);
> -    }
> -    return 0;
> -}
> -
>   static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon)
>   {
>       Pnv8Chip *chip8 = PNV8_CHIP(chip);
> +    int i;
>   
>       ics_pic_print_info(&chip8->psi.ics, mon);
> -    object_child_foreach(OBJECT(chip),
> -                         pnv_chip_power8_pic_print_info_child, mon);
> +
> +    for (i = 0; i < chip8->num_phbs; i++) {
> +        PnvPHB3 *phb3 = &chip8->phbs[i];
> +
> +        pnv_phb3_msi_pic_print_info(&phb3->msis, mon);
> +        ics_pic_print_info(&phb3->lsis, mon);
> +    }
>   }
>   
>   static int pnv_chip_power9_pic_print_info_child(Object *child, void *opaque)

Re: [PATCH v2 3/9] ppc/pnv: use dev->parent_bus->parent to get the PHB

[Mark Cave-Ayland]

On 18/06/2022 12:01, Daniel Henrique Barboza wrote:

> It is not advisable to execute an object_dynamic_cast() to poke into
> bus->qbus.parent and follow it up with a C cast into the PnvPHB type we
> think we got.
> 
> A better way is to access the PnvPHB object via a QOM macro accessing
> the existing parent links of the DeviceState. For a given
> pnv-phb3/4-root-port 'dev', dev->parent_bus will give us the PHB bus,
> and dev->parent_bus->parent is the PHB. Use the adequate QOM macro to
> assert the type, and keep the NULL check in case we didn't get the
> object we were expecting.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   hw/pci-host/pnv_phb3.c | 10 +++++++---
>   hw/pci-host/pnv_phb4.c | 10 +++++++---
>   2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index 4ba660f8b9..5e7f827415 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -1139,12 +1139,16 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
>   {
>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
>       PCIDevice *pci = PCI_DEVICE(dev);
> -    PCIBus *bus = pci_get_bus(pci);
>       PnvPHB3 *phb = NULL;
>       Error *local_err = NULL;
>   
> -    phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
> -                                          TYPE_PNV_PHB3);
> +    /*
> +     * dev->parent_bus gives access to the pnv-phb-root bus.
> +     * The PnvPHB3 is the owner (parent) of the bus.
> +     */
> +    if (dev->parent_bus) {

Here dev->parent_bus shouldn't be accessed directly: you should use 
qdev_get_parent_bus() instead.

> +        phb = PNV_PHB3(dev->parent_bus->parent);
> +    }

This one is a bit trickier, since part of the qdev design is that devices should only 
be aware of their immediate bus, and not the device parenting that bus i.e. 
dev->parent_bus->parent shouldn't be allowed.

What is really needed here is to use QOM links (or embed the device as a suitable QOM 
child) to get the PHB reference which I imagine will be changed as part of the 
follow-up series. So I think this can be left as-is for now, and fixed later.

>       if (!phb) {
>           error_setg(errp,
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index ffd9d8a947..a0ee52e820 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1782,12 +1782,16 @@ static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
>   {
>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
>       PCIDevice *pci = PCI_DEVICE(dev);
> -    PCIBus *bus = pci_get_bus(pci);
>       PnvPHB4 *phb = NULL;
>       Error *local_err = NULL;
>   
> -    phb = (PnvPHB4 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
> -                                          TYPE_PNV_PHB4);
> +    /*
> +     * dev->parent_bus gives access to the pnv-phb-root bus.
> +     * The PnvPHB4 is the owner (parent) of the bus.
> +     */
> +    if (dev->parent_bus) {
> +        phb = PNV_PHB4(dev->parent_bus->parent);
> +    }
>   
>       if (!phb) {
>           error_setg(errp, "%s must be connected to pnv-phb4 buses", dev->id);

I've had a quick look over the rest of the series and from what I can see this is 
definitely heading in the right direction :)


ATB,

Mark.

Re: [PATCH v2 3/9] ppc/pnv: use dev->parent_bus->parent to get the PHB

[Daniel Henrique Barboza]

On 6/20/22 04:27, Mark Cave-Ayland wrote:
> On 18/06/2022 12:01, Daniel Henrique Barboza wrote:
> 
>> It is not advisable to execute an object_dynamic_cast() to poke into
>> bus->qbus.parent and follow it up with a C cast into the PnvPHB type we
>> think we got.
>>
>> A better way is to access the PnvPHB object via a QOM macro accessing
>> the existing parent links of the DeviceState. For a given
>> pnv-phb3/4-root-port 'dev', dev->parent_bus will give us the PHB bus,
>> and dev->parent_bus->parent is the PHB. Use the adequate QOM macro to
>> assert the type, and keep the NULL check in case we didn't get the
>> object we were expecting.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hw/pci-host/pnv_phb3.c | 10 +++++++---
>>   hw/pci-host/pnv_phb4.c | 10 +++++++---
>>   2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
>> index 4ba660f8b9..5e7f827415 100644
>> --- a/hw/pci-host/pnv_phb3.c
>> +++ b/hw/pci-host/pnv_phb3.c
>> @@ -1139,12 +1139,16 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
>>   {
>>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
>>       PCIDevice *pci = PCI_DEVICE(dev);
>> -    PCIBus *bus = pci_get_bus(pci);
>>       PnvPHB3 *phb = NULL;
>>       Error *local_err = NULL;
>> -    phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
>> -                                          TYPE_PNV_PHB3);
>> +    /*
>> +     * dev->parent_bus gives access to the pnv-phb-root bus.
>> +     * The PnvPHB3 is the owner (parent) of the bus.
>> +     */
>> +    if (dev->parent_bus) {
> 
> Here dev->parent_bus shouldn't be accessed directly: you should use qdev_get_parent_bus() instead.
> 
>> +        phb = PNV_PHB3(dev->parent_bus->parent);
>> +    }
> 
> This one is a bit trickier, since part of the qdev design is that devices should only be aware of their immediate bus, and not the device parenting that bus i.e. dev->parent_bus->parent shouldn't be allowed.
> 
> What is really needed here is to use QOM links (or embed the device as a suitable QOM child) to get the PHB reference which I imagine will be changed as part of the follow-up series. So I think this can be left as-is for now, and fixed later.


In the previous patch (2) I've put the root port as a child of the bus,
giving us this hierarchy:


     /pnv-phb3[0] (pnv-phb3)  <====== PHB
       /lsi (ics)
       /msi (phb3-msi)
       /msi32[0] (memory-region)
       /msi64[0] (memory-region)
       /pbcq (pnv-pbcq)
     (...)
       /phb3_iommu[0] (pnv-phb3-iommu-memory-region)
       /pnv-phb3-root.0 (pnv-phb3-root)  <=== bus
         /pnv-phb3-root-port[0] (pnv-phb3-root-port) <==== root-port
           /bus master container[0] (memory-region)
           /bus master[0] (memory-region)
           /pci_bridge_io[0] (memory-region)
           /pci_bridge_io[1] (memory-region)
           /pci_bridge_mem[0] (memory-region)
           /pci_bridge_pci[0] (memory-region)
           /pci_bridge_pref_mem[0] (memory-region)
           /pci_bridge_vga_io_hi[0] (memory-region)
           /pci_bridge_vga_io_lo[0] (memory-region)
           /pci_bridge_vga_mem[0] (memory-region)
           /pcie.0 (PCIE)


I did it like this instead of the PHB for no particular reason. If the root port of
other PHBs are located as a direct child of the PHB I can change it.


All that said, thinking more about it, since I need to access the PHB just
to set "chassis" and "slot" of the device, and I'm already setting a QOM
parent for it, I guess I'll just set that before root_port_realize() and spare us
from having to accessing the parent of the parent bus of the root_port.



Thanks,


Daniel

> 
>>       if (!phb) {
>>           error_setg(errp,
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index ffd9d8a947..a0ee52e820 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1782,12 +1782,16 @@ static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
>>   {
>>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
>>       PCIDevice *pci = PCI_DEVICE(dev);
>> -    PCIBus *bus = pci_get_bus(pci);
>>       PnvPHB4 *phb = NULL;
>>       Error *local_err = NULL;
>> -    phb = (PnvPHB4 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
>> -                                          TYPE_PNV_PHB4);
>> +    /*
>> +     * dev->parent_bus gives access to the pnv-phb-root bus.
>> +     * The PnvPHB4 is the owner (parent) of the bus.
>> +     */
>> +    if (dev->parent_bus) {
>> +        phb = PNV_PHB4(dev->parent_bus->parent);
>> +    }
>>       if (!phb) {
>>           error_setg(errp, "%s must be connected to pnv-phb4 buses", dev->id);
> 
> I've had a quick look over the rest of the series and from what I can see this is definitely heading in the right direction :)
> 
> 
> ATB,
> 
> Mark.

Re: [PATCH v2 5/9] ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array

[Frederic Barrat]

On 18/06/2022 13:01, Daniel Henrique Barboza wrote:
> The function is working today by getting all the child objects of the
> chip, interacting with each of them to check whether the child is a PHB,
> and then doing what needs to be done.
> 
> We have all the chip PHBs in the phbs[] array so interacting with all
> child objects is unneeded. Open code pnv_ics_get_phb_ics() into
> pnv_ics_get() and remove both pnv_ics_get_phb_ics() and the
> ForeachPhb3Args struct.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/ppc/pnv.c | 38 +++++++++++---------------------------
>   1 file changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 40e0cbd84d..ff7f803662 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1939,44 +1939,28 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir)
>       return NULL;
>   }
>   
> -typedef struct ForeachPhb3Args {
> -    int irq;
> -    ICSState *ics;
> -} ForeachPhb3Args;
> -
> -static int pnv_ics_get_child(Object *child, void *opaque)
> -{
> -    ForeachPhb3Args *args = opaque;
> -    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
> -
> -    if (phb3) {
> -        if (ics_valid_irq(&phb3->lsis, args->irq)) {
> -            args->ics = &phb3->lsis;
> -        }
> -        if (ics_valid_irq(ICS(&phb3->msis), args->irq)) {
> -            args->ics = ICS(&phb3->msis);
> -        }
> -    }
> -    return args->ics ? 1 : 0;
> -}
> -
>   static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
>   {
>       PnvMachineState *pnv = PNV_MACHINE(xi);
> -    ForeachPhb3Args args = { irq, NULL };
> -    int i;
> +    int i, j;
>   
>       for (i = 0; i < pnv->num_chips; i++) {
> -        PnvChip *chip = pnv->chips[i];
>           Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
>   
>           if (ics_valid_irq(&chip8->psi.ics, irq)) {
>               return &chip8->psi.ics;
>           }
>   
> -        object_child_foreach(OBJECT(chip), pnv_ics_get_child, &args);
> -        if (args.ics) {
> -            return args.ics;
> +        for (j = 0; j < chip8->num_phbs; j++) {
> +            PnvPHB3 *phb3 = &chip8->phbs[j];
> +
> +            if (ics_valid_irq(&phb3->lsis, irq)) {
> +                return &phb3->lsis;
> +            }
> +
> +            if (ics_valid_irq(ICS(&phb3->msis), irq)) {
> +                return ICS(&phb3->msis);
> +            }
>           }
>       }
>       return NULL;

Re: [PATCH v2 6/9] ppc/pnv: make pnv_ics_resend() use chip8->phbs[]

[Frederic Barrat]

On 18/06/2022 13:01, Daniel Henrique Barboza wrote:
> pnv_ics_resend() is scrolling through all the child objects of the chip
> to search for the PHBs. It's faster and simpler to just use the phbs[]
> array.
> 
> pnv_ics_resend_child() was folded into pnv_ics_resend() since it's too
> simple to justify its own function.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred

>   hw/ppc/pnv.c | 22 ++++++++--------------
>   1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index ff7f803662..08136def8e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1979,28 +1979,22 @@ PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
>       return NULL;
>   }
>   
> -static int pnv_ics_resend_child(Object *child, void *opaque)
> -{
> -    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
> -
> -    if (phb3) {
> -        ics_resend(&phb3->lsis);
> -        ics_resend(ICS(&phb3->msis));
> -    }
> -    return 0;
> -}
> -
>   static void pnv_ics_resend(XICSFabric *xi)
>   {
>       PnvMachineState *pnv = PNV_MACHINE(xi);
> -    int i;
> +    int i, j;
>   
>       for (i = 0; i < pnv->num_chips; i++) {
> -        PnvChip *chip = pnv->chips[i];
>           Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
>   
>           ics_resend(&chip8->psi.ics);
> -        object_child_foreach(OBJECT(chip), pnv_ics_resend_child, NULL);
> +
> +        for (j = 0; j < chip8->num_phbs; j++) {
> +            PnvPHB3 *phb3 = &chip8->phbs[j];
> +
> +            ics_resend(&phb3->lsis);
> +            ics_resend(ICS(&phb3->msis));
> +        }
>       }
>   }
>   

Re: [PATCH v2 8/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb3 root bus

[Frederic Barrat]

On 18/06/2022 13:02, Daniel Henrique Barboza wrote:
> It's unneeded. No other PCIE_BUS implements this interface.
> 
> Fixes: 9ae1329ee2fe ("ppc/pnv: Add models for POWER8 PHB3 PCIe Host bridge")
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---

It seems indeed not needed.
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb3.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index 8c03cc94f2..696c9bef26 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -1129,10 +1129,6 @@ static const TypeInfo pnv_phb3_root_bus_info = {
>       .name = TYPE_PNV_PHB3_ROOT_BUS,
>       .parent = TYPE_PCIE_BUS,
>       .class_init = pnv_phb3_root_bus_class_init,
> -    .interfaces = (InterfaceInfo[]) {
> -        { INTERFACE_PCIE_DEVICE },
> -        { }
> -    },
>   };
>   
>   static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)

Re: [PATCH v2 9/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb4 root bus

[Frederic Barrat]

On 18/06/2022 13:02, Daniel Henrique Barboza wrote:
> It's unneeded. No other PCIE_BUS implements this interface.
> 
> Fixes: 4f9924c4d4cf ("ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge")
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb4.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 61b45fe33c..81f7c1fe8f 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1751,10 +1751,6 @@ static const TypeInfo pnv_phb4_root_bus_info = {
>       .name = TYPE_PNV_PHB4_ROOT_BUS,
>       .parent = TYPE_PCIE_BUS,
>       .class_init = pnv_phb4_root_bus_class_init,
> -    .interfaces = (InterfaceInfo[]) {
> -        { INTERFACE_PCIE_DEVICE },
> -        { }
> -    },
>   };
>   
>   static void pnv_phb4_root_port_reset(DeviceState *dev)