Re: [PATCH 02/11] ppc/pnv: attach phb3/phb4 root ports in QOM tree

[Cédric Le Goater]

On 6/14/22 11:09, Frederic Barrat wrote:

On 13/06/2022 17:44, Daniel Henrique Barboza wrote:

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)

Signed-off-by: Daniel Henrique Barboza <danie...@linux.ibm.com>
---

I've always wondered if there was a good reason to have them detached. Glad to 
see there was none :-)

Wasn't it for libvirt integration ?

C.

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

   Fred

  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;