[Qemu-devel] [PATCH v3 0/2] ppc/pnv: new Pnv8Chip and Pnv9Chip models

[Qemu-devel] [PATCH v3 0/2] ppc/pnv: new Pnv8Chip and Pnv9Chip models

[Cédric Le Goater]

Hello,

Here comes new chip models for the different processor the PowerNV
machine supports and some more cleanups around the device tree of the
ISA bus of the machine.

Thanks,

C. 

Changes since v2:

  - introduced a parent_realize
  - removed the machines
  
Changes since v1:

 - reworked the ISABus creation interface with the chip, the machine
   is not aware anymore of the chip controllers.
 - removed the bizarre controllers under the PnvChip base class.
 - kept back some changes on the ISA device tree name. They will come
   in time with the LPC controller for P9

Cédric Le Goater (2):
  ppc/pnv: introduce Pnv8Chip and Pnv9Chip models
  ppc/pnv: consolidate the creation of the ISA bus device tree

 include/hw/ppc/pnv.h |  24 +++-
 hw/ppc/pnv.c         | 332 +++++++++++++++++++++++++++++++--------------------
 2 files changed, 225 insertions(+), 131 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH v3 1/2] ppc/pnv: introduce Pnv8Chip and Pnv9Chip models

[Cédric Le Goater]

It introduces a base PnvChip class from which the specific processor
chip classes, Pnv8Chip and Pnv9Chip, inherit. Each of them needs to
define an init and a realize routine which will create the controllers
of the target processor. For the moment, the base PnvChip class
handles the XSCOM bus and the cores.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h |  24 ++++-
 hw/ppc/pnv.c         | 281 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 202 insertions(+), 103 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 563279f3e00c..86d5f54e5459 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -57,12 +57,32 @@ typedef struct PnvChip {
     MemoryRegion xscom_mmio;
     MemoryRegion xscom;
     AddressSpace xscom_as;
+} PnvChip;
+
+#define TYPE_PNV8_CHIP "pnv8-chip"
+#define PNV8_CHIP(obj) OBJECT_CHECK(Pnv8Chip, (obj), TYPE_PNV8_CHIP)
+
+typedef struct Pnv8Chip {
+    /*< private >*/
+    PnvChip      parent_obj;
+
+    /*< public >*/
     MemoryRegion icp_mmio;
 
     PnvLpcController lpc;
     PnvPsi       psi;
     PnvOCC       occ;
-} PnvChip;
+} Pnv8Chip;
+
+#define TYPE_PNV9_CHIP "pnv9-chip"
+#define PNV9_CHIP(obj) OBJECT_CHECK(Pnv9Chip, (obj), TYPE_PNV9_CHIP)
+
+typedef struct Pnv9Chip {
+    /*< private >*/
+    PnvChip      parent_obj;
+
+    /*< public >*/
+} Pnv9Chip;
 
 typedef struct PnvChipClass {
     /*< private >*/
@@ -75,6 +95,8 @@ typedef struct PnvChipClass {
 
     hwaddr       xscom_base;
 
+    DeviceRealize parent_realize;
+
     uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
     Object *(*intc_create)(PnvChip *chip, Object *child, Error **errp);
     ISABus *(*isa_create)(PnvChip *chip, Error **errp);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index ac828d133173..a29ea996b45d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -531,12 +531,14 @@ static void pnv_reset(void)
 
 static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
 {
-    return pnv_lpc_isa_create(&chip->lpc, true, errp);
+    Pnv8Chip *chip8 = PNV8_CHIP(chip);
+    return pnv_lpc_isa_create(&chip8->lpc, true, errp);
 }
 
 static ISABus *pnv_chip_power8nvl_isa_create(PnvChip *chip, Error **errp)
 {
-    return pnv_lpc_isa_create(&chip->lpc, false, errp);
+    Pnv8Chip *chip8 = PNV8_CHIP(chip);
+    return pnv_lpc_isa_create(&chip8->lpc, false, errp);
 }
 
 static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp)
@@ -725,6 +727,103 @@ static Object *pnv_chip_power9_intc_create(PnvChip *chip, Object *child,
  */
 #define POWER9_CORE_MASK   (0xffffffffffffffull)
 
+static void pnv_chip_power8_instance_init(Object *obj)
+{
+    Pnv8Chip *chip8 = PNV8_CHIP(obj);
+
+    object_initialize(&chip8->psi, sizeof(chip8->psi), TYPE_PNV_PSI);
+    object_property_add_child(obj, "psi", OBJECT(&chip8->psi), NULL);
+    object_property_add_const_link(OBJECT(&chip8->psi), "xics",
+                                   OBJECT(qdev_get_machine()), &error_abort);
+
+    object_initialize(&chip8->lpc, sizeof(chip8->lpc), TYPE_PNV_LPC);
+    object_property_add_child(obj, "lpc", OBJECT(&chip8->lpc), NULL);
+    object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
+                                   OBJECT(&chip8->psi), &error_abort);
+
+    object_initialize(&chip8->occ, sizeof(chip8->occ), TYPE_PNV_OCC);
+    object_property_add_child(obj, "occ", OBJECT(&chip8->occ), NULL);
+    object_property_add_const_link(OBJECT(&chip8->occ), "psi",
+                                   OBJECT(&chip8->psi), &error_abort);
+}
+
+static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
+ {
+    PnvChip *chip = PNV_CHIP(chip8);
+    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+    const char *typename = pnv_chip_core_typename(chip);
+    size_t typesize = object_type_get_instance_size(typename);
+    int i, j;
+    char *name;
+    XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
+
+    name = g_strdup_printf("icp-%x", chip->chip_id);
+    memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
+    g_free(name);
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
+
+    /* Map the ICP registers for each thread */
+    for (i = 0; i < chip->nr_cores; i++) {
+        PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
+        int core_hwid = CPU_CORE(pnv_core)->core_id;
+
+        for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
+            uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
+            PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
+
+            memory_region_add_subregion(&chip8->icp_mmio, pir << 12,
+                                        &icp->mmio);
+        }
+    }
+}
+
+static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
+{
+    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
+    PnvChip *chip = PNV_CHIP(dev);
+    Pnv8Chip *chip8 = PNV8_CHIP(dev);
+    Error *local_err = NULL;
+
+    pcc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /* Processor Service Interface (PSI) Host Bridge */
+    object_property_set_int(OBJECT(&chip8->psi), PNV_PSIHB_BASE(chip),
+                            "bar", &error_fatal);
+    object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV_XSCOM_PSIHB_BASE, &chip8->psi.xscom_regs);
+
+    /* Create LPC controller */
+    object_property_set_bool(OBJECT(&chip8->lpc), true, "realized",
+                             &error_fatal);
+    pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip8->lpc.xscom_regs);
+
+    /* Interrupt Management Area. This is the memory region holding
+     * all the Interrupt Control Presenter (ICP) registers */
+    pnv_chip_icp_realize(chip8, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /* Create the simplified OCC model */
+    object_property_set_bool(OBJECT(&chip8->occ), true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
+}
+
 static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -738,6 +837,9 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
     k->isa_create = pnv_chip_power8_isa_create;
     k->xscom_base = 0x003fc0000000000ull;
     dc->desc = "PowerNV Chip POWER8E";
+
+    device_class_set_parent_realize(dc, pnv_chip_power8_realize,
+                                    &k->parent_realize);
 }
 
 static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
@@ -753,6 +855,9 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
     k->isa_create = pnv_chip_power8_isa_create;
     k->xscom_base = 0x003fc0000000000ull;
     dc->desc = "PowerNV Chip POWER8";
+
+    device_class_set_parent_realize(dc, pnv_chip_power8_realize,
+                                    &k->parent_realize);
 }
 
 static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
@@ -768,6 +873,25 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
     k->isa_create = pnv_chip_power8nvl_isa_create;
     k->xscom_base = 0x003fc0000000000ull;
     dc->desc = "PowerNV Chip POWER8NVL";
+
+    device_class_set_parent_realize(dc, pnv_chip_power8_realize,
+                                    &k->parent_realize);
+}
+
+static void pnv_chip_power9_instance_init(Object *obj)
+{
+}
+
+static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
+{
+    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    pcc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 }
 
 static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
@@ -783,6 +907,9 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
     k->isa_create = pnv_chip_power9_isa_create;
     k->xscom_base = 0x00603fc00000000ull;
     dc->desc = "PowerNV Chip POWER9";
+
+    device_class_set_parent_realize(dc, pnv_chip_power9_realize,
+                                    &k->parent_realize);
 }
 
 static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
@@ -815,59 +942,9 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
     }
 }
 
-static void pnv_chip_init(Object *obj)
+static void pnv_chip_instance_init(Object *obj)
 {
-    PnvChip *chip = PNV_CHIP(obj);
-    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
-
-    chip->xscom_base = pcc->xscom_base;
-
-    object_initialize(&chip->lpc, sizeof(chip->lpc), TYPE_PNV_LPC);
-    object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL);
-
-    object_initialize(&chip->psi, sizeof(chip->psi), TYPE_PNV_PSI);
-    object_property_add_child(obj, "psi", OBJECT(&chip->psi), NULL);
-    object_property_add_const_link(OBJECT(&chip->psi), "xics",
-                                   OBJECT(qdev_get_machine()), &error_abort);
-
-    object_initialize(&chip->occ, sizeof(chip->occ), TYPE_PNV_OCC);
-    object_property_add_child(obj, "occ", OBJECT(&chip->occ), NULL);
-    object_property_add_const_link(OBJECT(&chip->occ), "psi",
-                                   OBJECT(&chip->psi), &error_abort);
-
-    /* The LPC controller needs PSI to generate interrupts */
-    object_property_add_const_link(OBJECT(&chip->lpc), "psi",
-                                   OBJECT(&chip->psi), &error_abort);
-}
-
-static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
-{
-    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
-    const char *typename = pnv_chip_core_typename(chip);
-    size_t typesize = object_type_get_instance_size(typename);
-    int i, j;
-    char *name;
-    XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
-
-    name = g_strdup_printf("icp-%x", chip->chip_id);
-    memory_region_init(&chip->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
-    sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip->icp_mmio);
-    g_free(name);
-
-    sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
-
-    /* Map the ICP registers for each thread */
-    for (i = 0; i < chip->nr_cores; i++) {
-        PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
-        int core_hwid = CPU_CORE(pnv_core)->core_id;
-
-        for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
-            uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
-            PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
-
-            memory_region_add_subregion(&chip->icp_mmio, pir << 12, &icp->mmio);
-        }
-    }
+    PNV_CHIP(obj)->xscom_base = PNV_CHIP_GET_CLASS(obj)->xscom_base;
 }
 
 static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
@@ -951,37 +1028,6 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, error);
         return;
     }
-
-    /* Create LPC controller */
-    object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
-                             &error_fatal);
-    pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
-
-    /* Interrupt Management Area. This is the memory region holding
-     * all the Interrupt Control Presenter (ICP) registers */
-    pnv_chip_icp_realize(chip, &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-
-    /* Processor Service Interface (PSI) Host Bridge */
-    object_property_set_int(OBJECT(&chip->psi), PNV_PSIHB_BASE(chip),
-                            "bar", &error_fatal);
-    object_property_set_bool(OBJECT(&chip->psi), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-    pnv_xscom_add_subregion(chip, PNV_XSCOM_PSIHB_BASE, &chip->psi.xscom_regs);
-
-    /* Create the simplified OCC model */
-    object_property_set_bool(OBJECT(&chip->occ), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-    pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip->occ.xscom_regs);
 }
 
 static Property pnv_chip_properties[] = {
@@ -1009,8 +1055,10 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
     int i;
 
     for (i = 0; i < pnv->num_chips; i++) {
-        if (ics_valid_irq(&pnv->chips[i]->psi.ics, irq)) {
-            return &pnv->chips[i]->psi.ics;
+        Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
+
+        if (ics_valid_irq(&chip8->psi.ics, irq)) {
+            return &chip8->psi.ics;
         }
     }
     return NULL;
@@ -1022,7 +1070,8 @@ static void pnv_ics_resend(XICSFabric *xi)
     int i;
 
     for (i = 0; i < pnv->num_chips; i++) {
-        ics_resend(&pnv->chips[i]->psi.ics);
+        Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
+        ics_resend(&chip8->psi.ics);
     }
 }
 
@@ -1063,7 +1112,8 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
     }
 
     for (i = 0; i < pnv->num_chips; i++) {
-        ics_pic_print_info(&pnv->chips[i]->psi.ics, mon);
+        Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
+        ics_pic_print_info(&chip8->psi.ics, mon);
     }
 }
 
@@ -1098,7 +1148,7 @@ static void pnv_set_num_chips(Object *obj, Visitor *v, const char *name,
     pnv->num_chips = num_chips;
 }
 
-static void pnv_machine_initfn(Object *obj)
+static void pnv_machine_instance_init(Object *obj)
 {
     PnvMachineState *pnv = PNV_MACHINE(obj);
     pnv->num_chips = 1;
@@ -1138,11 +1188,18 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
     pnv_machine_class_props_init(oc);
 }
 
-#define DEFINE_PNV_CHIP_TYPE(type, class_initfn) \
-    {                                            \
-        .name          = type,                   \
-        .class_init    = class_initfn,           \
-        .parent        = TYPE_PNV_CHIP,          \
+#define DEFINE_PNV8_CHIP_TYPE(type, class_initfn) \
+    {                                             \
+        .name          = type,                    \
+        .class_init    = class_initfn,            \
+        .parent        = TYPE_PNV8_CHIP,          \
+    }
+
+#define DEFINE_PNV9_CHIP_TYPE(type, class_initfn) \
+    {                                             \
+        .name          = type,                    \
+        .class_init    = class_initfn,            \
+        .parent        = TYPE_PNV9_CHIP,          \
     }
 
 static const TypeInfo types[] = {
@@ -1150,7 +1207,7 @@ static const TypeInfo types[] = {
         .name          = TYPE_PNV_MACHINE,
         .parent        = TYPE_MACHINE,
         .instance_size = sizeof(PnvMachineState),
-        .instance_init = pnv_machine_initfn,
+        .instance_init = pnv_machine_instance_init,
         .class_init    = pnv_machine_class_init,
         .interfaces = (InterfaceInfo[]) {
             { TYPE_XICS_FABRIC },
@@ -1162,16 +1219,36 @@ static const TypeInfo types[] = {
         .name          = TYPE_PNV_CHIP,
         .parent        = TYPE_SYS_BUS_DEVICE,
         .class_init    = pnv_chip_class_init,
-        .instance_init = pnv_chip_init,
+        .instance_init = pnv_chip_instance_init,
         .instance_size = sizeof(PnvChip),
         .class_size    = sizeof(PnvChipClass),
         .abstract      = true,
     },
-    DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER9, pnv_chip_power9_class_init),
-    DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
-    DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
-    DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
-                         pnv_chip_power8nvl_class_init),
+
+    /*
+     * P9 chip and variants
+     */
+    {
+        .name          = TYPE_PNV9_CHIP,
+        .parent        = TYPE_PNV_CHIP,
+        .instance_init = pnv_chip_power9_instance_init,
+        .instance_size = sizeof(Pnv9Chip),
+    },
+    DEFINE_PNV9_CHIP_TYPE(TYPE_PNV_CHIP_POWER9, pnv_chip_power9_class_init),
+
+    /*
+     * P8 chip and variants
+     */
+    {
+        .name          = TYPE_PNV8_CHIP,
+        .parent        = TYPE_PNV_CHIP,
+        .instance_init = pnv_chip_power8_instance_init,
+        .instance_size = sizeof(Pnv8Chip),
+    },
+    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
+    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
+    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
+                          pnv_chip_power8nvl_class_init),
 };
 
 DEFINE_TYPES(types)
-- 
2.13.6

[Qemu-devel] [PATCH v3 2/2] ppc/pnv: consolidate the creation of the ISA bus device tree

[Cédric Le Goater]

The device tree node of the ISA bus was being partially done in
different places. Move all the nodes creation under the same routine.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index a29ea996b45d..7401ffe5b01c 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -265,18 +265,6 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
     g_free(reg);
 }
 
-static int pnv_chip_lpc_offset(PnvChip *chip, void *fdt)
-{
-    char *name;
-    int offset;
-
-    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
-                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
-    offset = fdt_path_offset(fdt, name);
-    g_free(name);
-    return offset;
-}
-
 static void pnv_dt_chip(PnvChip *chip, void *fdt)
 {
     const char *typename = pnv_chip_core_typename(chip);
@@ -285,16 +273,6 @@ static void pnv_dt_chip(PnvChip *chip, void *fdt)
 
     pnv_dt_xscom(chip, fdt, 0);
 
-    /* The default LPC bus of a multichip system is on chip 0. It's
-     * recognized by the firmware (skiboot) using a "primary"
-     * property.
-     */
-    if (chip->chip_id == 0x0) {
-        int lpc_offset = pnv_chip_lpc_offset(chip, fdt);
-
-        _FDT((fdt_setprop(fdt, lpc_offset, "primary", NULL, 0)));
-    }
-
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
 
@@ -418,16 +396,35 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
     return 0;
 }
 
-static void pnv_dt_isa(ISABus *bus, void *fdt, int lpc_offset)
+static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
+{
+    char *name;
+    int offset;
+
+    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
+                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
+    offset = fdt_path_offset(fdt, name);
+    g_free(name);
+    return offset;
+}
+
+/* The default LPC bus of a multichip system is on chip 0. It's
+ * recognized by the firmware (skiboot) using a "primary" property.
+ */
+static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
 {
+    int isa_offset = pnv_chip_isa_offset(pnv->chips[0], fdt);
     ForeachPopulateArgs args = {
         .fdt = fdt,
-        .offset = lpc_offset,
+        .offset = isa_offset,
     };
 
+    _FDT((fdt_setprop(fdt, isa_offset, "primary", NULL, 0)));
+
     /* ISA devices are not necessarily parented to the ISA bus so we
      * can not use object_child_foreach() */
-    qbus_walk_children(BUS(bus), pnv_dt_isa_device, NULL, NULL, NULL, &args);
+    qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
+                       &args);
 }
 
 static void *pnv_dt_create(MachineState *machine)
@@ -438,7 +435,6 @@ static void *pnv_dt_create(MachineState *machine)
     char *buf;
     int off;
     int i;
-    int lpc_offset;
 
     fdt = g_malloc0(FDT_MAX_SIZE);
     _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
@@ -480,8 +476,7 @@ static void *pnv_dt_create(MachineState *machine)
     }
 
     /* Populate ISA devices on chip 0 */
-    lpc_offset = pnv_chip_lpc_offset(pnv->chips[0], fdt);
-    pnv_dt_isa(pnv->isa_bus, fdt, lpc_offset);
+    pnv_dt_isa(pnv, fdt);
 
     if (pnv->bmc) {
         pnv_dt_bmc_sensors(pnv->bmc, fdt);
-- 
2.13.6

Re: [Qemu-devel] [PATCH v3 0/2] ppc/pnv: new Pnv8Chip and Pnv9Chip models

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On Mon, Jun 18, 2018 at 07:05:38PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here comes new chip models for the different processor the PowerNV
> machine supports and some more cleanups around the device tree of the
> ISA bus of the machine.

Applied to ppc-for-3.0, thanks.

> 
> Thanks,
> 
> C. 
> 
> Changes since v2:
> 
>   - introduced a parent_realize
>   - removed the machines
>   
> Changes since v1:
> 
>  - reworked the ISABus creation interface with the chip, the machine
>    is not aware anymore of the chip controllers.
>  - removed the bizarre controllers under the PnvChip base class.
>  - kept back some changes on the ISA device tree name. They will come
>    in time with the LPC controller for P9
> 
> Cédric Le Goater (2):
>   ppc/pnv: introduce Pnv8Chip and Pnv9Chip models
>   ppc/pnv: consolidate the creation of the ISA bus device tree
> 
>  include/hw/ppc/pnv.h |  24 +++-
>  hw/ppc/pnv.c         | 332 +++++++++++++++++++++++++++++++--------------------
>  2 files changed, 225 insertions(+), 131 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v3 2/2] ppc/pnv: consolidate the creation of the ISA bus device tree

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4055 bytes --]

On Mon, Jun 18, 2018 at 07:05:40PM +0200, Cédric Le Goater wrote:
> The device tree node of the ISA bus was being partially done in
> different places. Move all the nodes creation under the same routine.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c | 51 +++++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index a29ea996b45d..7401ffe5b01c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -265,18 +265,6 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
>      g_free(reg);
>  }
>  
> -static int pnv_chip_lpc_offset(PnvChip *chip, void *fdt)
> -{
> -    char *name;
> -    int offset;
> -
> -    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
> -                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
> -    offset = fdt_path_offset(fdt, name);
> -    g_free(name);
> -    return offset;
> -}
> -
>  static void pnv_dt_chip(PnvChip *chip, void *fdt)
>  {
>      const char *typename = pnv_chip_core_typename(chip);
> @@ -285,16 +273,6 @@ static void pnv_dt_chip(PnvChip *chip, void *fdt)
>  
>      pnv_dt_xscom(chip, fdt, 0);
>  
> -    /* The default LPC bus of a multichip system is on chip 0. It's
> -     * recognized by the firmware (skiboot) using a "primary"
> -     * property.
> -     */
> -    if (chip->chip_id == 0x0) {
> -        int lpc_offset = pnv_chip_lpc_offset(chip, fdt);
> -
> -        _FDT((fdt_setprop(fdt, lpc_offset, "primary", NULL, 0)));
> -    }
> -
>      for (i = 0; i < chip->nr_cores; i++) {
>          PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
>  
> @@ -418,16 +396,35 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
>      return 0;
>  }
>  
> -static void pnv_dt_isa(ISABus *bus, void *fdt, int lpc_offset)
> +static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)

AFAICT, this only has one caller, so you could probably fold it in
there.  Not a big deal, though.

> +{
> +    char *name;
> +    int offset;
> +
> +    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
> +                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
> +    offset = fdt_path_offset(fdt, name);
> +    g_free(name);
> +    return offset;
> +}
> +
> +/* The default LPC bus of a multichip system is on chip 0. It's
> + * recognized by the firmware (skiboot) using a "primary" property.
> + */
> +static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
>  {
> +    int isa_offset = pnv_chip_isa_offset(pnv->chips[0], fdt);
>      ForeachPopulateArgs args = {
>          .fdt = fdt,
> -        .offset = lpc_offset,
> +        .offset = isa_offset,
>      };
>  
> +    _FDT((fdt_setprop(fdt, isa_offset, "primary", NULL, 0)));
> +
>      /* ISA devices are not necessarily parented to the ISA bus so we
>       * can not use object_child_foreach() */
> -    qbus_walk_children(BUS(bus), pnv_dt_isa_device, NULL, NULL, NULL, &args);
> +    qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
> +                       &args);
>  }
>  
>  static void *pnv_dt_create(MachineState *machine)
> @@ -438,7 +435,6 @@ static void *pnv_dt_create(MachineState *machine)
>      char *buf;
>      int off;
>      int i;
> -    int lpc_offset;
>  
>      fdt = g_malloc0(FDT_MAX_SIZE);
>      _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
> @@ -480,8 +476,7 @@ static void *pnv_dt_create(MachineState *machine)
>      }
>  
>      /* Populate ISA devices on chip 0 */
> -    lpc_offset = pnv_chip_lpc_offset(pnv->chips[0], fdt);
> -    pnv_dt_isa(pnv->isa_bus, fdt, lpc_offset);
> +    pnv_dt_isa(pnv, fdt);
>  
>      if (pnv->bmc) {
>          pnv_dt_bmc_sensors(pnv->bmc, fdt);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v3 2/2] ppc/pnv: consolidate the creation of the ISA bus device tree

[Cédric Le Goater]

On 06/19/2018 02:28 AM, David Gibson wrote:
> On Mon, Jun 18, 2018 at 07:05:40PM +0200, Cédric Le Goater wrote:
>> The device tree node of the ISA bus was being partially done in
>> different places. Move all the nodes creation under the same routine.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ppc/pnv.c | 51 +++++++++++++++++++++++----------------------------
>>  1 file changed, 23 insertions(+), 28 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index a29ea996b45d..7401ffe5b01c 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -265,18 +265,6 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
>>      g_free(reg);
>>  }
>>  
>> -static int pnv_chip_lpc_offset(PnvChip *chip, void *fdt)
>> -{
>> -    char *name;
>> -    int offset;
>> -
>> -    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
>> -                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
>> -    offset = fdt_path_offset(fdt, name);
>> -    g_free(name);
>> -    return offset;
>> -}
>> -
>>  static void pnv_dt_chip(PnvChip *chip, void *fdt)
>>  {
>>      const char *typename = pnv_chip_core_typename(chip);
>> @@ -285,16 +273,6 @@ static void pnv_dt_chip(PnvChip *chip, void *fdt)
>>  
>>      pnv_dt_xscom(chip, fdt, 0);
>>  
>> -    /* The default LPC bus of a multichip system is on chip 0. It's
>> -     * recognized by the firmware (skiboot) using a "primary"
>> -     * property.
>> -     */
>> -    if (chip->chip_id == 0x0) {
>> -        int lpc_offset = pnv_chip_lpc_offset(chip, fdt);
>> -
>> -        _FDT((fdt_setprop(fdt, lpc_offset, "primary", NULL, 0)));
>> -    }
>> -
>>      for (i = 0; i < chip->nr_cores; i++) {
>>          PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
>>  
>> @@ -418,16 +396,35 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
>>      return 0;
>>  }
>>  
>> -static void pnv_dt_isa(ISABus *bus, void *fdt, int lpc_offset)
>> +static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
> 
> AFAICT, this only has one caller, so you could probably fold it in
> there.  Not a big deal, though.

yes. Next round I think. With P9, it becomes :

static int pnv_chip_isa_offset(PnvChip *chip, void *fdt)
{
    char *name;
    int offset;

    if (pnv_chip_is_power9(chip)) {
        name = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
                               (uint64_t) PNV9_LPCM_BASE(chip));
    } else {
        name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
                               (uint64_t) PNV_XSCOM_BASE(chip),
                               PNV_XSCOM_LPC_BASE);
    }
    offset = fdt_path_offset(fdt, name);
    g_free(name);
    return offset;
}

I need to simplify that. Probably with the patch I already sent
moving the name under the LPC model.

Thanks,

C.

> 
>> +{
>> +    char *name;
>> +    int offset;
>> +
>> +    name = g_strdup_printf("/xscom@%" PRIx64 "/isa@%x",
>> +                           (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE);
>> +    offset = fdt_path_offset(fdt, name);
>> +    g_free(name);
>> +    return offset;
>> +}
>> +
>> +/* The default LPC bus of a multichip system is on chip 0. It's
>> + * recognized by the firmware (skiboot) using a "primary" property.
>> + */
>> +static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
>>  {
>> +    int isa_offset = pnv_chip_isa_offset(pnv->chips[0], fdt);
>>      ForeachPopulateArgs args = {
>>          .fdt = fdt,
>> -        .offset = lpc_offset,
>> +        .offset = isa_offset,
>>      };
>>  
>> +    _FDT((fdt_setprop(fdt, isa_offset, "primary", NULL, 0)));
>> +
>>      /* ISA devices are not necessarily parented to the ISA bus so we
>>       * can not use object_child_foreach() */
>> -    qbus_walk_children(BUS(bus), pnv_dt_isa_device, NULL, NULL, NULL, &args);
>> +    qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
>> +                       &args);
>>  }
>>  
>>  static void *pnv_dt_create(MachineState *machine)
>> @@ -438,7 +435,6 @@ static void *pnv_dt_create(MachineState *machine)
>>      char *buf;
>>      int off;
>>      int i;
>> -    int lpc_offset;
>>  
>>      fdt = g_malloc0(FDT_MAX_SIZE);
>>      _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
>> @@ -480,8 +476,7 @@ static void *pnv_dt_create(MachineState *machine)
>>      }
>>  
>>      /* Populate ISA devices on chip 0 */
>> -    lpc_offset = pnv_chip_lpc_offset(pnv->chips[0], fdt);
>> -    pnv_dt_isa(pnv->isa_bus, fdt, lpc_offset);
>> +    pnv_dt_isa(pnv, fdt);
>>  
>>      if (pnv->bmc) {
>>          pnv_dt_bmc_sensors(pnv->bmc, fdt);
>