qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory()
@ 2019-12-19 18:11 Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine Cédric Le Goater
                   ` (13 more replies)
  0 siblings, 14 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

Hello,

The PowerNV and sPAPR machine use qdev_get_machine() and
get_system_memory() in some places. This is not a good modeling
pratice and it should be avoided. This series replaces the uses of
these routines with a set of QOM properties and aliases.

Thanks,

C.

Changes since v1:

 - fixed a missing assert(chip->system_memory)
 - introduced a XivePresenter link under XiveTCTX
 
Cédric Le Goater (5):
  ppc/pnv: Modify the powerdown notifier to get the PowerNV machine
  ppc/pnv: Introduce a "system-memory" property
  ppc/pnv: Introduce a "xics" property alias under the PSI model
  ppc/pnv: Introduce a "xics" property under the POWER8 chip
  xive: Add a "presenter" link property to the TCTX object

Greg Kurz (8):
  spapr/xive: Use device_class_set_parent_realize()
  pnv/xive: Use device_class_set_parent_realize()
  spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router
  xive: Use the XIVE fabric link under the XIVE router
  ppc/pnv: Add an "nr-threads" property to the base chip class
  ppc/pnv: Add a "pnor" const link property to the BMC internal
    simulator
  spapr/xive: Deduce the SpaprXive pointer from XiveTCTX::xptr
  pnv/xive: Deduce the PnvXive pointer from XiveTCTX::xptr

 include/hw/ppc/pnv.h        |  9 ++--
 include/hw/ppc/pnv_psi.h    |  1 +
 include/hw/ppc/pnv_xive.h   | 12 +++++
 include/hw/ppc/spapr_xive.h | 10 ++++
 include/hw/ppc/xive.h       | 13 +++--
 hw/intc/pnv_xive.c          | 23 ++++++---
 hw/intc/spapr_xive.c        | 14 +++++-
 hw/intc/spapr_xive_kvm.c    |  9 ++--
 hw/intc/xive.c              | 28 +++++++++--
 hw/ppc/pnv.c                | 94 ++++++++++++++++++++++++-------------
 hw/ppc/pnv_bmc.c            |  8 ++--
 hw/ppc/pnv_psi.c            | 22 ++++-----
 hw/ppc/spapr_irq.c          |  2 +
 13 files changed, 172 insertions(+), 73 deletions(-)

-- 
2.21.0



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

* [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-20  0:26   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property Cédric Le Goater
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

Use container_of() instead of qdev_get_machine()

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index f77e7ca84ede..855254f28263 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -561,7 +561,7 @@ static void *pnv_dt_create(MachineState *machine)
 
 static void pnv_powerdown_notify(Notifier *n, void *opaque)
 {
-    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    PnvMachineState *pnv = container_of(n, PnvMachineState, powerdown_notifier);
 
     if (pnv->bmc) {
         pnv_bmc_powerdown(pnv->bmc);
-- 
2.21.0



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

* [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-19 18:56   ` Greg Kurz
  2019-12-20  0:31   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model Cédric Le Goater
                   ` (11 subsequent siblings)
  13 siblings, 2 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

and use a link to pass the system memory to the device models that
require it to map/unmap BARs. This replace the use of get_system_memory()

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h      |  2 ++
 include/hw/ppc/pnv_psi.h  |  1 +
 include/hw/ppc/pnv_xive.h |  2 ++
 hw/intc/pnv_xive.c        |  5 ++++-
 hw/ppc/pnv.c              | 33 ++++++++++++++++++++++++++-------
 hw/ppc/pnv_psi.c          | 13 ++++++++++---
 6 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index f78fd0dd967c..f31180618672 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -56,6 +56,8 @@ typedef struct PnvChip {
     AddressSpace xscom_as;
 
     gchar        *dt_isa_nodename;
+
+    MemoryRegion *system_memory;
 } PnvChip;
 
 #define TYPE_PNV8_CHIP "pnv8-chip"
diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
index f0f5b5519767..f85babaff0be 100644
--- a/include/hw/ppc/pnv_psi.h
+++ b/include/hw/ppc/pnv_psi.h
@@ -35,6 +35,7 @@ typedef struct PnvPsi {
 
     MemoryRegion regs_mr;
     uint64_t bar;
+    MemoryRegion *system_memory;
 
     /* FSP region not supported */
     /* MemoryRegion fsp_mr; */
diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
index f4c7caad40ee..4d641db691c8 100644
--- a/include/hw/ppc/pnv_xive.h
+++ b/include/hw/ppc/pnv_xive.h
@@ -30,6 +30,8 @@ typedef struct PnvXive {
     /* Owning chip */
     struct PnvChip *chip;
 
+    MemoryRegion *system_memory;
+
     /* XSCOM addresses giving access to the controller registers */
     MemoryRegion  xscom_regs;
 
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index a0a69b98a713..66970a60733b 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -853,7 +853,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
                                   uint64_t val, unsigned size)
 {
     PnvXive *xive = PNV_XIVE(opaque);
-    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *sysmem = xive->system_memory;
     uint32_t reg = offset >> 3;
     bool is_chip0 = xive->chip->chip_id == 0;
 
@@ -1821,6 +1821,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
 
     assert(xive->chip);
+    assert(xive->system_memory);
 
     /*
      * The XiveSource and XiveENDSource objects are realized with the
@@ -1937,6 +1938,8 @@ static Property pnv_xive_properties[] = {
     DEFINE_PROP_UINT64("tm-bar", PnvXive, tm_base, 0),
     /* The PnvChip id identifies the XIVE interrupt controller. */
     DEFINE_PROP_LINK("chip", PnvXive, chip, TYPE_PNV_CHIP, PnvChip *),
+    DEFINE_PROP_LINK("system-memory", PnvXive, system_memory,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 855254f28263..1d8bfb164a32 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -674,6 +674,7 @@ static void pnv_chip_power10_pic_print_info(PnvChip *chip, Monitor *mon)
 
 static void pnv_init(MachineState *machine)
 {
+    MemoryRegion *sysmem = get_system_memory();
     PnvMachineState *pnv = PNV_MACHINE(machine);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     MemoryRegion *ram;
@@ -692,7 +693,7 @@ static void pnv_init(MachineState *machine)
     ram = g_new(MemoryRegion, 1);
     memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
                                          machine->ram_size);
-    memory_region_add_subregion(get_system_memory(), 0, ram);
+    memory_region_add_subregion(sysmem, 0, ram);
 
     /*
      * Create our simple PNOR device
@@ -790,6 +791,12 @@ static void pnv_init(MachineState *machine)
                                 &error_fatal);
         object_property_set_int(chip, machine->smp.cores,
                                 "nr-cores", &error_fatal);
+        /*
+         * TODO: Only the MMIO range should be of interest for the
+         * controllers
+         */
+        object_property_set_link(chip, OBJECT(sysmem), "system-memory",
+                                 &error_abort);
         object_property_set_bool(chip, true, "realized", &error_fatal);
     }
     g_free(chip_typename);
@@ -1060,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     /* Processor Service Interface (PSI) Host Bridge */
     object_property_set_int(OBJECT(&chip8->psi), PNV_PSIHB_BASE(chip),
                             "bar", &error_fatal);
+    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
+                             "system-memory", &error_abort);
     object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1100,7 +1109,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
 
     /* OCC SRAM model */
-    memory_region_add_subregion(get_system_memory(), PNV_OCC_SENSOR_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV_OCC_SENSOR_BASE(chip),
                                 &chip8->occ.sram_regs);
 
     /* HOMER */
@@ -1116,7 +1125,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     pnv_xscom_add_subregion(chip, PNV_XSCOM_PBA_BASE, &chip8->homer.pba_regs);
 
     /* Homer mmio region */
-    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV_HOMER_BASE(chip),
                                 &chip8->homer.regs);
 }
 
@@ -1280,6 +1289,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
                             "tm-bar", &error_fatal);
     object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip), "chip",
                              &error_abort);
+    object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip->system_memory),
+                             "system-memory", &error_abort);
     object_property_set_bool(OBJECT(&chip9->xive), true, "realized",
                              &local_err);
     if (local_err) {
@@ -1292,6 +1303,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     /* Processor Service Interface (PSI) Host Bridge */
     object_property_set_int(OBJECT(&chip9->psi), PNV9_PSIHB_BASE(chip),
                             "bar", &error_fatal);
+    object_property_set_link(OBJECT(&chip9->psi), OBJECT(chip->system_memory),
+                             "system-memory", &error_abort);
     object_property_set_bool(OBJECT(&chip9->psi), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1308,7 +1321,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, local_err);
         return;
     }
-    memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV9_LPCM_BASE(chip),
                                 &chip9->lpc.xscom_regs);
 
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
@@ -1325,7 +1338,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs);
 
     /* OCC SRAM model */
-    memory_region_add_subregion(get_system_memory(), PNV9_OCC_SENSOR_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV9_OCC_SENSOR_BASE(chip),
                                 &chip9->occ.sram_regs);
 
     /* HOMER */
@@ -1341,7 +1354,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     pnv_xscom_add_subregion(chip, PNV9_XSCOM_PBA_BASE, &chip9->homer.pba_regs);
 
     /* Homer mmio region */
-    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV9_HOMER_BASE(chip),
                                 &chip9->homer.regs);
 }
 
@@ -1408,6 +1421,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     /* Processor Service Interface (PSI) Host Bridge */
     object_property_set_int(OBJECT(&chip10->psi), PNV10_PSIHB_BASE(chip),
                             "bar", &error_fatal);
+    object_property_set_link(OBJECT(&chip10->psi), OBJECT(chip->system_memory),
+                             "system-memory", &error_abort);
     object_property_set_bool(OBJECT(&chip10->psi), true, "realized",
                              &local_err);
     if (local_err) {
@@ -1426,7 +1441,7 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, local_err);
         return;
     }
-    memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
+    memory_region_add_subregion(chip->system_memory, PNV10_LPCM_BASE(chip),
                                 &chip10->lpc.xscom_regs);
 
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
@@ -1556,6 +1571,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
     PnvChip *chip = PNV_CHIP(dev);
     Error *error = NULL;
 
+    assert(chip->system_memory);
+
     /* Cores */
     pnv_chip_core_realize(chip, &error);
     if (error) {
@@ -1570,6 +1587,8 @@ static Property pnv_chip_properties[] = {
     DEFINE_PROP_UINT64("ram-size", PnvChip, ram_size, 0),
     DEFINE_PROP_UINT32("nr-cores", PnvChip, nr_cores, 1),
     DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
+    DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 75e20d9da08b..28d34e5c193a 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -126,7 +126,7 @@
 static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar)
 {
     PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
-    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *sysmem = psi->system_memory;
     uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
 
     psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
@@ -489,6 +489,8 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
     Error *err = NULL;
     unsigned int i;
 
+    assert(psi->system_memory);
+
     obj = object_property_get_link(OBJECT(dev), "xics", &err);
     if (!obj) {
         error_setg(errp, "%s: required link 'xics' not found: %s",
@@ -562,6 +564,8 @@ static int pnv_psi_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
 static Property pnv_psi_properties[] = {
     DEFINE_PROP_UINT64("bar", PnvPsi, bar, 0),
     DEFINE_PROP_UINT64("fsp-bar", PnvPsi, fsp_bar, 0),
+    DEFINE_PROP_LINK("system-memory", PnvPsi, system_memory,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -701,7 +705,7 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
     PnvPsi *psi = PNV_PSI(opaque);
     Pnv9Psi *psi9 = PNV9_PSI(psi);
     uint32_t reg = PSIHB_REG(addr);
-    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *sysmem = psi->system_memory;
 
     switch (addr) {
     case PSIHB9_CR:
@@ -819,11 +823,12 @@ static void pnv_psi_power9_irq_set(PnvPsi *psi, int irq, bool state)
 static void pnv_psi_power9_reset(void *dev)
 {
     Pnv9Psi *psi = PNV9_PSI(dev);
+    MemoryRegion *sysmem = PNV_PSI(psi)->system_memory;
 
     pnv_psi_reset(dev);
 
     if (memory_region_is_mapped(&psi->source.esb_mmio)) {
-        memory_region_del_subregion(get_system_memory(), &psi->source.esb_mmio);
+        memory_region_del_subregion(sysmem, &psi->source.esb_mmio);
     }
 }
 
@@ -842,6 +847,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
     int i;
 
+    assert(psi->system_memory);
+
     /* This is the only device with 4k ESB pages */
     object_property_set_int(OBJECT(xsrc), XIVE_ESB_4K, "shift",
                             &error_fatal);
-- 
2.21.0



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

* [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-22  9:33   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip Cédric Le Goater
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

This removes the need of the intermediate link under PSI to pass the
XICS link to the underlying ICSState object.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c     |  4 ++--
 hw/ppc/pnv_psi.c | 11 ++---------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1d8bfb164a32..163a658806e2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -999,8 +999,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
                             TYPE_PNV8_PSI, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip8->psi), "xics",
-                                   OBJECT(qdev_get_machine()), &error_abort);
 
     object_initialize_child(obj, "lpc",  &chip8->lpc, sizeof(chip8->lpc),
                             TYPE_PNV8_LPC, &error_abort, NULL);
@@ -1069,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
                             "bar", &error_fatal);
     object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
                              "system-memory", &error_abort);
+    object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
+                             ICS_PROP_XICS, &error_abort);
     object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 28d34e5c193a..d3124f673571 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -470,6 +470,8 @@ static void pnv_psi_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "ics-psi",  &psi8->ics, sizeof(psi8->ics),
                             TYPE_ICS, &error_abort, NULL);
+    object_property_add_alias(obj, ICS_PROP_XICS, OBJECT(&psi8->ics),
+                              ICS_PROP_XICS, &error_abort);
 }
 
 static const uint8_t irq_to_xivr[] = {
@@ -485,21 +487,12 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
 {
     PnvPsi *psi = PNV_PSI(dev);
     ICSState *ics = &PNV8_PSI(psi)->ics;
-    Object *obj;
     Error *err = NULL;
     unsigned int i;
 
     assert(psi->system_memory);
 
-    obj = object_property_get_link(OBJECT(dev), "xics", &err);
-    if (!obj) {
-        error_setg(errp, "%s: required link 'xics' not found: %s",
-                   __func__, error_get_pretty(err));
-        return;
-    }
-
     /* Create PSI interrupt control source */
-    object_property_set_link(OBJECT(ics), obj, ICS_PROP_XICS, &error_abort);
     object_property_set_int(OBJECT(ics), PSI_NUM_INTERRUPTS, "nr-irqs", &err);
     if (err) {
         error_propagate(errp, err);
-- 
2.21.0



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

* [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (2 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  4:17   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize() Cédric Le Goater
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

POWER8 is the only chip using the XICS interface. Add a "xics" link
and a XICSFabric attribute under this chip to remove the use of
qdev_get_machine()

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h |  2 ++
 hw/ppc/pnv.c         | 26 ++++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index f31180618672..8b957dfb5736 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -74,6 +74,8 @@ typedef struct Pnv8Chip {
     Pnv8Psi      psi;
     PnvOCC       occ;
     PnvHomer     homer;
+
+    XICSFabric    *xics;
 } Pnv8Chip;
 
 #define TYPE_PNV9_CHIP "pnv9-chip"
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 163a658806e2..2a1b15a69aed 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -797,6 +797,13 @@ static void pnv_init(MachineState *machine)
          */
         object_property_set_link(chip, OBJECT(sysmem), "system-memory",
                                  &error_abort);
+        /*
+         * The POWER8 machine use the XICS interrupt interface.
+         * Propagate the XICS fabric to the chip and its controllers.
+         */
+        if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
+            object_property_set_link(chip, OBJECT(pnv), "xics", &error_abort);
+        }
         object_property_set_bool(chip, true, "realized", &error_fatal);
     }
     g_free(chip_typename);
@@ -838,12 +845,12 @@ static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, uint32_t core_id)
 static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu,
                                         Error **errp)
 {
+    Pnv8Chip *chip8 = PNV8_CHIP(chip);
     Error *local_err = NULL;
     Object *obj;
     PnvCPUState *pnv_cpu = pnv_cpu_state(cpu);
 
-    obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, XICS_FABRIC(qdev_get_machine()),
-                     &local_err);
+    obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, chip8->xics, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -997,6 +1004,12 @@ static void pnv_chip_power8_instance_init(Object *obj)
 {
     Pnv8Chip *chip8 = PNV8_CHIP(obj);
 
+    object_property_add_link(obj, "xics", TYPE_XICS_FABRIC,
+                             (Object **)&chip8->xics,
+                             object_property_allow_set_link,
+                             OBJ_PROP_LINK_STRONG,
+                             &error_abort);
+
     object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
                             TYPE_PNV8_PSI, &error_abort, NULL);
 
@@ -1016,7 +1029,6 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
     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);
@@ -1032,7 +1044,7 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
 
         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));
+            PnvICPState *icp = PNV_ICP(xics_icp_get(chip8->xics, pir));
 
             memory_region_add_subregion(&chip8->icp_mmio, pir << 12,
                                         &icp->mmio);
@@ -1048,6 +1060,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     Pnv8Psi *psi8 = &chip8->psi;
     Error *local_err = NULL;
 
+    assert(chip8->xics);
+
     /* XSCOM bridge is first */
     pnv_xscom_realize(chip, PNV_XSCOM_SIZE, &local_err);
     if (local_err) {
@@ -1067,8 +1081,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
                             "bar", &error_fatal);
     object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
                              "system-memory", &error_abort);
-    object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
-                             ICS_PROP_XICS, &error_abort);
+    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip8->xics),
+                              ICS_PROP_XICS, &error_abort);
     object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-- 
2.21.0



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

* [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize()
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (3 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  4:27   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 06/13] pnv/xive: " Cédric Le Goater
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

The XIVE router base class currently inherits an empty realize hook
from the sysbus device base class, but it will soon implement one
of its own to perform some sanity checks. Do the preliminary plumbing
to have it called.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/spapr_xive.h | 10 ++++++++++
 hw/intc/spapr_xive.c        | 12 +++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 3a103c224d44..93d09d68deb7 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -15,6 +15,10 @@
 
 #define TYPE_SPAPR_XIVE "spapr-xive"
 #define SPAPR_XIVE(obj) OBJECT_CHECK(SpaprXive, (obj), TYPE_SPAPR_XIVE)
+#define SPAPR_XIVE_CLASS(klass)                                         \
+    OBJECT_CLASS_CHECK(SpaprXiveClass, (klass), TYPE_SPAPR_XIVE)
+#define SPAPR_XIVE_GET_CLASS(obj)                               \
+    OBJECT_GET_CLASS(SpaprXiveClass, (obj), TYPE_SPAPR_XIVE)
 
 typedef struct SpaprXive {
     XiveRouter    parent;
@@ -47,6 +51,12 @@ typedef struct SpaprXive {
     VMChangeStateEntry *change;
 } SpaprXive;
 
+typedef struct SpaprXiveClass {
+    XiveRouterClass parent;
+
+    DeviceRealize parent_realize;
+} SpaprXiveClass;
+
 /*
  * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
  * to the controller block id value. It can nevertheless be changed
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 57305c56d707..32322470a8b8 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -286,10 +286,17 @@ static void spapr_xive_instance_init(Object *obj)
 static void spapr_xive_realize(DeviceState *dev, Error **errp)
 {
     SpaprXive *xive = SPAPR_XIVE(dev);
+    SpaprXiveClass *sxc = SPAPR_XIVE_GET_CLASS(xive);
     XiveSource *xsrc = &xive->source;
     XiveENDSource *end_xsrc = &xive->end_source;
     Error *local_err = NULL;
 
+    sxc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     if (!xive->nr_irqs) {
         error_setg(errp, "Number of interrupt needs to be greater 0");
         return;
@@ -760,10 +767,12 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
     XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
     SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass);
     XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
+    SpaprXiveClass *sxc = SPAPR_XIVE_CLASS(klass);
 
     dc->desc    = "sPAPR XIVE Interrupt Controller";
     dc->props   = spapr_xive_properties;
-    dc->realize = spapr_xive_realize;
+    device_class_set_parent_realize(dc, spapr_xive_realize,
+                                    &sxc->parent_realize);
     dc->vmsd    = &vmstate_spapr_xive;
 
     xrc->get_eas = spapr_xive_get_eas;
@@ -794,6 +803,7 @@ static const TypeInfo spapr_xive_info = {
     .instance_init = spapr_xive_instance_init,
     .instance_size = sizeof(SpaprXive),
     .class_init = spapr_xive_class_init,
+    .class_size = sizeof(SpaprXiveClass),
     .interfaces = (InterfaceInfo[]) {
         { TYPE_SPAPR_INTC },
         { }
-- 
2.21.0



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

* [PATCH v2 06/13] pnv/xive: Use device_class_set_parent_realize()
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (4 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize() Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  4:29   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router Cédric Le Goater
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

The XIVE router base class currently inherits an empty realize hook
from the sysbus device base class, but it will soon implement one
of its own to perform some sanity checks. Do the preliminary plumbing
to have it called.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv_xive.h | 10 ++++++++++
 hw/intc/pnv_xive.c        | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
index 4d641db691c8..ba9bbeab88c3 100644
--- a/include/hw/ppc/pnv_xive.h
+++ b/include/hw/ppc/pnv_xive.h
@@ -16,6 +16,10 @@ struct PnvChip;
 
 #define TYPE_PNV_XIVE "pnv-xive"
 #define PNV_XIVE(obj) OBJECT_CHECK(PnvXive, (obj), TYPE_PNV_XIVE)
+#define PNV_XIVE_CLASS(klass)                                   \
+    OBJECT_CLASS_CHECK(PnvXiveClass, (klass), TYPE_PNV_XIVE)
+#define PNV_XIVE_GET_CLASS(obj)                                 \
+    OBJECT_GET_CLASS(PnvXiveClass, (obj), TYPE_PNV_XIVE)
 
 #define XIVE_BLOCK_MAX      16
 
@@ -87,6 +91,12 @@ typedef struct PnvXive {
     uint64_t      edt[XIVE_TABLE_EDT_MAX];
 } PnvXive;
 
+typedef struct PnvXiveClass {
+    XiveRouterClass parent_class;
+
+    DeviceRealize parent_realize;
+} PnvXiveClass;
+
 void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon);
 
 #endif /* PPC_PNV_XIVE_H */
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 66970a60733b..1962f884d6de 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1816,10 +1816,17 @@ static void pnv_xive_init(Object *obj)
 static void pnv_xive_realize(DeviceState *dev, Error **errp)
 {
     PnvXive *xive = PNV_XIVE(dev);
+    PnvXiveClass *pxc = PNV_XIVE_GET_CLASS(dev);
     XiveSource *xsrc = &xive->ipi_source;
     XiveENDSource *end_xsrc = &xive->end_source;
     Error *local_err = NULL;
 
+    pxc->parent_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     assert(xive->chip);
     assert(xive->system_memory);
 
@@ -1950,10 +1957,12 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
     XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
     XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
     XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
+    PnvXiveClass *pxc = PNV_XIVE_CLASS(klass);
 
     xdc->dt_xscom = pnv_xive_dt_xscom;
 
     dc->desc = "PowerNV XIVE Interrupt Controller";
+    device_class_set_parent_realize(dc, pnv_xive_realize, &pxc->parent_realize);
     dc->realize = pnv_xive_realize;
     dc->props = pnv_xive_properties;
 
@@ -1974,6 +1983,7 @@ static const TypeInfo pnv_xive_info = {
     .instance_init = pnv_xive_init,
     .instance_size = sizeof(PnvXive),
     .class_init    = pnv_xive_class_init,
+    .class_size    = sizeof(PnvXiveClass),
     .interfaces    = (InterfaceInfo[]) {
         { TYPE_PNV_XSCOM_INTERFACE },
         { }
-- 
2.21.0



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

* [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (5 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 06/13] pnv/xive: " Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  6:10   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 08/13] xive: Use the XIVE fabric link under " Cédric Le Goater
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

In order to get rid of qdev_get_machine(), first add a pointer to the
XIVE fabric under the XIVE router and make it configurable through a
QOM link property.

Configure it in the spapr and pnv machine. In the case of pnv, the XIVE
routers are under the chip, so this is done with a QOM alias property of
the POWER9 pnv chip.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 5 +++--
 hw/intc/xive.c        | 8 ++++++++
 hw/ppc/pnv.c          | 6 ++++++
 hw/ppc/spapr_irq.c    | 2 ++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 1b7b89098f71..1ded82b1cda8 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -324,9 +324,12 @@ typedef struct XiveTCTX {
 /*
  * XIVE Router
  */
+typedef struct XiveFabric XiveFabric;
 
 typedef struct XiveRouter {
     SysBusDevice    parent;
+
+    XiveFabric *xfb;
 } XiveRouter;
 
 #define TYPE_XIVE_ROUTER "xive-router"
@@ -402,8 +405,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
  * XIVE Fabric (Interface between Interrupt Controller and Machine)
  */
 
-typedef struct XiveFabric XiveFabric;
-
 #define TYPE_XIVE_FABRIC "xive-fabric"
 #define XIVE_FABRIC(obj)                                     \
     INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index d4c6e21703b3..6df89b06da38 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1714,12 +1714,19 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
                            xive_get_field64(EAS_END_DATA,  eas.w));
 }
 
+static Property xive_router_properties[] = {
+    DEFINE_PROP_LINK("xive-fabric", XiveRouter, xfb,
+                     TYPE_XIVE_FABRIC, XiveFabric *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void xive_router_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
 
     dc->desc    = "XIVE Router Engine";
+    dc->props   = xive_router_properties;
     xnc->notify = xive_router_notify;
 }
 
@@ -1727,6 +1734,7 @@ static const TypeInfo xive_router_info = {
     .name          = TYPE_XIVE_ROUTER,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .abstract      = true,
+    .instance_size = sizeof(XiveRouter),
     .class_size    = sizeof(XiveRouterClass),
     .class_init    = xive_router_class_init,
     .interfaces    = (InterfaceInfo[]) {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 2a1b15a69aed..915c80a24b3e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -804,6 +804,10 @@ static void pnv_init(MachineState *machine)
         if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
             object_property_set_link(chip, OBJECT(pnv), "xics", &error_abort);
         }
+        if (object_dynamic_cast(OBJECT(pnv), TYPE_XIVE_FABRIC)) {
+            object_property_set_link(chip, OBJECT(pnv), "xive-fabric",
+                                     &error_abort);
+        }
         object_property_set_bool(chip, true, "realized", &error_fatal);
     }
     g_free(chip_typename);
@@ -1224,6 +1228,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "xive", &chip9->xive, sizeof(chip9->xive),
                             TYPE_PNV_XIVE, &error_abort, NULL);
+    object_property_add_alias(obj, "xive-fabric", OBJECT(&chip9->xive),
+                              "xive-fabric", &error_abort);
 
     object_initialize_child(obj, "psi",  &chip9->psi, sizeof(chip9->psi),
                             TYPE_PNV9_PSI, &error_abort, NULL);
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 07e08d6544a0..2b656649ad6a 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -340,6 +340,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
          * priority
          */
         qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
+        object_property_set_link(OBJECT(dev), OBJECT(spapr), "xive-fabric",
+                                 &error_abort);
         qdev_init_nofail(dev);
 
         spapr->xive = SPAPR_XIVE(dev);
-- 
2.21.0



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

* [PATCH v2 08/13] xive: Use the XIVE fabric link under the XIVE router
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (6 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  6:11   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class Cédric Le Goater
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

Now that the spapr and pnv machines do set the "xive-fabric" link, the
use of the XIVE fabric pointer becomes mandatory. This is checked with
an assert() in a new realize hook. Since the XIVE router is realized at
machine init for the all the machine's life time, no risk to abort an
already running guest (ie. not a hotplug path).

This gets rid of a qdev_get_machine() call.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 6df89b06da38..12a362b681a6 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1378,6 +1378,13 @@ static int xive_router_get_block_id(XiveRouter *xrtr)
    return xrc->get_block_id(xrtr);
 }
 
+static void xive_router_realize(DeviceState *dev, Error **errp)
+{
+    XiveRouter *xrtr = XIVE_ROUTER(dev);
+
+    assert(xrtr->xfb);
+}
+
 /*
  * Encode the HW CAM line in the block group mode format :
  *
@@ -1470,12 +1477,11 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
  *
  * The parameters represent what is sent on the PowerBus
  */
-static bool xive_presenter_notify(uint8_t format,
+static bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
                                   uint8_t nvt_blk, uint32_t nvt_idx,
                                   bool cam_ignore, uint8_t priority,
                                   uint32_t logic_serv)
 {
-    XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine());
     XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb);
     XiveTCTXMatch match = { .tctx = NULL, .ring = 0 };
     int count;
@@ -1607,7 +1613,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
         return;
     }
 
-    found = xive_presenter_notify(format, nvt_blk, nvt_idx,
+    found = xive_presenter_notify(xrtr->xfb, format, nvt_blk, nvt_idx,
                           xive_get_field32(END_W7_F0_IGNORE, end.w7),
                           priority,
                           xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
@@ -1727,6 +1733,8 @@ static void xive_router_class_init(ObjectClass *klass, void *data)
 
     dc->desc    = "XIVE Router Engine";
     dc->props   = xive_router_properties;
+    /* Parent is SysBusDeviceClass. No need to call its realize hook */
+    dc->realize = xive_router_realize;
     xnc->notify = xive_router_notify;
 }
 
-- 
2.21.0



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

* [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (7 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 08/13] xive: Use the XIVE fabric link under " Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  6:12   ` David Gibson
  2019-12-19 18:11 ` [PATCH v2 10/13] ppc/pnv: Add a "pnor" const link property to the BMC internal simulator Cédric Le Goater
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

Set it at chip creation and forward it to the cores. This allows to drop
a call to qdev_get_machine().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h | 1 +
 hw/ppc/pnv.c         | 8 +++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 8b957dfb5736..4c13d4394a11 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -48,6 +48,7 @@ typedef struct PnvChip {
     uint64_t     ram_size;
 
     uint32_t     nr_cores;
+    uint32_t     nr_threads;
     uint64_t     cores_mask;
     PnvCore      **cores;
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 915c80a24b3e..e638cdc93091 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -791,6 +791,8 @@ static void pnv_init(MachineState *machine)
                                 &error_fatal);
         object_property_set_int(chip, machine->smp.cores,
                                 "nr-cores", &error_fatal);
+        object_property_set_int(chip, machine->smp.threads,
+                                "nr-threads", &error_fatal);
         /*
          * TODO: Only the MMIO range should be of interest for the
          * controllers
@@ -1529,7 +1531,6 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
 
 static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
 {
-    MachineState *ms = MACHINE(qdev_get_machine());
     Error *error = NULL;
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
     const char *typename = pnv_chip_core_typename(chip);
@@ -1565,8 +1566,8 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
         object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
                                   &error_abort);
         chip->cores[i] = pnv_core;
-        object_property_set_int(OBJECT(pnv_core), ms->smp.threads, "nr-threads",
-                                &error_fatal);
+        object_property_set_int(OBJECT(pnv_core), chip->nr_threads,
+                                "nr-threads", &error_fatal);
         object_property_set_int(OBJECT(pnv_core), core_hwid,
                                 CPU_CORE_PROP_CORE_ID, &error_fatal);
         object_property_set_int(OBJECT(pnv_core),
@@ -1609,6 +1610,7 @@ static Property pnv_chip_properties[] = {
     DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
     DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
                      TYPE_MEMORY_REGION, MemoryRegion *),
+    DEFINE_PROP_UINT32("nr-threads", PnvChip, nr_threads, 1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.21.0



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

* [PATCH v2 10/13] ppc/pnv: Add a "pnor" const link property to the BMC internal simulator
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (8 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 11/13] xive: Add a "presenter" link property to the TCTX object Cédric Le Goater
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

This allows to get rid of a call to qdev_get_machine().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h | 2 +-
 hw/ppc/pnv.c         | 2 +-
 hw/ppc/pnv_bmc.c     | 8 +++++---
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 4c13d4394a11..d016ab0d0319 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -231,7 +231,7 @@ PnvChip *pnv_get_chip(uint32_t chip_id);
  */
 void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
 void pnv_bmc_powerdown(IPMIBmc *bmc);
-IPMIBmc *pnv_bmc_create(void);
+IPMIBmc *pnv_bmc_create(PnvPnor *pnor);
 
 /*
  * POWER8 MMIO base addresses
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index e638cdc93091..34549aefa2fc 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -815,7 +815,7 @@ static void pnv_init(MachineState *machine)
     g_free(chip_typename);
 
     /* Create the machine BMC simulator */
-    pnv->bmc = pnv_bmc_create();
+    pnv->bmc = pnv_bmc_create(pnv->pnor);
 
     /* Instantiate ISA bus on chip 0 */
     pnv->isa_bus = pnv_isa_create(pnv->chips[0], &error_fatal);
diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index 07fa1e1c7e45..8863354c1c08 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -143,8 +143,8 @@ static uint16_t bytes_to_blocks(uint32_t bytes)
 static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len,
                        RspBuffer *rsp)
 {
-    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
-    PnvPnor *pnor = pnv->pnor;
+    PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor",
+                                                      &error_abort));
     uint32_t pnor_size = pnor->size;
     uint32_t pnor_addr = PNOR_SPI_OFFSET;
     bool readonly = false;
@@ -217,11 +217,13 @@ static const IPMINetfn hiomap_netfn = {
  * Instantiate the machine BMC. PowerNV uses the QEMU internal
  * simulator but it could also be external.
  */
-IPMIBmc *pnv_bmc_create(void)
+IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
 {
     Object *obj;
 
     obj = object_new(TYPE_IPMI_BMC_SIMULATOR);
+    object_ref(OBJECT(pnor));
+    object_property_add_const_link(obj, "pnor", OBJECT(pnor), &error_abort);
     object_property_set_bool(obj, true, "realized", &error_fatal);
 
     /* Install the HIOMAP protocol handlers to access the PNOR */
-- 
2.21.0



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

* [PATCH v2 11/13] xive: Add a "presenter" link property to the TCTX object
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (9 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 10/13] ppc/pnv: Add a "pnor" const link property to the BMC internal simulator Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 12/13] spapr/xive: Deduce the SpaprXive pointer from XiveTCTX::xptr Cédric Le Goater
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

This will be used in subsequent patches to access the XIVE associated to
a TCTX without reaching out to the machine through qdev_get_machine().

Signed-off-by: Cédric Le Goater <clg@kaod.org>
[ groug: - split patch
         - write subject and changelog ]
Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 8 +++++---
 hw/intc/spapr_xive.c  | 2 +-
 hw/intc/xive.c        | 6 +++++-
 hw/ppc/pnv.c          | 3 ++-
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 1ded82b1cda8..705cf48176fc 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -311,6 +311,8 @@ void xive_source_set_irq(void *opaque, int srcno, int val);
 #define XIVE_TM_RING_COUNT      4
 #define XIVE_TM_RING_SIZE       0x10
 
+typedef struct XivePresenter XivePresenter;
+
 typedef struct XiveTCTX {
     DeviceState parent_obj;
 
@@ -319,6 +321,8 @@ typedef struct XiveTCTX {
     qemu_irq    os_output;
 
     uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
+
+    XivePresenter *xptr;
 } XiveTCTX;
 
 /*
@@ -378,8 +382,6 @@ typedef struct XiveTCTXMatch {
     uint8_t ring;
 } XiveTCTXMatch;
 
-typedef struct XivePresenter XivePresenter;
-
 #define TYPE_XIVE_PRESENTER "xive-presenter"
 #define XIVE_PRESENTER(obj)                                     \
     INTERFACE_CHECK(XivePresenter, (obj), TYPE_XIVE_PRESENTER)
@@ -467,7 +469,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
                            unsigned size);
 
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
+Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, Error **errp);
 void xive_tctx_reset(XiveTCTX *tctx);
 void xive_tctx_destroy(XiveTCTX *tctx);
 void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 32322470a8b8..76631238783e 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -601,7 +601,7 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
     Object *obj;
     SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
 
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
+    obj = xive_tctx_create(OBJECT(cpu), XIVE_PRESENTER(xive), errp);
     if (!obj) {
         return -1;
     }
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 12a362b681a6..bc8019c4c973 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -654,6 +654,7 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
 
     assert(tctx->cs);
+    assert(tctx->xptr);
 
     cpu = POWERPC_CPU(tctx->cs);
     env = &cpu->env;
@@ -727,6 +728,8 @@ static const VMStateDescription vmstate_xive_tctx = {
 
 static Property xive_tctx_properties[] = {
     DEFINE_PROP_LINK("cpu", XiveTCTX, cs, TYPE_CPU, CPUState *),
+    DEFINE_PROP_LINK("presenter", XiveTCTX, xptr, TYPE_XIVE_PRESENTER,
+                     XivePresenter *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -752,7 +755,7 @@ static const TypeInfo xive_tctx_info = {
     .class_init    = xive_tctx_class_init,
 };
 
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
+Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, Error **errp)
 {
     Error *local_err = NULL;
     Object *obj;
@@ -761,6 +764,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
     object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
     object_unref(obj);
     object_property_set_link(obj, cpu, "cpu", &error_abort);
+    object_property_set_link(obj, OBJECT(xptr), "presenter", &error_abort);
     object_property_set_bool(obj, true, "realized", &local_err);
     if (local_err) {
         goto error;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 34549aefa2fc..d100b6f559e2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -920,7 +920,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
      * controller object is initialized afterwards. Hopefully, it's
      * only used at runtime.
      */
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
+    obj = xive_tctx_create(OBJECT(cpu), XIVE_PRESENTER(&chip9->xive),
+                           &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
-- 
2.21.0



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

* [PATCH v2 12/13] spapr/xive: Deduce the SpaprXive pointer from XiveTCTX::xptr
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (10 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 11/13] xive: Add a "presenter" link property to the TCTX object Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-19 18:11 ` [PATCH v2 13/13] pnv/xive: Deduce the PnvXive " Cédric Le Goater
  2019-12-23  6:16 ` [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() David Gibson
  13 siblings, 0 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

And use it instead of reaching out to the machine. This allows to get rid
of a call to qdev_get_machine() and to reduce the scope of another one so
that it is only used within the argument list of error_append_hint(). This
is an acceptable tradeoff compared to all it would require to know about
the maximum number of CPUs here without calling qdev_get_machine().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/spapr_xive_kvm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index 32b2809210a0..edb7ee0e74f1 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -75,7 +75,7 @@ static void kvm_cpu_disable_all(void)
 
 void kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp)
 {
-    SpaprXive *xive = SPAPR_MACHINE(qdev_get_machine())->xive;
+    SpaprXive *xive = SPAPR_XIVE(tctx->xptr);
     uint64_t state[2];
     int ret;
 
@@ -97,7 +97,7 @@ void kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp)
 
 void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
 {
-    SpaprXive *xive = SPAPR_MACHINE(qdev_get_machine())->xive;
+    SpaprXive *xive = SPAPR_XIVE(tctx->xptr);
     uint64_t state[2] = { 0 };
     int ret;
 
@@ -152,8 +152,7 @@ void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp)
 
 void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
 {
-    MachineState *ms = MACHINE(qdev_get_machine());
-    SpaprXive *xive = SPAPR_MACHINE(ms)->xive;
+    SpaprXive *xive = SPAPR_XIVE(tctx->xptr);
     unsigned long vcpu_id;
     int ret;
 
@@ -179,7 +178,7 @@ void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp)
                    vcpu_id, strerror(errno));
         if (errno == ENOSPC) {
             error_append_hint(&local_err, "Try -smp maxcpus=N with N < %u\n",
-                              ms->smp.max_cpus);
+                              MACHINE(qdev_get_machine())->smp.max_cpus);
         }
         error_propagate(errp, local_err);
         return;
-- 
2.21.0



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

* [PATCH v2 13/13] pnv/xive: Deduce the PnvXive pointer from XiveTCTX::xptr
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (11 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 12/13] spapr/xive: Deduce the SpaprXive pointer from XiveTCTX::xptr Cédric Le Goater
@ 2019-12-19 18:11 ` Cédric Le Goater
  2019-12-23  6:16 ` [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() David Gibson
  13 siblings, 0 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-19 18:11 UTC (permalink / raw)
  To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel

From: Greg Kurz <groug@kaod.org>

And use it instead of reaching out to the machine. This allows to get
rid of pnv_get_chip().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h |  2 --
 hw/intc/pnv_xive.c   |  8 ++------
 hw/ppc/pnv.c         | 14 --------------
 3 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index d016ab0d0319..5f0b7d3374ef 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -221,8 +221,6 @@ struct PnvMachineState {
     PnvPnor      *pnor;
 };
 
-PnvChip *pnv_get_chip(uint32_t chip_id);
-
 #define PNV_FDT_ADDR          0x01000000
 #define PNV_TIMEBASE_FREQ     512000000ULL
 
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 1962f884d6de..79a5dfe9a858 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -472,12 +472,8 @@ static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
 static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
 {
     int pir = ppc_cpu_pir(cpu);
-    PnvChip *chip;
-    PnvXive *xive;
-
-    chip = pnv_get_chip(PNV9_PIR2CHIP(pir));
-    assert(chip);
-    xive = &PNV9_CHIP(chip)->xive;
+    XivePresenter *xptr = XIVE_TCTX(pnv_cpu_state(cpu)->intc)->xptr;
+    PnvXive *xive = PNV_XIVE(xptr);
 
     if (!pnv_xive_is_cpu_enabled(xive, cpu)) {
         xive_error(xive, "IC: CPU %x is not enabled", pir);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d100b6f559e2..e1c218dc03f5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1724,20 +1724,6 @@ static int pnv_match_nvt(XiveFabric *xfb, uint8_t format,
     return total_count;
 }
 
-PnvChip *pnv_get_chip(uint32_t chip_id)
-{
-    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
-    int i;
-
-    for (i = 0; i < pnv->num_chips; i++) {
-        PnvChip *chip = pnv->chips[i];
-        if (chip->chip_id == chip_id) {
-            return chip;
-        }
-    }
-    return NULL;
-}
-
 static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
-- 
2.21.0



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

* Re: [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property
  2019-12-19 18:11 ` [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property Cédric Le Goater
@ 2019-12-19 18:56   ` Greg Kurz
  2019-12-20  0:31   ` David Gibson
  1 sibling, 0 replies; 26+ messages in thread
From: Greg Kurz @ 2019-12-19 18:56 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson

On Thu, 19 Dec 2019 19:11:44 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> and use a link to pass the system memory to the device models that
> require it to map/unmap BARs. This replace the use of get_system_memory()
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  include/hw/ppc/pnv.h      |  2 ++
>  include/hw/ppc/pnv_psi.h  |  1 +
>  include/hw/ppc/pnv_xive.h |  2 ++
>  hw/intc/pnv_xive.c        |  5 ++++-
>  hw/ppc/pnv.c              | 33 ++++++++++++++++++++++++++-------
>  hw/ppc/pnv_psi.c          | 13 ++++++++++---
>  6 files changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index f78fd0dd967c..f31180618672 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -56,6 +56,8 @@ typedef struct PnvChip {
>      AddressSpace xscom_as;
>  
>      gchar        *dt_isa_nodename;
> +
> +    MemoryRegion *system_memory;
>  } PnvChip;
>  
>  #define TYPE_PNV8_CHIP "pnv8-chip"
> diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
> index f0f5b5519767..f85babaff0be 100644
> --- a/include/hw/ppc/pnv_psi.h
> +++ b/include/hw/ppc/pnv_psi.h
> @@ -35,6 +35,7 @@ typedef struct PnvPsi {
>  
>      MemoryRegion regs_mr;
>      uint64_t bar;
> +    MemoryRegion *system_memory;
>  
>      /* FSP region not supported */
>      /* MemoryRegion fsp_mr; */
> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
> index f4c7caad40ee..4d641db691c8 100644
> --- a/include/hw/ppc/pnv_xive.h
> +++ b/include/hw/ppc/pnv_xive.h
> @@ -30,6 +30,8 @@ typedef struct PnvXive {
>      /* Owning chip */
>      struct PnvChip *chip;
>  
> +    MemoryRegion *system_memory;
> +
>      /* XSCOM addresses giving access to the controller registers */
>      MemoryRegion  xscom_regs;
>  
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index a0a69b98a713..66970a60733b 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -853,7 +853,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
>                                    uint64_t val, unsigned size)
>  {
>      PnvXive *xive = PNV_XIVE(opaque);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = xive->system_memory;
>      uint32_t reg = offset >> 3;
>      bool is_chip0 = xive->chip->chip_id == 0;
>  
> @@ -1821,6 +1821,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>  
>      assert(xive->chip);
> +    assert(xive->system_memory);
>  
>      /*
>       * The XiveSource and XiveENDSource objects are realized with the
> @@ -1937,6 +1938,8 @@ static Property pnv_xive_properties[] = {
>      DEFINE_PROP_UINT64("tm-bar", PnvXive, tm_base, 0),
>      /* The PnvChip id identifies the XIVE interrupt controller. */
>      DEFINE_PROP_LINK("chip", PnvXive, chip, TYPE_PNV_CHIP, PnvChip *),
> +    DEFINE_PROP_LINK("system-memory", PnvXive, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 855254f28263..1d8bfb164a32 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -674,6 +674,7 @@ static void pnv_chip_power10_pic_print_info(PnvChip *chip, Monitor *mon)
>  
>  static void pnv_init(MachineState *machine)
>  {
> +    MemoryRegion *sysmem = get_system_memory();
>      PnvMachineState *pnv = PNV_MACHINE(machine);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      MemoryRegion *ram;
> @@ -692,7 +693,7 @@ static void pnv_init(MachineState *machine)
>      ram = g_new(MemoryRegion, 1);
>      memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
>                                           machine->ram_size);
> -    memory_region_add_subregion(get_system_memory(), 0, ram);
> +    memory_region_add_subregion(sysmem, 0, ram);
>  
>      /*
>       * Create our simple PNOR device
> @@ -790,6 +791,12 @@ static void pnv_init(MachineState *machine)
>                                  &error_fatal);
>          object_property_set_int(chip, machine->smp.cores,
>                                  "nr-cores", &error_fatal);
> +        /*
> +         * TODO: Only the MMIO range should be of interest for the
> +         * controllers
> +         */
> +        object_property_set_link(chip, OBJECT(sysmem), "system-memory",
> +                                 &error_abort);
>          object_property_set_bool(chip, true, "realized", &error_fatal);
>      }
>      g_free(chip_typename);
> @@ -1060,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip8->psi), PNV_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -1100,7 +1109,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
>  
>      /* OCC SRAM model */
> -    memory_region_add_subregion(get_system_memory(), PNV_OCC_SENSOR_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV_OCC_SENSOR_BASE(chip),
>                                  &chip8->occ.sram_regs);
>  
>      /* HOMER */
> @@ -1116,7 +1125,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV_XSCOM_PBA_BASE, &chip8->homer.pba_regs);
>  
>      /* Homer mmio region */
> -    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV_HOMER_BASE(chip),
>                                  &chip8->homer.regs);
>  }
>  
> @@ -1280,6 +1289,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>                              "tm-bar", &error_fatal);
>      object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip), "chip",
>                               &error_abort);
> +    object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip9->xive), true, "realized",
>                               &local_err);
>      if (local_err) {
> @@ -1292,6 +1303,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip9->psi), PNV9_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip9->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip9->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -1308,7 +1321,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>          error_propagate(errp, local_err);
>          return;
>      }
> -    memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_LPCM_BASE(chip),
>                                  &chip9->lpc.xscom_regs);
>  
>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> @@ -1325,7 +1338,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs);
>  
>      /* OCC SRAM model */
> -    memory_region_add_subregion(get_system_memory(), PNV9_OCC_SENSOR_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_OCC_SENSOR_BASE(chip),
>                                  &chip9->occ.sram_regs);
>  
>      /* HOMER */
> @@ -1341,7 +1354,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_PBA_BASE, &chip9->homer.pba_regs);
>  
>      /* Homer mmio region */
> -    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_HOMER_BASE(chip),
>                                  &chip9->homer.regs);
>  }
>  
> @@ -1408,6 +1421,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip10->psi), PNV10_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip10->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip10->psi), true, "realized",
>                               &local_err);
>      if (local_err) {
> @@ -1426,7 +1441,7 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>          error_propagate(errp, local_err);
>          return;
>      }
> -    memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV10_LPCM_BASE(chip),
>                                  &chip10->lpc.xscom_regs);
>  
>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> @@ -1556,6 +1571,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>      PnvChip *chip = PNV_CHIP(dev);
>      Error *error = NULL;
>  
> +    assert(chip->system_memory);
> +
>      /* Cores */
>      pnv_chip_core_realize(chip, &error);
>      if (error) {
> @@ -1570,6 +1587,8 @@ static Property pnv_chip_properties[] = {
>      DEFINE_PROP_UINT64("ram-size", PnvChip, ram_size, 0),
>      DEFINE_PROP_UINT32("nr-cores", PnvChip, nr_cores, 1),
>      DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
> +    DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 75e20d9da08b..28d34e5c193a 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -126,7 +126,7 @@
>  static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar)
>  {
>      PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = psi->system_memory;
>      uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
>  
>      psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
> @@ -489,6 +489,8 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
>      Error *err = NULL;
>      unsigned int i;
>  
> +    assert(psi->system_memory);
> +
>      obj = object_property_get_link(OBJECT(dev), "xics", &err);
>      if (!obj) {
>          error_setg(errp, "%s: required link 'xics' not found: %s",
> @@ -562,6 +564,8 @@ static int pnv_psi_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>  static Property pnv_psi_properties[] = {
>      DEFINE_PROP_UINT64("bar", PnvPsi, bar, 0),
>      DEFINE_PROP_UINT64("fsp-bar", PnvPsi, fsp_bar, 0),
> +    DEFINE_PROP_LINK("system-memory", PnvPsi, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -701,7 +705,7 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
>      PnvPsi *psi = PNV_PSI(opaque);
>      Pnv9Psi *psi9 = PNV9_PSI(psi);
>      uint32_t reg = PSIHB_REG(addr);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = psi->system_memory;
>  
>      switch (addr) {
>      case PSIHB9_CR:
> @@ -819,11 +823,12 @@ static void pnv_psi_power9_irq_set(PnvPsi *psi, int irq, bool state)
>  static void pnv_psi_power9_reset(void *dev)
>  {
>      Pnv9Psi *psi = PNV9_PSI(dev);
> +    MemoryRegion *sysmem = PNV_PSI(psi)->system_memory;
>  
>      pnv_psi_reset(dev);
>  
>      if (memory_region_is_mapped(&psi->source.esb_mmio)) {
> -        memory_region_del_subregion(get_system_memory(), &psi->source.esb_mmio);
> +        memory_region_del_subregion(sysmem, &psi->source.esb_mmio);
>      }
>  }
>  
> @@ -842,6 +847,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>      int i;
>  
> +    assert(psi->system_memory);
> +
>      /* This is the only device with 4k ESB pages */
>      object_property_set_int(OBJECT(xsrc), XIVE_ESB_4K, "shift",
>                              &error_fatal);



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

* Re: [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine
  2019-12-19 18:11 ` [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine Cédric Le Goater
@ 2019-12-20  0:26   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-20  0:26 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:43PM +0100, Cédric Le Goater wrote:
> Use container_of() instead of qdev_get_machine()
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-5.0.

> ---
>  hw/ppc/pnv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index f77e7ca84ede..855254f28263 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -561,7 +561,7 @@ static void *pnv_dt_create(MachineState *machine)
>  
>  static void pnv_powerdown_notify(Notifier *n, void *opaque)
>  {
> -    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    PnvMachineState *pnv = container_of(n, PnvMachineState, powerdown_notifier);
>  
>      if (pnv->bmc) {
>          pnv_bmc_powerdown(pnv->bmc);

-- 
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 --]

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

* Re: [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property
  2019-12-19 18:11 ` [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property Cédric Le Goater
  2019-12-19 18:56   ` Greg Kurz
@ 2019-12-20  0:31   ` David Gibson
  2019-12-20  6:49     ` Cédric Le Goater
  1 sibling, 1 reply; 26+ messages in thread
From: David Gibson @ 2019-12-20  0:31 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:44PM +0100, Cédric Le Goater wrote:
> and use a link to pass the system memory to the device models that
> require it to map/unmap BARs. This replace the use of get_system_memory()
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

I don't really see the rationale for this.  The system memory really
is global, not per-chip in any way, and just using get_system_memory()
appears to be idiomatic throughout qemu.

> ---
>  include/hw/ppc/pnv.h      |  2 ++
>  include/hw/ppc/pnv_psi.h  |  1 +
>  include/hw/ppc/pnv_xive.h |  2 ++
>  hw/intc/pnv_xive.c        |  5 ++++-
>  hw/ppc/pnv.c              | 33 ++++++++++++++++++++++++++-------
>  hw/ppc/pnv_psi.c          | 13 ++++++++++---
>  6 files changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index f78fd0dd967c..f31180618672 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -56,6 +56,8 @@ typedef struct PnvChip {
>      AddressSpace xscom_as;
>  
>      gchar        *dt_isa_nodename;
> +
> +    MemoryRegion *system_memory;
>  } PnvChip;
>  
>  #define TYPE_PNV8_CHIP "pnv8-chip"
> diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
> index f0f5b5519767..f85babaff0be 100644
> --- a/include/hw/ppc/pnv_psi.h
> +++ b/include/hw/ppc/pnv_psi.h
> @@ -35,6 +35,7 @@ typedef struct PnvPsi {
>  
>      MemoryRegion regs_mr;
>      uint64_t bar;
> +    MemoryRegion *system_memory;
>  
>      /* FSP region not supported */
>      /* MemoryRegion fsp_mr; */
> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
> index f4c7caad40ee..4d641db691c8 100644
> --- a/include/hw/ppc/pnv_xive.h
> +++ b/include/hw/ppc/pnv_xive.h
> @@ -30,6 +30,8 @@ typedef struct PnvXive {
>      /* Owning chip */
>      struct PnvChip *chip;
>  
> +    MemoryRegion *system_memory;
> +
>      /* XSCOM addresses giving access to the controller registers */
>      MemoryRegion  xscom_regs;
>  
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index a0a69b98a713..66970a60733b 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -853,7 +853,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
>                                    uint64_t val, unsigned size)
>  {
>      PnvXive *xive = PNV_XIVE(opaque);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = xive->system_memory;
>      uint32_t reg = offset >> 3;
>      bool is_chip0 = xive->chip->chip_id == 0;
>  
> @@ -1821,6 +1821,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>  
>      assert(xive->chip);
> +    assert(xive->system_memory);
>  
>      /*
>       * The XiveSource and XiveENDSource objects are realized with the
> @@ -1937,6 +1938,8 @@ static Property pnv_xive_properties[] = {
>      DEFINE_PROP_UINT64("tm-bar", PnvXive, tm_base, 0),
>      /* The PnvChip id identifies the XIVE interrupt controller. */
>      DEFINE_PROP_LINK("chip", PnvXive, chip, TYPE_PNV_CHIP, PnvChip *),
> +    DEFINE_PROP_LINK("system-memory", PnvXive, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 855254f28263..1d8bfb164a32 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -674,6 +674,7 @@ static void pnv_chip_power10_pic_print_info(PnvChip *chip, Monitor *mon)
>  
>  static void pnv_init(MachineState *machine)
>  {
> +    MemoryRegion *sysmem = get_system_memory();
>      PnvMachineState *pnv = PNV_MACHINE(machine);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      MemoryRegion *ram;
> @@ -692,7 +693,7 @@ static void pnv_init(MachineState *machine)
>      ram = g_new(MemoryRegion, 1);
>      memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
>                                           machine->ram_size);
> -    memory_region_add_subregion(get_system_memory(), 0, ram);
> +    memory_region_add_subregion(sysmem, 0, ram);
>  
>      /*
>       * Create our simple PNOR device
> @@ -790,6 +791,12 @@ static void pnv_init(MachineState *machine)
>                                  &error_fatal);
>          object_property_set_int(chip, machine->smp.cores,
>                                  "nr-cores", &error_fatal);
> +        /*
> +         * TODO: Only the MMIO range should be of interest for the
> +         * controllers
> +         */
> +        object_property_set_link(chip, OBJECT(sysmem), "system-memory",
> +                                 &error_abort);
>          object_property_set_bool(chip, true, "realized", &error_fatal);
>      }
>      g_free(chip_typename);
> @@ -1060,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip8->psi), PNV_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -1100,7 +1109,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
>  
>      /* OCC SRAM model */
> -    memory_region_add_subregion(get_system_memory(), PNV_OCC_SENSOR_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV_OCC_SENSOR_BASE(chip),
>                                  &chip8->occ.sram_regs);
>  
>      /* HOMER */
> @@ -1116,7 +1125,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV_XSCOM_PBA_BASE, &chip8->homer.pba_regs);
>  
>      /* Homer mmio region */
> -    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV_HOMER_BASE(chip),
>                                  &chip8->homer.regs);
>  }
>  
> @@ -1280,6 +1289,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>                              "tm-bar", &error_fatal);
>      object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip), "chip",
>                               &error_abort);
> +    object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip9->xive), true, "realized",
>                               &local_err);
>      if (local_err) {
> @@ -1292,6 +1303,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip9->psi), PNV9_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip9->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip9->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -1308,7 +1321,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>          error_propagate(errp, local_err);
>          return;
>      }
> -    memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_LPCM_BASE(chip),
>                                  &chip9->lpc.xscom_regs);
>  
>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> @@ -1325,7 +1338,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs);
>  
>      /* OCC SRAM model */
> -    memory_region_add_subregion(get_system_memory(), PNV9_OCC_SENSOR_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_OCC_SENSOR_BASE(chip),
>                                  &chip9->occ.sram_regs);
>  
>      /* HOMER */
> @@ -1341,7 +1354,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_PBA_BASE, &chip9->homer.pba_regs);
>  
>      /* Homer mmio region */
> -    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV9_HOMER_BASE(chip),
>                                  &chip9->homer.regs);
>  }
>  
> @@ -1408,6 +1421,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>      /* Processor Service Interface (PSI) Host Bridge */
>      object_property_set_int(OBJECT(&chip10->psi), PNV10_PSIHB_BASE(chip),
>                              "bar", &error_fatal);
> +    object_property_set_link(OBJECT(&chip10->psi), OBJECT(chip->system_memory),
> +                             "system-memory", &error_abort);
>      object_property_set_bool(OBJECT(&chip10->psi), true, "realized",
>                               &local_err);
>      if (local_err) {
> @@ -1426,7 +1441,7 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>          error_propagate(errp, local_err);
>          return;
>      }
> -    memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
> +    memory_region_add_subregion(chip->system_memory, PNV10_LPCM_BASE(chip),
>                                  &chip10->lpc.xscom_regs);
>  
>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> @@ -1556,6 +1571,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>      PnvChip *chip = PNV_CHIP(dev);
>      Error *error = NULL;
>  
> +    assert(chip->system_memory);
> +
>      /* Cores */
>      pnv_chip_core_realize(chip, &error);
>      if (error) {
> @@ -1570,6 +1587,8 @@ static Property pnv_chip_properties[] = {
>      DEFINE_PROP_UINT64("ram-size", PnvChip, ram_size, 0),
>      DEFINE_PROP_UINT32("nr-cores", PnvChip, nr_cores, 1),
>      DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
> +    DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 75e20d9da08b..28d34e5c193a 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -126,7 +126,7 @@
>  static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar)
>  {
>      PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = psi->system_memory;
>      uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
>  
>      psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
> @@ -489,6 +489,8 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
>      Error *err = NULL;
>      unsigned int i;
>  
> +    assert(psi->system_memory);
> +
>      obj = object_property_get_link(OBJECT(dev), "xics", &err);
>      if (!obj) {
>          error_setg(errp, "%s: required link 'xics' not found: %s",
> @@ -562,6 +564,8 @@ static int pnv_psi_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>  static Property pnv_psi_properties[] = {
>      DEFINE_PROP_UINT64("bar", PnvPsi, bar, 0),
>      DEFINE_PROP_UINT64("fsp-bar", PnvPsi, fsp_bar, 0),
> +    DEFINE_PROP_LINK("system-memory", PnvPsi, system_memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -701,7 +705,7 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
>      PnvPsi *psi = PNV_PSI(opaque);
>      Pnv9Psi *psi9 = PNV9_PSI(psi);
>      uint32_t reg = PSIHB_REG(addr);
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sysmem = psi->system_memory;
>  
>      switch (addr) {
>      case PSIHB9_CR:
> @@ -819,11 +823,12 @@ static void pnv_psi_power9_irq_set(PnvPsi *psi, int irq, bool state)
>  static void pnv_psi_power9_reset(void *dev)
>  {
>      Pnv9Psi *psi = PNV9_PSI(dev);
> +    MemoryRegion *sysmem = PNV_PSI(psi)->system_memory;
>  
>      pnv_psi_reset(dev);
>  
>      if (memory_region_is_mapped(&psi->source.esb_mmio)) {
> -        memory_region_del_subregion(get_system_memory(), &psi->source.esb_mmio);
> +        memory_region_del_subregion(sysmem, &psi->source.esb_mmio);
>      }
>  }
>  
> @@ -842,6 +847,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>      int i;
>  
> +    assert(psi->system_memory);
> +
>      /* This is the only device with 4k ESB pages */
>      object_property_set_int(OBJECT(xsrc), XIVE_ESB_4K, "shift",
>                              &error_fatal);

-- 
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 --]

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

* Re: [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property
  2019-12-20  0:31   ` David Gibson
@ 2019-12-20  6:49     ` Cédric Le Goater
  0 siblings, 0 replies; 26+ messages in thread
From: Cédric Le Goater @ 2019-12-20  6:49 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, Greg Kurz, qemu-devel

On 20/12/2019 01:31, David Gibson wrote:
> On Thu, Dec 19, 2019 at 07:11:44PM +0100, Cédric Le Goater wrote:
>> and use a link to pass the system memory to the device models that
>> require it to map/unmap BARs. This replace the use of get_system_memory()
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> I don't really see the rationale for this.  The system memory really
> is global, not per-chip in any way, and just using get_system_memory()
> appears to be idiomatic throughout qemu.

OK. Let's drop it.

C. 

> 
>> ---
>>  include/hw/ppc/pnv.h      |  2 ++
>>  include/hw/ppc/pnv_psi.h  |  1 +
>>  include/hw/ppc/pnv_xive.h |  2 ++
>>  hw/intc/pnv_xive.c        |  5 ++++-
>>  hw/ppc/pnv.c              | 33 ++++++++++++++++++++++++++-------
>>  hw/ppc/pnv_psi.c          | 13 ++++++++++---
>>  6 files changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>> index f78fd0dd967c..f31180618672 100644
>> --- a/include/hw/ppc/pnv.h
>> +++ b/include/hw/ppc/pnv.h
>> @@ -56,6 +56,8 @@ typedef struct PnvChip {
>>      AddressSpace xscom_as;
>>  
>>      gchar        *dt_isa_nodename;
>> +
>> +    MemoryRegion *system_memory;
>>  } PnvChip;
>>  
>>  #define TYPE_PNV8_CHIP "pnv8-chip"
>> diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
>> index f0f5b5519767..f85babaff0be 100644
>> --- a/include/hw/ppc/pnv_psi.h
>> +++ b/include/hw/ppc/pnv_psi.h
>> @@ -35,6 +35,7 @@ typedef struct PnvPsi {
>>  
>>      MemoryRegion regs_mr;
>>      uint64_t bar;
>> +    MemoryRegion *system_memory;
>>  
>>      /* FSP region not supported */
>>      /* MemoryRegion fsp_mr; */
>> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
>> index f4c7caad40ee..4d641db691c8 100644
>> --- a/include/hw/ppc/pnv_xive.h
>> +++ b/include/hw/ppc/pnv_xive.h
>> @@ -30,6 +30,8 @@ typedef struct PnvXive {
>>      /* Owning chip */
>>      struct PnvChip *chip;
>>  
>> +    MemoryRegion *system_memory;
>> +
>>      /* XSCOM addresses giving access to the controller registers */
>>      MemoryRegion  xscom_regs;
>>  
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index a0a69b98a713..66970a60733b 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -853,7 +853,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
>>                                    uint64_t val, unsigned size)
>>  {
>>      PnvXive *xive = PNV_XIVE(opaque);
>> -    MemoryRegion *sysmem = get_system_memory();
>> +    MemoryRegion *sysmem = xive->system_memory;
>>      uint32_t reg = offset >> 3;
>>      bool is_chip0 = xive->chip->chip_id == 0;
>>  
>> @@ -1821,6 +1821,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
>>      Error *local_err = NULL;
>>  
>>      assert(xive->chip);
>> +    assert(xive->system_memory);
>>  
>>      /*
>>       * The XiveSource and XiveENDSource objects are realized with the
>> @@ -1937,6 +1938,8 @@ static Property pnv_xive_properties[] = {
>>      DEFINE_PROP_UINT64("tm-bar", PnvXive, tm_base, 0),
>>      /* The PnvChip id identifies the XIVE interrupt controller. */
>>      DEFINE_PROP_LINK("chip", PnvXive, chip, TYPE_PNV_CHIP, PnvChip *),
>> +    DEFINE_PROP_LINK("system-memory", PnvXive, system_memory,
>> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 855254f28263..1d8bfb164a32 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -674,6 +674,7 @@ static void pnv_chip_power10_pic_print_info(PnvChip *chip, Monitor *mon)
>>  
>>  static void pnv_init(MachineState *machine)
>>  {
>> +    MemoryRegion *sysmem = get_system_memory();
>>      PnvMachineState *pnv = PNV_MACHINE(machine);
>>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>>      MemoryRegion *ram;
>> @@ -692,7 +693,7 @@ static void pnv_init(MachineState *machine)
>>      ram = g_new(MemoryRegion, 1);
>>      memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
>>                                           machine->ram_size);
>> -    memory_region_add_subregion(get_system_memory(), 0, ram);
>> +    memory_region_add_subregion(sysmem, 0, ram);
>>  
>>      /*
>>       * Create our simple PNOR device
>> @@ -790,6 +791,12 @@ static void pnv_init(MachineState *machine)
>>                                  &error_fatal);
>>          object_property_set_int(chip, machine->smp.cores,
>>                                  "nr-cores", &error_fatal);
>> +        /*
>> +         * TODO: Only the MMIO range should be of interest for the
>> +         * controllers
>> +         */
>> +        object_property_set_link(chip, OBJECT(sysmem), "system-memory",
>> +                                 &error_abort);
>>          object_property_set_bool(chip, true, "realized", &error_fatal);
>>      }
>>      g_free(chip_typename);
>> @@ -1060,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>>      /* Processor Service Interface (PSI) Host Bridge */
>>      object_property_set_int(OBJECT(&chip8->psi), PNV_PSIHB_BASE(chip),
>>                              "bar", &error_fatal);
>> +    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
>> +                             "system-memory", &error_abort);
>>      object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>> @@ -1100,7 +1109,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>>      pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
>>  
>>      /* OCC SRAM model */
>> -    memory_region_add_subregion(get_system_memory(), PNV_OCC_SENSOR_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV_OCC_SENSOR_BASE(chip),
>>                                  &chip8->occ.sram_regs);
>>  
>>      /* HOMER */
>> @@ -1116,7 +1125,7 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>>      pnv_xscom_add_subregion(chip, PNV_XSCOM_PBA_BASE, &chip8->homer.pba_regs);
>>  
>>      /* Homer mmio region */
>> -    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV_HOMER_BASE(chip),
>>                                  &chip8->homer.regs);
>>  }
>>  
>> @@ -1280,6 +1289,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>                              "tm-bar", &error_fatal);
>>      object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip), "chip",
>>                               &error_abort);
>> +    object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip->system_memory),
>> +                             "system-memory", &error_abort);
>>      object_property_set_bool(OBJECT(&chip9->xive), true, "realized",
>>                               &local_err);
>>      if (local_err) {
>> @@ -1292,6 +1303,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>      /* Processor Service Interface (PSI) Host Bridge */
>>      object_property_set_int(OBJECT(&chip9->psi), PNV9_PSIHB_BASE(chip),
>>                              "bar", &error_fatal);
>> +    object_property_set_link(OBJECT(&chip9->psi), OBJECT(chip->system_memory),
>> +                             "system-memory", &error_abort);
>>      object_property_set_bool(OBJECT(&chip9->psi), true, "realized", &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>> @@ -1308,7 +1321,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>          error_propagate(errp, local_err);
>>          return;
>>      }
>> -    memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV9_LPCM_BASE(chip),
>>                                  &chip9->lpc.xscom_regs);
>>  
>>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>> @@ -1325,7 +1338,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs);
>>  
>>      /* OCC SRAM model */
>> -    memory_region_add_subregion(get_system_memory(), PNV9_OCC_SENSOR_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV9_OCC_SENSOR_BASE(chip),
>>                                  &chip9->occ.sram_regs);
>>  
>>      /* HOMER */
>> @@ -1341,7 +1354,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>      pnv_xscom_add_subregion(chip, PNV9_XSCOM_PBA_BASE, &chip9->homer.pba_regs);
>>  
>>      /* Homer mmio region */
>> -    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV9_HOMER_BASE(chip),
>>                                  &chip9->homer.regs);
>>  }
>>  
>> @@ -1408,6 +1421,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>>      /* Processor Service Interface (PSI) Host Bridge */
>>      object_property_set_int(OBJECT(&chip10->psi), PNV10_PSIHB_BASE(chip),
>>                              "bar", &error_fatal);
>> +    object_property_set_link(OBJECT(&chip10->psi), OBJECT(chip->system_memory),
>> +                             "system-memory", &error_abort);
>>      object_property_set_bool(OBJECT(&chip10->psi), true, "realized",
>>                               &local_err);
>>      if (local_err) {
>> @@ -1426,7 +1441,7 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>>          error_propagate(errp, local_err);
>>          return;
>>      }
>> -    memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
>> +    memory_region_add_subregion(chip->system_memory, PNV10_LPCM_BASE(chip),
>>                                  &chip10->lpc.xscom_regs);
>>  
>>      chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>> @@ -1556,6 +1571,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>>      PnvChip *chip = PNV_CHIP(dev);
>>      Error *error = NULL;
>>  
>> +    assert(chip->system_memory);
>> +
>>      /* Cores */
>>      pnv_chip_core_realize(chip, &error);
>>      if (error) {
>> @@ -1570,6 +1587,8 @@ static Property pnv_chip_properties[] = {
>>      DEFINE_PROP_UINT64("ram-size", PnvChip, ram_size, 0),
>>      DEFINE_PROP_UINT32("nr-cores", PnvChip, nr_cores, 1),
>>      DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
>> +    DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
>> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
>> index 75e20d9da08b..28d34e5c193a 100644
>> --- a/hw/ppc/pnv_psi.c
>> +++ b/hw/ppc/pnv_psi.c
>> @@ -126,7 +126,7 @@
>>  static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar)
>>  {
>>      PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
>> -    MemoryRegion *sysmem = get_system_memory();
>> +    MemoryRegion *sysmem = psi->system_memory;
>>      uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
>>  
>>      psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
>> @@ -489,6 +489,8 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
>>      Error *err = NULL;
>>      unsigned int i;
>>  
>> +    assert(psi->system_memory);
>> +
>>      obj = object_property_get_link(OBJECT(dev), "xics", &err);
>>      if (!obj) {
>>          error_setg(errp, "%s: required link 'xics' not found: %s",
>> @@ -562,6 +564,8 @@ static int pnv_psi_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>>  static Property pnv_psi_properties[] = {
>>      DEFINE_PROP_UINT64("bar", PnvPsi, bar, 0),
>>      DEFINE_PROP_UINT64("fsp-bar", PnvPsi, fsp_bar, 0),
>> +    DEFINE_PROP_LINK("system-memory", PnvPsi, system_memory,
>> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> @@ -701,7 +705,7 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
>>      PnvPsi *psi = PNV_PSI(opaque);
>>      Pnv9Psi *psi9 = PNV9_PSI(psi);
>>      uint32_t reg = PSIHB_REG(addr);
>> -    MemoryRegion *sysmem = get_system_memory();
>> +    MemoryRegion *sysmem = psi->system_memory;
>>  
>>      switch (addr) {
>>      case PSIHB9_CR:
>> @@ -819,11 +823,12 @@ static void pnv_psi_power9_irq_set(PnvPsi *psi, int irq, bool state)
>>  static void pnv_psi_power9_reset(void *dev)
>>  {
>>      Pnv9Psi *psi = PNV9_PSI(dev);
>> +    MemoryRegion *sysmem = PNV_PSI(psi)->system_memory;
>>  
>>      pnv_psi_reset(dev);
>>  
>>      if (memory_region_is_mapped(&psi->source.esb_mmio)) {
>> -        memory_region_del_subregion(get_system_memory(), &psi->source.esb_mmio);
>> +        memory_region_del_subregion(sysmem, &psi->source.esb_mmio);
>>      }
>>  }
>>  
>> @@ -842,6 +847,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
>>      Error *local_err = NULL;
>>      int i;
>>  
>> +    assert(psi->system_memory);
>> +
>>      /* This is the only device with 4k ESB pages */
>>      object_property_set_int(OBJECT(xsrc), XIVE_ESB_4K, "shift",
>>                              &error_fatal);
> 



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

* Re: [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model
  2019-12-19 18:11 ` [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model Cédric Le Goater
@ 2019-12-22  9:33   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-22  9:33 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:45PM +0100, Cédric Le Goater wrote:
> This removes the need of the intermediate link under PSI to pass the
> XICS link to the underlying ICSState object.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

LGTM, but will need a rebase if we're dropping the previous patch.

> ---
>  hw/ppc/pnv.c     |  4 ++--
>  hw/ppc/pnv_psi.c | 11 ++---------
>  2 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 1d8bfb164a32..163a658806e2 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -999,8 +999,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
>  
>      object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
>                              TYPE_PNV8_PSI, &error_abort, NULL);
> -    object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> -                                   OBJECT(qdev_get_machine()), &error_abort);
>  
>      object_initialize_child(obj, "lpc",  &chip8->lpc, sizeof(chip8->lpc),
>                              TYPE_PNV8_LPC, &error_abort, NULL);
> @@ -1069,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>                              "bar", &error_fatal);
>      object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
>                               "system-memory", &error_abort);
> +    object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
> +                             ICS_PROP_XICS, &error_abort);
>      object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 28d34e5c193a..d3124f673571 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -470,6 +470,8 @@ static void pnv_psi_power8_instance_init(Object *obj)
>  
>      object_initialize_child(obj, "ics-psi",  &psi8->ics, sizeof(psi8->ics),
>                              TYPE_ICS, &error_abort, NULL);
> +    object_property_add_alias(obj, ICS_PROP_XICS, OBJECT(&psi8->ics),
> +                              ICS_PROP_XICS, &error_abort);
>  }
>  
>  static const uint8_t irq_to_xivr[] = {
> @@ -485,21 +487,12 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp)
>  {
>      PnvPsi *psi = PNV_PSI(dev);
>      ICSState *ics = &PNV8_PSI(psi)->ics;
> -    Object *obj;
>      Error *err = NULL;
>      unsigned int i;
>  
>      assert(psi->system_memory);
>  
> -    obj = object_property_get_link(OBJECT(dev), "xics", &err);
> -    if (!obj) {
> -        error_setg(errp, "%s: required link 'xics' not found: %s",
> -                   __func__, error_get_pretty(err));
> -        return;
> -    }
> -
>      /* Create PSI interrupt control source */
> -    object_property_set_link(OBJECT(ics), obj, ICS_PROP_XICS, &error_abort);
>      object_property_set_int(OBJECT(ics), PSI_NUM_INTERRUPTS, "nr-irqs", &err);
>      if (err) {
>          error_propagate(errp, err);

-- 
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 --]

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

* Re: [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip
  2019-12-19 18:11 ` [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip Cédric Le Goater
@ 2019-12-23  4:17   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  4:17 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:46PM +0100, Cédric Le Goater wrote:
> POWER8 is the only chip using the XICS interface. Add a "xics" link
> and a XICSFabric attribute under this chip to remove the use of
> qdev_get_machine()
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

but will need a rebase.

> ---
>  include/hw/ppc/pnv.h |  2 ++
>  hw/ppc/pnv.c         | 26 ++++++++++++++++++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index f31180618672..8b957dfb5736 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -74,6 +74,8 @@ typedef struct Pnv8Chip {
>      Pnv8Psi      psi;
>      PnvOCC       occ;
>      PnvHomer     homer;
> +
> +    XICSFabric    *xics;
>  } Pnv8Chip;
>  
>  #define TYPE_PNV9_CHIP "pnv9-chip"
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 163a658806e2..2a1b15a69aed 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -797,6 +797,13 @@ static void pnv_init(MachineState *machine)
>           */
>          object_property_set_link(chip, OBJECT(sysmem), "system-memory",
>                                   &error_abort);
> +        /*
> +         * The POWER8 machine use the XICS interrupt interface.
> +         * Propagate the XICS fabric to the chip and its controllers.
> +         */
> +        if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
> +            object_property_set_link(chip, OBJECT(pnv), "xics", &error_abort);
> +        }
>          object_property_set_bool(chip, true, "realized", &error_fatal);
>      }
>      g_free(chip_typename);
> @@ -838,12 +845,12 @@ static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, uint32_t core_id)
>  static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>                                          Error **errp)
>  {
> +    Pnv8Chip *chip8 = PNV8_CHIP(chip);
>      Error *local_err = NULL;
>      Object *obj;
>      PnvCPUState *pnv_cpu = pnv_cpu_state(cpu);
>  
> -    obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, XICS_FABRIC(qdev_get_machine()),
> -                     &local_err);
> +    obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, chip8->xics, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -997,6 +1004,12 @@ static void pnv_chip_power8_instance_init(Object *obj)
>  {
>      Pnv8Chip *chip8 = PNV8_CHIP(obj);
>  
> +    object_property_add_link(obj, "xics", TYPE_XICS_FABRIC,
> +                             (Object **)&chip8->xics,
> +                             object_property_allow_set_link,
> +                             OBJ_PROP_LINK_STRONG,
> +                             &error_abort);
> +
>      object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
>                              TYPE_PNV8_PSI, &error_abort, NULL);
>  
> @@ -1016,7 +1029,6 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
>      PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>      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);
> @@ -1032,7 +1044,7 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
>  
>          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));
> +            PnvICPState *icp = PNV_ICP(xics_icp_get(chip8->xics, pir));
>  
>              memory_region_add_subregion(&chip8->icp_mmio, pir << 12,
>                                          &icp->mmio);
> @@ -1048,6 +1060,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      Pnv8Psi *psi8 = &chip8->psi;
>      Error *local_err = NULL;
>  
> +    assert(chip8->xics);
> +
>      /* XSCOM bridge is first */
>      pnv_xscom_realize(chip, PNV_XSCOM_SIZE, &local_err);
>      if (local_err) {
> @@ -1067,8 +1081,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>                              "bar", &error_fatal);
>      object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip->system_memory),
>                               "system-memory", &error_abort);
> -    object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
> -                             ICS_PROP_XICS, &error_abort);
> +    object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip8->xics),
> +                              ICS_PROP_XICS, &error_abort);
>      object_property_set_bool(OBJECT(&chip8->psi), true, "realized", &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);

-- 
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 --]

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

* Re: [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize()
  2019-12-19 18:11 ` [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize() Cédric Le Goater
@ 2019-12-23  4:27   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  4:27 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:47PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> The XIVE router base class currently inherits an empty realize hook
> from the sysbus device base class, but it will soon implement one
> of its own to perform some sanity checks. Do the preliminary plumbing
> to have it called.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-5.0, thanks.

> ---
>  include/hw/ppc/spapr_xive.h | 10 ++++++++++
>  hw/intc/spapr_xive.c        | 12 +++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 3a103c224d44..93d09d68deb7 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -15,6 +15,10 @@
>  
>  #define TYPE_SPAPR_XIVE "spapr-xive"
>  #define SPAPR_XIVE(obj) OBJECT_CHECK(SpaprXive, (obj), TYPE_SPAPR_XIVE)
> +#define SPAPR_XIVE_CLASS(klass)                                         \
> +    OBJECT_CLASS_CHECK(SpaprXiveClass, (klass), TYPE_SPAPR_XIVE)
> +#define SPAPR_XIVE_GET_CLASS(obj)                               \
> +    OBJECT_GET_CLASS(SpaprXiveClass, (obj), TYPE_SPAPR_XIVE)
>  
>  typedef struct SpaprXive {
>      XiveRouter    parent;
> @@ -47,6 +51,12 @@ typedef struct SpaprXive {
>      VMChangeStateEntry *change;
>  } SpaprXive;
>  
> +typedef struct SpaprXiveClass {
> +    XiveRouterClass parent;
> +
> +    DeviceRealize parent_realize;
> +} SpaprXiveClass;
> +
>  /*
>   * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>   * to the controller block id value. It can nevertheless be changed
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 57305c56d707..32322470a8b8 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -286,10 +286,17 @@ static void spapr_xive_instance_init(Object *obj)
>  static void spapr_xive_realize(DeviceState *dev, Error **errp)
>  {
>      SpaprXive *xive = SPAPR_XIVE(dev);
> +    SpaprXiveClass *sxc = SPAPR_XIVE_GET_CLASS(xive);
>      XiveSource *xsrc = &xive->source;
>      XiveENDSource *end_xsrc = &xive->end_source;
>      Error *local_err = NULL;
>  
> +    sxc->parent_realize(dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
>      if (!xive->nr_irqs) {
>          error_setg(errp, "Number of interrupt needs to be greater 0");
>          return;
> @@ -760,10 +767,12 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
>      XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>      SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass);
>      XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
> +    SpaprXiveClass *sxc = SPAPR_XIVE_CLASS(klass);
>  
>      dc->desc    = "sPAPR XIVE Interrupt Controller";
>      dc->props   = spapr_xive_properties;
> -    dc->realize = spapr_xive_realize;
> +    device_class_set_parent_realize(dc, spapr_xive_realize,
> +                                    &sxc->parent_realize);
>      dc->vmsd    = &vmstate_spapr_xive;
>  
>      xrc->get_eas = spapr_xive_get_eas;
> @@ -794,6 +803,7 @@ static const TypeInfo spapr_xive_info = {
>      .instance_init = spapr_xive_instance_init,
>      .instance_size = sizeof(SpaprXive),
>      .class_init = spapr_xive_class_init,
> +    .class_size = sizeof(SpaprXiveClass),
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_SPAPR_INTC },
>          { }

-- 
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 --]

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

* Re: [PATCH v2 06/13] pnv/xive: Use device_class_set_parent_realize()
  2019-12-19 18:11 ` [PATCH v2 06/13] pnv/xive: " Cédric Le Goater
@ 2019-12-23  4:29   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  4:29 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:48PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> The XIVE router base class currently inherits an empty realize hook
> from the sysbus device base class, but it will soon implement one
> of its own to perform some sanity checks. Do the preliminary plumbing
> to have it called.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

But will need a rebase due to changes earlier in the series.

> ---
>  include/hw/ppc/pnv_xive.h | 10 ++++++++++
>  hw/intc/pnv_xive.c        | 10 ++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
> index 4d641db691c8..ba9bbeab88c3 100644
> --- a/include/hw/ppc/pnv_xive.h
> +++ b/include/hw/ppc/pnv_xive.h
> @@ -16,6 +16,10 @@ struct PnvChip;
>  
>  #define TYPE_PNV_XIVE "pnv-xive"
>  #define PNV_XIVE(obj) OBJECT_CHECK(PnvXive, (obj), TYPE_PNV_XIVE)
> +#define PNV_XIVE_CLASS(klass)                                   \
> +    OBJECT_CLASS_CHECK(PnvXiveClass, (klass), TYPE_PNV_XIVE)
> +#define PNV_XIVE_GET_CLASS(obj)                                 \
> +    OBJECT_GET_CLASS(PnvXiveClass, (obj), TYPE_PNV_XIVE)
>  
>  #define XIVE_BLOCK_MAX      16
>  
> @@ -87,6 +91,12 @@ typedef struct PnvXive {
>      uint64_t      edt[XIVE_TABLE_EDT_MAX];
>  } PnvXive;
>  
> +typedef struct PnvXiveClass {
> +    XiveRouterClass parent_class;
> +
> +    DeviceRealize parent_realize;
> +} PnvXiveClass;
> +
>  void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon);
>  
>  #endif /* PPC_PNV_XIVE_H */
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 66970a60733b..1962f884d6de 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1816,10 +1816,17 @@ static void pnv_xive_init(Object *obj)
>  static void pnv_xive_realize(DeviceState *dev, Error **errp)
>  {
>      PnvXive *xive = PNV_XIVE(dev);
> +    PnvXiveClass *pxc = PNV_XIVE_GET_CLASS(dev);
>      XiveSource *xsrc = &xive->ipi_source;
>      XiveENDSource *end_xsrc = &xive->end_source;
>      Error *local_err = NULL;
>  
> +    pxc->parent_realize(dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
>      assert(xive->chip);
>      assert(xive->system_memory);
>  
> @@ -1950,10 +1957,12 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
>      XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>      XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
>      XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
> +    PnvXiveClass *pxc = PNV_XIVE_CLASS(klass);
>  
>      xdc->dt_xscom = pnv_xive_dt_xscom;
>  
>      dc->desc = "PowerNV XIVE Interrupt Controller";
> +    device_class_set_parent_realize(dc, pnv_xive_realize, &pxc->parent_realize);
>      dc->realize = pnv_xive_realize;
>      dc->props = pnv_xive_properties;
>  
> @@ -1974,6 +1983,7 @@ static const TypeInfo pnv_xive_info = {
>      .instance_init = pnv_xive_init,
>      .instance_size = sizeof(PnvXive),
>      .class_init    = pnv_xive_class_init,
> +    .class_size    = sizeof(PnvXiveClass),
>      .interfaces    = (InterfaceInfo[]) {
>          { TYPE_PNV_XSCOM_INTERFACE },
>          { }

-- 
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 --]

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

* Re: [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router
  2019-12-19 18:11 ` [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router Cédric Le Goater
@ 2019-12-23  6:10   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  6:10 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:49PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> In order to get rid of qdev_get_machine(), first add a pointer to the
> XIVE fabric under the XIVE router and make it configurable through a
> QOM link property.
> 
> Configure it in the spapr and pnv machine. In the case of pnv, the XIVE
> routers are under the chip, so this is done with a QOM alias property of
> the POWER9 pnv chip.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

LGTM, but will need rebase.

> ---
>  include/hw/ppc/xive.h | 5 +++--
>  hw/intc/xive.c        | 8 ++++++++
>  hw/ppc/pnv.c          | 6 ++++++
>  hw/ppc/spapr_irq.c    | 2 ++
>  4 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 1b7b89098f71..1ded82b1cda8 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -324,9 +324,12 @@ typedef struct XiveTCTX {
>  /*
>   * XIVE Router
>   */
> +typedef struct XiveFabric XiveFabric;
>  
>  typedef struct XiveRouter {
>      SysBusDevice    parent;
> +
> +    XiveFabric *xfb;
>  } XiveRouter;
>  
>  #define TYPE_XIVE_ROUTER "xive-router"
> @@ -402,8 +405,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>   * XIVE Fabric (Interface between Interrupt Controller and Machine)
>   */
>  
> -typedef struct XiveFabric XiveFabric;
> -
>  #define TYPE_XIVE_FABRIC "xive-fabric"
>  #define XIVE_FABRIC(obj)                                     \
>      INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index d4c6e21703b3..6df89b06da38 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1714,12 +1714,19 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>                             xive_get_field64(EAS_END_DATA,  eas.w));
>  }
>  
> +static Property xive_router_properties[] = {
> +    DEFINE_PROP_LINK("xive-fabric", XiveRouter, xfb,
> +                     TYPE_XIVE_FABRIC, XiveFabric *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void xive_router_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
>  
>      dc->desc    = "XIVE Router Engine";
> +    dc->props   = xive_router_properties;
>      xnc->notify = xive_router_notify;
>  }
>  
> @@ -1727,6 +1734,7 @@ static const TypeInfo xive_router_info = {
>      .name          = TYPE_XIVE_ROUTER,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .abstract      = true,
> +    .instance_size = sizeof(XiveRouter),
>      .class_size    = sizeof(XiveRouterClass),
>      .class_init    = xive_router_class_init,
>      .interfaces    = (InterfaceInfo[]) {
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 2a1b15a69aed..915c80a24b3e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -804,6 +804,10 @@ static void pnv_init(MachineState *machine)
>          if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
>              object_property_set_link(chip, OBJECT(pnv), "xics", &error_abort);
>          }
> +        if (object_dynamic_cast(OBJECT(pnv), TYPE_XIVE_FABRIC)) {
> +            object_property_set_link(chip, OBJECT(pnv), "xive-fabric",
> +                                     &error_abort);
> +        }
>          object_property_set_bool(chip, true, "realized", &error_fatal);
>      }
>      g_free(chip_typename);
> @@ -1224,6 +1228,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
>  
>      object_initialize_child(obj, "xive", &chip9->xive, sizeof(chip9->xive),
>                              TYPE_PNV_XIVE, &error_abort, NULL);
> +    object_property_add_alias(obj, "xive-fabric", OBJECT(&chip9->xive),
> +                              "xive-fabric", &error_abort);
>  
>      object_initialize_child(obj, "psi",  &chip9->psi, sizeof(chip9->psi),
>                              TYPE_PNV9_PSI, &error_abort, NULL);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 07e08d6544a0..2b656649ad6a 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -340,6 +340,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>           * priority
>           */
>          qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
> +        object_property_set_link(OBJECT(dev), OBJECT(spapr), "xive-fabric",
> +                                 &error_abort);
>          qdev_init_nofail(dev);
>  
>          spapr->xive = SPAPR_XIVE(dev);

-- 
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 --]

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

* Re: [PATCH v2 08/13] xive: Use the XIVE fabric link under the XIVE router
  2019-12-19 18:11 ` [PATCH v2 08/13] xive: Use the XIVE fabric link under " Cédric Le Goater
@ 2019-12-23  6:11   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  6:11 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:50PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> Now that the spapr and pnv machines do set the "xive-fabric" link, the
> use of the XIVE fabric pointer becomes mandatory. This is checked with
> an assert() in a new realize hook. Since the XIVE router is realized at
> machine init for the all the machine's life time, no risk to abort an
> already running guest (ie. not a hotplug path).
> 
> This gets rid of a qdev_get_machine() call.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

LGTM, but will need rebase.

> ---
>  hw/intc/xive.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 6df89b06da38..12a362b681a6 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1378,6 +1378,13 @@ static int xive_router_get_block_id(XiveRouter *xrtr)
>     return xrc->get_block_id(xrtr);
>  }
>  
> +static void xive_router_realize(DeviceState *dev, Error **errp)
> +{
> +    XiveRouter *xrtr = XIVE_ROUTER(dev);
> +
> +    assert(xrtr->xfb);
> +}
> +
>  /*
>   * Encode the HW CAM line in the block group mode format :
>   *
> @@ -1470,12 +1477,11 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>   *
>   * The parameters represent what is sent on the PowerBus
>   */
> -static bool xive_presenter_notify(uint8_t format,
> +static bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>                                    uint8_t nvt_blk, uint32_t nvt_idx,
>                                    bool cam_ignore, uint8_t priority,
>                                    uint32_t logic_serv)
>  {
> -    XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine());
>      XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb);
>      XiveTCTXMatch match = { .tctx = NULL, .ring = 0 };
>      int count;
> @@ -1607,7 +1613,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>          return;
>      }
>  
> -    found = xive_presenter_notify(format, nvt_blk, nvt_idx,
> +    found = xive_presenter_notify(xrtr->xfb, format, nvt_blk, nvt_idx,
>                            xive_get_field32(END_W7_F0_IGNORE, end.w7),
>                            priority,
>                            xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
> @@ -1727,6 +1733,8 @@ static void xive_router_class_init(ObjectClass *klass, void *data)
>  
>      dc->desc    = "XIVE Router Engine";
>      dc->props   = xive_router_properties;
> +    /* Parent is SysBusDeviceClass. No need to call its realize hook */
> +    dc->realize = xive_router_realize;
>      xnc->notify = xive_router_notify;
>  }
>  

-- 
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 --]

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

* Re: [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class
  2019-12-19 18:11 ` [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class Cédric Le Goater
@ 2019-12-23  6:12   ` David Gibson
  0 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  6:12 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:51PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> Set it at chip creation and forward it to the cores. This allows to drop
> a call to qdev_get_machine().
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

But will need rebase.

> ---
>  include/hw/ppc/pnv.h | 1 +
>  hw/ppc/pnv.c         | 8 +++++---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 8b957dfb5736..4c13d4394a11 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -48,6 +48,7 @@ typedef struct PnvChip {
>      uint64_t     ram_size;
>  
>      uint32_t     nr_cores;
> +    uint32_t     nr_threads;
>      uint64_t     cores_mask;
>      PnvCore      **cores;
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 915c80a24b3e..e638cdc93091 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -791,6 +791,8 @@ static void pnv_init(MachineState *machine)
>                                  &error_fatal);
>          object_property_set_int(chip, machine->smp.cores,
>                                  "nr-cores", &error_fatal);
> +        object_property_set_int(chip, machine->smp.threads,
> +                                "nr-threads", &error_fatal);
>          /*
>           * TODO: Only the MMIO range should be of interest for the
>           * controllers
> @@ -1529,7 +1531,6 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
>  
>  static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>  {
> -    MachineState *ms = MACHINE(qdev_get_machine());
>      Error *error = NULL;
>      PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>      const char *typename = pnv_chip_core_typename(chip);
> @@ -1565,8 +1566,8 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>          object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
>                                    &error_abort);
>          chip->cores[i] = pnv_core;
> -        object_property_set_int(OBJECT(pnv_core), ms->smp.threads, "nr-threads",
> -                                &error_fatal);
> +        object_property_set_int(OBJECT(pnv_core), chip->nr_threads,
> +                                "nr-threads", &error_fatal);
>          object_property_set_int(OBJECT(pnv_core), core_hwid,
>                                  CPU_CORE_PROP_CORE_ID, &error_fatal);
>          object_property_set_int(OBJECT(pnv_core),
> @@ -1609,6 +1610,7 @@ static Property pnv_chip_properties[] = {
>      DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
>      DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
>                       TYPE_MEMORY_REGION, MemoryRegion *),
> +    DEFINE_PROP_UINT32("nr-threads", PnvChip, nr_threads, 1),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  

-- 
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 --]

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

* Re: [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory()
  2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
                   ` (12 preceding siblings ...)
  2019-12-19 18:11 ` [PATCH v2 13/13] pnv/xive: Deduce the PnvXive " Cédric Le Goater
@ 2019-12-23  6:16 ` David Gibson
  13 siblings, 0 replies; 26+ messages in thread
From: David Gibson @ 2019-12-23  6:16 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel

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

On Thu, Dec 19, 2019 at 07:11:42PM +0100, Cédric Le Goater wrote:
> Hello,
> 
> The PowerNV and sPAPR machine use qdev_get_machine() and
> get_system_memory() in some places. This is not a good modeling
> pratice and it should be avoided. This series replaces the uses of
> these routines with a set of QOM properties and aliases.

Remaining patches LGTM, but will need a rebase to account for the
changes suggested in some of the earlier ones.

-- 
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 --]

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

end of thread, other threads:[~2019-12-23  6:17 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 18:11 [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() Cédric Le Goater
2019-12-19 18:11 ` [PATCH v2 01/13] ppc/pnv: Modify the powerdown notifier to get the PowerNV machine Cédric Le Goater
2019-12-20  0:26   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 02/13] ppc/pnv: Introduce a "system-memory" property Cédric Le Goater
2019-12-19 18:56   ` Greg Kurz
2019-12-20  0:31   ` David Gibson
2019-12-20  6:49     ` Cédric Le Goater
2019-12-19 18:11 ` [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model Cédric Le Goater
2019-12-22  9:33   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip Cédric Le Goater
2019-12-23  4:17   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize() Cédric Le Goater
2019-12-23  4:27   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 06/13] pnv/xive: " Cédric Le Goater
2019-12-23  4:29   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router Cédric Le Goater
2019-12-23  6:10   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 08/13] xive: Use the XIVE fabric link under " Cédric Le Goater
2019-12-23  6:11   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class Cédric Le Goater
2019-12-23  6:12   ` David Gibson
2019-12-19 18:11 ` [PATCH v2 10/13] ppc/pnv: Add a "pnor" const link property to the BMC internal simulator Cédric Le Goater
2019-12-19 18:11 ` [PATCH v2 11/13] xive: Add a "presenter" link property to the TCTX object Cédric Le Goater
2019-12-19 18:11 ` [PATCH v2 12/13] spapr/xive: Deduce the SpaprXive pointer from XiveTCTX::xptr Cédric Le Goater
2019-12-19 18:11 ` [PATCH v2 13/13] pnv/xive: Deduce the PnvXive " Cédric Le Goater
2019-12-23  6:16 ` [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory() David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).