All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] powernv: introduce pnv-phb unified devices
@ 2022-05-07 19:06 Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2* Daniel Henrique Barboza
                   ` (18 more replies)
  0 siblings, 19 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

Hi,

Since the 7.0.0 release cycle we have a desire to use the powernv
emulation with libvirt. To do that we need to enable user creatable
pnv-phb devices to allow more user customization an to avoid spamming
multiple default devices in the domain XML. In the middle of the
previous cycle we experimented with user created
pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
end result, although functional, is that the user needs to deal with a
lot of versioned devices that, from the user perspective, does the same
thing. In a way we outsourced the implementation details of the PHBs
(e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
devices also puts an extra burden in the libvirt support we want to
have.

To solve this, Cedric and Frederic gave the idea of adding a common
virtual pnv-phb device that the user can add in the command line, and
QEMU handles the details internally. Unfortunatelly this idea turned out
to be way harder than anticipated. Between creating a device that would
just forward the callbacks to the existing devices internally, creating
a PnvPHB device with the minimal attributes and making the other devices
inherit from it, and making an 'abstract' device that could be casted
for both phb3 and phb4 PHBs, all sorts of very obscure problems occured:
PHBs not being detected, interrupts not being delivered and memory
regions not being able to read/write registers. My initial impression is
that there are assumptions made both in ppc/pnv and hw/pci-host that
requires the memory region and the bus being in the same device. Even
if we somehow figure all this out, the resulting code is hacky and
annoying to maitain.

This brings us to this series. The cleaner way I found to accomplish
what we want to do is to create real, unified pnv-phb/phb-phb-root-port
devices, and get rid of all the versioned ones. This is done by
merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
files end up using the same pnv-phb and phb-phb-root-port unified devices,
with the difference that pnv_phb3* only cares about version 3 attributes
and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
versions in the future will be a matter of adding any non-existent
attributes in the unified pnv-phb* devices and using them in separated
pnv_phbN* files. 

The pnv-phb implementation per se is just a router for either phb3 or
phb4 logic, done in init() and realize() time, after we check which powernv
machine we're running. If running with powernv8 we redirect control to
pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
unified device does not do anything per se other than handling control
to the right version.

After this series, this same powernv8 command line that boots a powernv8
machine with some phbs and root ports and with network:

./qemu-system-ppc64 -m 4G \
-machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
-accel tcg,thread=multi -bios skiboot.lid  \
-kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
-nodefaults  -serial mon:stdio -nographic  \
-device pnv-phb,chip-id=0,index=0,id=pcie.0 \
-device pnv-phb,chip-id=0,index=1,id=pcie.1 \
-device pnv-phb,chip-id=1,index=2,id=pcie.2 \
-device pnv-phb-root-port,id=root0,bus=pcie.2 \
-device pnv-phb-root-port,id=root1,bus=pcie.1 \
-device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
-device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
-drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
-device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
-netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
-device nec-usb-xhci,bus=bridge1,addr=0x2


Can be used to boot powernv9 and powernv10 machines with the same attributes
just by changing the machine type. 


Daniel Henrique Barboza (17):
  ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
  ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
  ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
  ppc/pnv: add unified pnv-phb header
  ppc/pnv: add pnv-phb device
  ppc/pnv: remove PnvPHB3
  ppc/pnv: user created pnv-phb for powernv8
  ppc/pnv: remove PnvPHB4
  ppc/pnv: user creatable pnv-phb for powernv9
  ppc/pnv: use PnvPHB.version
  ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
  ppc/pnv: user creatable pnv-phb for powernv10
  ppc/pnv: add pnv_phb_get_current_machine()
  ppc/pnv: add pnv-phb-root-port device
  ppc/pnv: remove pnv-phb3-root-port
  ppc/pnv: remove pnv-phb4-root-port
  ppc/pnv: remove pecc->rp_model

 hw/pci-host/meson.build        |   3 +-
 hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
 hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
 hw/pci-host/pnv_phb3_msi.c     |  12 +-
 hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
 hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
 hw/pci-host/pnv_phb4_pec.c     |  14 +-
 hw/ppc/pnv.c                   |  41 ++++-
 include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
 include/hw/pci-host/pnv_phb3.h | 118 +------------
 include/hw/pci-host/pnv_phb4.h | 108 ++----------
 include/hw/ppc/pnv.h           |   3 +-
 12 files changed, 757 insertions(+), 586 deletions(-)
 create mode 100644 hw/pci-host/pnv_phb.c
 create mode 100644 include/hw/pci-host/pnv_phb.h

-- 
2.32.0



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 02/17] ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[] Daniel Henrique Barboza
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

We're going to merge all the existing pnv-phb models into a single
pnv-phb model. Users will be able to add phbs by using the same pnv-phb
device, regardless of which powernv machine is being used, and
internally we'll handle which PHB version the device needs to have.

The unified pnv-phb model needs to be usable by the existing pnv_phb3
and pnv_phb4 code base. One way of accomplishing that is to merge
PnvPHB3 and PnvPHB4 into a single PnvPHB struct. To do that we need to
handle the cases where the same attribute might have different
meaning/semantics depending on the version.

One of these attributes is the 'ioda' arrays. This patch renames
PnvPHB3.ioda* arrays to PnvPHB3.ioda2*. The reason why we're calling
'ioda2' is because PnvPHB3 uses IODA version 2.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c         | 18 +++++++++---------
 include/hw/pci-host/pnv_phb3.h | 12 ++++++------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 3f03467dde..860f8846df 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -165,7 +165,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index)
     }
 
     /* Get table entry */
-    m64 = phb->ioda_M64BT[index];
+    m64 = phb->ioda2_M64BT[index];
 
     if (!(m64 & IODA2_M64BT_ENABLE)) {
         return;
@@ -215,7 +215,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val)
 {
     uint8_t server, prio;
 
-    phb->ioda_LXIVT[idx] = val & (IODA2_LXIVT_SERVER |
+    phb->ioda2_LXIVT[idx] = val & (IODA2_LXIVT_SERVER |
                                   IODA2_LXIVT_PRIORITY |
                                   IODA2_LXIVT_NODE_ID);
     server = GETFIELD(IODA2_LXIVT_SERVER, val);
@@ -241,11 +241,11 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
 
     switch (table) {
     case IODA2_TBL_LIST:
-        tptr = phb->ioda_LIST;
+        tptr = phb->ioda2_LIST;
         mask = 7;
         break;
     case IODA2_TBL_LXIVT:
-        tptr = phb->ioda_LXIVT;
+        tptr = phb->ioda2_LXIVT;
         mask = 7;
         break;
     case IODA2_TBL_IVC_CAM:
@@ -263,7 +263,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
         mask = 255;
         break;
     case IODA2_TBL_TVT:
-        tptr = phb->ioda_TVT;
+        tptr = phb->ioda2_TVT;
         mask = 511;
         break;
     case IODA2_TBL_TCAM:
@@ -271,15 +271,15 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
         mask = 63;
         break;
     case IODA2_TBL_M64BT:
-        tptr = phb->ioda_M64BT;
+        tptr = phb->ioda2_M64BT;
         mask = 15;
         break;
     case IODA2_TBL_M32DT:
-        tptr = phb->ioda_MDT;
+        tptr = phb->ioda2_MDT;
         mask = 255;
         break;
     case IODA2_TBL_PEEV:
-        tptr = phb->ioda_PEEV;
+        tptr = phb->ioda2_PEEV;
         mask = 3;
         break;
     default:
@@ -869,7 +869,7 @@ static IOMMUTLBEntry pnv_phb3_translate_iommu(IOMMUMemoryRegion *iommu,
         }
         /* Choose TVE XXX Use PHB3 Control Register */
         tve_sel = (addr >> 59) & 1;
-        tve = ds->phb->ioda_TVT[ds->pe_num * 2 + tve_sel];
+        tve = ds->phb->ioda2_TVT[ds->pe_num * 2 + tve_sel];
         pnv_phb3_translate_tve(ds, addr, flag & IOMMU_WO, tve, &ret);
         break;
     case 01:
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index af6ec83cf6..73da31fbd2 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -141,12 +141,12 @@ struct PnvPHB3 {
     MemoryRegion pci_mmio;
     MemoryRegion pci_io;
 
-    uint64_t ioda_LIST[8];
-    uint64_t ioda_LXIVT[8];
-    uint64_t ioda_TVT[512];
-    uint64_t ioda_M64BT[16];
-    uint64_t ioda_MDT[256];
-    uint64_t ioda_PEEV[4];
+    uint64_t ioda2_LIST[8];
+    uint64_t ioda2_LXIVT[8];
+    uint64_t ioda2_TVT[512];
+    uint64_t ioda2_M64BT[16];
+    uint64_t ioda2_MDT[256];
+    uint64_t ioda2_PEEV[4];
 
     uint32_t total_irq;
     ICSState lsis;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 02/17] ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2* Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 03/17] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces Daniel Henrique Barboza
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

Another redundancy between PnvPHB3 and PnvPHB4 that we should take care
before merging all together in an unified model is the regs[] array.

Rename the regs[] array used by the PHB3 code to 'regs3'.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c         | 54 +++++++++++++++++-----------------
 hw/pci-host/pnv_phb3_msi.c     |  6 ++--
 include/hw/pci-host/pnv_phb3.h |  4 +--
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 860f8846df..77ee2325be 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -27,7 +27,7 @@
 static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
-    uint64_t addr = phb->regs[PHB_CONFIG_ADDRESS >> 3];
+    uint64_t addr = phb->regs3[PHB_CONFIG_ADDRESS >> 3];
     uint8_t bus, devfn;
 
     if (!(addr >> 63)) {
@@ -53,7 +53,7 @@ static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off,
     if (!pdev) {
         return;
     }
-    cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
+    cfg_addr = (phb->regs3[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
     cfg_addr |= off;
     limit = pci_config_size(pdev);
     if (limit <= cfg_addr) {
@@ -89,7 +89,7 @@ static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off,
     if (!pdev) {
         return ~0ull;
     }
-    cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
+    cfg_addr = (phb->regs3[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
     cfg_addr |= off;
     limit = pci_config_size(pdev);
     if (limit <= cfg_addr) {
@@ -122,14 +122,14 @@ static void pnv_phb3_check_m32(PnvPHB3 *phb)
         memory_region_del_subregion(phb->mr_m32.container, &phb->mr_m32);
     }
 
-    if (!(phb->regs[PHB_PHB3_CONFIG >> 3] & PHB_PHB3C_M32_EN)) {
+    if (!(phb->regs3[PHB_PHB3_CONFIG >> 3] & PHB_PHB3C_M32_EN)) {
         return;
     }
 
     /* Grab geometry from registers */
-    base = phb->regs[PHB_M32_BASE_ADDR >> 3];
-    start = phb->regs[PHB_M32_START_ADDR >> 3];
-    size = ~(phb->regs[PHB_M32_BASE_MASK >> 3] | 0xfffc000000000000ull) + 1;
+    base = phb->regs3[PHB_M32_BASE_ADDR >> 3];
+    start = phb->regs3[PHB_M32_START_ADDR >> 3];
+    size = ~(phb->regs3[PHB_M32_BASE_MASK >> 3] | 0xfffc000000000000ull) + 1;
 
     /* Check if it matches an enabled MMIO region in the PBCQ */
     if (memory_region_is_mapped(&pbcq->mmbar0) &&
@@ -179,7 +179,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index)
     size = GETFIELD(IODA2_M64BT_MASK, m64) << 20;
     size |= 0xfffc000000000000ull;
     size = ~size + 1;
-    start = base | (phb->regs[PHB_M64_UPPER_BITS >> 3]);
+    start = base | (phb->regs3[PHB_M64_UPPER_BITS >> 3]);
 
     /* Check if it matches an enabled MMIO region in the PBCQ */
     if (memory_region_is_mapped(&pbcq->mmbar0) &&
@@ -233,7 +233,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val)
 static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
                                       unsigned *out_table, unsigned *out_idx)
 {
-    uint64_t adreg = phb->regs[PHB_IODA_ADDR >> 3];
+    uint64_t adreg = phb->regs3[PHB_IODA_ADDR >> 3];
     unsigned int index = GETFIELD(PHB_IODA_AD_TADR, adreg);
     unsigned int table = GETFIELD(PHB_IODA_AD_TSEL, adreg);
     unsigned int mask;
@@ -300,7 +300,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
         index = (index + 1) & mask;
         adreg = SETFIELD(PHB_IODA_AD_TADR, adreg, index);
     }
-    phb->regs[PHB_IODA_ADDR >> 3] = adreg;
+    phb->regs3[PHB_IODA_ADDR >> 3] = adreg;
     return tptr;
 }
 
@@ -364,7 +364,7 @@ void pnv_phb3_remap_irqs(PnvPHB3 *phb)
     }
 
     /* Grab local LSI source ID */
-    local = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]) << 3;
+    local = GETFIELD(PHB_LSI_SRC_ID, phb->regs3[PHB_LSI_SOURCE_ID >> 3]) << 3;
 
     /* Grab global one and compare */
     global = GETFIELD(PBCQ_NEST_LSI_SRC,
@@ -412,7 +412,7 @@ static void pnv_phb3_lsi_src_id_write(PnvPHB3 *phb, uint64_t val)
 {
     /* Sanitize content */
     val &= PHB_LSI_SRC_ID;
-    phb->regs[PHB_LSI_SOURCE_ID >> 3] = val;
+    phb->regs3[PHB_LSI_SOURCE_ID >> 3] = val;
     pnv_phb3_remap_irqs(phb);
 }
 
@@ -429,7 +429,7 @@ static void pnv_phb3_rtc_invalidate(PnvPHB3 *phb, uint64_t val)
 
 static void pnv_phb3_update_msi_regions(PnvPhb3DMASpace *ds)
 {
-    uint64_t cfg = ds->phb->regs[PHB_PHB3_CONFIG >> 3];
+    uint64_t cfg = ds->phb->regs3[PHB_PHB3_CONFIG >> 3];
 
     if (cfg & PHB_PHB3C_32BIT_MSI_EN) {
         if (!memory_region_is_mapped(&ds->msi32_mr)) {
@@ -501,16 +501,16 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size)
         break;
     /* LEM stuff */
     case PHB_LEM_FIR_AND_MASK:
-        phb->regs[PHB_LEM_FIR_ACCUM >> 3] &= val;
+        phb->regs3[PHB_LEM_FIR_ACCUM >> 3] &= val;
         return;
     case PHB_LEM_FIR_OR_MASK:
-        phb->regs[PHB_LEM_FIR_ACCUM >> 3] |= val;
+        phb->regs3[PHB_LEM_FIR_ACCUM >> 3] |= val;
         return;
     case PHB_LEM_ERROR_AND_MASK:
-        phb->regs[PHB_LEM_ERROR_MASK >> 3] &= val;
+        phb->regs3[PHB_LEM_ERROR_MASK >> 3] &= val;
         return;
     case PHB_LEM_ERROR_OR_MASK:
-        phb->regs[PHB_LEM_ERROR_MASK >> 3] |= val;
+        phb->regs3[PHB_LEM_ERROR_MASK >> 3] |= val;
         return;
     case PHB_LEM_WOF:
         val = 0;
@@ -518,10 +518,10 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size)
     }
 
     /* Record whether it changed */
-    changed = phb->regs[off >> 3] != val;
+    changed = phb->regs3[off >> 3] != val;
 
     /* Store in register cache first */
-    phb->regs[off >> 3] = val;
+    phb->regs3[off >> 3] = val;
 
     /* Handle side effects */
     switch (off) {
@@ -605,7 +605,7 @@ uint64_t pnv_phb3_reg_read(void *opaque, hwaddr off, unsigned size)
     }
 
     /* Default read from cache */
-    val = phb->regs[off >> 3];
+    val = phb->regs3[off >> 3];
 
     switch (off) {
     /* Simulate venice DD2.0 */
@@ -628,7 +628,7 @@ uint64_t pnv_phb3_reg_read(void *opaque, hwaddr off, unsigned size)
     /* FFI Lock */
     case PHB_FFI_LOCK:
         /* Set lock and return previous value */
-        phb->regs[off >> 3] |= PHB_FFI_LOCK_STATE;
+        phb->regs3[off >> 3] |= PHB_FFI_LOCK_STATE;
         return val;
 
     /* DMA read sync: make it look like it's complete */
@@ -704,7 +704,7 @@ static bool pnv_phb3_resolve_pe(PnvPhb3DMASpace *ds)
     }
 
     /* We need to lookup the RTT */
-    rtt = ds->phb->regs[PHB_RTT_BAR >> 3];
+    rtt = ds->phb->regs3[PHB_RTT_BAR >> 3];
     if (!(rtt & PHB_RTT_BAR_ENABLE)) {
         phb3_error(ds->phb, "DMA with RTT BAR disabled !");
         /* Set error bits ? fence ? ... */
@@ -861,7 +861,7 @@ static IOMMUTLBEntry pnv_phb3_translate_iommu(IOMMUMemoryRegion *iommu,
     switch (addr >> 60) {
     case 00:
         /* DMA or 32-bit MSI ? */
-        cfg = ds->phb->regs[PHB_PHB3_CONFIG >> 3];
+        cfg = ds->phb->regs3[PHB_PHB3_CONFIG >> 3];
         if ((cfg & PHB_PHB3C_32BIT_MSI_EN) &&
             ((addr & 0xffffffffffff0000ull) == 0xffff0000ull)) {
             phb3_error(phb, "xlate on 32-bit MSI region");
@@ -1032,7 +1032,7 @@ static void pnv_phb3_realize(DeviceState *dev, Error **errp)
     }
 
     /* Controller Registers */
-    memory_region_init_io(&phb->mr_regs, OBJECT(phb), &pnv_phb3_reg_ops, phb,
+    memory_region_init_io(&phb->mr_regs3, OBJECT(phb), &pnv_phb3_reg_ops, phb,
                           "phb3-regs", 0x1000);
 
     /*
@@ -1060,14 +1060,14 @@ void pnv_phb3_update_regions(PnvPHB3 *phb)
     PnvPBCQState *pbcq = &phb->pbcq;
 
     /* Unmap first always */
-    if (memory_region_is_mapped(&phb->mr_regs)) {
-        memory_region_del_subregion(&pbcq->phbbar, &phb->mr_regs);
+    if (memory_region_is_mapped(&phb->mr_regs3)) {
+        memory_region_del_subregion(&pbcq->phbbar, &phb->mr_regs3);
     }
 
     /* Map registers if enabled */
     if (memory_region_is_mapped(&pbcq->phbbar)) {
         /* TODO: We should use the PHB BAR 2 register but we don't ... */
-        memory_region_add_subregion(&pbcq->phbbar, 0, &phb->mr_regs);
+        memory_region_add_subregion(&pbcq->phbbar, 0, &phb->mr_regs3);
     }
 
     /* Check/update m32 */
diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
index 2f4112907b..d8534376f8 100644
--- a/hw/pci-host/pnv_phb3_msi.c
+++ b/hw/pci-host/pnv_phb3_msi.c
@@ -20,8 +20,8 @@
 
 static uint64_t phb3_msi_ive_addr(PnvPHB3 *phb, int srcno)
 {
-    uint64_t ivtbar = phb->regs[PHB_IVT_BAR >> 3];
-    uint64_t phbctl = phb->regs[PHB_CONTROL >> 3];
+    uint64_t ivtbar = phb->regs3[PHB_IVT_BAR >> 3];
+    uint64_t phbctl = phb->regs3[PHB_CONTROL >> 3];
 
     if (!(ivtbar & PHB_IVT_BAR_ENABLE)) {
         qemu_log_mask(LOG_GUEST_ERROR, "Failed access to disable IVT BAR !");
@@ -188,7 +188,7 @@ void pnv_phb3_msi_ffi(Phb3MsiState *msi, uint64_t val)
     pnv_phb3_msi_send(msi, val, 0, -1);
 
     /* Clear FFI lock */
-    msi->phb->regs[PHB_FFI_LOCK >> 3] = 0;
+    msi->phb->regs3[PHB_FFI_LOCK >> 3] = 0;
 }
 
 static void phb3_msi_reject(ICSState *ics, uint32_t nr)
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index 73da31fbd2..486dbbefee 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -133,8 +133,8 @@ struct PnvPHB3 {
     uint32_t phb_id;
     char bus_path[8];
 
-    uint64_t regs[PNV_PHB3_NUM_REGS];
-    MemoryRegion mr_regs;
+    uint64_t regs3[PNV_PHB3_NUM_REGS];
+    MemoryRegion mr_regs3;
 
     MemoryRegion mr_m32;
     MemoryRegion mr_m64[PNV_PHB3_NUM_M64];
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 03/17] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2* Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 02/17] ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[] Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 04/17] ppc/pnv: add unified pnv-phb header Daniel Henrique Barboza
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The last common attribute that has a different meaning/semantic between
PnvPHB3 and PnvPHB4 devices is the 'dma_spaces' QLIST.

Rename the PHB3 version to 'v3_dma_spaces'. The reason why we chose that
instead of 'dma3_spaces' or similar is to avoid any misunderstanding
about this being related to DMA version 3.

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

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 77ee2325be..70d92edd94 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -421,7 +421,7 @@ static void pnv_phb3_rtc_invalidate(PnvPHB3 *phb, uint64_t val)
     PnvPhb3DMASpace *ds;
 
     /* Always invalidate all for now ... */
-    QLIST_FOREACH(ds, &phb->dma_spaces, list) {
+    QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) {
         ds->pe_num = PHB_INVALID_PE;
     }
 }
@@ -460,7 +460,7 @@ static void pnv_phb3_update_all_msi_regions(PnvPHB3 *phb)
 {
     PnvPhb3DMASpace *ds;
 
-    QLIST_FOREACH(ds, &phb->dma_spaces, list) {
+    QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) {
         pnv_phb3_update_msi_regions(ds);
     }
 }
@@ -938,7 +938,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     PnvPHB3 *phb = opaque;
     PnvPhb3DMASpace *ds;
 
-    QLIST_FOREACH(ds, &phb->dma_spaces, list) {
+    QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) {
         if (ds->bus == bus && ds->devfn == devfn) {
             break;
         }
@@ -961,7 +961,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn)
                               ds, "msi64", 0x100000);
         pnv_phb3_update_msi_regions(ds);
 
-        QLIST_INSERT_HEAD(&phb->dma_spaces, ds, list);
+        QLIST_INSERT_HEAD(&phb->v3_dma_spaces, ds, list);
     }
     return &ds->dma_as;
 }
@@ -970,7 +970,7 @@ static void pnv_phb3_instance_init(Object *obj)
 {
     PnvPHB3 *phb = PNV_PHB3(obj);
 
-    QLIST_INIT(&phb->dma_spaces);
+    QLIST_INIT(&phb->v3_dma_spaces);
 
     /* LSI sources */
     object_initialize_child(obj, "lsi", &phb->lsis, TYPE_ICS);
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index 486dbbefee..35483e59c3 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -155,7 +155,7 @@ struct PnvPHB3 {
 
     PnvPBCQState pbcq;
 
-    QLIST_HEAD(, PnvPhb3DMASpace) dma_spaces;
+    QLIST_HEAD(, PnvPhb3DMASpace) v3_dma_spaces;
 
     PnvChip *chip;
 };
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 04/17] ppc/pnv: add unified pnv-phb header
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (2 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 03/17] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 05/17] ppc/pnv: add pnv-phb device Daniel Henrique Barboza
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

This patch adds the pnv_phb.h header with the declarations we're going
to use in this soon to be added device.

It consists of an union between all attributes of PnvPHB3 and PnvPHB4
devices. This will allow for the same PnvPHB device to be used for PHB3
and PHB4 code.

Some struct definitions from PnvPHB3 had to be moved to the new header
due to scope constraints.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 include/hw/pci-host/pnv_phb.h  | 211 +++++++++++++++++++++++++++++++++
 include/hw/pci-host/pnv_phb3.h |  65 +---------
 2 files changed, 212 insertions(+), 64 deletions(-)
 create mode 100644 include/hw/pci-host/pnv_phb.h

diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h
new file mode 100644
index 0000000000..2a8bf9a66d
--- /dev/null
+++ b/include/hw/pci-host/pnv_phb.h
@@ -0,0 +1,211 @@
+/*
+ * QEMU PowerPC PowerNV Unified PHB model
+ *
+ * Copyright (c) 2022, IBM Corporation.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef PCI_HOST_PNV_PHB_H
+#define PCI_HOST_PNV_PHB_H
+
+#include "hw/pci/pcie_host.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/ppc/xics.h"
+#include "hw/ppc/xive.h"
+#include "qom/object.h"
+
+/* pnv_phb3.h types */
+#define PNV_PHB3_NUM_M64      16
+#define PNV_PHB3_NUM_REGS     (0x1000 >> 3)
+#define PHB3_MAX_MSI     2048
+
+typedef struct PnvPHB3 PnvPHB3;
+typedef struct PnvChip PnvChip;
+
+typedef struct Phb3MsiState {
+    ICSState ics;
+    qemu_irq *qirqs;
+
+    PnvPHB3 *phb;
+    uint64_t rba[PHB3_MAX_MSI / 64];
+    uint32_t rba_sum;
+} Phb3MsiState;
+
+typedef struct PnvPBCQState {
+    DeviceState parent;
+
+    uint32_t nest_xbase;
+    uint32_t spci_xbase;
+    uint32_t pci_xbase;
+#define PBCQ_NEST_REGS_COUNT    0x46
+#define PBCQ_PCI_REGS_COUNT     0x15
+#define PBCQ_SPCI_REGS_COUNT    0x5
+
+    uint64_t nest_regs[PBCQ_NEST_REGS_COUNT];
+    uint64_t spci_regs[PBCQ_SPCI_REGS_COUNT];
+    uint64_t pci_regs[PBCQ_PCI_REGS_COUNT];
+    MemoryRegion mmbar0;
+    MemoryRegion mmbar1;
+    MemoryRegion phbbar;
+    uint64_t mmio0_base;
+    uint64_t mmio0_size;
+    uint64_t mmio1_base;
+    uint64_t mmio1_size;
+    PnvPHB3 *phb;
+
+    MemoryRegion xscom_nest_regs;
+    MemoryRegion xscom_pci_regs;
+    MemoryRegion xscom_spci_regs;
+} PnvPBCQState;
+
+/*
+ * We have one such address space wrapper per possible device under
+ * the PHB since they need to be assigned statically at qemu device
+ * creation time. The relationship to a PE is done later dynamically.
+ * This means we can potentially create a lot of these guys. Q35
+ * stores them as some kind of radix tree but we never really need to
+ * do fast lookups so instead we simply keep a QLIST of them for now,
+ * we can add the radix if needed later on.
+ *
+ * We do cache the PE number to speed things up a bit though.
+ */
+typedef struct PnvPhb3DMASpace {
+    PCIBus *bus;
+    uint8_t devfn;
+    int pe_num;         /* Cached PE number */
+#define PHB_INVALID_PE (-1)
+    PnvPHB3 *phb;
+    AddressSpace dma_as;
+    IOMMUMemoryRegion dma_mr;
+    MemoryRegion msi32_mr;
+    MemoryRegion msi64_mr;
+    QLIST_ENTRY(PnvPhb3DMASpace) list;
+} PnvPhb3DMASpace;
+
+/* pnv_phb4.h types */
+#define PNV_PHB4_MAX_LSIs          8
+#define PNV_PHB4_MAX_INTs          4096
+#define PNV_PHB4_MAX_MIST          (PNV_PHB4_MAX_INTs >> 2)
+#define PNV_PHB4_MAX_MMIO_WINDOWS  32
+#define PNV_PHB4_MIN_MMIO_WINDOWS  16
+#define PNV_PHB4_NUM_REGS          (0x3000 >> 3)
+#define PNV_PHB4_MAX_PEs           512
+#define PNV_PHB4_MAX_TVEs          (PNV_PHB4_MAX_PEs * 2)
+#define PNV_PHB4_MAX_PEEVs         (PNV_PHB4_MAX_PEs / 64)
+#define PNV_PHB4_MAX_MBEs          (PNV_PHB4_MAX_MMIO_WINDOWS * 2)
+typedef struct PnvPhb4PecState PnvPhb4PecState;
+
+/*
+ * Unified PHB PCIe Host Bridge for PowerNV machines
+ */
+typedef struct PnvPHB PnvPHB;
+#define TYPE_PNV_PHB "pnv-phb"
+OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB)
+
+struct PnvPHB {
+    PCIExpressHost parent_obj;
+
+    uint32_t chip_id;
+    uint32_t phb_id;
+    char bus_path[8];
+    MemoryRegion pci_mmio;
+    MemoryRegion pci_io;
+    qemu_irq *qirqs;
+
+    /*
+     * PnvPHB3 attributes
+     */
+    uint64_t regs3[PNV_PHB3_NUM_REGS];
+    MemoryRegion mr_regs3;
+
+    MemoryRegion mr_m32;
+    MemoryRegion mr_m64[PNV_PHB3_NUM_M64];
+
+    uint64_t ioda2_LIST[8];
+    uint64_t ioda2_LXIVT[8];
+    uint64_t ioda2_TVT[512];
+    uint64_t ioda2_M64BT[16];
+    uint64_t ioda2_MDT[256];
+    uint64_t ioda2_PEEV[4];
+
+    uint32_t total_irq;
+    ICSState lsis;
+    Phb3MsiState msis;
+
+    PnvPBCQState pbcq;
+
+    QLIST_HEAD(, PnvPhb3DMASpace) v3_dma_spaces;
+
+    PnvChip *chip;
+
+    /*
+     * PnvPHB4 attributes
+     */
+    uint64_t version;
+
+    /* The owner PEC */
+    PnvPhb4PecState *pec;
+
+    /* Main register images */
+    uint64_t regs[PNV_PHB4_NUM_REGS];
+    MemoryRegion mr_regs;
+
+    /* Extra SCOM-only register */
+    uint64_t scom_hv_ind_addr_reg;
+
+    /*
+     * Geometry of the PHB. There are two types, small and big PHBs, a
+     * number of resources (number of PEs, windows etc...) are doubled
+     * for a big PHB
+     */
+    bool big_phb;
+
+    /* Memory regions for MMIO space */
+    MemoryRegion mr_mmio[PNV_PHB4_MAX_MMIO_WINDOWS];
+
+    /* PCI registers (excluding pass-through) */
+#define PHB4_PEC_PCI_STK_REGS_COUNT  0xf
+    uint64_t pci_regs[PHB4_PEC_PCI_STK_REGS_COUNT];
+    MemoryRegion pci_regs_mr;
+
+    /* Nest registers */
+#define PHB4_PEC_NEST_STK_REGS_COUNT  0x17
+    uint64_t nest_regs[PHB4_PEC_NEST_STK_REGS_COUNT];
+    MemoryRegion nest_regs_mr;
+
+    /* PHB pass-through XSCOM */
+    MemoryRegion phb_regs_mr;
+
+    /* Memory windows from PowerBus to PHB */
+    MemoryRegion phbbar;
+    MemoryRegion intbar;
+    MemoryRegion mmbar0;
+    MemoryRegion mmbar1;
+    uint64_t mmio0_base;
+    uint64_t mmio0_size;
+    uint64_t mmio1_base;
+    uint64_t mmio1_size;
+
+    /* On-chip IODA tables */
+    uint64_t ioda_LIST[PNV_PHB4_MAX_LSIs];
+    uint64_t ioda_MIST[PNV_PHB4_MAX_MIST];
+    uint64_t ioda_TVT[PNV_PHB4_MAX_TVEs];
+    uint64_t ioda_MBT[PNV_PHB4_MAX_MBEs];
+    uint64_t ioda_MDT[PNV_PHB4_MAX_PEs];
+    uint64_t ioda_PEEV[PNV_PHB4_MAX_PEEVs];
+
+    /*
+     * The internal PESTA/B is 2 bits per PE split into two tables, we
+     * store them in a single array here to avoid wasting space.
+     */
+    uint8_t  ioda_PEST_AB[PNV_PHB4_MAX_PEs];
+
+    /* P9 Interrupt generation */
+    XiveSource xsrc;
+
+    QLIST_HEAD(, PnvPhb4DMASpace) dma_spaces;
+};
+
+#endif /* PCI_HOST_PNV_PHB_H */
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index 35483e59c3..b6b5f91684 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -12,6 +12,7 @@
 
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pcie_port.h"
+#include "hw/pci-host/pnv_phb.h"
 #include "hw/ppc/xics.h"
 #include "qom/object.h"
 
@@ -22,21 +23,9 @@ typedef struct PnvChip PnvChip;
  * PHB3 XICS Source for MSIs
  */
 #define TYPE_PHB3_MSI "phb3-msi"
-typedef struct Phb3MsiState Phb3MsiState;
 DECLARE_INSTANCE_CHECKER(Phb3MsiState, PHB3_MSI,
                          TYPE_PHB3_MSI)
 
-#define PHB3_MAX_MSI     2048
-
-struct Phb3MsiState {
-    ICSState ics;
-    qemu_irq *qirqs;
-
-    PnvPHB3 *phb;
-    uint64_t rba[PHB3_MAX_MSI / 64];
-    uint32_t rba_sum;
-};
-
 void pnv_phb3_msi_update_config(Phb3MsiState *msis, uint32_t base,
                                 uint32_t count);
 void pnv_phb3_msi_send(Phb3MsiState *msis, uint64_t addr, uint16_t data,
@@ -44,64 +33,12 @@ void pnv_phb3_msi_send(Phb3MsiState *msis, uint64_t addr, uint16_t data,
 void pnv_phb3_msi_ffi(Phb3MsiState *msis, uint64_t val);
 void pnv_phb3_msi_pic_print_info(Phb3MsiState *msis, Monitor *mon);
 
-
-/*
- * We have one such address space wrapper per possible device under
- * the PHB since they need to be assigned statically at qemu device
- * creation time. The relationship to a PE is done later dynamically.
- * This means we can potentially create a lot of these guys. Q35
- * stores them as some kind of radix tree but we never really need to
- * do fast lookups so instead we simply keep a QLIST of them for now,
- * we can add the radix if needed later on.
- *
- * We do cache the PE number to speed things up a bit though.
- */
-typedef struct PnvPhb3DMASpace {
-    PCIBus *bus;
-    uint8_t devfn;
-    int pe_num;         /* Cached PE number */
-#define PHB_INVALID_PE (-1)
-    PnvPHB3 *phb;
-    AddressSpace dma_as;
-    IOMMUMemoryRegion dma_mr;
-    MemoryRegion msi32_mr;
-    MemoryRegion msi64_mr;
-    QLIST_ENTRY(PnvPhb3DMASpace) list;
-} PnvPhb3DMASpace;
-
 /*
  * PHB3 Power Bus Common Queue
  */
 #define TYPE_PNV_PBCQ "pnv-pbcq"
 OBJECT_DECLARE_SIMPLE_TYPE(PnvPBCQState, PNV_PBCQ)
 
-struct PnvPBCQState {
-    DeviceState parent;
-
-    uint32_t nest_xbase;
-    uint32_t spci_xbase;
-    uint32_t pci_xbase;
-#define PBCQ_NEST_REGS_COUNT    0x46
-#define PBCQ_PCI_REGS_COUNT     0x15
-#define PBCQ_SPCI_REGS_COUNT    0x5
-
-    uint64_t nest_regs[PBCQ_NEST_REGS_COUNT];
-    uint64_t spci_regs[PBCQ_SPCI_REGS_COUNT];
-    uint64_t pci_regs[PBCQ_PCI_REGS_COUNT];
-    MemoryRegion mmbar0;
-    MemoryRegion mmbar1;
-    MemoryRegion phbbar;
-    uint64_t mmio0_base;
-    uint64_t mmio0_size;
-    uint64_t mmio1_base;
-    uint64_t mmio1_size;
-    PnvPHB3 *phb;
-
-    MemoryRegion xscom_nest_regs;
-    MemoryRegion xscom_pci_regs;
-    MemoryRegion xscom_spci_regs;
-};
-
 /*
  * PHB3 PCIe Root port
  */
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 05/17] ppc/pnv: add pnv-phb device
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (3 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 04/17] ppc/pnv: add unified pnv-phb header Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 06/17] ppc/pnv: remove PnvPHB3 Daniel Henrique Barboza
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

This device works as a generic pnv-phb that redirects the control flow
to one of the implemented PHB versions (PHB3 and PHB4 at this moment).

The control redirection happens in the instance_init() and realize()
callbacks, where we check which powernv machine we're running and
execute the PnvPHB3 callbacks if running powernv8 or PnvPHB4 if running
powernv9/10.  This will allow us to keep the existing PHB3 and PHB4 code
as is, just changing their device type to PnvPHB3/PnvPHB4 to PnvPHB when
we're ready.

For now we're putting logic to handle the PHB3 version. We'll add PHB4
later on.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/meson.build        |   3 +-
 hw/pci-host/pnv_phb.c          | 116 +++++++++++++++++++++++++++++++++
 hw/pci-host/pnv_phb3.c         |   4 +-
 include/hw/pci-host/pnv_phb3.h |   3 +
 4 files changed, 123 insertions(+), 3 deletions(-)
 create mode 100644 hw/pci-host/pnv_phb.c

diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index 4c4f39c15c..b4107b7a2a 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -32,5 +32,6 @@ specific_ss.add(when: 'CONFIG_PCI_POWERNV', if_true: files(
   'pnv_phb3_msi.c',
   'pnv_phb3_pbcq.c',
   'pnv_phb4.c',
-  'pnv_phb4_pec.c'
+  'pnv_phb4_pec.c',
+  'pnv_phb.c',
 ))
diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
new file mode 100644
index 0000000000..3dd08f768f
--- /dev/null
+++ b/hw/pci-host/pnv_phb.c
@@ -0,0 +1,116 @@
+/*
+ * QEMU PowerPC PowerNV Unified PHB model
+ *
+ * Copyright (c) 2022, IBM Corporation.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/visitor.h"
+#include "qapi/error.h"
+#include "hw/pci-host/pnv_phb.h"
+#include "hw/pci/pcie_host.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/ppc/pnv.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "qom/object.h"
+#include "sysemu/sysemu.h"
+
+
+static char *pnv_phb_get_chip_typename(void)
+{
+    Object *qdev_machine = qdev_get_machine();
+    PnvMachineState *pnv = PNV_MACHINE(qdev_machine);
+    MachineState *machine = MACHINE(pnv);
+    g_autofree char *chip_typename = NULL;
+    int i;
+
+    if (!machine->cpu_type) {
+        return NULL;
+    }
+
+    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
+    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
+                                    i, machine->cpu_type);
+
+    return g_steal_pointer(&chip_typename);
+}
+
+static void pnv_phb_instance_init(Object *obj)
+{
+    g_autofree char *chip_typename = pnv_phb_get_chip_typename();
+
+    /*
+     * When doing command line instrospection we won't have
+     * a valid machine->cpu_type value.
+     */
+    if (!chip_typename) {
+        return;
+    }
+
+    if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) ||
+        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
+        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
+        pnv_phb3_instance_init(obj);
+    }
+}
+
+static void pnv_phb_realize(DeviceState *dev, Error **errp)
+{
+    g_autofree char *chip_typename = pnv_phb_get_chip_typename();
+
+    g_assert(chip_typename != NULL);
+
+    if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) ||
+        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
+        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
+        /* PnvPHB3 */
+        pnv_phb3_realize(dev, errp);
+    }
+}
+
+static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge,
+                                          PCIBus *rootbus)
+{
+    PnvPHB *phb = PNV_PHB(host_bridge);
+
+    snprintf(phb->bus_path, sizeof(phb->bus_path), "00%02x:%02x",
+             phb->chip_id, phb->phb_id);
+    return phb->bus_path;
+}
+
+static Property pnv_phb_properties[] = {
+    DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0),
+    DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pnv_phb_class_init(ObjectClass *klass, void *data)
+{
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    hc->root_bus_path = pnv_phb_root_bus_path;
+    dc->realize = pnv_phb_realize;
+    device_class_set_props(dc, pnv_phb_properties);
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->user_creatable = true;
+}
+
+static const TypeInfo pnv_phb_type_info = {
+    .name          = TYPE_PNV_PHB,
+    .parent        = TYPE_PCIE_HOST_BRIDGE,
+    .instance_size = sizeof(PnvPHB),
+    .class_init    = pnv_phb_class_init,
+    .instance_init = pnv_phb_instance_init,
+};
+
+static void pnv_phb_register_types(void)
+{
+    type_register_static(&pnv_phb_type_info);
+}
+
+type_init(pnv_phb_register_types)
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 70d92edd94..8e31a69083 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -966,7 +966,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     return &ds->dma_as;
 }
 
-static void pnv_phb3_instance_init(Object *obj)
+void pnv_phb3_instance_init(Object *obj)
 {
     PnvPHB3 *phb = PNV_PHB3(obj);
 
@@ -986,7 +986,7 @@ static void pnv_phb3_instance_init(Object *obj)
 
 }
 
-static void pnv_phb3_realize(DeviceState *dev, Error **errp)
+void pnv_phb3_realize(DeviceState *dev, Error **errp)
 {
     PnvPHB3 *phb = PNV_PHB3(dev);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index b6b5f91684..aba26f4f7c 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -102,4 +102,7 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size);
 void pnv_phb3_update_regions(PnvPHB3 *phb);
 void pnv_phb3_remap_irqs(PnvPHB3 *phb);
 
+void pnv_phb3_instance_init(Object *obj);
+void pnv_phb3_realize(DeviceState *dev, Error **errp);
+
 #endif /* PCI_HOST_PNV_PHB3_H */
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 06/17] ppc/pnv: remove PnvPHB3
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (4 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 05/17] ppc/pnv: add pnv-phb device Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 07/17] ppc/pnv: user created pnv-phb for powernv8 Daniel Henrique Barboza
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The unified PnvPHB model is able to replace PnvPHB3 in all PHB3 related
code. Let's do that while also removing the PnvPHB3 device entirely.
The device wasn't user creatable in any QEMU release so there's no ABI
concerns in doing so.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c          |  1 +
 hw/pci-host/pnv_phb3.c         | 92 ++++++++++------------------------
 hw/pci-host/pnv_phb3_msi.c     |  6 +--
 hw/pci-host/pnv_phb3_pbcq.c    |  8 +--
 hw/ppc/pnv.c                   | 12 ++---
 include/hw/pci-host/pnv_phb.h  |  9 ++--
 include/hw/pci-host/pnv_phb3.h | 43 +---------------
 include/hw/ppc/pnv.h           |  2 +-
 8 files changed, 48 insertions(+), 125 deletions(-)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 3dd08f768f..e4c4cca311 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -85,6 +85,7 @@ static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge,
 static Property pnv_phb_properties[] = {
     DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0),
     DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0),
+    DEFINE_PROP_LINK("chip", PnvPHB, chip, TYPE_PNV_CHIP, PnvChip *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 8e31a69083..d2405d4f5a 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -24,7 +24,7 @@
     qemu_log_mask(LOG_GUEST_ERROR, "phb3[%d:%d]: " fmt "\n",            \
                   (phb)->chip_id, (phb)->phb_id, ## __VA_ARGS__)
 
-static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb)
+static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB *phb)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
     uint64_t addr = phb->regs3[PHB_CONFIG_ADDRESS >> 3];
@@ -43,7 +43,7 @@ static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb)
  * The CONFIG_DATA register expects little endian accesses, but as the
  * region is big endian, we have to swap the value.
  */
-static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off,
+static void pnv_phb3_config_write(PnvPHB *phb, unsigned off,
                                   unsigned size, uint64_t val)
 {
     uint32_t cfg_addr, limit;
@@ -78,7 +78,7 @@ static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off,
     pci_host_config_write_common(pdev, cfg_addr, limit, val, size);
 }
 
-static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off,
+static uint64_t pnv_phb3_config_read(PnvPHB *phb, unsigned off,
                                      unsigned size)
 {
     uint32_t cfg_addr, limit;
@@ -112,7 +112,7 @@ static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off,
     }
 }
 
-static void pnv_phb3_check_m32(PnvPHB3 *phb)
+static void pnv_phb3_check_m32(PnvPHB *phb)
 {
     uint64_t base, start, size;
     MemoryRegion *parent;
@@ -152,7 +152,7 @@ static void pnv_phb3_check_m32(PnvPHB3 *phb)
     memory_region_add_subregion(parent, base, &phb->mr_m32);
 }
 
-static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index)
+static void pnv_phb3_check_m64(PnvPHB *phb, uint32_t index)
 {
     uint64_t base, start, size, m64;
     MemoryRegion *parent;
@@ -202,7 +202,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index)
     memory_region_add_subregion(parent, base, &phb->mr_m64[index]);
 }
 
-static void pnv_phb3_check_all_m64s(PnvPHB3 *phb)
+static void pnv_phb3_check_all_m64s(PnvPHB *phb)
 {
     uint64_t i;
 
@@ -211,7 +211,7 @@ static void pnv_phb3_check_all_m64s(PnvPHB3 *phb)
     }
 }
 
-static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val)
+static void pnv_phb3_lxivt_write(PnvPHB *phb, unsigned idx, uint64_t val)
 {
     uint8_t server, prio;
 
@@ -230,7 +230,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val)
     ics_write_xive(&phb->lsis, idx, server, prio, prio);
 }
 
-static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
+static uint64_t *pnv_phb3_ioda_access(PnvPHB *phb,
                                       unsigned *out_table, unsigned *out_idx)
 {
     uint64_t adreg = phb->regs3[PHB_IODA_ADDR >> 3];
@@ -304,7 +304,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb,
     return tptr;
 }
 
-static uint64_t pnv_phb3_ioda_read(PnvPHB3 *phb)
+static uint64_t pnv_phb3_ioda_read(PnvPHB *phb)
 {
         unsigned table;
         uint64_t *tptr;
@@ -317,7 +317,7 @@ static uint64_t pnv_phb3_ioda_read(PnvPHB3 *phb)
         return *tptr;
 }
 
-static void pnv_phb3_ioda_write(PnvPHB3 *phb, uint64_t val)
+static void pnv_phb3_ioda_write(PnvPHB *phb, uint64_t val)
 {
         unsigned table, idx;
         uint64_t *tptr;
@@ -345,7 +345,7 @@ static void pnv_phb3_ioda_write(PnvPHB3 *phb, uint64_t val)
  * This is called whenever the PHB LSI, MSI source ID register or
  * the PBCQ irq filters are written.
  */
-void pnv_phb3_remap_irqs(PnvPHB3 *phb)
+void pnv_phb3_remap_irqs(PnvPHB *phb)
 {
     ICSState *ics = &phb->lsis;
     uint32_t local, global, count, mask, comp;
@@ -408,7 +408,7 @@ void pnv_phb3_remap_irqs(PnvPHB3 *phb)
     pnv_phb3_msi_update_config(&phb->msis, comp, count - PNV_PHB3_NUM_LSI);
 }
 
-static void pnv_phb3_lsi_src_id_write(PnvPHB3 *phb, uint64_t val)
+static void pnv_phb3_lsi_src_id_write(PnvPHB *phb, uint64_t val)
 {
     /* Sanitize content */
     val &= PHB_LSI_SRC_ID;
@@ -416,7 +416,7 @@ static void pnv_phb3_lsi_src_id_write(PnvPHB3 *phb, uint64_t val)
     pnv_phb3_remap_irqs(phb);
 }
 
-static void pnv_phb3_rtc_invalidate(PnvPHB3 *phb, uint64_t val)
+static void pnv_phb3_rtc_invalidate(PnvPHB *phb, uint64_t val)
 {
     PnvPhb3DMASpace *ds;
 
@@ -456,7 +456,7 @@ static void pnv_phb3_update_msi_regions(PnvPhb3DMASpace *ds)
     }
 }
 
-static void pnv_phb3_update_all_msi_regions(PnvPHB3 *phb)
+static void pnv_phb3_update_all_msi_regions(PnvPHB *phb)
 {
     PnvPhb3DMASpace *ds;
 
@@ -467,7 +467,7 @@ static void pnv_phb3_update_all_msi_regions(PnvPHB3 *phb)
 
 void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size)
 {
-    PnvPHB3 *phb = opaque;
+    PnvPHB *phb = opaque;
     bool changed;
 
     /* Special case configuration data */
@@ -589,7 +589,7 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size)
 
 uint64_t pnv_phb3_reg_read(void *opaque, hwaddr off, unsigned size)
 {
-    PnvPHB3 *phb = opaque;
+    PnvPHB *phb = opaque;
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
     uint64_t val;
 
@@ -683,7 +683,7 @@ static int pnv_phb3_map_irq(PCIDevice *pci_dev, int irq_num)
 
 static void pnv_phb3_set_irq(void *opaque, int irq_num, int level)
 {
-    PnvPHB3 *phb = opaque;
+    PnvPHB *phb = opaque;
 
     /* LSI only ... */
     if (irq_num > 3) {
@@ -741,7 +741,7 @@ static void pnv_phb3_translate_tve(PnvPhb3DMASpace *ds, hwaddr addr,
     int32_t  lev = GETFIELD(IODA2_TVT_NUM_LEVELS, tve);
     uint32_t tts = GETFIELD(IODA2_TVT_TCE_TABLE_SIZE, tve);
     uint32_t tps = GETFIELD(IODA2_TVT_IO_PSIZE, tve);
-    PnvPHB3 *phb = ds->phb;
+    PnvPHB *phb = ds->phb;
 
     /* Invalid levels */
     if (lev > 4) {
@@ -848,7 +848,7 @@ static IOMMUTLBEntry pnv_phb3_translate_iommu(IOMMUMemoryRegion *iommu,
         .addr_mask = ~(hwaddr)0,
         .perm = IOMMU_NONE,
     };
-    PnvPHB3 *phb = ds->phb;
+    PnvPHB *phb = ds->phb;
 
     /* Resolve PE# */
     if (!pnv_phb3_resolve_pe(ds)) {
@@ -935,7 +935,7 @@ static const MemoryRegionOps pnv_phb3_msi_ops = {
 
 static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
-    PnvPHB3 *phb = opaque;
+    PnvPHB *phb = opaque;
     PnvPhb3DMASpace *ds;
 
     QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) {
@@ -968,7 +968,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 
 void pnv_phb3_instance_init(Object *obj)
 {
-    PnvPHB3 *phb = PNV_PHB3(obj);
+    PnvPHB *phb = PNV_PHB(obj);
 
     QLIST_INIT(&phb->v3_dma_spaces);
 
@@ -988,7 +988,7 @@ void pnv_phb3_instance_init(Object *obj)
 
 void pnv_phb3_realize(DeviceState *dev, Error **errp)
 {
-    PnvPHB3 *phb = PNV_PHB3(dev);
+    PnvPHB *phb = PNV_PHB(dev);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
     int i;
@@ -1055,7 +1055,7 @@ void pnv_phb3_realize(DeviceState *dev, Error **errp)
     pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB3_ROOT_PORT);
 }
 
-void pnv_phb3_update_regions(PnvPHB3 *phb)
+void pnv_phb3_update_regions(PnvPHB *phb)
 {
     PnvPBCQState *pbcq = &phb->pbcq;
 
@@ -1077,43 +1077,6 @@ void pnv_phb3_update_regions(PnvPHB3 *phb)
     pnv_phb3_check_all_m64s(phb);
 }
 
-static const char *pnv_phb3_root_bus_path(PCIHostState *host_bridge,
-                                          PCIBus *rootbus)
-{
-    PnvPHB3 *phb = PNV_PHB3(host_bridge);
-
-    snprintf(phb->bus_path, sizeof(phb->bus_path), "00%02x:%02x",
-             phb->chip_id, phb->phb_id);
-    return phb->bus_path;
-}
-
-static Property pnv_phb3_properties[] = {
-        DEFINE_PROP_UINT32("index", PnvPHB3, phb_id, 0),
-        DEFINE_PROP_UINT32("chip-id", PnvPHB3, chip_id, 0),
-        DEFINE_PROP_LINK("chip", PnvPHB3, chip, TYPE_PNV_CHIP, PnvChip *),
-        DEFINE_PROP_END_OF_LIST(),
-};
-
-static void pnv_phb3_class_init(ObjectClass *klass, void *data)
-{
-    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    hc->root_bus_path = pnv_phb3_root_bus_path;
-    dc->realize = pnv_phb3_realize;
-    device_class_set_props(dc, pnv_phb3_properties);
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    dc->user_creatable = false;
-}
-
-static const TypeInfo pnv_phb3_type_info = {
-    .name          = TYPE_PNV_PHB3,
-    .parent        = TYPE_PCIE_HOST_BRIDGE,
-    .instance_size = sizeof(PnvPHB3),
-    .class_init    = pnv_phb3_class_init,
-    .instance_init = pnv_phb3_instance_init,
-};
-
 static void pnv_phb3_root_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
@@ -1140,15 +1103,15 @@ 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;
+    PnvPHB *phb = NULL;
     Error *local_err = NULL;
 
-    phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                          TYPE_PNV_PHB3);
+    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
+                                          TYPE_PNV_PHB);
 
     if (!phb) {
         error_setg(errp,
-"pnv_phb3_root_port devices must be connected to pnv-phb3 buses");
+"pnv_phb3_root_port devices must be connected to pnv-phb buses");
         return;
     }
 
@@ -1195,7 +1158,6 @@ static void pnv_phb3_register_types(void)
 {
     type_register_static(&pnv_phb3_root_bus_info);
     type_register_static(&pnv_phb3_root_port_info);
-    type_register_static(&pnv_phb3_type_info);
     type_register_static(&pnv_phb3_iommu_memory_region_info);
 }
 
diff --git a/hw/pci-host/pnv_phb3_msi.c b/hw/pci-host/pnv_phb3_msi.c
index d8534376f8..84475f8dc1 100644
--- a/hw/pci-host/pnv_phb3_msi.c
+++ b/hw/pci-host/pnv_phb3_msi.c
@@ -18,7 +18,7 @@
 #include "hw/qdev-properties.h"
 #include "sysemu/reset.h"
 
-static uint64_t phb3_msi_ive_addr(PnvPHB3 *phb, int srcno)
+static uint64_t phb3_msi_ive_addr(PnvPHB *phb, int srcno)
 {
     uint64_t ivtbar = phb->regs3[PHB_IVT_BAR >> 3];
     uint64_t phbctl = phb->regs3[PHB_CONTROL >> 3];
@@ -43,7 +43,7 @@ static uint64_t phb3_msi_ive_addr(PnvPHB3 *phb, int srcno)
     }
 }
 
-static bool phb3_msi_read_ive(PnvPHB3 *phb, int srcno, uint64_t *out_ive)
+static bool phb3_msi_read_ive(PnvPHB *phb, int srcno, uint64_t *out_ive)
 {
     uint64_t ive_addr, ive;
 
@@ -281,7 +281,7 @@ static void phb3_msi_instance_init(Object *obj)
     Phb3MsiState *msi = PHB3_MSI(obj);
     ICSState *ics = ICS(obj);
 
-    object_property_add_link(obj, "phb", TYPE_PNV_PHB3,
+    object_property_add_link(obj, "phb", TYPE_PNV_PHB,
                              (Object **)&msi->phb,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_STRONG);
diff --git a/hw/pci-host/pnv_phb3_pbcq.c b/hw/pci-host/pnv_phb3_pbcq.c
index 82f70efa43..39f02c158f 100644
--- a/hw/pci-host/pnv_phb3_pbcq.c
+++ b/hw/pci-host/pnv_phb3_pbcq.c
@@ -238,7 +238,7 @@ static const MemoryRegionOps pnv_pbcq_spci_xscom_ops = {
 static void pnv_pbcq_default_bars(PnvPBCQState *pbcq)
 {
     uint64_t mm0, mm1, reg;
-    PnvPHB3 *phb = pbcq->phb;
+    PnvPHB *phb = pbcq->phb;
 
     mm0 = 0x3d00000000000ull + 0x4000000000ull * phb->chip_id +
             0x1000000000ull * phb->phb_id;
@@ -258,7 +258,7 @@ static void pnv_pbcq_default_bars(PnvPBCQState *pbcq)
 static void pnv_pbcq_realize(DeviceState *dev, Error **errp)
 {
     PnvPBCQState *pbcq = PNV_PBCQ(dev);
-    PnvPHB3 *phb;
+    PnvPHB *phb;
     char name[32];
 
     assert(pbcq->phb);
@@ -300,7 +300,7 @@ static int pnv_pbcq_dt_xscom(PnvXScomInterface *dev, void *fdt,
                              int xscom_offset)
 {
     const char compat[] = "ibm,power8-pbcq";
-    PnvPHB3 *phb = PNV_PBCQ(dev)->phb;
+    PnvPHB *phb = PNV_PBCQ(dev)->phb;
     char *name;
     int offset;
     uint32_t lpc_pcba = PNV_XSCOM_PBCQ_NEST_BASE + 0x400 * phb->phb_id;
@@ -331,7 +331,7 @@ static void phb3_pbcq_instance_init(Object *obj)
 {
     PnvPBCQState *pbcq = PNV_PBCQ(obj);
 
-    object_property_add_link(obj, "phb", TYPE_PNV_PHB3,
+    object_property_add_link(obj, "phb", TYPE_PNV_PHB,
                              (Object **)&pbcq->phb,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_STRONG);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7c08a78d6c..12a3fe4920 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -655,7 +655,7 @@ static ISABus *pnv_isa_create(PnvChip *chip, Error **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);
+    PnvPHB *phb3 = (PnvPHB *) object_dynamic_cast(child, TYPE_PNV_PHB);
 
     if (phb3) {
         pnv_phb3_msi_pic_print_info(&phb3->msis, mon);
@@ -1155,7 +1155,7 @@ static void pnv_chip_power8_instance_init(Object *obj)
     chip8->num_phbs = pcc->num_phbs;
 
     for (i = 0; i < chip8->num_phbs; i++) {
-        object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB3);
+        object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB);
     }
 
 }
@@ -1277,9 +1277,9 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
                                 &chip8->homer.regs);
 
-    /* PHB3 controllers */
+    /* PHB version 3 controllers */
     for (i = 0; i < chip8->num_phbs; i++) {
-        PnvPHB3 *phb = &chip8->phbs[i];
+        PnvPHB *phb = &chip8->phbs[i];
 
         object_property_set_int(OBJECT(phb), "index", i, &error_fatal);
         object_property_set_int(OBJECT(phb), "chip-id", chip->chip_id,
@@ -1942,7 +1942,7 @@ typedef struct ForeachPhb3Args {
 static int pnv_ics_get_child(Object *child, void *opaque)
 {
     ForeachPhb3Args *args = opaque;
-    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
+    PnvPHB *phb3 = (PnvPHB *) object_dynamic_cast(child, TYPE_PNV_PHB);
 
     if (phb3) {
         if (ics_valid_irq(&phb3->lsis, args->irq)) {
@@ -1992,7 +1992,7 @@ PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
 
 static int pnv_ics_resend_child(Object *child, void *opaque)
 {
-    PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
+    PnvPHB *phb3 = (PnvPHB *) object_dynamic_cast(child, TYPE_PNV_PHB);
 
     if (phb3) {
         ics_resend(&phb3->lsis);
diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h
index 2a8bf9a66d..46158e124f 100644
--- a/include/hw/pci-host/pnv_phb.h
+++ b/include/hw/pci-host/pnv_phb.h
@@ -21,14 +21,14 @@
 #define PNV_PHB3_NUM_REGS     (0x1000 >> 3)
 #define PHB3_MAX_MSI     2048
 
-typedef struct PnvPHB3 PnvPHB3;
 typedef struct PnvChip PnvChip;
+typedef struct PnvPHB PnvPHB;
 
 typedef struct Phb3MsiState {
     ICSState ics;
     qemu_irq *qirqs;
 
-    PnvPHB3 *phb;
+    PnvPHB *phb;
     uint64_t rba[PHB3_MAX_MSI / 64];
     uint32_t rba_sum;
 } Phb3MsiState;
@@ -53,7 +53,7 @@ typedef struct PnvPBCQState {
     uint64_t mmio0_size;
     uint64_t mmio1_base;
     uint64_t mmio1_size;
-    PnvPHB3 *phb;
+    PnvPHB *phb;
 
     MemoryRegion xscom_nest_regs;
     MemoryRegion xscom_pci_regs;
@@ -76,7 +76,7 @@ typedef struct PnvPhb3DMASpace {
     uint8_t devfn;
     int pe_num;         /* Cached PE number */
 #define PHB_INVALID_PE (-1)
-    PnvPHB3 *phb;
+    PnvPHB *phb;
     AddressSpace dma_as;
     IOMMUMemoryRegion dma_mr;
     MemoryRegion msi32_mr;
@@ -100,7 +100,6 @@ typedef struct PnvPhb4PecState PnvPhb4PecState;
 /*
  * Unified PHB PCIe Host Bridge for PowerNV machines
  */
-typedef struct PnvPHB PnvPHB;
 #define TYPE_PNV_PHB "pnv-phb"
 OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB)
 
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index aba26f4f7c..1c4af98fee 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -16,7 +16,6 @@
 #include "hw/ppc/xics.h"
 #include "qom/object.h"
 
-typedef struct PnvPHB3 PnvPHB3;
 typedef struct PnvChip PnvChip;
 
 /*
@@ -50,11 +49,6 @@ typedef struct PnvPHB3RootPort {
     PCIESlot parent_obj;
 } PnvPHB3RootPort;
 
-/*
- * PHB3 PCIe Host Bridge for PowerNV machines (POWER8)
- */
-#define TYPE_PNV_PHB3 "pnv-phb3"
-OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB3, PNV_PHB3)
 
 #define PNV_PHB3_NUM_M64      16
 #define PNV_PHB3_NUM_REGS     (0x1000 >> 3)
@@ -63,44 +57,11 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB3, PNV_PHB3)
 
 #define PCI_MMIO_TOTAL_SIZE   (0x1ull << 60)
 
-struct PnvPHB3 {
-    PCIExpressHost parent_obj;
-
-    uint32_t chip_id;
-    uint32_t phb_id;
-    char bus_path[8];
-
-    uint64_t regs3[PNV_PHB3_NUM_REGS];
-    MemoryRegion mr_regs3;
-
-    MemoryRegion mr_m32;
-    MemoryRegion mr_m64[PNV_PHB3_NUM_M64];
-    MemoryRegion pci_mmio;
-    MemoryRegion pci_io;
-
-    uint64_t ioda2_LIST[8];
-    uint64_t ioda2_LXIVT[8];
-    uint64_t ioda2_TVT[512];
-    uint64_t ioda2_M64BT[16];
-    uint64_t ioda2_MDT[256];
-    uint64_t ioda2_PEEV[4];
-
-    uint32_t total_irq;
-    ICSState lsis;
-    qemu_irq *qirqs;
-    Phb3MsiState msis;
-
-    PnvPBCQState pbcq;
-
-    QLIST_HEAD(, PnvPhb3DMASpace) v3_dma_spaces;
-
-    PnvChip *chip;
-};
 
 uint64_t pnv_phb3_reg_read(void *opaque, hwaddr off, unsigned size);
 void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size);
-void pnv_phb3_update_regions(PnvPHB3 *phb);
-void pnv_phb3_remap_irqs(PnvPHB3 *phb);
+void pnv_phb3_update_regions(PnvPHB *phb);
+void pnv_phb3_remap_irqs(PnvPHB *phb);
 
 void pnv_phb3_instance_init(Object *obj);
 void pnv_phb3_realize(DeviceState *dev, Error **errp);
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 86cb7d7f97..539454fe9d 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -80,7 +80,7 @@ struct Pnv8Chip {
     PnvHomer     homer;
 
 #define PNV8_CHIP_PHB3_MAX 4
-    PnvPHB3      phbs[PNV8_CHIP_PHB3_MAX];
+    PnvPHB       phbs[PNV8_CHIP_PHB3_MAX];
     uint32_t     num_phbs;
 
     XICSFabric    *xics;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 07/17] ppc/pnv: user created pnv-phb for powernv8
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (5 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 06/17] ppc/pnv: remove PnvPHB3 Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 08/17] ppc/pnv: remove PnvPHB4 Daniel Henrique Barboza
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

This patch reintroduces the powernv8 bits of the code what was removed
in commit 9c10d86fee "ppc/pnv: Remove user-created PHB{3,4,5} devices",
using the pnv-phb device instead of the now late pnv-phb3 device,
allowing us to enable user creatable pnv-phb devices for the powernv8
machine.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c | 31 +++++++++++++++++++++++++++++--
 hw/ppc/pnv.c           | 23 ++++++++++++++++++++++-
 include/hw/ppc/pnv.h   |  1 +
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index d2405d4f5a..a6d6a10c52 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -993,6 +993,30 @@ void pnv_phb3_realize(DeviceState *dev, Error **errp)
     PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
     int i;
 
+    /* User created devices */
+    if (!phb->chip) {
+        Error *local_err = NULL;
+        BusState *s;
+
+        phb->chip = pnv_get_chip(pnv, phb->chip_id);
+        if (!phb->chip) {
+            error_setg(errp, "invalid chip id: %d", phb->chip_id);
+            return;
+        }
+
+        /*
+         * Reparent user created devices to the chip to build
+         * correctly the device tree.
+         */
+        pnv_chip_parent_fixup(phb->chip, OBJECT(phb), phb->phb_id);
+
+        s = qdev_get_parent_bus(DEVICE(phb->chip));
+        if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
     if (phb->phb_id >= PNV_CHIP_GET_CLASS(phb->chip)->num_phbs) {
         error_setg(errp, "invalid PHB index: %d", phb->phb_id);
         return;
@@ -1052,7 +1076,10 @@ 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);
+    if (defaults_enabled()) {
+        pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb),
+                                 TYPE_PNV_PHB3_ROOT_PORT);
+    }
 }
 
 void pnv_phb3_update_regions(PnvPHB *phb)
@@ -1137,7 +1164,7 @@ static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data)
 
     device_class_set_parent_realize(dc, pnv_phb3_root_port_realize,
                                     &rpc->parent_realize);
-    dc->user_creatable = false;
+    dc->user_creatable = true;
 
     k->vendor_id = PCI_VENDOR_ID_IBM;
     k->device_id = 0x03dc;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 12a3fe4920..d9e7530cd3 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1152,7 +1152,9 @@ static void pnv_chip_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER);
 
-    chip8->num_phbs = pcc->num_phbs;
+    if (defaults_enabled()) {
+        chip8->num_phbs = pcc->num_phbs;
+    }
 
     for (i = 0; i < chip8->num_phbs; i++) {
         object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB);
@@ -1977,6 +1979,23 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
     return NULL;
 }
 
+void pnv_chip_parent_fixup(PnvChip *chip, Object *obj, int index)
+{
+    Object *parent = OBJECT(chip);
+    g_autofree char *default_id =
+        g_strdup_printf("%s[%d]", object_get_typename(obj), index);
+
+    if (obj->parent == parent) {
+        return;
+    }
+
+    object_ref(obj);
+    object_unparent(obj);
+    object_property_add_child(
+        parent, DEVICE(obj)->id ? DEVICE(obj)->id : default_id, obj);
+    object_unref(obj);
+}
+
 PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
 {
     int i;
@@ -2116,6 +2135,8 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
 
     pmc->compat = compat;
     pmc->compat_size = sizeof(compat);
+
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
 }
 
 static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 539454fe9d..e489bec019 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -190,6 +190,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, 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_chip_parent_fixup(PnvChip *chip, Object *obj, int index);
 
 #define TYPE_PNV_MACHINE       MACHINE_TYPE_NAME("powernv")
 typedef struct PnvMachineClass PnvMachineClass;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 08/17] ppc/pnv: remove PnvPHB4
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (6 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 07/17] ppc/pnv: user created pnv-phb for powernv8 Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 09/17] ppc/pnv: user creatable pnv-phb for powernv9 Daniel Henrique Barboza
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The PnvPHB device is able to replace the PnvPHB4 device in all
instances, while also being usable by the PHB3 logic.

The PnvPHB4 device wasn't user creatable in any official QEMU release,
so we can remove it without breaking ABI.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c          |  15 ++++
 hw/pci-host/pnv_phb4.c         | 138 +++++++++++----------------------
 hw/pci-host/pnv_phb4_pec.c     |   4 +-
 hw/ppc/pnv.c                   |   2 +-
 include/hw/pci-host/pnv_phb4.h |  98 +++--------------------
 5 files changed, 75 insertions(+), 182 deletions(-)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index e4c4cca311..9583c703d4 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -55,7 +55,10 @@ static void pnv_phb_instance_init(Object *obj)
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
         pnv_phb3_instance_init(obj);
+        return;
     }
+
+    pnv_phb4_instance_init(obj);
 }
 
 static void pnv_phb_realize(DeviceState *dev, Error **errp)
@@ -69,7 +72,10 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
         /* PnvPHB3 */
         pnv_phb3_realize(dev, errp);
+        return;
     }
+
+    pnv_phb4_realize(dev, errp);
 }
 
 static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge,
@@ -86,6 +92,8 @@ static Property pnv_phb_properties[] = {
     DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0),
     DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0),
     DEFINE_PROP_LINK("chip", PnvPHB, chip, TYPE_PNV_CHIP, PnvChip *),
+    DEFINE_PROP_LINK("pec", PnvPHB, pec, TYPE_PNV_PHB4_PEC,
+                     PnvPhb4PecState *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -93,12 +101,15 @@ static void pnv_phb_class_init(ObjectClass *klass, void *data)
 {
     PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
+    XiveNotifierClass *xfc = XIVE_NOTIFIER_CLASS(klass);
 
     hc->root_bus_path = pnv_phb_root_bus_path;
     dc->realize = pnv_phb_realize;
     device_class_set_props(dc, pnv_phb_properties);
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->user_creatable = true;
+
+    xfc->notify         = pnv_phb4_xive_notify;
 }
 
 static const TypeInfo pnv_phb_type_info = {
@@ -107,6 +118,10 @@ static const TypeInfo pnv_phb_type_info = {
     .instance_size = sizeof(PnvPHB),
     .class_init    = pnv_phb_class_init,
     .instance_init = pnv_phb_instance_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_XIVE_NOTIFIER },
+        { },
+    },
 };
 
 static void pnv_phb_register_types(void)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 13ba9e45d8..becfd366f1 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -47,7 +47,7 @@ static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
     return (word & ~mask) | ((value << ctz64(mask)) & mask);
 }
 
-static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB4 *phb)
+static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB *phb)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
     uint64_t addr = phb->regs[PHB_CONFIG_ADDRESS >> 3];
@@ -70,7 +70,7 @@ static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB4 *phb)
  * The CONFIG_DATA register expects little endian accesses, but as the
  * region is big endian, we have to swap the value.
  */
-static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off,
+static void pnv_phb4_config_write(PnvPHB *phb, unsigned off,
                                   unsigned size, uint64_t val)
 {
     uint32_t cfg_addr, limit;
@@ -105,7 +105,7 @@ static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off,
     pci_host_config_write_common(pdev, cfg_addr, limit, val, size);
 }
 
-static uint64_t pnv_phb4_config_read(PnvPHB4 *phb, unsigned off,
+static uint64_t pnv_phb4_config_read(PnvPHB *phb, unsigned off,
                                      unsigned size)
 {
     uint32_t cfg_addr, limit;
@@ -142,7 +142,7 @@ static uint64_t pnv_phb4_config_read(PnvPHB4 *phb, unsigned off,
 /*
  * Root complex register accesses are memory mapped.
  */
-static void pnv_phb4_rc_config_write(PnvPHB4 *phb, unsigned off,
+static void pnv_phb4_rc_config_write(PnvPHB *phb, unsigned off,
                                      unsigned size, uint64_t val)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
@@ -163,7 +163,7 @@ static void pnv_phb4_rc_config_write(PnvPHB4 *phb, unsigned off,
                                  bswap32(val), 4);
 }
 
-static uint64_t pnv_phb4_rc_config_read(PnvPHB4 *phb, unsigned off,
+static uint64_t pnv_phb4_rc_config_read(PnvPHB *phb, unsigned off,
                                         unsigned size)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(phb);
@@ -185,7 +185,7 @@ static uint64_t pnv_phb4_rc_config_read(PnvPHB4 *phb, unsigned off,
     return bswap32(val);
 }
 
-static void pnv_phb4_check_mbt(PnvPHB4 *phb, uint32_t index)
+static void pnv_phb4_check_mbt(PnvPHB *phb, uint32_t index)
 {
     uint64_t base, start, size, mbe0, mbe1;
     MemoryRegion *parent;
@@ -248,7 +248,7 @@ static void pnv_phb4_check_mbt(PnvPHB4 *phb, uint32_t index)
     memory_region_add_subregion(parent, base, &phb->mr_mmio[index]);
 }
 
-static void pnv_phb4_check_all_mbt(PnvPHB4 *phb)
+static void pnv_phb4_check_all_mbt(PnvPHB *phb)
 {
     uint64_t i;
     uint32_t num_windows = phb->big_phb ? PNV_PHB4_MAX_MMIO_WINDOWS :
@@ -259,7 +259,7 @@ static void pnv_phb4_check_all_mbt(PnvPHB4 *phb)
     }
 }
 
-static uint64_t *pnv_phb4_ioda_access(PnvPHB4 *phb,
+static uint64_t *pnv_phb4_ioda_access(PnvPHB *phb,
                                       unsigned *out_table, unsigned *out_idx)
 {
     uint64_t adreg = phb->regs[PHB_IODA_ADDR >> 3];
@@ -336,7 +336,7 @@ static uint64_t *pnv_phb4_ioda_access(PnvPHB4 *phb,
     return tptr;
 }
 
-static uint64_t pnv_phb4_ioda_read(PnvPHB4 *phb)
+static uint64_t pnv_phb4_ioda_read(PnvPHB *phb)
 {
     unsigned table, idx;
     uint64_t *tptr;
@@ -355,7 +355,7 @@ static uint64_t pnv_phb4_ioda_read(PnvPHB4 *phb)
     return *tptr;
 }
 
-static void pnv_phb4_ioda_write(PnvPHB4 *phb, uint64_t val)
+static void pnv_phb4_ioda_write(PnvPHB *phb, uint64_t val)
 {
     unsigned table, idx;
     uint64_t *tptr;
@@ -419,7 +419,7 @@ static void pnv_phb4_ioda_write(PnvPHB4 *phb, uint64_t val)
     }
 }
 
-static void pnv_phb4_rtc_invalidate(PnvPHB4 *phb, uint64_t val)
+static void pnv_phb4_rtc_invalidate(PnvPHB *phb, uint64_t val)
 {
     PnvPhb4DMASpace *ds;
 
@@ -458,7 +458,7 @@ static void pnv_phb4_update_msi_regions(PnvPhb4DMASpace *ds)
     }
 }
 
-static void pnv_phb4_update_all_msi_regions(PnvPHB4 *phb)
+static void pnv_phb4_update_all_msi_regions(PnvPHB *phb)
 {
     PnvPhb4DMASpace *ds;
 
@@ -467,7 +467,7 @@ static void pnv_phb4_update_all_msi_regions(PnvPHB4 *phb)
     }
 }
 
-static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
+static void pnv_phb4_update_xsrc(PnvPHB *phb)
 {
     int shift, flags, i, lsi_base;
     XiveSource *xsrc = &phb->xsrc;
@@ -518,7 +518,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
 static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
                                unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     bool changed;
 
     /* Special case outbound configuration data */
@@ -656,7 +656,7 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
 
 static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint64_t val;
 
     if ((off & 0xfffc) == PHB_CONFIG_DATA) {
@@ -752,7 +752,7 @@ static const MemoryRegionOps pnv_phb4_reg_ops = {
 
 static uint64_t pnv_phb4_xscom_read(void *opaque, hwaddr addr, unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t reg = addr >> 3;
     uint64_t val;
     hwaddr offset;
@@ -805,7 +805,7 @@ static uint64_t pnv_phb4_xscom_read(void *opaque, hwaddr addr, unsigned size)
 static void pnv_phb4_xscom_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t reg = addr >> 3;
     hwaddr offset;
 
@@ -868,7 +868,7 @@ const MemoryRegionOps pnv_phb4_xscom_ops = {
 static uint64_t pnv_pec_stk_nest_xscom_read(void *opaque, hwaddr addr,
                                             unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t reg = addr >> 3;
 
     /* TODO: add list of allowed registers and error out if not */
@@ -876,14 +876,14 @@ static uint64_t pnv_pec_stk_nest_xscom_read(void *opaque, hwaddr addr,
 }
 
 /*
- * Return the 'stack_no' of a PHB4. 'stack_no' is the order
+ * Return the 'stack_no' of a PHB. 'stack_no' is the order
  * the PHB4 occupies in the PEC. This is the reverse of what
  * pnv_phb4_pec_get_phb_id() does.
  *
  * E.g. a phb with phb_id = 4 and pec->index = 1 (PEC1) will
  * be the second phb (stack_no = 1) of the PEC.
  */
-static int pnv_phb4_get_phb_stack_no(PnvPHB4 *phb)
+static int pnv_phb4_get_phb_stack_no(PnvPHB *phb)
 {
     PnvPhb4PecState *pec = phb->pec;
     PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
@@ -897,7 +897,7 @@ static int pnv_phb4_get_phb_stack_no(PnvPHB4 *phb)
     return stack_no;
 }
 
-static void pnv_phb4_update_regions(PnvPHB4 *phb)
+static void pnv_phb4_update_regions(PnvPHB *phb)
 {
     /* Unmap first always */
     if (memory_region_is_mapped(&phb->mr_regs)) {
@@ -921,7 +921,7 @@ static void pnv_phb4_update_regions(PnvPHB4 *phb)
     pnv_phb4_check_all_mbt(phb);
 }
 
-static void pnv_pec_phb_update_map(PnvPHB4 *phb)
+static void pnv_pec_phb_update_map(PnvPHB *phb)
 {
     PnvPhb4PecState *pec = phb->pec;
     MemoryRegion *sysmem = get_system_memory();
@@ -1010,7 +1010,7 @@ static void pnv_pec_phb_update_map(PnvPHB4 *phb)
 static void pnv_pec_stk_nest_xscom_write(void *opaque, hwaddr addr,
                                          uint64_t val, unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     PnvPhb4PecState *pec = phb->pec;
     uint32_t reg = addr >> 3;
 
@@ -1099,7 +1099,7 @@ static const MemoryRegionOps pnv_pec_stk_nest_xscom_ops = {
 static uint64_t pnv_pec_stk_pci_xscom_read(void *opaque, hwaddr addr,
                                            unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t reg = addr >> 3;
 
     /* TODO: add list of allowed registers and error out if not */
@@ -1109,7 +1109,7 @@ static uint64_t pnv_pec_stk_pci_xscom_read(void *opaque, hwaddr addr,
 static void pnv_pec_stk_pci_xscom_write(void *opaque, hwaddr addr,
                                         uint64_t val, unsigned size)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t reg = addr >> 3;
 
     switch (reg) {
@@ -1172,7 +1172,7 @@ static int pnv_phb4_map_irq(PCIDevice *pci_dev, int irq_num)
 
 static void pnv_phb4_set_irq(void *opaque, int irq_num, int level)
 {
-    PnvPHB4 *phb = PNV_PHB4(opaque);
+    PnvPHB *phb = PNV_PHB(opaque);
     uint32_t lsi_base;
 
     /* LSI only ... */
@@ -1407,7 +1407,7 @@ static void pnv_phb4_msi_write(void *opaque, hwaddr addr,
                                uint64_t data, unsigned size)
 {
     PnvPhb4DMASpace *ds = opaque;
-    PnvPHB4 *phb = ds->phb;
+    PnvPHB *phb = ds->phb;
 
     uint32_t src = ((addr >> 4) & 0xffff) | (data & 0x1f);
 
@@ -1444,7 +1444,7 @@ static const MemoryRegionOps pnv_phb4_msi_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN
 };
 
-static PnvPhb4DMASpace *pnv_phb4_dma_find(PnvPHB4 *phb, PCIBus *bus, int devfn)
+static PnvPhb4DMASpace *pnv_phb4_dma_find(PnvPHB *phb, PCIBus *bus, int devfn)
 {
     PnvPhb4DMASpace *ds;
 
@@ -1458,7 +1458,7 @@ static PnvPhb4DMASpace *pnv_phb4_dma_find(PnvPHB4 *phb, PCIBus *bus, int devfn)
 
 static AddressSpace *pnv_phb4_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
-    PnvPHB4 *phb = opaque;
+    PnvPHB *phb = opaque;
     PnvPhb4DMASpace *ds;
     char name[32];
 
@@ -1488,7 +1488,7 @@ static AddressSpace *pnv_phb4_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     return &ds->dma_as;
 }
 
-static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
+static void pnv_phb4_xscom_realize(PnvPHB *phb)
 {
     PnvPhb4PecState *pec = phb->pec;
     PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
@@ -1534,9 +1534,9 @@ static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
                             &phb->phb_regs_mr);
 }
 
-static void pnv_phb4_instance_init(Object *obj)
+void pnv_phb4_instance_init(Object *obj)
 {
-    PnvPHB4 *phb = PNV_PHB4(obj);
+    PnvPHB *phb = PNV_PHB(obj);
 
     QLIST_INIT(&phb->dma_spaces);
 
@@ -1544,9 +1544,9 @@ static void pnv_phb4_instance_init(Object *obj)
     object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
 }
 
-static void pnv_phb4_realize(DeviceState *dev, Error **errp)
+void pnv_phb4_realize(DeviceState *dev, Error **errp)
 {
-    PnvPHB4 *phb = PNV_PHB4(dev);
+    PnvPHB *phb = PNV_PHB(dev);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     XiveSource *xsrc = &phb->xsrc;
     int nr_irqs;
@@ -1602,22 +1602,12 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
     pnv_phb4_xscom_realize(phb);
 }
 
-static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
-                                          PCIBus *rootbus)
-{
-    PnvPHB4 *phb = PNV_PHB4(host_bridge);
-
-    snprintf(phb->bus_path, sizeof(phb->bus_path), "00%02x:%02x",
-             phb->chip_id, phb->phb_id);
-    return phb->bus_path;
-}
-
 /*
  * Address base trigger mode (POWER10)
  *
  * Trigger directly the IC ESB page
  */
-static void pnv_phb4_xive_notify_abt(PnvPHB4 *phb, uint32_t srcno,
+static void pnv_phb4_xive_notify_abt(PnvPHB *phb, uint32_t srcno,
                                      bool pq_checked)
 {
     uint64_t notif_port = phb->regs[PHB_INT_NOTIFY_ADDR >> 3];
@@ -1657,7 +1647,7 @@ static void pnv_phb4_xive_notify_abt(PnvPHB4 *phb, uint32_t srcno,
     }
 }
 
-static void pnv_phb4_xive_notify_ic(PnvPHB4 *phb, uint32_t srcno,
+static void pnv_phb4_xive_notify_ic(PnvPHB *phb, uint32_t srcno,
                                     bool pq_checked)
 {
     uint64_t notif_port = phb->regs[PHB_INT_NOTIFY_ADDR >> 3];
@@ -1679,10 +1669,10 @@ static void pnv_phb4_xive_notify_ic(PnvPHB4 *phb, uint32_t srcno,
     }
 }
 
-static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
-                                 bool pq_checked)
+void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
+                          bool pq_checked)
 {
-    PnvPHB4 *phb = PNV_PHB4(xf);
+    PnvPHB *phb = PNV_PHB(xf);
 
     if (phb->regs[PHB_CTRLR >> 3] & PHB_CTRLR_IRQ_ABT_MODE) {
         pnv_phb4_xive_notify_abt(phb, srcno, pq_checked);
@@ -1691,45 +1681,10 @@ static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
     }
 }
 
-static Property pnv_phb4_properties[] = {
-        DEFINE_PROP_UINT32("index", PnvPHB4, phb_id, 0),
-        DEFINE_PROP_UINT32("chip-id", PnvPHB4, chip_id, 0),
-        DEFINE_PROP_LINK("pec", PnvPHB4, pec, TYPE_PNV_PHB4_PEC,
-                         PnvPhb4PecState *),
-        DEFINE_PROP_END_OF_LIST(),
-};
-
-static void pnv_phb4_class_init(ObjectClass *klass, void *data)
-{
-    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    XiveNotifierClass *xfc = XIVE_NOTIFIER_CLASS(klass);
-
-    hc->root_bus_path   = pnv_phb4_root_bus_path;
-    dc->realize         = pnv_phb4_realize;
-    device_class_set_props(dc, pnv_phb4_properties);
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    dc->user_creatable  = false;
-
-    xfc->notify         = pnv_phb4_xive_notify;
-}
-
-static const TypeInfo pnv_phb4_type_info = {
-    .name          = TYPE_PNV_PHB4,
-    .parent        = TYPE_PCIE_HOST_BRIDGE,
-    .instance_init = pnv_phb4_instance_init,
-    .instance_size = sizeof(PnvPHB4),
-    .class_init    = pnv_phb4_class_init,
-    .interfaces = (InterfaceInfo[]) {
-            { TYPE_XIVE_NOTIFIER },
-            { },
-    }
-};
-
 static const TypeInfo pnv_phb5_type_info = {
     .name          = TYPE_PNV_PHB5,
-    .parent        = TYPE_PNV_PHB4,
-    .instance_size = sizeof(PnvPHB4),
+    .parent        = TYPE_PNV_PHB,
+    .instance_size = sizeof(PnvPHB),
 };
 
 static void pnv_phb4_root_bus_class_init(ObjectClass *klass, void *data)
@@ -1779,14 +1734,14 @@ 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;
+    PnvPHB *phb = NULL;
     Error *local_err = NULL;
 
-    phb = (PnvPHB4 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                          TYPE_PNV_PHB4);
+    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
+                                         TYPE_PNV_PHB);
 
     if (!phb) {
-        error_setg(errp, "%s must be connected to pnv-phb4 buses", dev->id);
+        error_setg(errp, "%s must be connected to pnv-phb buses", dev->id);
         return;
     }
 
@@ -1856,14 +1811,13 @@ static void pnv_phb4_register_types(void)
     type_register_static(&pnv_phb4_root_bus_info);
     type_register_static(&pnv_phb5_root_port_info);
     type_register_static(&pnv_phb4_root_port_info);
-    type_register_static(&pnv_phb4_type_info);
     type_register_static(&pnv_phb5_type_info);
     type_register_static(&pnv_phb4_iommu_memory_region_info);
 }
 
 type_init(pnv_phb4_register_types);
 
-void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon)
+void pnv_phb4_pic_print_info(PnvPHB *phb, Monitor *mon)
 {
     uint64_t notif_port =
         phb->regs[PHB_INT_NOTIFY_ADDR >> 3] & ~PHB_INT_NOTIFY_ADDR_64K;
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 61bc0b503e..3eed560e44 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -116,7 +116,7 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
                                         Error **errp)
 {
     PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
-    PnvPHB4 *phb = PNV_PHB4(qdev_new(pecc->phb_type));
+    PnvPHB *phb = PNV_PHB(qdev_new(pecc->phb_type));
     int phb_id = pnv_phb4_pec_get_phb_id(pec, stack_no);
 
     object_property_add_child(OBJECT(pec), "phb[*]", OBJECT(phb));
@@ -262,7 +262,7 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data)
     pecc->stk_compat = stk_compat;
     pecc->stk_compat_size = sizeof(stk_compat);
     pecc->version = PNV_PHB4_VERSION;
-    pecc->phb_type = TYPE_PNV_PHB4;
+    pecc->phb_type = TYPE_PNV_PHB;
     pecc->num_phbs = pnv_pec_num_phbs;
     pecc->rp_model = TYPE_PNV_PHB4_ROOT_PORT;
 }
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d9e7530cd3..34a200a29c 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -676,7 +676,7 @@ static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon)
 static int pnv_chip_power9_pic_print_info_child(Object *child, void *opaque)
 {
     Monitor *mon = opaque;
-    PnvPHB4 *phb4 = (PnvPHB4 *) object_dynamic_cast(child, TYPE_PNV_PHB4);
+    PnvPHB *phb4 = (PnvPHB *) object_dynamic_cast(child, TYPE_PNV_PHB);
 
     if (phb4) {
         pnv_phb4_pic_print_info(phb4, mon);
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 19dcbd6f87..65a16f2067 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -12,12 +12,12 @@
 
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pcie_port.h"
+#include "hw/pci-host/pnv_phb.h"
 #include "hw/ppc/xive.h"
 #include "qom/object.h"
 
 typedef struct PnvPhb4PecState PnvPhb4PecState;
 typedef struct PnvPhb4PecStack PnvPhb4PecStack;
-typedef struct PnvPHB4 PnvPHB4;
 typedef struct PnvChip PnvChip;
 
 /*
@@ -36,7 +36,7 @@ typedef struct PnvPhb4DMASpace {
     uint8_t devfn;
     int pe_num;         /* Cached PE number */
 #define PHB_INVALID_PE (-1)
-    PnvPHB4 *phb;
+    PnvPHB *phb;
     AddressSpace dma_as;
     IOMMUMemoryRegion dma_mr;
     MemoryRegion msi32_mr;
@@ -55,11 +55,9 @@ typedef struct PnvPHB4RootPort {
     PCIESlot parent_obj;
 } PnvPHB4RootPort;
 
-/*
- * PHB4 PCIe Host Bridge for PowerNV machines (POWER9)
- */
-#define TYPE_PNV_PHB4 "pnv-phb4"
-OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB4, PNV_PHB4)
+struct PnvPHB4DeviceClass {
+    DeviceClass parent_class;
+};
 
 #define PNV_PHB4_MAX_LSIs          8
 #define PNV_PHB4_MAX_INTs          4096
@@ -77,85 +75,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB4, PNV_PHB4)
 
 #define PCI_MMIO_TOTAL_SIZE        (0x1ull << 60)
 
-struct PnvPHB4 {
-    PCIExpressHost parent_obj;
-
-    uint32_t chip_id;
-    uint32_t phb_id;
-
-    uint64_t version;
-
-    /* The owner PEC */
-    PnvPhb4PecState *pec;
-
-    char bus_path[8];
-
-    /* Main register images */
-    uint64_t regs[PNV_PHB4_NUM_REGS];
-    MemoryRegion mr_regs;
-
-    /* Extra SCOM-only register */
-    uint64_t scom_hv_ind_addr_reg;
-
-    /*
-     * Geometry of the PHB. There are two types, small and big PHBs, a
-     * number of resources (number of PEs, windows etc...) are doubled
-     * for a big PHB
-     */
-    bool big_phb;
-
-    /* Memory regions for MMIO space */
-    MemoryRegion mr_mmio[PNV_PHB4_MAX_MMIO_WINDOWS];
-
-    /* PCI side space */
-    MemoryRegion pci_mmio;
-    MemoryRegion pci_io;
-
-    /* PCI registers (excluding pass-through) */
-#define PHB4_PEC_PCI_STK_REGS_COUNT  0xf
-    uint64_t pci_regs[PHB4_PEC_PCI_STK_REGS_COUNT];
-    MemoryRegion pci_regs_mr;
-
-    /* Nest registers */
-#define PHB4_PEC_NEST_STK_REGS_COUNT  0x17
-    uint64_t nest_regs[PHB4_PEC_NEST_STK_REGS_COUNT];
-    MemoryRegion nest_regs_mr;
-
-    /* PHB pass-through XSCOM */
-    MemoryRegion phb_regs_mr;
-
-    /* Memory windows from PowerBus to PHB */
-    MemoryRegion phbbar;
-    MemoryRegion intbar;
-    MemoryRegion mmbar0;
-    MemoryRegion mmbar1;
-    uint64_t mmio0_base;
-    uint64_t mmio0_size;
-    uint64_t mmio1_base;
-    uint64_t mmio1_size;
-
-    /* On-chip IODA tables */
-    uint64_t ioda_LIST[PNV_PHB4_MAX_LSIs];
-    uint64_t ioda_MIST[PNV_PHB4_MAX_MIST];
-    uint64_t ioda_TVT[PNV_PHB4_MAX_TVEs];
-    uint64_t ioda_MBT[PNV_PHB4_MAX_MBEs];
-    uint64_t ioda_MDT[PNV_PHB4_MAX_PEs];
-    uint64_t ioda_PEEV[PNV_PHB4_MAX_PEEVs];
-
-    /*
-     * The internal PESTA/B is 2 bits per PE split into two tables, we
-     * store them in a single array here to avoid wasting space.
-     */
-    uint8_t  ioda_PEST_AB[PNV_PHB4_MAX_PEs];
-
-    /* P9 Interrupt generation */
-    XiveSource xsrc;
-    qemu_irq *qirqs;
-
-    QLIST_HEAD(, PnvPhb4DMASpace) dma_spaces;
-};
-
-void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
+void pnv_phb4_pic_print_info(PnvPHB *phb, Monitor *mon);
 int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
 extern const MemoryRegionOps pnv_phb4_xscom_ops;
 
@@ -214,7 +134,7 @@ struct PnvPhb4PecClass {
 
 #define TYPE_PNV_PHB5 "pnv-phb5"
 #define PNV_PHB5(obj) \
-    OBJECT_CHECK(PnvPhb4, (obj), TYPE_PNV_PHB5)
+    OBJECT_CHECK(PnvPhb, (obj), TYPE_PNV_PHB5)
 
 #define PNV_PHB5_VERSION           0x000000a500000001ull
 #define PNV_PHB5_DEVICE_ID         0x0652
@@ -223,4 +143,8 @@ struct PnvPhb4PecClass {
 #define PNV_PHB5_PEC(obj) \
     OBJECT_CHECK(PnvPhb4PecState, (obj), TYPE_PNV_PHB5_PEC)
 
+void pnv_phb4_instance_init(Object *obj);
+void pnv_phb4_realize(DeviceState *dev, Error **errp);
+void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno, bool pq_checked);
+
 #endif /* PCI_HOST_PNV_PHB4_H */
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 09/17] ppc/pnv: user creatable pnv-phb for powernv9
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (7 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 08/17] ppc/pnv: remove PnvPHB4 Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 10/17] ppc/pnv: use PnvPHB.version Daniel Henrique Barboza
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

This patch reintroduces the powernv8 bits of the code what was removed
in commit 9c10d86fee "ppc/pnv: Remove user-created PHB{3,4,5} devices",
using the pnv-phb device instead of the now late pnv-phb4 device,
allowing us to enable user creatable pnv-phb devices for the powernv9
machine.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4.c     | 58 +++++++++++++++++++++++++++++++++++++-
 hw/pci-host/pnv_phb4_pec.c |  6 ++--
 hw/ppc/pnv.c               |  2 ++
 3 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index becfd366f1..262251ebcf 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1544,14 +1544,70 @@ void pnv_phb4_instance_init(Object *obj)
     object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
 }
 
+static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb,
+                                         Error **errp)
+{
+    Pnv9Chip *chip9 = PNV9_CHIP(chip);
+    int chip_id = phb->chip_id;
+    int index = phb->phb_id;
+    int i, j;
+
+    for (i = 0; i < chip->num_pecs; i++) {
+        /*
+         * For each PEC, check the amount of phbs it supports
+         * and see if the given phb4 index matches an index.
+         */
+        PnvPhb4PecState *pec = &chip9->pecs[i];
+
+        for (j = 0; j < pec->num_phbs; j++) {
+            if (index == pnv_phb4_pec_get_phb_id(pec, j)) {
+                return pec;
+            }
+        }
+    }
+
+    error_setg(errp,
+               "pnv-phb chip-id %d index %d didn't match any existing PEC",
+               chip_id, index);
+
+    return NULL;
+}
+
 void pnv_phb4_realize(DeviceState *dev, Error **errp)
 {
     PnvPHB *phb = PNV_PHB(dev);
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     XiveSource *xsrc = &phb->xsrc;
+    BusState *s;
+    Error *local_err = NULL;
     int nr_irqs;
     char name[32];
 
+    if (!chip) {
+        error_setg(errp, "invalid chip id: %d", phb->chip_id);
+        return;
+    }
+
+    /* User created PHBs need to be assigned to a PEC */
+    if (!phb->pec) {
+        phb->pec = pnv_phb4_get_pec(chip, phb, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
+    /* Reparent the PHB to the chip to build the device tree */
+    pnv_chip_parent_fixup(chip, OBJECT(phb), phb->phb_id);
+
+    s = qdev_get_parent_bus(DEVICE(chip));
+    if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     /* Set the "big_phb" flag */
     phb->big_phb = phb->phb_id == 0 || phb->phb_id == 3;
 
@@ -1763,7 +1819,7 @@ static void pnv_phb4_root_port_class_init(ObjectClass *klass, void *data)
     PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
 
     dc->desc     = "IBM PHB4 PCIE Root Port";
-    dc->user_creatable = false;
+    dc->user_creatable = true;
 
     device_class_set_parent_realize(dc, pnv_phb4_root_port_realize,
                                     &rpc->parent_realize);
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 3eed560e44..243a079ea7 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -150,8 +150,10 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
     pec->num_phbs = pecc->num_phbs[pec->index];
 
     /* Create PHBs if running with defaults */
-    for (i = 0; i < pec->num_phbs; i++) {
-        pnv_pec_default_phb_realize(pec, i, errp);
+    if (defaults_enabled()) {
+        for (i = 0; i < pec->num_phbs; i++) {
+            pnv_pec_default_phb_realize(pec, i, errp);
+        }
     }
 
     /* Initialize the XSCOM regions for the PEC registers */
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 34a200a29c..1a3cafcb7a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2155,6 +2155,8 @@ static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
     pmc->compat = compat;
     pmc->compat_size = sizeof(compat);
     pmc->dt_power_mgt = pnv_dt_power_mgt;
+
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
 }
 
 static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 10/17] ppc/pnv: use PnvPHB.version
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (8 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 09/17] ppc/pnv: user creatable pnv-phb for powernv9 Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 11/17] ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs Daniel Henrique Barboza
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The 'version' attribute of the PnvPHB was never used. Instead of
removing it, let's make use of it by setting the PHB version the PnvPHB
device is currently running.

This distinction will be used next patch.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c         | 11 +++++++++++
 include/hw/pci-host/pnv_phb.h |  7 +++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index 9583c703d4..cef6a57d50 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -63,6 +63,7 @@ static void pnv_phb_instance_init(Object *obj)
 
 static void pnv_phb_realize(DeviceState *dev, Error **errp)
 {
+    PnvPHB *phb = PNV_PHB(dev);
     g_autofree char *chip_typename = pnv_phb_get_chip_typename();
 
     g_assert(chip_typename != NULL);
@@ -71,10 +72,20 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
         /* PnvPHB3 */
+        phb->version = PHB_VERSION_3;
         pnv_phb3_realize(dev, errp);
         return;
     }
 
+    if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) {
+        phb->version = PHB_VERSION_4;
+    } else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) {
+        phb->version = PHB_VERSION_5;
+    } else {
+        error_setg(errp, "unknown PNV chip: %s", chip_typename);
+        return;
+    }
+
     pnv_phb4_realize(dev, errp);
 }
 
diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h
index 46158e124f..cceb37d03c 100644
--- a/include/hw/pci-host/pnv_phb.h
+++ b/include/hw/pci-host/pnv_phb.h
@@ -103,9 +103,14 @@ typedef struct PnvPhb4PecState PnvPhb4PecState;
 #define TYPE_PNV_PHB "pnv-phb"
 OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB)
 
+#define PHB_VERSION_3    3
+#define PHB_VERSION_4    4
+#define PHB_VERSION_5    5
+
 struct PnvPHB {
     PCIExpressHost parent_obj;
 
+    uint64_t version;
     uint32_t chip_id;
     uint32_t phb_id;
     char bus_path[8];
@@ -142,8 +147,6 @@ struct PnvPHB {
     /*
      * PnvPHB4 attributes
      */
-    uint64_t version;
-
     /* The owner PEC */
     PnvPhb4PecState *pec;
 
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 11/17] ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (9 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 10/17] ppc/pnv: use PnvPHB.version Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 12/17] ppc/pnv: user creatable pnv-phb for powernv10 Daniel Henrique Barboza
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The function assumes that we're always dealing with a PNV9_CHIP()
object. This is not the case when the pnv-phb device belongs to a
powernv10 machine.

Change pnv_phb4_get_pec() to be able to work with PNV10_CHIP() if
necessary.

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

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 262251ebcf..f911957f10 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1547,17 +1547,29 @@ void pnv_phb4_instance_init(Object *obj)
 static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb,
                                          Error **errp)
 {
-    Pnv9Chip *chip9 = PNV9_CHIP(chip);
+    PnvPhb4PecState *pecs = NULL;
     int chip_id = phb->chip_id;
     int index = phb->phb_id;
     int i, j;
 
+    if (phb->version == PHB_VERSION_4) {
+        Pnv9Chip *chip9 = PNV9_CHIP(chip);
+
+        pecs = chip9->pecs;
+    } else if (phb->version == PHB_VERSION_5) {
+        Pnv10Chip *chip10 = PNV10_CHIP(chip);
+
+        pecs = chip10->pecs;
+    } else {
+        return NULL;
+    }
+
     for (i = 0; i < chip->num_pecs; i++) {
         /*
          * For each PEC, check the amount of phbs it supports
          * and see if the given phb4 index matches an index.
          */
-        PnvPhb4PecState *pec = &chip9->pecs[i];
+        PnvPhb4PecState *pec = &pecs[i];
 
         for (j = 0; j < pec->num_phbs; j++) {
             if (index == pnv_phb4_pec_get_phb_id(pec, j)) {
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 12/17] ppc/pnv: user creatable pnv-phb for powernv10
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (10 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 11/17] ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 13/17] ppc/pnv: add pnv_phb_get_current_machine() Daniel Henrique Barboza
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

Given that powernv9 and powernv10 uses the same pnv-phb backend, the logic to
allow user created pnv-phbs for powernv10 is already in place. This
patch just flips the switch.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4.c | 2 +-
 hw/ppc/pnv.c           | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index f911957f10..fb3222d458 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1861,7 +1861,7 @@ static void pnv_phb5_root_port_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     dc->desc     = "IBM PHB5 PCIE Root Port";
-    dc->user_creatable = false;
+    dc->user_creatable = true;
 
     k->vendor_id = PCI_VENDOR_ID_IBM;
     k->device_id = PNV_PHB5_DEVICE_ID;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1a3cafcb7a..bb193b1ed3 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2174,6 +2174,8 @@ static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
     pmc->dt_power_mgt = pnv_dt_power_mgt;
 
     xfc->match_nvt = pnv10_xive_match_nvt;
+
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
 }
 
 static bool pnv_machine_get_hb(Object *obj, Error **errp)
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 13/17] ppc/pnv: add pnv_phb_get_current_machine()
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (11 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 12/17] ppc/pnv: user creatable pnv-phb for powernv10 Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 14/17] ppc/pnv: add pnv-phb-root-port device Daniel Henrique Barboza
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

Add a simple helper to avoid hardcoding strcmp() comparisons all around
pnv_phb.c.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index cef6a57d50..e03062a494 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -20,6 +20,10 @@
 #include "sysemu/sysemu.h"
 
 
+#define PNV_MACHINE_POWER8    1
+#define PNV_MACHINE_POWER9    2
+#define PNV_MACHINE_POWER10   3
+
 static char *pnv_phb_get_chip_typename(void)
 {
     Object *qdev_machine = qdev_get_machine();
@@ -39,7 +43,7 @@ static char *pnv_phb_get_chip_typename(void)
     return g_steal_pointer(&chip_typename);
 }
 
-static void pnv_phb_instance_init(Object *obj)
+static int pnv_phb_get_current_machine(void)
 {
     g_autofree char *chip_typename = pnv_phb_get_chip_typename();
 
@@ -48,12 +52,31 @@ static void pnv_phb_instance_init(Object *obj)
      * a valid machine->cpu_type value.
      */
     if (!chip_typename) {
-        return;
+        return 0;
     }
 
     if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) ||
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
         !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
+        return PNV_MACHINE_POWER8;
+    } else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) {
+        return PNV_MACHINE_POWER9;
+    } else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) {
+        return PNV_MACHINE_POWER10;
+    }
+
+    return 0;
+}
+
+static void pnv_phb_instance_init(Object *obj)
+{
+    int pnv_current_machine = pnv_phb_get_current_machine();
+
+    if (pnv_current_machine == 0) {
+        return;
+    }
+
+    if (pnv_current_machine == PNV_MACHINE_POWER8) {
         pnv_phb3_instance_init(obj);
         return;
     }
@@ -63,25 +86,24 @@ static void pnv_phb_instance_init(Object *obj)
 
 static void pnv_phb_realize(DeviceState *dev, Error **errp)
 {
+    int pnv_current_machine = pnv_phb_get_current_machine();
     PnvPHB *phb = PNV_PHB(dev);
-    g_autofree char *chip_typename = pnv_phb_get_chip_typename();
 
-    g_assert(chip_typename != NULL);
+    g_assert(pnv_current_machine != 0);
 
-    if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) ||
-        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) ||
-        !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) {
+    if (pnv_current_machine == PNV_MACHINE_POWER8) {
         /* PnvPHB3 */
         phb->version = PHB_VERSION_3;
         pnv_phb3_realize(dev, errp);
         return;
     }
 
-    if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) {
+    if (pnv_current_machine == PNV_MACHINE_POWER9) {
         phb->version = PHB_VERSION_4;
-    } else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) {
+    } else if (pnv_current_machine == PNV_MACHINE_POWER10) {
         phb->version = PHB_VERSION_5;
     } else {
+        g_autofree char *chip_typename = pnv_phb_get_chip_typename();
         error_setg(errp, "unknown PNV chip: %s", chip_typename);
         return;
     }
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 14/17] ppc/pnv: add pnv-phb-root-port device
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (12 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 13/17] ppc/pnv: add pnv_phb_get_current_machine() Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 15/17] ppc/pnv: remove pnv-phb3-root-port Daniel Henrique Barboza
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

We have two very similar root-port devices, pnv-phb3-root-port and
pnv-phb4-root-port. Both consist of a wrapper around the PCIESlot device
that, until now, has no additional attributes.

The main difference between the PHB3 and PHB4 root ports is that
pnv-phb4-root-port has the pnv_phb4_root_port_reset() callback. All
other differences can be merged in a single device without too much
trouble.

This patch introduces the unified pnv-phb-root-port that, in time, will
be used as the default root port for the pnv-phb device.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb.c         | 93 +++++++++++++++++++++++++++++++++++
 include/hw/pci-host/pnv_phb.h | 11 +++++
 2 files changed, 104 insertions(+)

diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
index e03062a494..369dc21931 100644
--- a/hw/pci-host/pnv_phb.c
+++ b/hw/pci-host/pnv_phb.c
@@ -157,9 +157,102 @@ static const TypeInfo pnv_phb_type_info = {
     },
 };
 
+static void pnv_phb_root_port_reset(DeviceState *dev)
+{
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
+    PCIDevice *d = PCI_DEVICE(dev);
+    uint8_t *conf = d->config;
+    int pnv_current_machine = pnv_phb_get_current_machine();
+
+    rpc->parent_reset(dev);
+
+    if (pnv_current_machine == PNV_MACHINE_POWER8) {
+        return;
+    }
+
+    pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
+                               PCI_IO_RANGE_MASK & 0xff);
+    pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+                                 PCI_IO_RANGE_MASK & 0xff);
+    pci_set_word(conf + PCI_MEMORY_BASE, 0);
+    pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0);
+    pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1);
+    pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1);
+    pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */
+    pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff);
+    pci_config_set_interrupt_pin(conf, 0);
+}
+
+static void pnv_phb_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);
+    PnvPHB *phb = NULL;
+    Error *local_err = NULL;
+
+    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
+                                          TYPE_PNV_PHB);
+
+    if (!phb) {
+        error_setg(errp,
+"pnv_phb_root_port devices must be connected to pnv-phb buses");
+        return;
+    }
+
+    /* 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);
+
+    rpc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    pci_config_set_interrupt_pin(pci->config, 0);
+}
+
+static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
+
+    dc->desc     = "IBM PHB PCIE Root Port";
+
+    device_class_set_parent_realize(dc, pnv_phb_root_port_realize,
+                                    &rpc->parent_realize);
+
+    device_class_set_parent_reset(dc, pnv_phb_root_port_reset,
+                                  &rpc->parent_reset);
+    dc->reset = &pnv_phb_root_port_reset;
+
+    dc->user_creatable = true;
+
+    k->vendor_id = PCI_VENDOR_ID_IBM;
+    /*
+     * k->device_id is defaulted to PNV_PHB3_DEVICE_ID. We'll fix
+     * it during instance_init() when we are aware of what machine
+     * we're running.
+     */
+    k->device_id = 0x03dc;
+    k->revision  = 0;
+
+    rpc->exp_offset = 0x48;
+    rpc->aer_offset = 0x100;
+}
+
+static const TypeInfo pnv_phb_root_port_info = {
+    .name          = TYPE_PNV_PHB_ROOT_PORT,
+    .parent        = TYPE_PCIE_ROOT_PORT,
+    .instance_size = sizeof(PnvPHBRootPort),
+    .class_init    = pnv_phb_root_port_class_init,
+};
+
 static void pnv_phb_register_types(void)
 {
     type_register_static(&pnv_phb_type_info);
+    type_register_static(&pnv_phb_root_port_info);
 }
 
 type_init(pnv_phb_register_types)
diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h
index cceb37d03c..ff90a9c200 100644
--- a/include/hw/pci-host/pnv_phb.h
+++ b/include/hw/pci-host/pnv_phb.h
@@ -210,4 +210,15 @@ struct PnvPHB {
     QLIST_HEAD(, PnvPhb4DMASpace) dma_spaces;
 };
 
+/*
+ * PHB PCIe Root port
+ */
+typedef struct PnvPHBRootPort {
+    PCIESlot parent_obj;
+} PnvPHBRootPort;
+
+#define TYPE_PNV_PHB_ROOT_PORT "pnv-phb-root-port"
+#define PNV_PHB_ROOT_PORT(obj) \
+    OBJECT_CHECK(PnvPHBRootPort, obj, TYPE_PNV_PHB_ROOT_PORT)
+
 #endif /* PCI_HOST_PNV_PHB_H */
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 15/17] ppc/pnv: remove pnv-phb3-root-port
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (13 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 14/17] ppc/pnv: add pnv-phb-root-port device Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 16/17] ppc/pnv: remove pnv-phb4-root-port Daniel Henrique Barboza
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The unified pnv-phb-root-port can be used in its place. There is no ABI
breakage in doing so because no official QEMU release introduced user
creatable pnv-phb3-root-port devices.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb3.c         | 59 +---------------------------------
 include/hw/pci-host/pnv_phb3.h |  7 ----
 2 files changed, 1 insertion(+), 65 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index a6d6a10c52..1c52df4c3f 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1078,7 +1078,7 @@ void pnv_phb3_realize(DeviceState *dev, Error **errp)
 
     if (defaults_enabled()) {
         pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb),
-                                 TYPE_PNV_PHB3_ROOT_PORT);
+                                 TYPE_PNV_PHB_ROOT_PORT);
     }
 }
 
@@ -1125,66 +1125,9 @@ static const TypeInfo pnv_phb3_root_bus_info = {
     },
 };
 
-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);
-    PnvPHB *phb = NULL;
-    Error *local_err = NULL;
-
-    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                          TYPE_PNV_PHB);
-
-    if (!phb) {
-        error_setg(errp,
-"pnv_phb3_root_port devices must be connected to pnv-phb buses");
-        return;
-    }
-
-    /* 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);
-
-    rpc->parent_realize(dev, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    pci_config_set_interrupt_pin(pci->config, 0);
-}
-
-static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
-
-    dc->desc     = "IBM PHB3 PCIE Root Port";
-
-    device_class_set_parent_realize(dc, pnv_phb3_root_port_realize,
-                                    &rpc->parent_realize);
-    dc->user_creatable = true;
-
-    k->vendor_id = PCI_VENDOR_ID_IBM;
-    k->device_id = 0x03dc;
-    k->revision  = 0;
-
-    rpc->exp_offset = 0x48;
-    rpc->aer_offset = 0x100;
-}
-
-static const TypeInfo pnv_phb3_root_port_info = {
-    .name          = TYPE_PNV_PHB3_ROOT_PORT,
-    .parent        = TYPE_PCIE_ROOT_PORT,
-    .instance_size = sizeof(PnvPHB3RootPort),
-    .class_init    = pnv_phb3_root_port_class_init,
-};
-
 static void pnv_phb3_register_types(void)
 {
     type_register_static(&pnv_phb3_root_bus_info);
-    type_register_static(&pnv_phb3_root_port_info);
     type_register_static(&pnv_phb3_iommu_memory_region_info);
 }
 
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
index 1c4af98fee..417b0f99a7 100644
--- a/include/hw/pci-host/pnv_phb3.h
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -43,13 +43,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPBCQState, PNV_PBCQ)
  */
 #define TYPE_PNV_PHB3_ROOT_BUS "pnv-phb3-root"
 
-#define TYPE_PNV_PHB3_ROOT_PORT "pnv-phb3-root-port"
-
-typedef struct PnvPHB3RootPort {
-    PCIESlot parent_obj;
-} PnvPHB3RootPort;
-
-
 #define PNV_PHB3_NUM_M64      16
 #define PNV_PHB3_NUM_REGS     (0x1000 >> 3)
 #define PNV_PHB3_NUM_LSI      8
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 16/17] ppc/pnv: remove pnv-phb4-root-port
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (14 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 15/17] ppc/pnv: remove pnv-phb3-root-port Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-07 19:06 ` [PATCH 17/17] ppc/pnv: remove pecc->rp_model Daniel Henrique Barboza
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The unified pnv-phb-root-port can be used instead. THe
pnv-phb4-root-port device isn't exposed to the user in any official QEMU
release so there's no ABI breakage in removing it.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4.c         | 100 ---------------------------------
 hw/pci-host/pnv_phb4_pec.c     |   4 +-
 include/hw/pci-host/pnv_phb4.h |   9 ---
 3 files changed, 2 insertions(+), 111 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index fb3222d458..2dce10830d 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1776,109 +1776,9 @@ static const TypeInfo pnv_phb4_root_bus_info = {
     },
 };
 
-static void pnv_phb4_root_port_reset(DeviceState *dev)
-{
-    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
-    PCIDevice *d = PCI_DEVICE(dev);
-    uint8_t *conf = d->config;
-
-    rpc->parent_reset(dev);
-
-    pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
-                               PCI_IO_RANGE_MASK & 0xff);
-    pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
-                                 PCI_IO_RANGE_MASK & 0xff);
-    pci_set_word(conf + PCI_MEMORY_BASE, 0);
-    pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0);
-    pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1);
-    pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1);
-    pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */
-    pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff);
-    pci_config_set_interrupt_pin(conf, 0);
-}
-
-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);
-    PnvPHB *phb = NULL;
-    Error *local_err = NULL;
-
-    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-                                         TYPE_PNV_PHB);
-
-    if (!phb) {
-        error_setg(errp, "%s must be connected to pnv-phb buses", dev->id);
-        return;
-    }
-
-    /* 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);
-
-    rpc->parent_realize(dev, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-}
-
-static void pnv_phb4_root_port_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
-
-    dc->desc     = "IBM PHB4 PCIE Root Port";
-    dc->user_creatable = true;
-
-    device_class_set_parent_realize(dc, pnv_phb4_root_port_realize,
-                                    &rpc->parent_realize);
-    device_class_set_parent_reset(dc, pnv_phb4_root_port_reset,
-                                  &rpc->parent_reset);
-
-    k->vendor_id = PCI_VENDOR_ID_IBM;
-    k->device_id = PNV_PHB4_DEVICE_ID;
-    k->revision  = 0;
-
-    rpc->exp_offset = 0x48;
-    rpc->aer_offset = 0x100;
-
-    dc->reset = &pnv_phb4_root_port_reset;
-}
-
-static const TypeInfo pnv_phb4_root_port_info = {
-    .name          = TYPE_PNV_PHB4_ROOT_PORT,
-    .parent        = TYPE_PCIE_ROOT_PORT,
-    .instance_size = sizeof(PnvPHB4RootPort),
-    .class_init    = pnv_phb4_root_port_class_init,
-};
-
-static void pnv_phb5_root_port_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    dc->desc     = "IBM PHB5 PCIE Root Port";
-    dc->user_creatable = true;
-
-    k->vendor_id = PCI_VENDOR_ID_IBM;
-    k->device_id = PNV_PHB5_DEVICE_ID;
-}
-
-static const TypeInfo pnv_phb5_root_port_info = {
-    .name          = TYPE_PNV_PHB5_ROOT_PORT,
-    .parent        = TYPE_PNV_PHB4_ROOT_PORT,
-    .instance_size = sizeof(PnvPHB4RootPort),
-    .class_init    = pnv_phb5_root_port_class_init,
-};
-
 static void pnv_phb4_register_types(void)
 {
     type_register_static(&pnv_phb4_root_bus_info);
-    type_register_static(&pnv_phb5_root_port_info);
-    type_register_static(&pnv_phb4_root_port_info);
     type_register_static(&pnv_phb5_type_info);
     type_register_static(&pnv_phb4_iommu_memory_region_info);
 }
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 243a079ea7..51821276e9 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -266,7 +266,7 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data)
     pecc->version = PNV_PHB4_VERSION;
     pecc->phb_type = TYPE_PNV_PHB;
     pecc->num_phbs = pnv_pec_num_phbs;
-    pecc->rp_model = TYPE_PNV_PHB4_ROOT_PORT;
+    pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT;
 }
 
 static const TypeInfo pnv_pec_type_info = {
@@ -319,7 +319,7 @@ static void pnv_phb5_pec_class_init(ObjectClass *klass, void *data)
     pecc->version = PNV_PHB5_VERSION;
     pecc->phb_type = TYPE_PNV_PHB5;
     pecc->num_phbs = pnv_phb5_pec_num_stacks;
-    pecc->rp_model = TYPE_PNV_PHB5_ROOT_PORT;
+    pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT;
 }
 
 static const TypeInfo pnv_phb5_pec_type_info = {
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 65a16f2067..8c57d836d1 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -44,16 +44,7 @@ typedef struct PnvPhb4DMASpace {
     QLIST_ENTRY(PnvPhb4DMASpace) list;
 } PnvPhb4DMASpace;
 
-/*
- * PHB4 PCIe Root port
- */
 #define TYPE_PNV_PHB4_ROOT_BUS "pnv-phb4-root"
-#define TYPE_PNV_PHB4_ROOT_PORT "pnv-phb4-root-port"
-#define TYPE_PNV_PHB5_ROOT_PORT "pnv-phb5-root-port"
-
-typedef struct PnvPHB4RootPort {
-    PCIESlot parent_obj;
-} PnvPHB4RootPort;
 
 struct PnvPHB4DeviceClass {
     DeviceClass parent_class;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 17/17] ppc/pnv: remove pecc->rp_model
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (15 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 16/17] ppc/pnv: remove pnv-phb4-root-port Daniel Henrique Barboza
@ 2022-05-07 19:06 ` Daniel Henrique Barboza
  2022-05-09 21:17 ` [PATCH 00/17] powernv: introduce pnv-phb unified devices Mark Cave-Ayland
  2022-05-10  9:07 ` Frederic Barrat
  18 siblings, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-07 19:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat, Daniel Henrique Barboza

The attribute is always being set to TYPE_PNV_PHB_ROOT_PORT.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/pnv_phb4_pec.c     | 4 +---
 include/hw/pci-host/pnv_phb4.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 51821276e9..509039bfe6 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -132,7 +132,7 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
     }
 
     /* Add a single Root port if running with defaults */
-    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
+    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB_ROOT_PORT);
 }
 
 static void pnv_pec_realize(DeviceState *dev, Error **errp)
@@ -266,7 +266,6 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data)
     pecc->version = PNV_PHB4_VERSION;
     pecc->phb_type = TYPE_PNV_PHB;
     pecc->num_phbs = pnv_pec_num_phbs;
-    pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT;
 }
 
 static const TypeInfo pnv_pec_type_info = {
@@ -319,7 +318,6 @@ static void pnv_phb5_pec_class_init(ObjectClass *klass, void *data)
     pecc->version = PNV_PHB5_VERSION;
     pecc->phb_type = TYPE_PNV_PHB5;
     pecc->num_phbs = pnv_phb5_pec_num_stacks;
-    pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT;
 }
 
 static const TypeInfo pnv_phb5_pec_type_info = {
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 8c57d836d1..b2c59ea1a0 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -116,7 +116,6 @@ struct PnvPhb4PecClass {
     uint64_t version;
     const char *phb_type;
     const uint32_t *num_phbs;
-    const char *rp_model;
 };
 
 /*
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (16 preceding siblings ...)
  2022-05-07 19:06 ` [PATCH 17/17] ppc/pnv: remove pecc->rp_model Daniel Henrique Barboza
@ 2022-05-09 21:17 ` Mark Cave-Ayland
  2022-05-09 22:30   ` Daniel Henrique Barboza
  2022-05-10  9:07 ` Frederic Barrat
  18 siblings, 1 reply; 24+ messages in thread
From: Mark Cave-Ayland @ 2022-05-09 21:17 UTC (permalink / raw)
  To: Daniel Henrique Barboza, qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat

On 07/05/2022 20:06, Daniel Henrique Barboza wrote:

> Hi,
> 
> Since the 7.0.0 release cycle we have a desire to use the powernv
> emulation with libvirt. To do that we need to enable user creatable
> pnv-phb devices to allow more user customization an to avoid spamming
> multiple default devices in the domain XML. In the middle of the
> previous cycle we experimented with user created
> pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
> end result, although functional, is that the user needs to deal with a
> lot of versioned devices that, from the user perspective, does the same
> thing. In a way we outsourced the implementation details of the PHBs
> (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
> devices also puts an extra burden in the libvirt support we want to
> have.
> 
> To solve this, Cedric and Frederic gave the idea of adding a common
> virtual pnv-phb device that the user can add in the command line, and
> QEMU handles the details internally. Unfortunatelly this idea turned out
> to be way harder than anticipated. Between creating a device that would
> just forward the callbacks to the existing devices internally, creating
> a PnvPHB device with the minimal attributes and making the other devices
> inherit from it, and making an 'abstract' device that could be casted
> for both phb3 and phb4 PHBs,

This bit sounds all good...

> all sorts of very obscure problems occured:
> PHBs not being detected, interrupts not being delivered and memory
> regions not being able to read/write registers. My initial impression is
> that there are assumptions made both in ppc/pnv and hw/pci-host that
> requires the memory region and the bus being in the same device. Even
> if we somehow figure all this out, the resulting code is hacky and
> annoying to maitain.

But this seems really surprising since there are many examples of similar QOM 
patterns within the codebase, and in my experience they work really well. Do you have 
a particular example that you can share to demonstrate why the result of the QOM 
mapping is making things more difficult?

> This brings us to this series. The cleaner way I found to accomplish
> what we want to do is to create real, unified pnv-phb/phb-phb-root-port
> devices, and get rid of all the versioned ones. This is done by
> merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
> files end up using the same pnv-phb and phb-phb-root-port unified devices,
> with the difference that pnv_phb3* only cares about version 3 attributes
> and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
> versions in the future will be a matter of adding any non-existent
> attributes in the unified pnv-phb* devices and using them in separated
> pnv_phbN* files.
> 
> The pnv-phb implementation per se is just a router for either phb3 or
> phb4 logic, done in init() and realize() time, after we check which powernv
> machine we're running. If running with powernv8 we redirect control to
> pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
> unified device does not do anything per se other than handling control
> to the right version.
> 
> After this series, this same powernv8 command line that boots a powernv8
> machine with some phbs and root ports and with network:
> 
> ./qemu-system-ppc64 -m 4G \
> -machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
> -accel tcg,thread=multi -bios skiboot.lid  \
> -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
> -nodefaults  -serial mon:stdio -nographic  \
> -device pnv-phb,chip-id=0,index=0,id=pcie.0 \
> -device pnv-phb,chip-id=0,index=1,id=pcie.1 \
> -device pnv-phb,chip-id=1,index=2,id=pcie.2 \
> -device pnv-phb-root-port,id=root0,bus=pcie.2 \
> -device pnv-phb-root-port,id=root1,bus=pcie.1 \
> -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
> -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
> -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
> -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
> -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
> -device nec-usb-xhci,bus=bridge1,addr=0x2
> 
> 
> Can be used to boot powernv9 and powernv10 machines with the same attributes
> just by changing the machine type.
> 
> 
> Daniel Henrique Barboza (17):
>    ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
>    ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
>    ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
>    ppc/pnv: add unified pnv-phb header
>    ppc/pnv: add pnv-phb device
>    ppc/pnv: remove PnvPHB3
>    ppc/pnv: user created pnv-phb for powernv8
>    ppc/pnv: remove PnvPHB4
>    ppc/pnv: user creatable pnv-phb for powernv9
>    ppc/pnv: use PnvPHB.version
>    ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
>    ppc/pnv: user creatable pnv-phb for powernv10
>    ppc/pnv: add pnv_phb_get_current_machine()
>    ppc/pnv: add pnv-phb-root-port device
>    ppc/pnv: remove pnv-phb3-root-port
>    ppc/pnv: remove pnv-phb4-root-port
>    ppc/pnv: remove pecc->rp_model
> 
>   hw/pci-host/meson.build        |   3 +-
>   hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
>   hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
>   hw/pci-host/pnv_phb3_msi.c     |  12 +-
>   hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
>   hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
>   hw/pci-host/pnv_phb4_pec.c     |  14 +-
>   hw/ppc/pnv.c                   |  41 ++++-
>   include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
>   include/hw/pci-host/pnv_phb3.h | 118 +------------
>   include/hw/pci-host/pnv_phb4.h | 108 ++----------
>   include/hw/ppc/pnv.h           |   3 +-
>   12 files changed, 757 insertions(+), 586 deletions(-)
>   create mode 100644 hw/pci-host/pnv_phb.c
>   create mode 100644 include/hw/pci-host/pnv_phb.h

I'm completely on-board with having a proxy-like PHB device that maps to the correct 
underlying PHB version, but to me it feels like combining multiple separate devices 
into a single one is going to make things more complicated to maintain in the long term.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-09 21:17 ` [PATCH 00/17] powernv: introduce pnv-phb unified devices Mark Cave-Ayland
@ 2022-05-09 22:30   ` Daniel Henrique Barboza
  2022-05-10  7:57     ` Mark Cave-Ayland
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-09 22:30 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat



On 5/9/22 18:17, Mark Cave-Ayland wrote:
> On 07/05/2022 20:06, Daniel Henrique Barboza wrote:
> 
>> Hi,
>>
>> Since the 7.0.0 release cycle we have a desire to use the powernv
>> emulation with libvirt. To do that we need to enable user creatable
>> pnv-phb devices to allow more user customization an to avoid spamming
>> multiple default devices in the domain XML. In the middle of the
>> previous cycle we experimented with user created
>> pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
>> end result, although functional, is that the user needs to deal with a
>> lot of versioned devices that, from the user perspective, does the same
>> thing. In a way we outsourced the implementation details of the PHBs
>> (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
>> devices also puts an extra burden in the libvirt support we want to
>> have.
>>
>> To solve this, Cedric and Frederic gave the idea of adding a common
>> virtual pnv-phb device that the user can add in the command line, and
>> QEMU handles the details internally. Unfortunatelly this idea turned out
>> to be way harder than anticipated. Between creating a device that would
>> just forward the callbacks to the existing devices internally, creating
>> a PnvPHB device with the minimal attributes and making the other devices
>> inherit from it, and making an 'abstract' device that could be casted
>> for both phb3 and phb4 PHBs,
> 
> This bit sounds all good...
> 
>> all sorts of very obscure problems occured:
>> PHBs not being detected, interrupts not being delivered and memory
>> regions not being able to read/write registers. My initial impression is
>> that there are assumptions made both in ppc/pnv and hw/pci-host that
>> requires the memory region and the bus being in the same device. Even
>> if we somehow figure all this out, the resulting code is hacky and
>> annoying to maitain.
> 
> But this seems really surprising since there are many examples of similar QOM patterns within the codebase, and in my experience they work really well. Do you have a particular example that you can share to demonstrate why the result of the QOM mapping is making things more difficult?


It's not so much about the OOM getting in the way, but rather myself not being
able to use QOM in a way to get what I want. There is a good probability that
QOM is able to do it.

Talking about the 2 PHBs pnv-phb3 (powernv8 only) and pnv-phb4 (powernv9 only),
what we want is a way to let the user instantiate both using an alias, or
an abstract object if you will, called "pnv-phb". This alias/abstraction would
instantiate either a pnv-phb3 or a pnv-phb4 depending on the actual machine
running.

QOM has the "interface" concept that is used internally to make the device behave
like the interface describes it, but I wasn't able to expose this kind of object
to the user. It also has the "abstract" concept that, by the documentation itself,
isn't supposed to be user created. Eventually I gave up this idea, accepting that
only real devices can be exposed to the user (please correct me if I'm wrong).

After that I tried to create a pnv-phb object that contains the common attributes
of both pnv-phb3 and pnv-phb4. This object would be the parent of phb3 and phb4,
and during realize time creating a pnv-phb object would create a pnv-phb3/4 and
we go from there. This attempt went far after a few tweaks but then I started
to hit the problems I mentioned above: some strange behaviors with interrupts,
PHBs not appearing and so on. I got a little farther by moving all the memory
regions from phb3/phb4 to the parent phb object and I got up to the guest login,
but without network. Since moving the memory regions in the same object as the
pci root bus got me farther I concluded that there might some relation/assumption
made somewhere that I was breaking before.

After all that I got more than halfway of what I ended up doing. I decided to
unify phb3/phb4 into a single device, renamed their attributes that has different
meanings between v3 and v4 code, and I ended up with this series.


> 
>> This brings us to this series. The cleaner way I found to accomplish
>> what we want to do is to create real, unified pnv-phb/phb-phb-root-port
>> devices, and get rid of all the versioned ones. This is done by
>> merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
>> files end up using the same pnv-phb and phb-phb-root-port unified devices,
>> with the difference that pnv_phb3* only cares about version 3 attributes
>> and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
>> versions in the future will be a matter of adding any non-existent
>> attributes in the unified pnv-phb* devices and using them in separated
>> pnv_phbN* files.
>>
>> The pnv-phb implementation per se is just a router for either phb3 or
>> phb4 logic, done in init() and realize() time, after we check which powernv
>> machine we're running. If running with powernv8 we redirect control to
>> pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
>> unified device does not do anything per se other than handling control
>> to the right version.
>>
>> After this series, this same powernv8 command line that boots a powernv8
>> machine with some phbs and root ports and with network:
>>
>> ./qemu-system-ppc64 -m 4G \
>> -machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
>> -accel tcg,thread=multi -bios skiboot.lid  \
>> -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
>> -nodefaults  -serial mon:stdio -nographic  \
>> -device pnv-phb,chip-id=0,index=0,id=pcie.0 \
>> -device pnv-phb,chip-id=0,index=1,id=pcie.1 \
>> -device pnv-phb,chip-id=1,index=2,id=pcie.2 \
>> -device pnv-phb-root-port,id=root0,bus=pcie.2 \
>> -device pnv-phb-root-port,id=root1,bus=pcie.1 \
>> -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
>> -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
>> -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
>> -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
>> -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
>> -device nec-usb-xhci,bus=bridge1,addr=0x2
>>
>>
>> Can be used to boot powernv9 and powernv10 machines with the same attributes
>> just by changing the machine type.
>>
>>
>> Daniel Henrique Barboza (17):
>>    ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
>>    ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
>>    ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
>>    ppc/pnv: add unified pnv-phb header
>>    ppc/pnv: add pnv-phb device
>>    ppc/pnv: remove PnvPHB3
>>    ppc/pnv: user created pnv-phb for powernv8
>>    ppc/pnv: remove PnvPHB4
>>    ppc/pnv: user creatable pnv-phb for powernv9
>>    ppc/pnv: use PnvPHB.version
>>    ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
>>    ppc/pnv: user creatable pnv-phb for powernv10
>>    ppc/pnv: add pnv_phb_get_current_machine()
>>    ppc/pnv: add pnv-phb-root-port device
>>    ppc/pnv: remove pnv-phb3-root-port
>>    ppc/pnv: remove pnv-phb4-root-port
>>    ppc/pnv: remove pecc->rp_model
>>
>>   hw/pci-host/meson.build        |   3 +-
>>   hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
>>   hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
>>   hw/pci-host/pnv_phb3_msi.c     |  12 +-
>>   hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
>>   hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
>>   hw/pci-host/pnv_phb4_pec.c     |  14 +-
>>   hw/ppc/pnv.c                   |  41 ++++-
>>   include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
>>   include/hw/pci-host/pnv_phb3.h | 118 +------------
>>   include/hw/pci-host/pnv_phb4.h | 108 ++----------
>>   include/hw/ppc/pnv.h           |   3 +-
>>   12 files changed, 757 insertions(+), 586 deletions(-)
>>   create mode 100644 hw/pci-host/pnv_phb.c
>>   create mode 100644 include/hw/pci-host/pnv_phb.h
> 
> I'm completely on-board with having a proxy-like PHB device that maps to the correct underlying PHB version, but to me it feels like combining multiple separate devices into a single one is going to make things more complicated to maintain in the long term.


I'm all up for a "virtual pnv-phb" device that can be used together with the
existing versioned phbs. I wasn't able to pull that off.



Thanks,


Daniel


> 
> 
> ATB,
> 
> Mark.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-09 22:30   ` Daniel Henrique Barboza
@ 2022-05-10  7:57     ` Mark Cave-Ayland
  2022-05-11 18:30       ` Daniel Henrique Barboza
  2022-05-12 15:03       ` Cédric Le Goater
  0 siblings, 2 replies; 24+ messages in thread
From: Mark Cave-Ayland @ 2022-05-10  7:57 UTC (permalink / raw)
  To: Daniel Henrique Barboza, qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat

On 09/05/2022 23:30, Daniel Henrique Barboza wrote:

> On 5/9/22 18:17, Mark Cave-Ayland wrote:
>> On 07/05/2022 20:06, Daniel Henrique Barboza wrote:
>>
>>> Hi,
>>>
>>> Since the 7.0.0 release cycle we have a desire to use the powernv
>>> emulation with libvirt. To do that we need to enable user creatable
>>> pnv-phb devices to allow more user customization an to avoid spamming
>>> multiple default devices in the domain XML. In the middle of the
>>> previous cycle we experimented with user created
>>> pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
>>> end result, although functional, is that the user needs to deal with a
>>> lot of versioned devices that, from the user perspective, does the same
>>> thing. In a way we outsourced the implementation details of the PHBs
>>> (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
>>> devices also puts an extra burden in the libvirt support we want to
>>> have.
>>>
>>> To solve this, Cedric and Frederic gave the idea of adding a common
>>> virtual pnv-phb device that the user can add in the command line, and
>>> QEMU handles the details internally. Unfortunatelly this idea turned out
>>> to be way harder than anticipated. Between creating a device that would
>>> just forward the callbacks to the existing devices internally, creating
>>> a PnvPHB device with the minimal attributes and making the other devices
>>> inherit from it, and making an 'abstract' device that could be casted
>>> for both phb3 and phb4 PHBs,
>>
>> This bit sounds all good...
>>
>>> all sorts of very obscure problems occured:
>>> PHBs not being detected, interrupts not being delivered and memory
>>> regions not being able to read/write registers. My initial impression is
>>> that there are assumptions made both in ppc/pnv and hw/pci-host that
>>> requires the memory region and the bus being in the same device. Even
>>> if we somehow figure all this out, the resulting code is hacky and
>>> annoying to maitain.
>>
>> But this seems really surprising since there are many examples of similar QOM 
>> patterns within the codebase, and in my experience they work really well. Do you 
>> have a particular example that you can share to demonstrate why the result of the 
>> QOM mapping is making things more difficult?
> 
> 
> It's not so much about the OOM getting in the way, but rather myself not being
> able to use QOM in a way to get what I want. There is a good probability that
> QOM is able to do it.
> 
> Talking about the 2 PHBs pnv-phb3 (powernv8 only) and pnv-phb4 (powernv9 only),
> what we want is a way to let the user instantiate both using an alias, or
> an abstract object if you will, called "pnv-phb". This alias/abstraction would
> instantiate either a pnv-phb3 or a pnv-phb4 depending on the actual machine
> running.
> 
> QOM has the "interface" concept that is used internally to make the device behave
> like the interface describes it, but I wasn't able to expose this kind of object
> to the user. It also has the "abstract" concept that, by the documentation itself,
> isn't supposed to be user created. Eventually I gave up this idea, accepting that
> only real devices can be exposed to the user (please correct me if I'm wrong).

Sorry, I should have clarified this a bit more: introducing an abstract type is a 
separate task from adding your proxy device type, but I think will definitely help 
maintaining the individual versions. Certainly it will make it easier to see which 
fields are in use by specific versions, and it also gives you a QOM cast for the 
superclass of all PHB devices to help with type checking.

> After that I tried to create a pnv-phb object that contains the common attributes
> of both pnv-phb3 and pnv-phb4. This object would be the parent of phb3 and phb4,
> and during realize time creating a pnv-phb object would create a pnv-phb3/4 and
> we go from there. This attempt went far after a few tweaks but then I started
> to hit the problems I mentioned above: some strange behaviors with interrupts,
> PHBs not appearing and so on. I got a little farther by moving all the memory
> regions from phb3/phb4 to the parent phb object and I got up to the guest login,
> but without network. Since moving the memory regions in the same object as the
> pci root bus got me farther I concluded that there might some relation/assumption
> made somewhere that I was breaking before.

That sounds really odd. The normal reasons for this in my experience are i) 
forgetting to update the class/instance size in the class_init function (an 
--enable-sanitizers build will catch this) and ii) not propagating device 
reset/realize functions to a parent device leading to uninitialised state.

> After all that I got more than halfway of what I ended up doing. I decided to
> unify phb3/phb4 into a single device, renamed their attributes that has different
> meanings between v3 and v4 code, and I ended up with this series.

As a rough outline for a pnv-phb device, I'd aim for creating a proxy for the 
underlying device rather than manually invoking the QOM instance and qdev-related 
functions:


struct PnvPHB {
     ....
     uint32_t version;
     Object *phb_dev;  /* Could be PHBCommonBase if it exists */
};

DECLARE_SIMPLE_OBJECT_TYPE(...)

...
...

static Property pnv_phb_properties[] = {
     DEFINE_PROP_UINT32("version", PnvPHB, version, 0),
     DEFINE_PROP_END_OF_LIST(),
};

static void pnv_phb_realize(DeviceState *dev, Error **errp)
{
     PnvPHB *pnv_phb = PNV_PHB(dev);
     g_autofree char *phb_typename;

     if (!pnv_phb->version) {
         error_setg("version not specified", errp);
         return;
     }

     switch (pnv_phb->version) {
     case 3:
         phb_typename = g_strdup(TYPE_PNV_PHB3);
         break;
     case 4:
         phb_typename = g_strdup(TYPE_PNV_PHB4);
         break;
     default:
         g_assert_unreached();
     }

     pnv_phb->phb_dev = object_new(phb_typename);
     object_property_add_child(OBJECT(dev), "phb-device", pnv_phb->phb_dev);

     if (!qdev_realize(DEVICE(pnv_phb->phb_dev), errp)) {
         return;
     }

     /* Passthrough child device properties to the proxy device */
     qdev_alias_all_properties(dev, OBJECT(pnv_phb->phb_dev));
}

Finally you can set the pnv-phb version on a per-machine basis by adding the version 
to the machine compat_props:

static GlobalProperty compat[] = {
     { TYPE_PHB_PNV, "version", 3},
};

>>> This brings us to this series. The cleaner way I found to accomplish
>>> what we want to do is to create real, unified pnv-phb/phb-phb-root-port
>>> devices, and get rid of all the versioned ones. This is done by
>>> merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
>>> files end up using the same pnv-phb and phb-phb-root-port unified devices,
>>> with the difference that pnv_phb3* only cares about version 3 attributes
>>> and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
>>> versions in the future will be a matter of adding any non-existent
>>> attributes in the unified pnv-phb* devices and using them in separated
>>> pnv_phbN* files.
>>>
>>> The pnv-phb implementation per se is just a router for either phb3 or
>>> phb4 logic, done in init() and realize() time, after we check which powernv
>>> machine we're running. If running with powernv8 we redirect control to
>>> pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
>>> unified device does not do anything per se other than handling control
>>> to the right version.
>>>
>>> After this series, this same powernv8 command line that boots a powernv8
>>> machine with some phbs and root ports and with network:
>>>
>>> ./qemu-system-ppc64 -m 4G \
>>> -machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
>>> -accel tcg,thread=multi -bios skiboot.lid  \
>>> -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
>>> -nodefaults  -serial mon:stdio -nographic  \
>>> -device pnv-phb,chip-id=0,index=0,id=pcie.0 \
>>> -device pnv-phb,chip-id=0,index=1,id=pcie.1 \
>>> -device pnv-phb,chip-id=1,index=2,id=pcie.2 \
>>> -device pnv-phb-root-port,id=root0,bus=pcie.2 \
>>> -device pnv-phb-root-port,id=root1,bus=pcie.1 \
>>> -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
>>> -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
>>> -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
>>> -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
>>> -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
>>> -device nec-usb-xhci,bus=bridge1,addr=0x2
>>>
>>>
>>> Can be used to boot powernv9 and powernv10 machines with the same attributes
>>> just by changing the machine type.
>>>
>>>
>>> Daniel Henrique Barboza (17):
>>>    ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
>>>    ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
>>>    ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
>>>    ppc/pnv: add unified pnv-phb header
>>>    ppc/pnv: add pnv-phb device
>>>    ppc/pnv: remove PnvPHB3
>>>    ppc/pnv: user created pnv-phb for powernv8
>>>    ppc/pnv: remove PnvPHB4
>>>    ppc/pnv: user creatable pnv-phb for powernv9
>>>    ppc/pnv: use PnvPHB.version
>>>    ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
>>>    ppc/pnv: user creatable pnv-phb for powernv10
>>>    ppc/pnv: add pnv_phb_get_current_machine()
>>>    ppc/pnv: add pnv-phb-root-port device
>>>    ppc/pnv: remove pnv-phb3-root-port
>>>    ppc/pnv: remove pnv-phb4-root-port
>>>    ppc/pnv: remove pecc->rp_model
>>>
>>>   hw/pci-host/meson.build        |   3 +-
>>>   hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
>>>   hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
>>>   hw/pci-host/pnv_phb3_msi.c     |  12 +-
>>>   hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
>>>   hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
>>>   hw/pci-host/pnv_phb4_pec.c     |  14 +-
>>>   hw/ppc/pnv.c                   |  41 ++++-
>>>   include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
>>>   include/hw/pci-host/pnv_phb3.h | 118 +------------
>>>   include/hw/pci-host/pnv_phb4.h | 108 ++----------
>>>   include/hw/ppc/pnv.h           |   3 +-
>>>   12 files changed, 757 insertions(+), 586 deletions(-)
>>>   create mode 100644 hw/pci-host/pnv_phb.c
>>>   create mode 100644 include/hw/pci-host/pnv_phb.h
>>
>> I'm completely on-board with having a proxy-like PHB device that maps to the 
>> correct underlying PHB version, but to me it feels like combining multiple separate 
>> devices into a single one is going to make things more complicated to maintain in 
>> the long term.
>  
> I'm all up for a "virtual pnv-phb" device that can be used together with the
> existing versioned phbs. I wasn't able to pull that off.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
                   ` (17 preceding siblings ...)
  2022-05-09 21:17 ` [PATCH 00/17] powernv: introduce pnv-phb unified devices Mark Cave-Ayland
@ 2022-05-10  9:07 ` Frederic Barrat
  18 siblings, 0 replies; 24+ messages in thread
From: Frederic Barrat @ 2022-05-10  9:07 UTC (permalink / raw)
  To: Daniel Henrique Barboza, qemu-devel
  Cc: qemu-ppc, david, clg, mark.cave-ayland



On 07/05/2022 21:06, Daniel Henrique Barboza wrote:
> Hi,
> 
> Since the 7.0.0 release cycle we have a desire to use the powernv
> emulation with libvirt. To do that we need to enable user creatable
> pnv-phb devices to allow more user customization an to avoid spamming
> multiple default devices in the domain XML. In the middle of the
> previous cycle we experimented with user created
> pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
> end result, although functional, is that the user needs to deal with a
> lot of versioned devices that, from the user perspective, does the same
> thing. In a way we outsourced the implementation details of the PHBs
> (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
> devices also puts an extra burden in the libvirt support we want to
> have.
> 
> To solve this, Cedric and Frederic gave the idea of adding a common
> virtual pnv-phb device that the user can add in the command line, and
> QEMU handles the details internally. Unfortunatelly this idea turned out
> to be way harder than anticipated. Between creating a device that would
> just forward the callbacks to the existing devices internally, creating
> a PnvPHB device with the minimal attributes and making the other devices
> inherit from it, and making an 'abstract' device that could be casted
> for both phb3 and phb4 PHBs, all sorts of very obscure problems occured:
> PHBs not being detected, interrupts not being delivered and memory
> regions not being able to read/write registers. My initial impression is
> that there are assumptions made both in ppc/pnv and hw/pci-host that
> requires the memory region and the bus being in the same device. Even
> if we somehow figure all this out, the resulting code is hacky and
> annoying to maitain.
> 
> This brings us to this series. The cleaner way I found to accomplish
> what we want to do is to create real, unified pnv-phb/phb-phb-root-port
> devices, and get rid of all the versioned ones. This is done by
> merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
> files end up using the same pnv-phb and phb-phb-root-port unified devices,
> with the difference that pnv_phb3* only cares about version 3 attributes
> and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
> versions in the future will be a matter of adding any non-existent
> attributes in the unified pnv-phb* devices and using them in separated
> pnv_phbN* files.
> 
> The pnv-phb implementation per se is just a router for either phb3 or
> phb4 logic, done in init() and realize() time, after we check which powernv
> machine we're running. If running with powernv8 we redirect control to
> pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
> unified device does not do anything per se other than handling control
> to the right version.
> 
> After this series, this same powernv8 command line that boots a powernv8
> machine with some phbs and root ports and with network:
> 
> ./qemu-system-ppc64 -m 4G \
> -machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
> -accel tcg,thread=multi -bios skiboot.lid  \
> -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
> -nodefaults  -serial mon:stdio -nographic  \
> -device pnv-phb,chip-id=0,index=0,id=pcie.0 \
> -device pnv-phb,chip-id=0,index=1,id=pcie.1 \
> -device pnv-phb,chip-id=1,index=2,id=pcie.2 \
> -device pnv-phb-root-port,id=root0,bus=pcie.2 \
> -device pnv-phb-root-port,id=root1,bus=pcie.1 \
> -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
> -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
> -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
> -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
> -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
> -device nec-usb-xhci,bus=bridge1,addr=0x2
> 
> 
> Can be used to boot powernv9 and powernv10 machines with the same attributes
> just by changing the machine type.
> 


Hello Daniel,

Thanks for trying to cleanup the phb3 and phb4 implementations!
Like you I'm sure, I'm not fond of that big struct PnvPHB, which is 
accumulating all the states of all the PHB versions. It's likely going 
to grow, with most of it being unused. I was about to suggest if it was 
possible to at least have a union for the phb3- or phb4-specific 
attributes, but I'm glad to see that Mark is helping and hasn't given up 
trying to fix it by using a parent object. Hopefully we can make it work.

Some random comments, which may or may not hold depending on how it is 
reworked:
- IODA2 is the I/O Design Architecture used by phb3 and IODA3 is used by 
phb4. So while it makes sense to use the "ioda2_" prefix for phb3, it is 
counter-intuitive to use the "ioda_" prefix for attributes related to phb4.

- the struct PnvPhb3DMASpace and struct PnvPhb4DMASpace are almost 
identical, save for the type of the backpointer to the PHB. It would be 
nice if we could merge them.

   Fred


> Daniel Henrique Barboza (17):
>    ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
>    ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
>    ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
>    ppc/pnv: add unified pnv-phb header
>    ppc/pnv: add pnv-phb device
>    ppc/pnv: remove PnvPHB3
>    ppc/pnv: user created pnv-phb for powernv8
>    ppc/pnv: remove PnvPHB4
>    ppc/pnv: user creatable pnv-phb for powernv9
>    ppc/pnv: use PnvPHB.version
>    ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
>    ppc/pnv: user creatable pnv-phb for powernv10
>    ppc/pnv: add pnv_phb_get_current_machine()
>    ppc/pnv: add pnv-phb-root-port device
>    ppc/pnv: remove pnv-phb3-root-port
>    ppc/pnv: remove pnv-phb4-root-port
>    ppc/pnv: remove pecc->rp_model
> 
>   hw/pci-host/meson.build        |   3 +-
>   hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
>   hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
>   hw/pci-host/pnv_phb3_msi.c     |  12 +-
>   hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
>   hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
>   hw/pci-host/pnv_phb4_pec.c     |  14 +-
>   hw/ppc/pnv.c                   |  41 ++++-
>   include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
>   include/hw/pci-host/pnv_phb3.h | 118 +------------
>   include/hw/pci-host/pnv_phb4.h | 108 ++----------
>   include/hw/ppc/pnv.h           |   3 +-
>   12 files changed, 757 insertions(+), 586 deletions(-)
>   create mode 100644 hw/pci-host/pnv_phb.c
>   create mode 100644 include/hw/pci-host/pnv_phb.h
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-10  7:57     ` Mark Cave-Ayland
@ 2022-05-11 18:30       ` Daniel Henrique Barboza
  2022-05-12 15:03       ` Cédric Le Goater
  1 sibling, 0 replies; 24+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-11 18:30 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel; +Cc: qemu-ppc, david, clg, fbarrat



On 5/10/22 04:57, Mark Cave-Ayland wrote:
> On 09/05/2022 23:30, Daniel Henrique Barboza wrote:
> 
>> On 5/9/22 18:17, Mark Cave-Ayland wrote:
>>> On 07/05/2022 20:06, Daniel Henrique Barboza wrote:
>>>
>>>> Hi,
>>>>
>>>> Since the 7.0.0 release cycle we have a desire to use the powernv
>>>> emulation with libvirt. To do that we need to enable user creatable
>>>> pnv-phb devices to allow more user customization an to avoid spamming
>>>> multiple default devices in the domain XML. In the middle of the
>>>> previous cycle we experimented with user created
>>>> pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The
>>>> end result, although functional, is that the user needs to deal with a
>>>> lot of versioned devices that, from the user perspective, does the same
>>>> thing. In a way we outsourced the implementation details of the PHBs
>>>> (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more
>>>> devices also puts an extra burden in the libvirt support we want to
>>>> have.
>>>>
>>>> To solve this, Cedric and Frederic gave the idea of adding a common
>>>> virtual pnv-phb device that the user can add in the command line, and
>>>> QEMU handles the details internally. Unfortunatelly this idea turned out
>>>> to be way harder than anticipated. Between creating a device that would
>>>> just forward the callbacks to the existing devices internally, creating
>>>> a PnvPHB device with the minimal attributes and making the other devices
>>>> inherit from it, and making an 'abstract' device that could be casted
>>>> for both phb3 and phb4 PHBs,
>>>
>>> This bit sounds all good...
>>>
>>>> all sorts of very obscure problems occured:
>>>> PHBs not being detected, interrupts not being delivered and memory
>>>> regions not being able to read/write registers. My initial impression is
>>>> that there are assumptions made both in ppc/pnv and hw/pci-host that
>>>> requires the memory region and the bus being in the same device. Even
>>>> if we somehow figure all this out, the resulting code is hacky and
>>>> annoying to maitain.
>>>
>>> But this seems really surprising since there are many examples of similar QOM patterns within the codebase, and in my experience they work really well. Do you have a particular example that you can share to demonstrate why the result of the QOM mapping is making things more difficult?
>>
>>
>> It's not so much about the OOM getting in the way, but rather myself not being
>> able to use QOM in a way to get what I want. There is a good probability that
>> QOM is able to do it.
>>
>> Talking about the 2 PHBs pnv-phb3 (powernv8 only) and pnv-phb4 (powernv9 only),
>> what we want is a way to let the user instantiate both using an alias, or
>> an abstract object if you will, called "pnv-phb". This alias/abstraction would
>> instantiate either a pnv-phb3 or a pnv-phb4 depending on the actual machine
>> running.
>>
>> QOM has the "interface" concept that is used internally to make the device behave
>> like the interface describes it, but I wasn't able to expose this kind of object
>> to the user. It also has the "abstract" concept that, by the documentation itself,
>> isn't supposed to be user created. Eventually I gave up this idea, accepting that
>> only real devices can be exposed to the user (please correct me if I'm wrong).
> 
> Sorry, I should have clarified this a bit more: introducing an abstract type is a separate task from adding your proxy device type, but I think will definitely help maintaining the individual versions. Certainly it will make it easier to see which fields are in use by specific versions, and it also gives you a QOM cast for the superclass of all PHB devices to help with type checking.
> 
>> After that I tried to create a pnv-phb object that contains the common attributes
>> of both pnv-phb3 and pnv-phb4. This object would be the parent of phb3 and phb4,
>> and during realize time creating a pnv-phb object would create a pnv-phb3/4 and
>> we go from there. This attempt went far after a few tweaks but then I started
>> to hit the problems I mentioned above: some strange behaviors with interrupts,
>> PHBs not appearing and so on. I got a little farther by moving all the memory
>> regions from phb3/phb4 to the parent phb object and I got up to the guest login,
>> but without network. Since moving the memory regions in the same object as the
>> pci root bus got me farther I concluded that there might some relation/assumption
>> made somewhere that I was breaking before.
> 
> That sounds really odd. The normal reasons for this in my experience are i) forgetting to update the class/instance size in the class_init function (an --enable-sanitizers build will catch this) and ii) not propagating device reset/realize functions to a parent device leading to uninitialised state.
> 
>> After all that I got more than halfway of what I ended up doing. I decided to
>> unify phb3/phb4 into a single device, renamed their attributes that has different
>> meanings between v3 and v4 code, and I ended up with this series.
> 
> As a rough outline for a pnv-phb device, I'd aim for creating a proxy for the underlying device rather than manually invoking the QOM instance and qdev-related functions:
> 
> 
> struct PnvPHB {
>      ....
>      uint32_t version;
>      Object *phb_dev;  /* Could be PHBCommonBase if it exists */
> };
> 
> DECLARE_SIMPLE_OBJECT_TYPE(...)
> 
> ...
> ...
> 
> static Property pnv_phb_properties[] = {
>      DEFINE_PROP_UINT32("version", PnvPHB, version, 0),
>      DEFINE_PROP_END_OF_LIST(),
> };
> 
> static void pnv_phb_realize(DeviceState *dev, Error **errp)
> {
>      PnvPHB *pnv_phb = PNV_PHB(dev);
>      g_autofree char *phb_typename;
> 
>      if (!pnv_phb->version) {
>          error_setg("version not specified", errp);
>          return;
>      }
> 
>      switch (pnv_phb->version) {
>      case 3:
>          phb_typename = g_strdup(TYPE_PNV_PHB3);
>          break;
>      case 4:
>          phb_typename = g_strdup(TYPE_PNV_PHB4);
>          break;
>      default:
>          g_assert_unreached();
>      }
> 
>      pnv_phb->phb_dev = object_new(phb_typename);
>      object_property_add_child(OBJECT(dev), "phb-device", pnv_phb->phb_dev);
> 
>      if (!qdev_realize(DEVICE(pnv_phb->phb_dev), errp)) {
>          return;
>      }
> 
>      /* Passthrough child device properties to the proxy device */
>      qdev_alias_all_properties(dev, OBJECT(pnv_phb->phb_dev));
> }
> 
> Finally you can set the pnv-phb version on a per-machine basis by adding the version to the machine compat_props:
> 
> static GlobalProperty compat[] = {
>      { TYPE_PHB_PNV, "version", 3},
> };


That's really interesting. I'll get a spin with your suggestion to see if I can
make it work. Thanks!



Daniel

> 
>>>> This brings us to this series. The cleaner way I found to accomplish
>>>> what we want to do is to create real, unified pnv-phb/phb-phb-root-port
>>>> devices, and get rid of all the versioned ones. This is done by
>>>> merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4*
>>>> files end up using the same pnv-phb and phb-phb-root-port unified devices,
>>>> with the difference that pnv_phb3* only cares about version 3 attributes
>>>> and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB
>>>> versions in the future will be a matter of adding any non-existent
>>>> attributes in the unified pnv-phb* devices and using them in separated
>>>> pnv_phbN* files.
>>>>
>>>> The pnv-phb implementation per se is just a router for either phb3 or
>>>> phb4 logic, done in init() and realize() time, after we check which powernv
>>>> machine we're running. If running with powernv8 we redirect control to
>>>> pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The
>>>> unified device does not do anything per se other than handling control
>>>> to the right version.
>>>>
>>>> After this series, this same powernv8 command line that boots a powernv8
>>>> machine with some phbs and root ports and with network:
>>>>
>>>> ./qemu-system-ppc64 -m 4G \
>>>> -machine powernv8 -smp 2,sockets=2,cores=1,threads=1  \
>>>> -accel tcg,thread=multi -bios skiboot.lid  \
>>>> -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on'  \
>>>> -nodefaults  -serial mon:stdio -nographic  \
>>>> -device pnv-phb,chip-id=0,index=0,id=pcie.0 \
>>>> -device pnv-phb,chip-id=0,index=1,id=pcie.1 \
>>>> -device pnv-phb,chip-id=1,index=2,id=pcie.2 \
>>>> -device pnv-phb-root-port,id=root0,bus=pcie.2 \
>>>> -device pnv-phb-root-port,id=root1,bus=pcie.1 \
>>>> -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0  \
>>>> -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234  \
>>>> -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none  \
>>>> -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \
>>>> -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \
>>>> -device nec-usb-xhci,bus=bridge1,addr=0x2
>>>>
>>>>
>>>> Can be used to boot powernv9 and powernv10 machines with the same attributes
>>>> just by changing the machine type.
>>>>
>>>>
>>>> Daniel Henrique Barboza (17):
>>>>    ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
>>>>    ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
>>>>    ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
>>>>    ppc/pnv: add unified pnv-phb header
>>>>    ppc/pnv: add pnv-phb device
>>>>    ppc/pnv: remove PnvPHB3
>>>>    ppc/pnv: user created pnv-phb for powernv8
>>>>    ppc/pnv: remove PnvPHB4
>>>>    ppc/pnv: user creatable pnv-phb for powernv9
>>>>    ppc/pnv: use PnvPHB.version
>>>>    ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
>>>>    ppc/pnv: user creatable pnv-phb for powernv10
>>>>    ppc/pnv: add pnv_phb_get_current_machine()
>>>>    ppc/pnv: add pnv-phb-root-port device
>>>>    ppc/pnv: remove pnv-phb3-root-port
>>>>    ppc/pnv: remove pnv-phb4-root-port
>>>>    ppc/pnv: remove pecc->rp_model
>>>>
>>>>   hw/pci-host/meson.build        |   3 +-
>>>>   hw/pci-host/pnv_phb.c          | 258 ++++++++++++++++++++++++++++
>>>>   hw/pci-host/pnv_phb3.c         | 256 +++++++++++-----------------
>>>>   hw/pci-host/pnv_phb3_msi.c     |  12 +-
>>>>   hw/pci-host/pnv_phb3_pbcq.c    |   8 +-
>>>>   hw/pci-host/pnv_phb4.c         | 298 ++++++++++++---------------------
>>>>   hw/pci-host/pnv_phb4_pec.c     |  14 +-
>>>>   hw/ppc/pnv.c                   |  41 ++++-
>>>>   include/hw/pci-host/pnv_phb.h  | 224 +++++++++++++++++++++++++
>>>>   include/hw/pci-host/pnv_phb3.h | 118 +------------
>>>>   include/hw/pci-host/pnv_phb4.h | 108 ++----------
>>>>   include/hw/ppc/pnv.h           |   3 +-
>>>>   12 files changed, 757 insertions(+), 586 deletions(-)
>>>>   create mode 100644 hw/pci-host/pnv_phb.c
>>>>   create mode 100644 include/hw/pci-host/pnv_phb.h
>>>
>>> I'm completely on-board with having a proxy-like PHB device that maps to the correct underlying PHB version, but to me it feels like combining multiple separate devices into a single one is going to make things more complicated to maintain in the long term.
>>
>> I'm all up for a "virtual pnv-phb" device that can be used together with the
>> existing versioned phbs. I wasn't able to pull that off.
> 
> 
> ATB,
> 
> Mark.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 00/17] powernv: introduce pnv-phb unified devices
  2022-05-10  7:57     ` Mark Cave-Ayland
  2022-05-11 18:30       ` Daniel Henrique Barboza
@ 2022-05-12 15:03       ` Cédric Le Goater
  1 sibling, 0 replies; 24+ messages in thread
From: Cédric Le Goater @ 2022-05-12 15:03 UTC (permalink / raw)
  To: Mark Cave-Ayland, Daniel Henrique Barboza, qemu-devel
  Cc: qemu-ppc, david, fbarrat

> As a rough outline for a pnv-phb device, I'd aim for creating a proxy for the underlying device rather than manually invoking the QOM instance and qdev-related functions:
> 
> 
> struct PnvPHB {
>      ....
>      uint32_t version;
>      Object *phb_dev;  /* Could be PHBCommonBase if it exists */
> };
> 
> DECLARE_SIMPLE_OBJECT_TYPE(...)
> 
> ...
> ...
> 
> static Property pnv_phb_properties[] = {
>      DEFINE_PROP_UINT32("version", PnvPHB, version, 0),

It could be the type name directly.

>      DEFINE_PROP_END_OF_LIST(),
> };
> 
> static void pnv_phb_realize(DeviceState *dev, Error **errp)
> {
>      PnvPHB *pnv_phb = PNV_PHB(dev);
>      g_autofree char *phb_typename;
> 
>      if (!pnv_phb->version) {
>          error_setg("version not specified", errp);
>          return;
>      }
> 
>      switch (pnv_phb->version) {
>      case 3:
>          phb_typename = g_strdup(TYPE_PNV_PHB3);
>          break;
>      case 4:
>          phb_typename = g_strdup(TYPE_PNV_PHB4);
>          break;
>      default:
>          g_assert_unreached();
>      }
> 
>      pnv_phb->phb_dev = object_new(phb_typename);
>      object_property_add_child(OBJECT(dev), "phb-device", pnv_phb->phb_dev);
> 
>      if (!qdev_realize(DEVICE(pnv_phb->phb_dev), errp)) {
>          return;
>      }
> 
>      /* Passthrough child device properties to the proxy device */
>      qdev_alias_all_properties(dev, OBJECT(pnv_phb->phb_dev));
> }
> 
> Finally you can set the pnv-phb version on a per-machine basis by adding the version to the machine compat_props:
> 
> static GlobalProperty compat[] = {
>      { TYPE_PHB_PNV, "version", 3},
> };
> 


That looks nice. Something to try for cold plugging phbs on the machine.

Thanks,

C.



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-05-12 15:04 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-07 19:06 [PATCH 00/17] powernv: introduce pnv-phb unified devices Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2* Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 02/17] ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[] Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 03/17] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 04/17] ppc/pnv: add unified pnv-phb header Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 05/17] ppc/pnv: add pnv-phb device Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 06/17] ppc/pnv: remove PnvPHB3 Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 07/17] ppc/pnv: user created pnv-phb for powernv8 Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 08/17] ppc/pnv: remove PnvPHB4 Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 09/17] ppc/pnv: user creatable pnv-phb for powernv9 Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 10/17] ppc/pnv: use PnvPHB.version Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 11/17] ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 12/17] ppc/pnv: user creatable pnv-phb for powernv10 Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 13/17] ppc/pnv: add pnv_phb_get_current_machine() Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 14/17] ppc/pnv: add pnv-phb-root-port device Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 15/17] ppc/pnv: remove pnv-phb3-root-port Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 16/17] ppc/pnv: remove pnv-phb4-root-port Daniel Henrique Barboza
2022-05-07 19:06 ` [PATCH 17/17] ppc/pnv: remove pecc->rp_model Daniel Henrique Barboza
2022-05-09 21:17 ` [PATCH 00/17] powernv: introduce pnv-phb unified devices Mark Cave-Ayland
2022-05-09 22:30   ` Daniel Henrique Barboza
2022-05-10  7:57     ` Mark Cave-Ayland
2022-05-11 18:30       ` Daniel Henrique Barboza
2022-05-12 15:03       ` Cédric Le Goater
2022-05-10  9:07 ` Frederic Barrat

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.