[PATCH v4 0/3] pnv nest1 chiplet model

[PATCH v4 0/3] pnv nest1 chiplet model

[Chalapathi V]

From: Chalapathi V <chalapathi.v@linux.ibm.com>

Hello,

For modularity reasons the P10 processor chip is split into multiple
chiplets individually controlled and managed by the pervasive logic.
The boundaries of these chiplets are defined based on physical design
parameters like clock grids, the nature of the functional units as well
as their pervasive requirements (e.g. clock domains). Examples of chiplet
in the P10 chip are processor cores and caches, memory controllers or IO
interfaces like PCIe. Partitioning the processor chip into these chiplets
allows the pervasive logic to test, initialize, control and manage these
chip partitions individually.

In this series, we create a nest1 chiplet model and implements the chiplet
control scom registers on nest1 chiplet. The chiplet control registers does
the initialization and configuration of a chiplet.

PATCH4: The review comments of PATCH3 has been addressed.

/nest1_chiplet (pnv-nest1-chiplet)
      /perv_chiplet (pnv-pervasive-chiplet)
        /xscom-chiplet-control-regs[0] (memory-region)

Chalapathi V (3):
  hw/ppc: Add pnv pervasive common chiplet units
  hw/ppc: Add nest1 chiplet model
  hw/ppc: Nest1 chiplet wiring

 hw/ppc/meson.build                |   2 +
 hw/ppc/pnv.c                      |  14 +++
 hw/ppc/pnv_nest1_chiplet.c        |  94 +++++++++++++++++
 hw/ppc/pnv_pervasive.c            | 213 ++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv_chip.h         |   2 +
 include/hw/ppc/pnv_nest_chiplet.h |  41 ++++++++
 include/hw/ppc/pnv_pervasive.h    |  39 +++++++
 include/hw/ppc/pnv_xscom.h        |   3 +
 8 files changed, 408 insertions(+)
 create mode 100644 hw/ppc/pnv_nest1_chiplet.c
 create mode 100644 hw/ppc/pnv_pervasive.c
 create mode 100644 include/hw/ppc/pnv_nest_chiplet.h
 create mode 100644 include/hw/ppc/pnv_pervasive.h

-- 
1.8.3.1

[PATCH v4 1/3] hw/ppc: Add pnv pervasive common chiplet units

[Chalapathi V]

From: Chalapathi V <chalapathi.v@linux.ibm.com>

This part of the patchset creates a common pervasive chiplet model where it
houses the common units of a chiplets.

The chiplet control unit is common across chiplets and this commit implements
the pervasive chiplet model with chiplet control registers.

Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
---
 hw/ppc/meson.build             |   1 +
 hw/ppc/pnv_pervasive.c         | 213 +++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv_pervasive.h |  39 ++++++++
 include/hw/ppc/pnv_xscom.h     |   3 +
 4 files changed, 256 insertions(+)
 create mode 100644 hw/ppc/pnv_pervasive.c
 create mode 100644 include/hw/ppc/pnv_pervasive.h

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 7c2c524..c80d2f6 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -50,6 +50,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
   'pnv_bmc.c',
   'pnv_homer.c',
   'pnv_pnor.c',
+  'pnv_pervasive.c',
 ))
 # PowerPC 4xx boards
 ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
diff --git a/hw/ppc/pnv_pervasive.c b/hw/ppc/pnv_pervasive.c
new file mode 100644
index 0000000..40f60b5
--- /dev/null
+++ b/hw/ppc/pnv_pervasive.c
@@ -0,0 +1,213 @@
+/*
+ * QEMU PowerPC pervasive common chiplet model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * 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 "hw/qdev-properties.h"
+#include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/pnv_pervasive.h"
+#include "hw/ppc/fdt.h"
+#include <libfdt.h>
+
+#define CPLT_CONF0               0x08
+#define CPLT_CONF0_OR            0x18
+#define CPLT_CONF0_CLEAR         0x28
+#define CPLT_CONF1               0x09
+#define CPLT_CONF1_OR            0x19
+#define CPLT_CONF1_CLEAR         0x29
+#define CPLT_STAT0               0x100
+#define CPLT_MASK0               0x101
+#define CPLT_PROTECT_MODE        0x3FE
+#define CPLT_ATOMIC_CLOCK        0x3FF
+
+static uint64_t pnv_chiplet_ctrl_read(void *opaque, hwaddr addr,
+                                 unsigned size)
+{
+    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(opaque);
+    int reg = addr >> 3;
+    uint64_t val = ~0ull;
+    /* CPLT_CTRL0 to CPLT_CTRL5 */
+    for (int i = 0; i <= 5; i++) {
+        if (reg == i) {
+            val = perv_chiplet->control_regs.cplt_ctrl[i];
+            return val;
+        } else if ((reg == (i + 0x10)) || (reg == (i + 0x20))) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
+                                           "xscom read at 0x%" PRIx64 "\n",
+                                           __func__, (unsigned long)reg);
+            return val;
+        }
+    }
+
+    switch (reg) {
+    case CPLT_CONF0:
+        val = perv_chiplet->control_regs.cplt_cfg0;
+        break;
+    case CPLT_CONF0_OR:
+    case CPLT_CONF0_CLEAR:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
+                                   "xscom read at 0x%" PRIx64 "\n",
+                                   __func__, (unsigned long)reg);
+        break;
+    case CPLT_CONF1:
+        val = perv_chiplet->control_regs.cplt_cfg1;
+        break;
+    case CPLT_CONF1_OR:
+    case CPLT_CONF1_CLEAR:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
+                                   "xscom read at 0x%" PRIx64 "\n",
+                                   __func__, (unsigned long)reg);
+        break;
+    case CPLT_STAT0:
+        val = perv_chiplet->control_regs.cplt_stat0;
+        break;
+    case CPLT_MASK0:
+        val = perv_chiplet->control_regs.cplt_mask0;
+        break;
+    case CPLT_PROTECT_MODE:
+        val = perv_chiplet->control_regs.ctrl_protect_mode;
+        break;
+    case CPLT_ATOMIC_CLOCK:
+        val = perv_chiplet->control_regs.ctrl_atomic_lock;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "%s: Chiplet_control_regs: Invalid xscom "
+                 "read at 0x%" PRIx64 "\n", __func__, (unsigned long)reg);
+    }
+    return val;
+}
+
+static void pnv_chiplet_ctrl_write(void *opaque, hwaddr addr,
+                                 uint64_t val, unsigned size)
+{
+    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(opaque);
+    int reg = addr >> 3;
+    /* CPLT_CTRL0 to CPLT_CTRL5 */
+    for (int i = 0; i <= 5; i++) {
+        if (reg == i) {
+            perv_chiplet->control_regs.cplt_ctrl[i] = val;
+            return;
+        } else if (reg == (i + 0x10)) {
+            perv_chiplet->control_regs.cplt_ctrl[i] |= val;
+            return;
+        } else if (reg == (i + 0x20)) {
+            perv_chiplet->control_regs.cplt_ctrl[i] &= ~val;
+            return;
+        }
+    }
+
+    switch (reg) {
+    case CPLT_CONF0:
+        perv_chiplet->control_regs.cplt_cfg0 = val;
+        break;
+    case CPLT_CONF0_OR:
+        perv_chiplet->control_regs.cplt_cfg0 |= val;
+        break;
+    case CPLT_CONF0_CLEAR:
+        perv_chiplet->control_regs.cplt_cfg0 &= ~val;
+        break;
+    case CPLT_CONF1:
+        perv_chiplet->control_regs.cplt_cfg1 = val;
+        break;
+    case CPLT_CONF1_OR:
+        perv_chiplet->control_regs.cplt_cfg1 |= val;
+        break;
+    case CPLT_CONF1_CLEAR:
+        perv_chiplet->control_regs.cplt_cfg1 &= ~val;
+        break;
+    case CPLT_STAT0:
+        perv_chiplet->control_regs.cplt_stat0 = val;
+        break;
+    case CPLT_MASK0:
+        perv_chiplet->control_regs.cplt_mask0 = val;
+        break;
+    case CPLT_PROTECT_MODE:
+        perv_chiplet->control_regs.ctrl_protect_mode = val;
+        break;
+    case CPLT_ATOMIC_CLOCK:
+        perv_chiplet->control_regs.ctrl_atomic_lock = val;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "%s: Chiplet_control_regs: Invalid xscom "
+                                 "write at 0x%" PRIx64 "\n",
+                                 __func__, (unsigned long)reg);
+    }
+    return;
+}
+
+static const MemoryRegionOps pnv_perv_chiplet_control_xscom_ops = {
+    .read = pnv_chiplet_ctrl_read,
+    .write = pnv_chiplet_ctrl_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void pnv_perv_chiplet_realize(DeviceState *dev, Error **errp)
+{
+    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(dev);
+
+    /* Chiplet control scoms */
+    pnv_xscom_region_init(&perv_chiplet->xscom_perv_ctrl_regs,
+                          OBJECT(perv_chiplet),
+                          &pnv_perv_chiplet_control_xscom_ops,
+                          perv_chiplet, "xscom-chiplet-control-regs",
+                          PNV10_XSCOM_CTRL_CHIPLET_SIZE);
+}
+
+void pnv_perv_dt(uint32_t base_addr, void *fdt,
+                             int offset)
+{
+    g_autofree char *name = NULL;
+    int perv_chiplet_offset;
+
+    const char compat[] = "ibm,power10-perv-chiplet";
+    uint32_t reg[] = {
+        base_addr,
+        cpu_to_be32(PNV10_XSCOM_CTRL_CHIPLET_SIZE)
+    };
+    name = g_strdup_printf("perv_chiplet@%x", base_addr);
+    perv_chiplet_offset = fdt_add_subnode(fdt, offset, name);
+    _FDT(perv_chiplet_offset);
+
+    _FDT(fdt_setprop(fdt, perv_chiplet_offset, "reg", reg, sizeof(reg)));
+    _FDT(fdt_setprop(fdt, perv_chiplet_offset, "compatible",
+                     compat, sizeof(compat)));
+}
+
+static void pnv_perv_chiplet_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "PowerNV perv chiplet";
+    dc->realize = pnv_perv_chiplet_realize;
+}
+
+static const TypeInfo pnv_perv_chiplet_info = {
+    .name          = TYPE_PNV_PERV_CHIPLET,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvPervChiplet),
+    .class_init    = pnv_perv_chiplet_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_perv_chiplet_register_types(void)
+{
+    type_register_static(&pnv_perv_chiplet_info);
+}
+
+type_init(pnv_perv_chiplet_register_types);
diff --git a/include/hw/ppc/pnv_pervasive.h b/include/hw/ppc/pnv_pervasive.h
new file mode 100644
index 0000000..61be4cf
--- /dev/null
+++ b/include/hw/ppc/pnv_pervasive.h
@@ -0,0 +1,39 @@
+/*
+ * QEMU PowerPC pervasive common chiplet model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef PPC_PNV_PERVASIVE_H
+#define PPC_PNV_PERVASIVE_H
+
+#define TYPE_PNV_PERV_CHIPLET "pnv-pervasive-chiplet"
+#define PNV_PERVCHIPLET(obj) OBJECT_CHECK(PnvPervChiplet, (obj), TYPE_PNV_PERV_CHIPLET)
+
+typedef struct ControlRegs {
+
+    uint64_t cplt_ctrl[6];
+    uint64_t cplt_cfg0;
+    uint64_t cplt_cfg1;
+    uint64_t cplt_stat0;
+    uint64_t cplt_mask0;
+    uint64_t ctrl_protect_mode;
+    uint64_t ctrl_atomic_lock;
+} ControlRegs;
+
+typedef struct PnvPervChiplet {
+
+    DeviceState parent;
+    struct PnvChip *chip;
+    MemoryRegion xscom_perv_ctrl_regs;
+    ControlRegs control_regs;
+
+} PnvPervChiplet;
+
+void pnv_perv_dt(uint32_t base, void *fdt, int offset);
+#endif /*PPC_PNV_PERVASIVE_H */
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 35b1961..87bbee8 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -164,6 +164,9 @@ struct PnvXScomInterfaceClass {
 #define PNV10_XSCOM_XIVE2_BASE     0x2010800
 #define PNV10_XSCOM_XIVE2_SIZE     0x400
 
+#define PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE      0x3000000
+#define PNV10_XSCOM_CTRL_CHIPLET_SIZE            0x400
+
 #define PNV10_XSCOM_PEC_NEST_BASE  0x3011800 /* index goes downwards ... */
 #define PNV10_XSCOM_PEC_NEST_SIZE  0x100
 
-- 
1.8.3.1

[PATCH v4 2/3] hw/ppc: Add nest1 chiplet model

[Chalapathi V]

From: Chalapathi V <chalapathi.v@linux.ibm.com>

The nest1 chiplet handle the high speed i/o traffic over PCIe and others.
The nest1 chiplet consists of PowerBus Fabric controller,
nest Memory Management Unit, chiplet control unit and more.

This commit creates a nest1 chiplet model and initialize and realize the
pervasive chiplet model where chiplet control registers are implemented.

Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
---
 hw/ppc/meson.build                |  1 +
 hw/ppc/pnv_nest1_chiplet.c        | 94 +++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv_nest_chiplet.h | 41 +++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 hw/ppc/pnv_nest1_chiplet.c
 create mode 100644 include/hw/ppc/pnv_nest_chiplet.h

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index c80d2f6..4e45e5c 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -51,6 +51,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
   'pnv_homer.c',
   'pnv_pnor.c',
   'pnv_pervasive.c',
+  'pnv_nest1_chiplet.c',
 ))
 # PowerPC 4xx boards
 ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
diff --git a/hw/ppc/pnv_nest1_chiplet.c b/hw/ppc/pnv_nest1_chiplet.c
new file mode 100644
index 0000000..e078076
--- /dev/null
+++ b/hw/ppc/pnv_nest1_chiplet.c
@@ -0,0 +1,94 @@
+/*
+ * QEMU PowerPC nest1 chiplet model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * 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 "hw/qdev-properties.h"
+#include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/pnv_nest_chiplet.h"
+#include "hw/ppc/pnv_pervasive.h"
+#include "hw/ppc/fdt.h"
+#include <libfdt.h>
+
+/*
+ * The nest1 chiplet contains chiplet control unit,
+ * PowerBus/RaceTrack/Bridge logic, nest Memory Management Unit(nMMU)
+ * and more.
+ */
+
+static void pnv_nest1_chiplet_realize(DeviceState *dev, Error **errp)
+{
+    PnvNest1Chiplet *nest1_chiplet = PNV_NEST1CHIPLET(dev);
+
+    object_initialize_child(OBJECT(nest1_chiplet), "perv_chiplet",
+                            &nest1_chiplet->perv_chiplet,
+                            TYPE_PNV_PERV_CHIPLET);
+
+    if (!qdev_realize(DEVICE(&nest1_chiplet->perv_chiplet), NULL, errp)) {
+        return;
+    }
+}
+
+static int pnv_nest1_chiplet_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                             int offset)
+{
+    g_autofree char *name = NULL;
+    int nest1_chiplet_offset;
+    const char compat[] = "ibm,power10-nest1-chiplet";
+
+    name = g_strdup_printf("nest1_chiplet@%x",
+                           PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE);
+    nest1_chiplet_offset = fdt_add_subnode(fdt, offset, name);
+    _FDT(nest1_chiplet_offset);
+
+    _FDT(fdt_setprop(fdt, nest1_chiplet_offset, "compatible",
+                            compat, sizeof(compat)));
+    return 0;
+}
+
+static void pnv_nest1_dt_populate(void *fdt)
+{
+
+    uint32_t nest1_base = cpu_to_be32(PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE);
+    pnv_perv_dt(nest1_base, fdt, 0);
+}
+
+static void pnv_nest1_chiplet_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PnvNest1Class *nest1_class = PNV_NEST1CHIPLET_CLASS(klass);
+    PnvXScomInterfaceClass *xscomc = PNV_XSCOM_INTERFACE_CLASS(klass);
+
+    xscomc->dt_xscom = pnv_nest1_chiplet_dt_xscom;
+
+    dc->desc = "PowerNV nest1 chiplet";
+    dc->realize = pnv_nest1_chiplet_realize;
+    nest1_class->nest1_dt_populate = pnv_nest1_dt_populate;
+}
+
+static const TypeInfo pnv_nest1_chiplet_info = {
+    .name          = TYPE_PNV_NEST1_CHIPLET,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvNest1Chiplet),
+    .class_init    = pnv_nest1_chiplet_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_nest1_chiplet_register_types(void)
+{
+    type_register_static(&pnv_nest1_chiplet_info);
+}
+
+type_init(pnv_nest1_chiplet_register_types);
diff --git a/include/hw/ppc/pnv_nest_chiplet.h b/include/hw/ppc/pnv_nest_chiplet.h
new file mode 100644
index 0000000..12525d3
--- /dev/null
+++ b/include/hw/ppc/pnv_nest_chiplet.h
@@ -0,0 +1,41 @@
+/*
+ * QEMU PowerPC nest chiplet model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef PPC_PNV_NEST1_CHIPLET_H
+#define PPC_PNV_NEST1_CHIPLET_H
+
+#include "hw/ppc/pnv_pervasive.h"
+
+#define TYPE_PNV_NEST1_CHIPLET "pnv-nest1-chiplet"
+typedef struct PnvNest1Class PnvNest1Class;
+typedef struct PnvNest1Chiplet PnvNest1Chiplet;
+DECLARE_OBJ_CHECKERS(PnvNest1Chiplet, PnvNest1Class,
+                     PNV_NEST1CHIPLET, TYPE_PNV_NEST1_CHIPLET)
+
+typedef struct PnvNest1Chiplet {
+    DeviceState parent;
+
+    struct PnvChip *chip;
+
+    /* common pervasive chiplet unit */
+    PnvPervChiplet perv_chiplet;
+} PnvNest1Chiplet;
+
+struct PnvNest1Class {
+    DeviceClass parent_class;
+
+    DeviceRealize parent_realize;
+
+    void (*nest1_dt_populate)(void *fdt);
+};
+
+#endif /*PPC_PNV_NEST1_CHIPLET_H */
-- 
1.8.3.1

[PATCH v4 3/3] hw/ppc: Nest1 chiplet wiring

[Chalapathi V]

From: Chalapathi V <chalapathi.v@linux.ibm.com>

This part of the patchset connects the nest1 chiplet model to p10 chip.

Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
---
 hw/ppc/pnv.c              | 14 ++++++++++++++
 include/hw/ppc/pnv_chip.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index c0e34ff..2b93cdd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -351,6 +351,8 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
     static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
     int i;
 
+    Pnv10Chip *chip10 = PNV10_CHIP(chip);
+
     pnv_dt_xscom(chip, fdt, 0,
                  cpu_to_be64(PNV10_XSCOM_BASE(chip)),
                  cpu_to_be64(PNV10_XSCOM_SIZE),
@@ -366,6 +368,9 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
         pnv_dt_memory(fdt, chip->chip_id, chip->ram_start, chip->ram_size);
     }
 
+    /* Populate nest1_chiplet device tree */
+    PNV_NEST1CHIPLET_GET_CLASS(&chip10->nest1_chiplet)->nest1_dt_populate(fdt);
+
     pnv_dt_lpc(chip, fdt, 0, PNV10_LPCM_BASE(chip), PNV10_LPCM_SIZE);
 }
 
@@ -1649,6 +1654,8 @@ static void pnv_chip_power10_instance_init(Object *obj)
     object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
     object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
     object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
+    object_initialize_child(obj, "nest1_chiplet", &chip10->nest1_chiplet,
+                            TYPE_PNV_NEST1_CHIPLET);
 
     chip->num_pecs = pcc->num_pecs;
 
@@ -1813,6 +1820,13 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
                                 &chip10->homer.regs);
 
+    /* nest1 chiplet control regs */
+    if (!qdev_realize(DEVICE(&chip10->nest1_chiplet), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE,
+                   &chip10->nest1_chiplet.perv_chiplet.xscom_perv_ctrl_regs);
+
     /* PHBs */
     pnv_chip_power10_phb_realize(chip, &local_err);
     if (local_err) {
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 53e1d92..4bcb925 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -4,6 +4,7 @@
 #include "hw/pci-host/pnv_phb4.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_homer.h"
+#include "hw/ppc/pnv_nest_chiplet.h"
 #include "hw/ppc/pnv_lpc.h"
 #include "hw/ppc/pnv_occ.h"
 #include "hw/ppc/pnv_psi.h"
@@ -109,6 +110,7 @@ struct Pnv10Chip {
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
+    PnvNest1Chiplet nest1_chiplet;
 
     uint32_t     nr_quads;
     PnvQuad      *quads;
-- 
1.8.3.1

Re: [PATCH v4 0/3] pnv nest1 chiplet model

[Cédric Le Goater]

Hello Chalapathi,

Please tune the "From: " email address of the series you send.
This one uses " Chalapathi V <chalap1@gfwr516.rchland.ibm.com>"
which is certainly from an internal IBM host. Unfortunately, we
can not reply to this user/sender.

On 11/7/23 08:41, Chalapathi V wrote:
> From: Chalapathi V <chalapathi.v@linux.ibm.com>
> 
> Hello,
> 
> For modularity reasons the P10 processor chip is split into multiple
> chiplets individually controlled and managed by the pervasive logic.
> The boundaries of these chiplets are defined based on physical design
> parameters like clock grids, the nature of the functional units as well
> as their pervasive requirements (e.g. clock domains). Examples of chiplet
> in the P10 chip are processor cores and caches, memory controllers or IO
> interfaces like PCIe. Partitioning the processor chip into these chiplets
> allows the pervasive logic to test, initialize, control and manage these
> chip partitions individually.
> 
> In this series, we create a nest1 chiplet model and implements the chiplet
> control scom registers on nest1 chiplet. The chiplet control registers does
> the initialization and configuration of a chiplet.
> 
> PATCH4: The review comments of PATCH3 has been addressed.

What do you mean by PATCH4 and PATCH3 ? Version 4 and 3 ?

Usually, people send a small changelog with the cover letter explaining
the differences between each respin. See the Zhenzhong's series "vfio:
Adopt iommufd" [1] for an example. Your series does not need that much
details, but "comments have been addressed" is not very useful.

Thanks,

C.


> 
> /nest1_chiplet (pnv-nest1-chiplet)
>        /perv_chiplet (pnv-pervasive-chiplet)
>          /xscom-chiplet-control-regs[0] (memory-region)
> 
> Chalapathi V (3):
>    hw/ppc: Add pnv pervasive common chiplet units
>    hw/ppc: Add nest1 chiplet model
>    hw/ppc: Nest1 chiplet wiring
> 
>   hw/ppc/meson.build                |   2 +
>   hw/ppc/pnv.c                      |  14 +++
>   hw/ppc/pnv_nest1_chiplet.c        |  94 +++++++++++++++++
>   hw/ppc/pnv_pervasive.c            | 213 ++++++++++++++++++++++++++++++++++++++
>   include/hw/ppc/pnv_chip.h         |   2 +
>   include/hw/ppc/pnv_nest_chiplet.h |  41 ++++++++
>   include/hw/ppc/pnv_pervasive.h    |  39 +++++++
>   include/hw/ppc/pnv_xscom.h        |   3 +
>   8 files changed, 408 insertions(+)
>   create mode 100644 hw/ppc/pnv_nest1_chiplet.c
>   create mode 100644 hw/ppc/pnv_pervasive.c
>   create mode 100644 include/hw/ppc/pnv_nest_chiplet.h
>   create mode 100644 include/hw/ppc/pnv_pervasive.h
> 

Re: [PATCH v4 1/3] hw/ppc: Add pnv pervasive common chiplet units

[Cédric Le Goater]

Hello Chalapathi,

Please add to your ~/.gitconfig :

   [diff]
	  orderFile = /path/to/qemu/scripts/git.orderfile

It will order the header file first in the patch.

On 11/7/23 08:41, Chalapathi V wrote:
> From: Chalapathi V <chalapathi.v@linux.ibm.com>
> 
> This part of the patchset creates a common pervasive chiplet model where it
> houses the common units of a chiplets.
> 
> The chiplet control unit is common across chiplets and this commit implements
> the pervasive chiplet model with chiplet control registers.
> 
> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> ---
>   hw/ppc/meson.build             |   1 +
>   hw/ppc/pnv_pervasive.c         | 213 +++++++++++++++++++++++++++++++++++++++++
>   include/hw/ppc/pnv_pervasive.h |  39 ++++++++
>   include/hw/ppc/pnv_xscom.h     |   3 +
>   4 files changed, 256 insertions(+)
>   create mode 100644 hw/ppc/pnv_pervasive.c
>   create mode 100644 include/hw/ppc/pnv_pervasive.h
> 
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 7c2c524..c80d2f6 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -50,6 +50,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
>     'pnv_bmc.c',
>     'pnv_homer.c',
>     'pnv_pnor.c',
> +  'pnv_pervasive.c',
>   ))
>   # PowerPC 4xx boards
>   ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
> diff --git a/hw/ppc/pnv_pervasive.c b/hw/ppc/pnv_pervasive.c
> new file mode 100644
> index 0000000..40f60b5
> --- /dev/null
> +++ b/hw/ppc/pnv_pervasive.c
> @@ -0,0 +1,213 @@
> +/*
> + * QEMU PowerPC pervasive common chiplet model
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * 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 "hw/qdev-properties.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/pnv_pervasive.h"
> +#include "hw/ppc/fdt.h"
> +#include <libfdt.h>
> +
> +#define CPLT_CONF0               0x08
> +#define CPLT_CONF0_OR            0x18
> +#define CPLT_CONF0_CLEAR         0x28
> +#define CPLT_CONF1               0x09
> +#define CPLT_CONF1_OR            0x19
> +#define CPLT_CONF1_CLEAR         0x29
> +#define CPLT_STAT0               0x100
> +#define CPLT_MASK0               0x101
> +#define CPLT_PROTECT_MODE        0x3FE
> +#define CPLT_ATOMIC_CLOCK        0x3FF
> +
> +static uint64_t pnv_chiplet_ctrl_read(void *opaque, hwaddr addr,
> +                                 unsigned size)
> +{
> +    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(opaque);
> +    int reg = addr >> 3;
> +    uint64_t val = ~0ull;

White line please

> +    /* CPLT_CTRL0 to CPLT_CTRL5 */
> +    for (int i = 0; i <= 5; i++) {

I would introduce a #define for this "5" value, and use it in ControlRegs also.

> +        if (reg == i) {
> +            val = perv_chiplet->control_regs.cplt_ctrl[i];
> +            return val;

or may be :

        return perv_chiplet->control_regs.cplt_ctrl[i];  ?
  

> +        } else if ((reg == (i + 0x10)) || (reg == (i + 0x20))) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
> +                                           "xscom read at 0x%" PRIx64 "\n",
> +                                           __func__, (unsigned long)reg);
> +            return val;
> +        }
> +    }
> +
> +    switch (reg) {
> +    case CPLT_CONF0:
> +        val = perv_chiplet->control_regs.cplt_cfg0;
> +        break;
> +    case CPLT_CONF0_OR:
> +    case CPLT_CONF0_CLEAR:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
> +                                   "xscom read at 0x%" PRIx64 "\n",
> +                                   __func__, (unsigned long)reg);
> +        break;
> +    case CPLT_CONF1:
> +        val = perv_chiplet->control_regs.cplt_cfg1;
> +        break;
> +    case CPLT_CONF1_OR:
> +    case CPLT_CONF1_CLEAR:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
> +                                   "xscom read at 0x%" PRIx64 "\n",
> +                                   __func__, (unsigned long)reg);
> +        break;
> +    case CPLT_STAT0:
> +        val = perv_chiplet->control_regs.cplt_stat0;
> +        break;
> +    case CPLT_MASK0:
> +        val = perv_chiplet->control_regs.cplt_mask0;
> +        break;
> +    case CPLT_PROTECT_MODE:
> +        val = perv_chiplet->control_regs.ctrl_protect_mode;
> +        break;
> +    case CPLT_ATOMIC_CLOCK:
> +        val = perv_chiplet->control_regs.ctrl_atomic_lock;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "%s: Chiplet_control_regs: Invalid xscom "
> +                 "read at 0x%" PRIx64 "\n", __func__, (unsigned long)reg);
> +    }
> +    return val;
> +}
> +
> +static void pnv_chiplet_ctrl_write(void *opaque, hwaddr addr,
> +                                 uint64_t val, unsigned size)
> +{
> +    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(opaque);
> +    int reg = addr >> 3;
> +    /* CPLT_CTRL0 to CPLT_CTRL5 */
> +    for (int i = 0; i <= 5; i++) {
> +        if (reg == i) {
> +            perv_chiplet->control_regs.cplt_ctrl[i] = val;
> +            return;
> +        } else if (reg == (i + 0x10)) {
> +            perv_chiplet->control_regs.cplt_ctrl[i] |= val;
> +            return;
> +        } else if (reg == (i + 0x20)) {
> +            perv_chiplet->control_regs.cplt_ctrl[i] &= ~val;
> +            return;
> +        }
> +    }
> +
> +    switch (reg) {
> +    case CPLT_CONF0:
> +        perv_chiplet->control_regs.cplt_cfg0 = val;
> +        break;
> +    case CPLT_CONF0_OR:
> +        perv_chiplet->control_regs.cplt_cfg0 |= val;
> +        break;
> +    case CPLT_CONF0_CLEAR:
> +        perv_chiplet->control_regs.cplt_cfg0 &= ~val;
> +        break;
> +    case CPLT_CONF1:
> +        perv_chiplet->control_regs.cplt_cfg1 = val;
> +        break;
> +    case CPLT_CONF1_OR:
> +        perv_chiplet->control_regs.cplt_cfg1 |= val;
> +        break;
> +    case CPLT_CONF1_CLEAR:
> +        perv_chiplet->control_regs.cplt_cfg1 &= ~val;
> +        break;
> +    case CPLT_STAT0:
> +        perv_chiplet->control_regs.cplt_stat0 = val;
> +        break;
> +    case CPLT_MASK0:
> +        perv_chiplet->control_regs.cplt_mask0 = val;
> +        break;
> +    case CPLT_PROTECT_MODE:
> +        perv_chiplet->control_regs.ctrl_protect_mode = val;
> +        break;
> +    case CPLT_ATOMIC_CLOCK:
> +        perv_chiplet->control_regs.ctrl_atomic_lock = val;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "%s: Chiplet_control_regs: Invalid xscom "
> +                                 "write at 0x%" PRIx64 "\n",
> +                                 __func__, (unsigned long)reg);
> +    }
> +    return;

This return is useless.

> +}
> +
> +static const MemoryRegionOps pnv_perv_chiplet_control_xscom_ops = {
> +    .read = pnv_chiplet_ctrl_read,
> +    .write = pnv_chiplet_ctrl_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void pnv_perv_chiplet_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvPervChiplet *perv_chiplet = PNV_PERVCHIPLET(dev);
> +
> +    /* Chiplet control scoms */
> +    pnv_xscom_region_init(&perv_chiplet->xscom_perv_ctrl_regs,
> +                          OBJECT(perv_chiplet),
> +                          &pnv_perv_chiplet_control_xscom_ops,
> +                          perv_chiplet, "xscom-chiplet-control-regs",
> +                          PNV10_XSCOM_CTRL_CHIPLET_SIZE);
> +}
> +
> +void pnv_perv_dt(uint32_t base_addr, void *fdt,
> +                             int offset)
> +{
> +    g_autofree char *name = NULL;
> +    int perv_chiplet_offset;
> +
> +    const char compat[] = "ibm,power10-perv-chiplet";
> +    uint32_t reg[] = {
> +        base_addr,
> +        cpu_to_be32(PNV10_XSCOM_CTRL_CHIPLET_SIZE)
> +    };
> +    name = g_strdup_printf("perv_chiplet@%x", base_addr);
> +    perv_chiplet_offset = fdt_add_subnode(fdt, offset, name);
> +    _FDT(perv_chiplet_offset);
> +
> +    _FDT(fdt_setprop(fdt, perv_chiplet_offset, "reg", reg, sizeof(reg)));
> +    _FDT(fdt_setprop(fdt, perv_chiplet_offset, "compatible",
> +                     compat, sizeof(compat)));
> +}
> +
> +static void pnv_perv_chiplet_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc = "PowerNV perv chiplet";
> +    dc->realize = pnv_perv_chiplet_realize;
> +}
> +
> +static const TypeInfo pnv_perv_chiplet_info = {
> +    .name          = TYPE_PNV_PERV_CHIPLET,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvPervChiplet),
> +    .class_init    = pnv_perv_chiplet_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_perv_chiplet_register_types(void)
> +{
> +    type_register_static(&pnv_perv_chiplet_info);
> +}
> +
> +type_init(pnv_perv_chiplet_register_types);
> diff --git a/include/hw/ppc/pnv_pervasive.h b/include/hw/ppc/pnv_pervasive.h
> new file mode 100644
> index 0000000..61be4cf
> --- /dev/null
> +++ b/include/hw/ppc/pnv_pervasive.h
> @@ -0,0 +1,39 @@
> +/*
> + * QEMU PowerPC pervasive common chiplet model
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef PPC_PNV_PERVASIVE_H
> +#define PPC_PNV_PERVASIVE_H
> +
> +#define TYPE_PNV_PERV_CHIPLET "pnv-pervasive-chiplet"
> +#define PNV_PERVCHIPLET(obj) OBJECT_CHECK(PnvPervChiplet, (obj), TYPE_PNV_PERV_CHIPLET)
> +
> +typedef struct ControlRegs {

This needs some prefix : PnvPervChipletRegs ?

> +

Please remove the extra white line.
  
> +    uint64_t cplt_ctrl[6];

Introduce a #define since you need it in the memops.

> +    uint64_t cplt_cfg0;
> +    uint64_t cplt_cfg1;
> +    uint64_t cplt_stat0;
> +    uint64_t cplt_mask0;
> +    uint64_t ctrl_protect_mode;
> +    uint64_t ctrl_atomic_lock;
> +} ControlRegs;
> +
> +typedef struct PnvPervChiplet {
> +
> +    DeviceState parent;
> +    struct PnvChip *chip;

chip is unused, please remove.

> +    MemoryRegion xscom_perv_ctrl_regs;
> +    ControlRegs control_regs;
> +

Drop white line.


> +} PnvPervChiplet;
> +
> +void pnv_perv_dt(uint32_t base, void *fdt, int offset);
> +#endif /*PPC_PNV_PERVASIVE_H */
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index 35b1961..87bbee8 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -164,6 +164,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV10_XSCOM_XIVE2_BASE     0x2010800
>   #define PNV10_XSCOM_XIVE2_SIZE     0x400
>   
> +#define PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE      0x3000000
> +#define PNV10_XSCOM_CTRL_CHIPLET_SIZE            0x400
> +
>   #define PNV10_XSCOM_PEC_NEST_BASE  0x3011800 /* index goes downwards ... */
>   #define PNV10_XSCOM_PEC_NEST_SIZE  0x100
>   

Re: [PATCH v4 2/3] hw/ppc: Add nest1 chiplet model

[Cédric Le Goater]

On 11/7/23 08:41, Chalapathi V wrote:
> From: Chalapathi V <chalapathi.v@linux.ibm.com>
> 
> The nest1 chiplet handle the high speed i/o traffic over PCIe and others.
> The nest1 chiplet consists of PowerBus Fabric controller,
> nest Memory Management Unit, chiplet control unit and more.
> 
> This commit creates a nest1 chiplet model and initialize and realize the
> pervasive chiplet model where chiplet control registers are implemented.
> 
> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> ---
>   hw/ppc/meson.build                |  1 +
>   hw/ppc/pnv_nest1_chiplet.c        | 94 +++++++++++++++++++++++++++++++++++++++
>   include/hw/ppc/pnv_nest_chiplet.h | 41 +++++++++++++++++
>   3 files changed, 136 insertions(+)
>   create mode 100644 hw/ppc/pnv_nest1_chiplet.c
>   create mode 100644 include/hw/ppc/pnv_nest_chiplet.h
> 
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index c80d2f6..4e45e5c 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -51,6 +51,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
>     'pnv_homer.c',
>     'pnv_pnor.c',
>     'pnv_pervasive.c',
> +  'pnv_nest1_chiplet.c',
>   ))
>   # PowerPC 4xx boards
>   ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
> diff --git a/hw/ppc/pnv_nest1_chiplet.c b/hw/ppc/pnv_nest1_chiplet.c
> new file mode 100644
> index 0000000..e078076
> --- /dev/null
> +++ b/hw/ppc/pnv_nest1_chiplet.c
> @@ -0,0 +1,94 @@
> +/*
> + * QEMU PowerPC nest1 chiplet model
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * 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 "hw/qdev-properties.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/pnv_nest_chiplet.h"
> +#include "hw/ppc/pnv_pervasive.h"
> +#include "hw/ppc/fdt.h"
> +#include <libfdt.h>
> +
> +/*
> + * The nest1 chiplet contains chiplet control unit,
> + * PowerBus/RaceTrack/Bridge logic, nest Memory Management Unit(nMMU)
> + * and more.
> + */
> +
> +static void pnv_nest1_chiplet_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvNest1Chiplet *nest1_chiplet = PNV_NEST1CHIPLET(dev);
> +
> +    object_initialize_child(OBJECT(nest1_chiplet), "perv_chiplet",
> +                            &nest1_chiplet->perv_chiplet,
> +                            TYPE_PNV_PERV_CHIPLET);
> +
> +    if (!qdev_realize(DEVICE(&nest1_chiplet->perv_chiplet), NULL, errp)) {

I think we will save some bytes and ease reading by removing the '_chiplet'
suffix in the variable names and types. A QEMU model is generally an HW
logic unit or sub-unit. We don't need to add a Chip* suffix.

PnvChip is an exception to this comment, because it represent a socket or
processor, and is referred to as a chip in the POWER documentation AFAICR.

For instance, this routine would become :


   static void pnv_nest1_realize(DeviceState *dev, Error **errp)
   {
       PnvNest1 *nest1 = PNV_NEST1(dev);
   
       object_initialize_child(OBJECT(nest1), "perv", &nest1->perv,
                               TYPE_PNV_PERV);
   
       if (!qdev_realize(DEVICE(&nest1->perv), NULL, errp)) {
	      return;
       }
   }


which is clear enough for me. What do you think ? We can still use the
term chiplet in the documentation.


> +        return;
> +    }
> +}
> +
> +static int pnv_nest1_chiplet_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                             int offset)
> +{
> +    g_autofree char *name = NULL;
> +    int nest1_chiplet_offset;
> +    const char compat[] = "ibm,power10-nest1-chiplet";
> +
> +    name = g_strdup_printf("nest1_chiplet@%x",
> +                           PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE);
> +    nest1_chiplet_offset = fdt_add_subnode(fdt, offset, name);
> +    _FDT(nest1_chiplet_offset);
> +
> +    _FDT(fdt_setprop(fdt, nest1_chiplet_offset, "compatible",
> +                            compat, sizeof(compat)));
> +    return 0;
> +}
> +
> +static void pnv_nest1_dt_populate(void *fdt)
> +{
> +

Please remove white line.

> +    uint32_t nest1_base = cpu_to_be32(PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE);
> +    pnv_perv_dt(nest1_base, fdt, 0);
> +}
> +
> +static void pnv_nest1_chiplet_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvNest1Class *nest1_class = PNV_NEST1CHIPLET_CLASS(klass);
> +    PnvXScomInterfaceClass *xscomc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +
> +    xscomc->dt_xscom = pnv_nest1_chiplet_dt_xscom;
> +
> +    dc->desc = "PowerNV nest1 chiplet";
> +    dc->realize = pnv_nest1_chiplet_realize;
> +    nest1_class->nest1_dt_populate = pnv_nest1_dt_populate;

Why do you need a class handler for the Nest1 unit ? Do you have plans
to have multiple implementations ?

> +}
> +
> +static const TypeInfo pnv_nest1_chiplet_info = {
> +    .name          = TYPE_PNV_NEST1_CHIPLET,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvNest1Chiplet),
> +    .class_init    = pnv_nest1_chiplet_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_nest1_chiplet_register_types(void)
> +{
> +    type_register_static(&pnv_nest1_chiplet_info);
> +}
> +
> +type_init(pnv_nest1_chiplet_register_types);
> diff --git a/include/hw/ppc/pnv_nest_chiplet.h b/include/hw/ppc/pnv_nest_chiplet.h
> new file mode 100644
> index 0000000..12525d3
> --- /dev/null
> +++ b/include/hw/ppc/pnv_nest_chiplet.h
> @@ -0,0 +1,41 @@
> +/*
> + * QEMU PowerPC nest chiplet model
> + *
> + * Copyright (c) 2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef PPC_PNV_NEST1_CHIPLET_H
> +#define PPC_PNV_NEST1_CHIPLET_H
> +
> +#include "hw/ppc/pnv_pervasive.h"
> +
> +#define TYPE_PNV_NEST1_CHIPLET "pnv-nest1-chiplet"
> +typedef struct PnvNest1Class PnvNest1Class;
> +typedef struct PnvNest1Chiplet PnvNest1Chiplet;
> +DECLARE_OBJ_CHECKERS(PnvNest1Chiplet, PnvNest1Class,
> +                     PNV_NEST1CHIPLET, TYPE_PNV_NEST1_CHIPLET)
> +
> +typedef struct PnvNest1Chiplet {
> +    DeviceState parent;
> +
> +    struct PnvChip *chip;

Please remove chip, it is unused.

> +
> +    /* common pervasive chiplet unit */
> +    PnvPervChiplet perv_chiplet;
> +} PnvNest1Chiplet;
> +
> +struct PnvNest1Class {
> +    DeviceClass parent_class;
> +
> +    DeviceRealize parent_realize;
> +
> +    void (*nest1_dt_populate)(void *fdt);

This class needs some justification.


Thanks,

C.



> +};
> +
> +#endif /*PPC_PNV_NEST1_CHIPLET_H */

Re: [PATCH v4 3/3] hw/ppc: Nest1 chiplet wiring

[Cédric Le Goater]

On 11/7/23 08:41, Chalapathi V wrote:
> From: Chalapathi V <chalapathi.v@linux.ibm.com>
> 
> This part of the patchset connects the nest1 chiplet model to p10 chip.
> 
> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> ---
>   hw/ppc/pnv.c              | 14 ++++++++++++++
>   include/hw/ppc/pnv_chip.h |  2 ++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index c0e34ff..2b93cdd 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -351,6 +351,8 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>       static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
>       int i;
>   
> +    Pnv10Chip *chip10 = PNV10_CHIP(chip);
> +
>       pnv_dt_xscom(chip, fdt, 0,
>                    cpu_to_be64(PNV10_XSCOM_BASE(chip)),
>                    cpu_to_be64(PNV10_XSCOM_SIZE),
> @@ -366,6 +368,9 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>           pnv_dt_memory(fdt, chip->chip_id, chip->ram_start, chip->ram_size);
>       }
>   
> +    /* Populate nest1_chiplet device tree */
> +    PNV_NEST1CHIPLET_GET_CLASS(&chip10->nest1_chiplet)->nest1_dt_populate(fdt);
you could call directly pnv_nest1_dt_populate() IMO.


Have you tried to start a 2 sockets machine ?

   -machine powernv10 -smp 16,sockets=2,cores=2,threads=4


Thanks,

C.



> +
>       pnv_dt_lpc(chip, fdt, 0, PNV10_LPCM_BASE(chip), PNV10_LPCM_SIZE);
>   }
>   
> @@ -1649,6 +1654,8 @@ static void pnv_chip_power10_instance_init(Object *obj)
>       object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>       object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>       object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
> +    object_initialize_child(obj, "nest1_chiplet", &chip10->nest1_chiplet,
> +                            TYPE_PNV_NEST1_CHIPLET);
>   
>       chip->num_pecs = pcc->num_pecs;
>   
> @@ -1813,6 +1820,13 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
>                                   &chip10->homer.regs);
>   
> +    /* nest1 chiplet control regs */
> +    if (!qdev_realize(DEVICE(&chip10->nest1_chiplet), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_NEST1_CTRL_CHIPLET_BASE,
> +                   &chip10->nest1_chiplet.perv_chiplet.xscom_perv_ctrl_regs);
> +
>       /* PHBs */
>       pnv_chip_power10_phb_realize(chip, &local_err);
>       if (local_err) {
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 53e1d92..4bcb925 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -4,6 +4,7 @@
>   #include "hw/pci-host/pnv_phb4.h"
>   #include "hw/ppc/pnv_core.h"
>   #include "hw/ppc/pnv_homer.h"
> +#include "hw/ppc/pnv_nest_chiplet.h"
>   #include "hw/ppc/pnv_lpc.h"
>   #include "hw/ppc/pnv_occ.h"
>   #include "hw/ppc/pnv_psi.h"
> @@ -109,6 +110,7 @@ struct Pnv10Chip {
>       PnvOCC       occ;
>       PnvSBE       sbe;
>       PnvHomer     homer;
> +    PnvNest1Chiplet nest1_chiplet;
>   
>       uint32_t     nr_quads;
>       PnvQuad      *quads;