[RFC PATCH 5/5] hw/ppc/e500: Pass array of CPUs as array of canonical QOM paths

[Philippe Mathieu-Daudé]

Devices should avoid calling qemu_get_cpu() because this call
doesn't work as expected with heterogeneous machines. Such
devices often iterate over a cluster of CPUs, which the device's
parent has direct access (when creating the child device).

We can pass QOM as 'link' between objects, but we can't pass an
array of links. Here we exploits QAPI simplicity, by using
DEFINE_PROP_ARRAY and a list of strings, each string being the
CPU canonical path in QOM tree (which is constant and unique).
When the device realizes itself, the original CPU pointer is
recovered via a object_resolve_path() call.

Inspired-by: Peter Maydell <peter.maydell@linaro.org>
Inspired-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Tested with:
$ make check-qtest-ppc{,64}
$ make check-avocado AVOCADO_TAGS='machine:ppce500 machine:mpc8544ds'

RFC: See cover

FIXME: Should we free spin_cpu_list using g_autoptr(QList)?
---
 hw/ppc/e500.c         |  6 ++++++
 hw/ppc/ppce500_spin.c | 48 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index e38f46df38..8b31143dca 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -18,6 +18,7 @@
 #include "qemu/datadir.h"
 #include "qemu/units.h"
 #include "qemu/guest-random.h"
+#include "qapi/qmp/qlist.h"
 #include "qapi/error.h"
 #include "e500.h"
 #include "e500-ccsr.h"
@@ -930,11 +931,13 @@ void ppce500_init(MachineState *machine)
     SysBusDevice *s;
     PPCE500CCSRState *ccsr;
     I2CBus *i2c;
+    QList *spin_cpu_list = qlist_new();
 
     irqs = g_new0(IrqLines, smp_cpus);
     for (i = 0; i < smp_cpus; i++) {
         PowerPCCPU *cpu;
         CPUState *cs;
+        g_autofree char *cpu_qompath;
 
         cpu = POWERPC_CPU(object_new(machine->cpu_type));
         env = &cpu->env;
@@ -954,6 +957,8 @@ void ppce500_init(MachineState *machine)
         object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
                                  &error_fatal);
         qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
+        cpu_qompath = object_get_canonical_path(OBJECT(cs));
+        qlist_append_str(spin_cpu_list, cpu_qompath);
 
         if (!firstenv) {
             firstenv = env;
@@ -1083,6 +1088,7 @@ void ppce500_init(MachineState *machine)
 
     /* Register spinning region */
     dev = qdev_new("e500-spin");
+    qdev_prop_set_array(dev, "cpus-qom-path", spin_cpu_list);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, pmc->spin_base);
 
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index e3608d8c16..a67046b2ea 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -30,11 +30,13 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qemu/units.h"
+#include "qapi/error.h"
 #include "hw/hw.h"
 #include "hw/sysbus.h"
 #include "sysemu/hw_accel.h"
 #include "e500.h"
 #include "qom/object.h"
+#include "hw/qdev-properties.h"
 
 #define MAX_CPUS 32
 
@@ -46,6 +48,10 @@ typedef struct spin_info {
     uint64_t reserved;
 } QEMU_PACKED SpinInfo;
 
+/*
+ * QEMU interface:
+ *  + QOM array property "cpus-qom-path": QOM canonical path of each CPU.
+ */
 #define TYPE_E500_SPIN "e500-spin"
 OBJECT_DECLARE_SIMPLE_TYPE(SpinState, E500_SPIN)
 
@@ -54,6 +60,9 @@ struct SpinState {
 
     MemoryRegion iomem;
     SpinInfo spin[MAX_CPUS];
+    uint32_t cpu_count;
+    char **cpu_canonical_path;
+    CPUState **cpu;
 };
 
 static void spin_reset(DeviceState *dev)
@@ -121,16 +130,10 @@ static void spin_write(void *opaque, hwaddr addr, 
uint64_t value,
 {
     SpinState *s = opaque;
     int env_idx = addr / sizeof(SpinInfo);
-    CPUState *cpu;
+    CPUState *cpu = s->cpu[env_idx];
     SpinInfo *curspin = &s->spin[env_idx];
     uint8_t *curspin_p = (uint8_t*)curspin;
 
-    cpu = qemu_get_cpu(env_idx);
-    if (cpu == NULL) {
-        /* Unknown CPU */
-        return;
-    }
-
     if (cpu->cpu_index == 0) {
         /* primary CPU doesn't spin */
         return;
@@ -188,11 +191,42 @@ static void ppce500_spin_initfn(Object *obj)
     sysbus_init_mmio(dev, &s->iomem);
 }
 
+static void ppce500_spin_realize(DeviceState *dev, Error **errp)
+{
+    SpinState *s = E500_SPIN(dev);
+
+    if (s->cpu_count == 0) {
+        error_setg(errp, "'cpus-qom-path' property array must be set");
+        return;
+    } else if (s->cpu_count > MAX_CPUS) {
+        error_setg(errp, "at most %d CPUs are supported", MAX_CPUS);
+        return;
+    }
+
+    s->cpu = g_new(CPUState *, s->cpu_count);
+    for (unsigned i = 0; i < s->cpu_count; i++) {
+        bool ambiguous;
+        Object *obj;
+
+        obj = object_resolve_path(s->cpu_canonical_path[i], &ambiguous);
+        assert(!ambiguous);
+        s->cpu[i] = CPU(obj);
+    }
+}
+
+static Property ppce500_spin_properties[] = {
+    DEFINE_PROP_ARRAY("cpus-qom-path", SpinState, cpu_count,
+                      cpu_canonical_path, qdev_prop_string, char *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ppce500_spin_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->reset = spin_reset;
+    dc->realize = ppce500_spin_realize;
+    device_class_set_props(dc, ppce500_spin_properties);
 }
 
 static const TypeInfo ppce500_spin_types[] = {
-- 
2.41.0