[RFC PATCH 00/18] user-creatable cpu clusters

[RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

Hi,

This series add devices to be able to user-create (coldplug) cpu
clusters. The existing cpu cluster dictates how cpus are exposed
in gdb, but it does not handle the cpu objects creation. This series
adds a new device to handle both issues and adds support for two
architectures: arm and riscv.

Please look at patches 2 and 3 for more details about the new device.

Last part concerning the riscv is rfc as I do non-backward compatible
updates. I'm not sure what migration (or other) constraints we have
on these machines and I probably need to make some changes to cope
with them.

This series almost deprecates the cpu-cluster type as all uses
but one are replaced.

It is organized as follows:

+ Patches 1 to 7 adds a new base device to replace cpu-cluster

+ Patches 8 and 9 adds an arm specific version and replace existing
  clusters in the xlnx-zynqmp machine.

+ patches 10 to 17 updates the riscv_array. It was already used to
  create cpus but was not a cpu cluster.

Thanks for any comments,
--
Damien

Damien Hedde (18):
  define MAX_CLUSTERS in cpu.h instead of cluster.h
  hw/cpu/cpus: introduce _cpus_ device
  hw/cpu/cpus: prepare to handle cpu clusters
  hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_
  gdbstub: deal with _cpus_ object instead of _cpu-cluster_
  hw/cpu/cluster: remove cluster_id now that gdbstub is updated
  hw/cpu/cpus: add a common start-powered-off property
  hw/arm/arm_cpus: add arm_cpus device
  hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus
  hw/riscv/riscv_hart: prepare transition to cpus
  hw/riscv: prepare riscv_hart transition to cpus
  hw/riscv/virt: prepare riscv_hart transition to cpus
  hw/riscv/spike: prepare riscv_hart transition to cpus
  hw/riscv/riscv_hart: use cpus as base class
  hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update
  hw/riscv: update remaining machines due to riscv_hart_array update
  hw/riscv/riscv_hart: remove temporary features
  add myself as reviewer of the newly added _cpus_

 include/hw/arm/arm_cpus.h          |  45 +++++++
 include/hw/arm/xlnx-zynqmp.h       |   8 +-
 include/hw/core/cpu.h              |   6 +
 include/hw/cpu/cluster.h           |  26 ++--
 include/hw/cpu/cpus.h              |  93 ++++++++++++++
 include/hw/riscv/microchip_pfsoc.h |   2 -
 include/hw/riscv/riscv_hart.h      |  25 +++-
 include/hw/riscv/sifive_u.h        |   2 -
 gdbstub.c                          |  12 +-
 hw/arm/arm_cpus.c                  |  63 ++++++++++
 hw/arm/xlnx-zynqmp.c               | 121 +++++++-----------
 hw/cpu/cluster.c                   |  53 ++++----
 hw/cpu/cpus.c                      | 195 +++++++++++++++++++++++++++++
 hw/riscv/boot.c                    |   2 +-
 hw/riscv/microchip_pfsoc.c         |  28 +----
 hw/riscv/opentitan.c               |   4 +-
 hw/riscv/riscv_hart.c              |  44 ++-----
 hw/riscv/shakti_c.c                |   4 +-
 hw/riscv/sifive_e.c                |   4 +-
 hw/riscv/sifive_u.c                |  31 ++---
 hw/riscv/spike.c                   |  18 +--
 hw/riscv/virt.c                    |  79 +++++++-----
 MAINTAINERS                        |   3 +
 hw/arm/meson.build                 |   1 +
 hw/cpu/meson.build                 |   2 +-
 25 files changed, 612 insertions(+), 259 deletions(-)
 create mode 100644 include/hw/arm/arm_cpus.h
 create mode 100644 include/hw/cpu/cpus.h
 create mode 100644 hw/arm/arm_cpus.c
 create mode 100644 hw/cpu/cpus.c

-- 
2.35.1

[RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

Hi,

This series add devices to be able to user-create (coldplug) cpu
clusters. The existing cpu cluster dictates how cpus are exposed
in gdb, but it does not handle the cpu objects creation. This series
adds a new device to handle both issues and adds support for two
architectures: arm and riscv.

Please look at patches 2 and 3 for more details about the new device.

Last part concerning the riscv is rfc as I do non-backward compatible
updates. I'm not sure what migration (or other) constraints we have
on these machines and I probably need to make some changes to cope
with them.

This series almost deprecates the cpu-cluster type as all uses
but one are replaced.

It is organized as follows:

+ Patches 1 to 7 adds a new base device to replace cpu-cluster

+ Patches 8 and 9 adds an arm specific version and replace existing
  clusters in the xlnx-zynqmp machine.

+ patches 10 to 17 updates the riscv_array. It was already used to
  create cpus but was not a cpu cluster.

Thanks for any comments,
--
Damien

Damien Hedde (18):
  define MAX_CLUSTERS in cpu.h instead of cluster.h
  hw/cpu/cpus: introduce _cpus_ device
  hw/cpu/cpus: prepare to handle cpu clusters
  hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_
  gdbstub: deal with _cpus_ object instead of _cpu-cluster_
  hw/cpu/cluster: remove cluster_id now that gdbstub is updated
  hw/cpu/cpus: add a common start-powered-off property
  hw/arm/arm_cpus: add arm_cpus device
  hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus
  hw/riscv/riscv_hart: prepare transition to cpus
  hw/riscv: prepare riscv_hart transition to cpus
  hw/riscv/virt: prepare riscv_hart transition to cpus
  hw/riscv/spike: prepare riscv_hart transition to cpus
  hw/riscv/riscv_hart: use cpus as base class
  hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update
  hw/riscv: update remaining machines due to riscv_hart_array update
  hw/riscv/riscv_hart: remove temporary features
  add myself as reviewer of the newly added _cpus_

 include/hw/arm/arm_cpus.h          |  45 +++++++
 include/hw/arm/xlnx-zynqmp.h       |   8 +-
 include/hw/core/cpu.h              |   6 +
 include/hw/cpu/cluster.h           |  26 ++--
 include/hw/cpu/cpus.h              |  93 ++++++++++++++
 include/hw/riscv/microchip_pfsoc.h |   2 -
 include/hw/riscv/riscv_hart.h      |  25 +++-
 include/hw/riscv/sifive_u.h        |   2 -
 gdbstub.c                          |  12 +-
 hw/arm/arm_cpus.c                  |  63 ++++++++++
 hw/arm/xlnx-zynqmp.c               | 121 +++++++-----------
 hw/cpu/cluster.c                   |  53 ++++----
 hw/cpu/cpus.c                      | 195 +++++++++++++++++++++++++++++
 hw/riscv/boot.c                    |   2 +-
 hw/riscv/microchip_pfsoc.c         |  28 +----
 hw/riscv/opentitan.c               |   4 +-
 hw/riscv/riscv_hart.c              |  44 ++-----
 hw/riscv/shakti_c.c                |   4 +-
 hw/riscv/sifive_e.c                |   4 +-
 hw/riscv/sifive_u.c                |  31 ++---
 hw/riscv/spike.c                   |  18 +--
 hw/riscv/virt.c                    |  79 +++++++-----
 MAINTAINERS                        |   3 +
 hw/arm/meson.build                 |   1 +
 hw/cpu/meson.build                 |   2 +-
 25 files changed, 612 insertions(+), 259 deletions(-)
 create mode 100644 include/hw/arm/arm_cpus.h
 create mode 100644 include/hw/cpu/cpus.h
 create mode 100644 hw/arm/arm_cpus.c
 create mode 100644 hw/cpu/cpus.c

-- 
2.35.1

[RFC PATCH 01/18] define MAX_CLUSTERS in cpu.h instead of cluster.h

[Damien Hedde]

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/core/cpu.h    | 6 ++++++
 include/hw/cpu/cluster.h | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 136973655c..38c72cbda1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1054,5 +1054,11 @@ extern const VMStateDescription vmstate_cpu_common;
 
 #define UNASSIGNED_CPU_INDEX -1
 #define UNASSIGNED_CLUSTER_INDEX -1
+/*
+ * This limit is imposed by TCG, which puts the cluster ID into an
+ * 8 bit field. Default being all-1s, custom index cannot be set to
+ * more than 254.
+ */
+#define MAX_CLUSTERS 255
 
 #endif
diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 53fbf36af5..09a2b86832 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -57,12 +57,6 @@
 #define TYPE_CPU_CLUSTER "cpu-cluster"
 OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
 
-/*
- * This limit is imposed by TCG, which puts the cluster ID into an
- * 8 bit field (and uses all-1s for the default "not in any cluster").
- */
-#define MAX_CLUSTERS 255
-
 /**
  * CPUClusterState:
  * @cluster_id: The cluster ID. This value is for internal use only and should
-- 
2.35.1

[RFC PATCH 01/18] define MAX_CLUSTERS in cpu.h instead of cluster.h

[Damien Hedde]

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/core/cpu.h    | 6 ++++++
 include/hw/cpu/cluster.h | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 136973655c..38c72cbda1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1054,5 +1054,11 @@ extern const VMStateDescription vmstate_cpu_common;
 
 #define UNASSIGNED_CPU_INDEX -1
 #define UNASSIGNED_CLUSTER_INDEX -1
+/*
+ * This limit is imposed by TCG, which puts the cluster ID into an
+ * 8 bit field. Default being all-1s, custom index cannot be set to
+ * more than 254.
+ */
+#define MAX_CLUSTERS 255
 
 #endif
diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 53fbf36af5..09a2b86832 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -57,12 +57,6 @@
 #define TYPE_CPU_CLUSTER "cpu-cluster"
 OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
 
-/*
- * This limit is imposed by TCG, which puts the cluster ID into an
- * 8 bit field (and uses all-1s for the default "not in any cluster").
- */
-#define MAX_CLUSTERS 255
-
 /**
  * CPUClusterState:
  * @cluster_id: The cluster ID. This value is for internal use only and should
-- 
2.35.1

[RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Damien Hedde]

This object will be a _cpu-cluster_ generalization and
is meant to allow create cpus of the same type.

The main goal is that this object, on contrary to _cpu-cluster-_,
can be used to dynamically create cpus: it does not rely on
external code to populate the object with cpus.

Allowing the user to create a cpu cluster and each _cpu_
separately would be hard because of the following reasons:
+ cpu reset need to be handled
+ instantiation and realize of cpu-cluster and the cpus
  are interleaved
+ cpu cluster must contains only identical cpus and it seems
  difficult to check that at runtime.
Therefore we add a new type solving all this constraints.

_cpu-cluster_ will be updated to inherit from this class
in following commits.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
 hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
 hw/cpu/meson.build    |   2 +-
 3 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/cpu/cpus.h
 create mode 100644 hw/cpu/cpus.c

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
new file mode 100644
index 0000000000..c65f568ef8
--- /dev/null
+++ b/include/hw/cpu/cpus.h
@@ -0,0 +1,71 @@
+/*
+ * QEMU CPUs type
+ *
+ * Copyright (c) 2022 GreenSocs
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_CPU_CPUS_H
+#define HW_CPU_CPUS_H
+
+#include "qemu/typedefs.h"
+#include "hw/qdev-core.h"
+#include "qom/object.h"
+
+/*
+ * This object represent several CPUs which are all identical.
+ *
+ * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 CPUs in an
+ * Arm big.LITTLE system) they should be in different groups. If the CPUs do
+ * not have the same view of memory (for example the main CPU and a management
+ * controller processor) they should be in different groups.
+ *
+ * This is an abstract class, subclasses are supposed to be created on
+ * per-architecture basis to handle the specifics of the cpu architecture.
+ * Subclasses are meant to be user-creatable (for cold-plug).
+ */
+
+#define TYPE_CPUS "cpus"
+OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
+
+/**
+ * CpusState:
+ * @cpu_type: The type of cpu.
+ * @topology.cpus: The number of cpus in this group.
+ *      Explicity put this field into a topology structure in
+ *      order to eventually update this smoothly with a full
+ *      CpuTopology structure in the future.
+ * @cpus: Array of pointer to cpu objects.
+ */
+struct CpusState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    char *cpu_type;
+    struct {
+        uint16_t cpus;
+    } topology;
+    CPUState **cpus;
+};
+
+typedef void (*CpusConfigureCpu)(CpusState *s, CPUState *cpu, unsigned idx);
+
+/**
+ * CpusClass:
+ * @base_cpu_type: base cpu type accepted by this cpu group
+ *     (the state cpu_type will be tested against it).
+ * @configure_cpu: method to configure a cpu (called between
+ *     cpu init and realize)
+ * @skip_cpus_creation: CPUCLuster do not rely on creating
+ *     cpus internally. This flag disables this feature.
+ */
+struct CpusClass {
+    DeviceClass parent_class;
+    const char *base_cpu_type;
+    CpusConfigureCpu configure_cpu;
+    bool skip_cpus_creation;
+};
+
+#endif /* HW_CPU_CPUS_H */
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
new file mode 100644
index 0000000000..5fad1de2c7
--- /dev/null
+++ b/hw/cpu/cpus.c
@@ -0,0 +1,127 @@
+/*
+ * QEMU CPUs type
+ *
+ * Copyright (c) 2022 GreenSocs
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qemu/cutils.h"
+#include "hw/cpu/cpus.h"
+#include "hw/core/cpu.h"
+#include "hw/resettable.h"
+#include "sysemu/reset.h"
+
+static Property cpus_properties[] = {
+    DEFINE_PROP_STRING("cpu-type", CpusState, cpu_type),
+    DEFINE_PROP_UINT16("num-cpus", CpusState, topology.cpus, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpus_reset(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+    for (unsigned i = 0; i < s->topology.cpus; i++) {
+        cpu_reset(s->cpus[i]);
+    }
+}
+
+static void cpus_create_cpus(CpusState *s, Error **errp)
+{
+    Error *err = NULL;
+    CpusClass *cgc = CPUS_GET_CLASS(s);
+    s->cpus = g_new0(CPUState *, s->topology.cpus);
+
+    for (unsigned i = 0; i < s->topology.cpus; i++) {
+        CPUState *cpu = CPU(object_new(s->cpu_type));
+        s->cpus[i] = cpu;
+
+        /* set child property and release the initial ref */
+        object_property_add_child(OBJECT(s), "cpu[*]", OBJECT(cpu));
+        object_unref(OBJECT(cpu));
+
+        /* let subclass configure the cpu */
+        if (cgc->configure_cpu) {
+            cgc->configure_cpu(s, cpu, i);
+        }
+
+        /* finally realize the cpu */
+        qdev_realize(DEVICE(cpu), NULL, &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
+}
+
+static void cpus_realize(DeviceState *dev, Error **errp)
+{
+    CpusState *s = CPUS(dev);
+    CpusClass *cgc = CPUS_GET_CLASS(s);
+
+    /* if subclass defined a base type, let's check it */
+    if (cgc->base_cpu_type &&
+        !object_class_dynamic_cast(object_class_by_name(s->cpu_type),
+                                   cgc->base_cpu_type)) {
+        error_setg(errp, "bad cpu-type '%s' (expected '%s')", s->cpu_type,
+                   cgc->base_cpu_type);
+        return;
+    }
+
+    if (s->topology.cpus == 0) {
+        error_setg(errp, "num-cpus is zero");
+        return;
+    }
+
+    /* create the cpus if needed */
+    if (!cgc->skip_cpus_creation) {
+        cpus_create_cpus(s, errp);
+        qemu_register_reset(resettable_cold_reset_fn, s);
+    }
+}
+
+static void cpus_finalize(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+
+    g_free(s->cpus);
+}
+
+static void cpus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    device_class_set_props(dc, cpus_properties);
+    dc->realize = cpus_realize;
+
+    rc->phases.exit = cpus_reset;
+
+    /*
+     * Subclasses are expected to be user-creatable.
+     * They may provide support to hotplug cpus, but they are
+     * not expected to be hotpluggable themselves.
+     */
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo cpus_type_info = {
+    .name              = TYPE_CPUS,
+    .parent            = TYPE_DEVICE,
+    .abstract          = true,
+    .instance_size     = sizeof(CpusState),
+    .instance_finalize = cpus_finalize,
+    .class_size        = sizeof(CpusClass),
+    .class_init        = cpus_class_init,
+};
+
+static void cpus_register_types(void)
+{
+    type_register_static(&cpus_type_info);
+}
+
+type_init(cpus_register_types)
diff --git a/hw/cpu/meson.build b/hw/cpu/meson.build
index 9e52fee9e7..ca4dda4f88 100644
--- a/hw/cpu/meson.build
+++ b/hw/cpu/meson.build
@@ -1,4 +1,4 @@
-softmmu_ss.add(files('core.c', 'cluster.c'))
+softmmu_ss.add(files('core.c', 'cluster.c', 'cpus.c'))
 
 specific_ss.add(when: 'CONFIG_ARM11MPCORE', if_true: files('arm11mpcore.c'))
 specific_ss.add(when: 'CONFIG_REALVIEW', if_true: files('realview_mpcore.c'))
-- 
2.35.1

[RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Damien Hedde]

This object will be a _cpu-cluster_ generalization and
is meant to allow create cpus of the same type.

The main goal is that this object, on contrary to _cpu-cluster-_,
can be used to dynamically create cpus: it does not rely on
external code to populate the object with cpus.

Allowing the user to create a cpu cluster and each _cpu_
separately would be hard because of the following reasons:
+ cpu reset need to be handled
+ instantiation and realize of cpu-cluster and the cpus
  are interleaved
+ cpu cluster must contains only identical cpus and it seems
  difficult to check that at runtime.
Therefore we add a new type solving all this constraints.

_cpu-cluster_ will be updated to inherit from this class
in following commits.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
 hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
 hw/cpu/meson.build    |   2 +-
 3 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/cpu/cpus.h
 create mode 100644 hw/cpu/cpus.c

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
new file mode 100644
index 0000000000..c65f568ef8
--- /dev/null
+++ b/include/hw/cpu/cpus.h
@@ -0,0 +1,71 @@
+/*
+ * QEMU CPUs type
+ *
+ * Copyright (c) 2022 GreenSocs
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_CPU_CPUS_H
+#define HW_CPU_CPUS_H
+
+#include "qemu/typedefs.h"
+#include "hw/qdev-core.h"
+#include "qom/object.h"
+
+/*
+ * This object represent several CPUs which are all identical.
+ *
+ * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 CPUs in an
+ * Arm big.LITTLE system) they should be in different groups. If the CPUs do
+ * not have the same view of memory (for example the main CPU and a management
+ * controller processor) they should be in different groups.
+ *
+ * This is an abstract class, subclasses are supposed to be created on
+ * per-architecture basis to handle the specifics of the cpu architecture.
+ * Subclasses are meant to be user-creatable (for cold-plug).
+ */
+
+#define TYPE_CPUS "cpus"
+OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
+
+/**
+ * CpusState:
+ * @cpu_type: The type of cpu.
+ * @topology.cpus: The number of cpus in this group.
+ *      Explicity put this field into a topology structure in
+ *      order to eventually update this smoothly with a full
+ *      CpuTopology structure in the future.
+ * @cpus: Array of pointer to cpu objects.
+ */
+struct CpusState {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    char *cpu_type;
+    struct {
+        uint16_t cpus;
+    } topology;
+    CPUState **cpus;
+};
+
+typedef void (*CpusConfigureCpu)(CpusState *s, CPUState *cpu, unsigned idx);
+
+/**
+ * CpusClass:
+ * @base_cpu_type: base cpu type accepted by this cpu group
+ *     (the state cpu_type will be tested against it).
+ * @configure_cpu: method to configure a cpu (called between
+ *     cpu init and realize)
+ * @skip_cpus_creation: CPUCLuster do not rely on creating
+ *     cpus internally. This flag disables this feature.
+ */
+struct CpusClass {
+    DeviceClass parent_class;
+    const char *base_cpu_type;
+    CpusConfigureCpu configure_cpu;
+    bool skip_cpus_creation;
+};
+
+#endif /* HW_CPU_CPUS_H */
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
new file mode 100644
index 0000000000..5fad1de2c7
--- /dev/null
+++ b/hw/cpu/cpus.c
@@ -0,0 +1,127 @@
+/*
+ * QEMU CPUs type
+ *
+ * Copyright (c) 2022 GreenSocs
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qemu/cutils.h"
+#include "hw/cpu/cpus.h"
+#include "hw/core/cpu.h"
+#include "hw/resettable.h"
+#include "sysemu/reset.h"
+
+static Property cpus_properties[] = {
+    DEFINE_PROP_STRING("cpu-type", CpusState, cpu_type),
+    DEFINE_PROP_UINT16("num-cpus", CpusState, topology.cpus, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpus_reset(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+    for (unsigned i = 0; i < s->topology.cpus; i++) {
+        cpu_reset(s->cpus[i]);
+    }
+}
+
+static void cpus_create_cpus(CpusState *s, Error **errp)
+{
+    Error *err = NULL;
+    CpusClass *cgc = CPUS_GET_CLASS(s);
+    s->cpus = g_new0(CPUState *, s->topology.cpus);
+
+    for (unsigned i = 0; i < s->topology.cpus; i++) {
+        CPUState *cpu = CPU(object_new(s->cpu_type));
+        s->cpus[i] = cpu;
+
+        /* set child property and release the initial ref */
+        object_property_add_child(OBJECT(s), "cpu[*]", OBJECT(cpu));
+        object_unref(OBJECT(cpu));
+
+        /* let subclass configure the cpu */
+        if (cgc->configure_cpu) {
+            cgc->configure_cpu(s, cpu, i);
+        }
+
+        /* finally realize the cpu */
+        qdev_realize(DEVICE(cpu), NULL, &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
+}
+
+static void cpus_realize(DeviceState *dev, Error **errp)
+{
+    CpusState *s = CPUS(dev);
+    CpusClass *cgc = CPUS_GET_CLASS(s);
+
+    /* if subclass defined a base type, let's check it */
+    if (cgc->base_cpu_type &&
+        !object_class_dynamic_cast(object_class_by_name(s->cpu_type),
+                                   cgc->base_cpu_type)) {
+        error_setg(errp, "bad cpu-type '%s' (expected '%s')", s->cpu_type,
+                   cgc->base_cpu_type);
+        return;
+    }
+
+    if (s->topology.cpus == 0) {
+        error_setg(errp, "num-cpus is zero");
+        return;
+    }
+
+    /* create the cpus if needed */
+    if (!cgc->skip_cpus_creation) {
+        cpus_create_cpus(s, errp);
+        qemu_register_reset(resettable_cold_reset_fn, s);
+    }
+}
+
+static void cpus_finalize(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+
+    g_free(s->cpus);
+}
+
+static void cpus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    device_class_set_props(dc, cpus_properties);
+    dc->realize = cpus_realize;
+
+    rc->phases.exit = cpus_reset;
+
+    /*
+     * Subclasses are expected to be user-creatable.
+     * They may provide support to hotplug cpus, but they are
+     * not expected to be hotpluggable themselves.
+     */
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo cpus_type_info = {
+    .name              = TYPE_CPUS,
+    .parent            = TYPE_DEVICE,
+    .abstract          = true,
+    .instance_size     = sizeof(CpusState),
+    .instance_finalize = cpus_finalize,
+    .class_size        = sizeof(CpusClass),
+    .class_init        = cpus_class_init,
+};
+
+static void cpus_register_types(void)
+{
+    type_register_static(&cpus_type_info);
+}
+
+type_init(cpus_register_types)
diff --git a/hw/cpu/meson.build b/hw/cpu/meson.build
index 9e52fee9e7..ca4dda4f88 100644
--- a/hw/cpu/meson.build
+++ b/hw/cpu/meson.build
@@ -1,4 +1,4 @@
-softmmu_ss.add(files('core.c', 'cluster.c'))
+softmmu_ss.add(files('core.c', 'cluster.c', 'cpus.c'))
 
 specific_ss.add(when: 'CONFIG_ARM11MPCORE', if_true: files('arm11mpcore.c'))
 specific_ss.add(when: 'CONFIG_REALVIEW', if_true: files('realview_mpcore.c'))
-- 
2.35.1

Re: [RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> This object will be a _cpu-cluster_ generalization and
> is meant to allow create cpus of the same type.
> 
> The main goal is that this object, on contrary to _cpu-cluster-_,
> can be used to dynamically create cpus: it does not rely on
> external code to populate the object with cpus.
> 
> Allowing the user to create a cpu cluster and each _cpu_
> separately would be hard because of the following reasons:
> + cpu reset need to be handled
> + instantiation and realize of cpu-cluster and the cpus
>    are interleaved
> + cpu cluster must contains only identical cpus and it seems
>    difficult to check that at runtime.
> Therefore we add a new type solving all this constraints.
> 
> _cpu-cluster_ will be updated to inherit from this class
> in following commits.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
>   hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
>   hw/cpu/meson.build    |   2 +-
>   3 files changed, 199 insertions(+), 1 deletion(-)
>   create mode 100644 include/hw/cpu/cpus.h
>   create mode 100644 hw/cpu/cpus.c
> 
> diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
> new file mode 100644
> index 0000000000..c65f568ef8
> --- /dev/null
> +++ b/include/hw/cpu/cpus.h
> @@ -0,0 +1,71 @@
> +/*
> + * QEMU CPUs type
> + *
> + * Copyright (c) 2022 GreenSocs
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_CPU_CPUS_H
> +#define HW_CPU_CPUS_H
> +
> +#include "qemu/typedefs.h"
> +#include "hw/qdev-core.h"
> +#include "qom/object.h"
> +
> +/*
> + * This object represent several CPUs which are all identical.

Typo "represents".

> + *
> + * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 CPUs in an
> + * Arm big.LITTLE system) they should be in different groups. If the CPUs do
> + * not have the same view of memory (for example the main CPU and a management
> + * controller processor) they should be in different groups.

This description calls for a clearer CpusGroupState name instead
of CpusState (which confuses me with CPUState). Alternatively
CpusArrayState.

> + *
> + * This is an abstract class, subclasses are supposed to be created on
> + * per-architecture basis to handle the specifics of the cpu architecture.
> + * Subclasses are meant to be user-creatable (for cold-plug).
> + */
> +
> +#define TYPE_CPUS "cpus"
> +OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
> +
> +/**
> + * CpusState:
> + * @cpu_type: The type of cpu.
> + * @topology.cpus: The number of cpus in this group.
> + *      Explicity put this field into a topology structure in
> + *      order to eventually update this smoothly with a full
> + *      CpuTopology structure in the future.
> + * @cpus: Array of pointer to cpu objects.
> + */
> +struct CpusState {
> +    /*< private >*/
> +    DeviceState parent_obj;
> +
> +    /*< public >*/
> +    char *cpu_type;
> +    struct {
> +        uint16_t cpus;
> +    } topology;
> +    CPUState **cpus;
> +};

Re: [RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> This object will be a _cpu-cluster_ generalization and
> is meant to allow create cpus of the same type.
> 
> The main goal is that this object, on contrary to _cpu-cluster-_,
> can be used to dynamically create cpus: it does not rely on
> external code to populate the object with cpus.
> 
> Allowing the user to create a cpu cluster and each _cpu_
> separately would be hard because of the following reasons:
> + cpu reset need to be handled
> + instantiation and realize of cpu-cluster and the cpus
>    are interleaved
> + cpu cluster must contains only identical cpus and it seems
>    difficult to check that at runtime.
> Therefore we add a new type solving all this constraints.
> 
> _cpu-cluster_ will be updated to inherit from this class
> in following commits.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
>   hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
>   hw/cpu/meson.build    |   2 +-
>   3 files changed, 199 insertions(+), 1 deletion(-)
>   create mode 100644 include/hw/cpu/cpus.h
>   create mode 100644 hw/cpu/cpus.c
> 
> diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
> new file mode 100644
> index 0000000000..c65f568ef8
> --- /dev/null
> +++ b/include/hw/cpu/cpus.h
> @@ -0,0 +1,71 @@
> +/*
> + * QEMU CPUs type
> + *
> + * Copyright (c) 2022 GreenSocs
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_CPU_CPUS_H
> +#define HW_CPU_CPUS_H
> +
> +#include "qemu/typedefs.h"
> +#include "hw/qdev-core.h"
> +#include "qom/object.h"
> +
> +/*
> + * This object represent several CPUs which are all identical.

Typo "represents".

> + *
> + * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 CPUs in an
> + * Arm big.LITTLE system) they should be in different groups. If the CPUs do
> + * not have the same view of memory (for example the main CPU and a management
> + * controller processor) they should be in different groups.

This description calls for a clearer CpusGroupState name instead
of CpusState (which confuses me with CPUState). Alternatively
CpusArrayState.

> + *
> + * This is an abstract class, subclasses are supposed to be created on
> + * per-architecture basis to handle the specifics of the cpu architecture.
> + * Subclasses are meant to be user-creatable (for cold-plug).
> + */
> +
> +#define TYPE_CPUS "cpus"
> +OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
> +
> +/**
> + * CpusState:
> + * @cpu_type: The type of cpu.
> + * @topology.cpus: The number of cpus in this group.
> + *      Explicity put this field into a topology structure in
> + *      order to eventually update this smoothly with a full
> + *      CpuTopology structure in the future.
> + * @cpus: Array of pointer to cpu objects.
> + */
> +struct CpusState {
> +    /*< private >*/
> +    DeviceState parent_obj;
> +
> +    /*< public >*/
> +    char *cpu_type;
> +    struct {
> +        uint16_t cpus;
> +    } topology;
> +    CPUState **cpus;
> +};

Re: [RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Damien Hedde]

On 4/16/22 19:52, Philippe Mathieu-Daudé wrote:
> On 30/3/22 14:56, Damien Hedde wrote:
>> This object will be a _cpu-cluster_ generalization and
>> is meant to allow create cpus of the same type.
>>
>> The main goal is that this object, on contrary to _cpu-cluster-_,
>> can be used to dynamically create cpus: it does not rely on
>> external code to populate the object with cpus.
>>
>> Allowing the user to create a cpu cluster and each _cpu_
>> separately would be hard because of the following reasons:
>> + cpu reset need to be handled
>> + instantiation and realize of cpu-cluster and the cpus
>>    are interleaved
>> + cpu cluster must contains only identical cpus and it seems
>>    difficult to check that at runtime.
>> Therefore we add a new type solving all this constraints.
>>
>> _cpu-cluster_ will be updated to inherit from this class
>> in following commits.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>> ---
>>   include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
>>   hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/cpu/meson.build    |   2 +-
>>   3 files changed, 199 insertions(+), 1 deletion(-)
>>   create mode 100644 include/hw/cpu/cpus.h
>>   create mode 100644 hw/cpu/cpus.c
>>
>> diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
>> new file mode 100644
>> index 0000000000..c65f568ef8
>> --- /dev/null
>> +++ b/include/hw/cpu/cpus.h
>> @@ -0,0 +1,71 @@
>> +/*
>> + * QEMU CPUs type
>> + *
>> + * Copyright (c) 2022 GreenSocs
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HW_CPU_CPUS_H
>> +#define HW_CPU_CPUS_H
>> +
>> +#include "qemu/typedefs.h"
>> +#include "hw/qdev-core.h"
>> +#include "qom/object.h"
>> +
>> +/*
>> + * This object represent several CPUs which are all identical.
> 
> Typo "represents".
> 
>> + *
>> + * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 
>> CPUs in an
>> + * Arm big.LITTLE system) they should be in different groups. If the 
>> CPUs do
>> + * not have the same view of memory (for example the main CPU and a 
>> management
>> + * controller processor) they should be in different groups.
> 
> This description calls for a clearer CpusGroupState name instead
> of CpusState (which confuses me with CPUState). Alternatively
> CpusArrayState.

Your are right, I'll add the "group" suffix.

Thanks,
Damien

Re: [RFC PATCH 02/18] hw/cpu/cpus: introduce _cpus_ device

[Damien Hedde]

On 4/16/22 19:52, Philippe Mathieu-Daudé wrote:
> On 30/3/22 14:56, Damien Hedde wrote:
>> This object will be a _cpu-cluster_ generalization and
>> is meant to allow create cpus of the same type.
>>
>> The main goal is that this object, on contrary to _cpu-cluster-_,
>> can be used to dynamically create cpus: it does not rely on
>> external code to populate the object with cpus.
>>
>> Allowing the user to create a cpu cluster and each _cpu_
>> separately would be hard because of the following reasons:
>> + cpu reset need to be handled
>> + instantiation and realize of cpu-cluster and the cpus
>>    are interleaved
>> + cpu cluster must contains only identical cpus and it seems
>>    difficult to check that at runtime.
>> Therefore we add a new type solving all this constraints.
>>
>> _cpu-cluster_ will be updated to inherit from this class
>> in following commits.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>> ---
>>   include/hw/cpu/cpus.h |  71 +++++++++++++++++++++++
>>   hw/cpu/cpus.c         | 127 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/cpu/meson.build    |   2 +-
>>   3 files changed, 199 insertions(+), 1 deletion(-)
>>   create mode 100644 include/hw/cpu/cpus.h
>>   create mode 100644 hw/cpu/cpus.c
>>
>> diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
>> new file mode 100644
>> index 0000000000..c65f568ef8
>> --- /dev/null
>> +++ b/include/hw/cpu/cpus.h
>> @@ -0,0 +1,71 @@
>> +/*
>> + * QEMU CPUs type
>> + *
>> + * Copyright (c) 2022 GreenSocs
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HW_CPU_CPUS_H
>> +#define HW_CPU_CPUS_H
>> +
>> +#include "qemu/typedefs.h"
>> +#include "hw/qdev-core.h"
>> +#include "qom/object.h"
>> +
>> +/*
>> + * This object represent several CPUs which are all identical.
> 
> Typo "represents".
> 
>> + *
>> + * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 
>> CPUs in an
>> + * Arm big.LITTLE system) they should be in different groups. If the 
>> CPUs do
>> + * not have the same view of memory (for example the main CPU and a 
>> management
>> + * controller processor) they should be in different groups.
> 
> This description calls for a clearer CpusGroupState name instead
> of CpusState (which confuses me with CPUState). Alternatively
> CpusArrayState.

Your are right, I'll add the "group" suffix.

Thanks,
Damien

[RFC PATCH 03/18] hw/cpu/cpus: prepare to handle cpu clusters

[Damien Hedde]

Some group of cpus need to form a cpu cluster to be exposed
in gdb as inferiors.

Note: 'is_cluster' field is required at least to
transition the riscv_hart_array (see following commits about that)

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h | 19 +++++++++++++
 hw/cpu/cpus.c         | 65 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
index c65f568ef8..295d7537e2 100644
--- a/include/hw/cpu/cpus.h
+++ b/include/hw/cpu/cpus.h
@@ -24,6 +24,10 @@
  * This is an abstract class, subclasses are supposed to be created on
  * per-architecture basis to handle the specifics of the cpu architecture.
  * Subclasses are meant to be user-creatable (for cold-plug).
+ *
+ * Optionnaly a group of cpus may correspond to a cpu cluster and be
+ * exposed as a gdbstub's inferior. In that case cpus must have the
+ * same memory view.
  */
 
 #define TYPE_CPUS "cpus"
@@ -37,10 +41,18 @@ OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
  *      order to eventually update this smoothly with a full
  *      CpuTopology structure in the future.
  * @cpus: Array of pointer to cpu objects.
+ * @cluster_node: node in the global cluster list.
+ * @is_cluster: true if the object corresponds to a cpu cluster. It can be
+ *      written before realize in order to enable/disable clustering.
+ * @cluster_index: The cluster ID. This value is for internal use only and
+ *      should not be exposed directly to the user or to the guest.
  */
 struct CpusState {
     /*< private >*/
     DeviceState parent_obj;
+    bool is_cluster;
+    int32_t cluster_index;
+    QLIST_ENTRY(CpusState) cluster_node;
 
     /*< public >*/
     char *cpu_type;
@@ -68,4 +80,11 @@ struct CpusClass {
     bool skip_cpus_creation;
 };
 
+/**
+ * cpus_disable_clustering:
+ * Disable clustering for this object.
+ * Has to be called before realize step.
+ */
+void cpus_disable_clustering(CpusState *s);
+
 #endif /* HW_CPU_CPUS_H */
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
index 5fad1de2c7..ed9402c100 100644
--- a/hw/cpu/cpus.c
+++ b/hw/cpu/cpus.c
@@ -16,9 +16,17 @@
 #include "hw/resettable.h"
 #include "sysemu/reset.h"
 
+static QLIST_HEAD(, CpusState) clusters =
+    QLIST_HEAD_INITIALIZER(clusters);
+
 static Property cpus_properties[] = {
     DEFINE_PROP_STRING("cpu-type", CpusState, cpu_type),
     DEFINE_PROP_UINT16("num-cpus", CpusState, topology.cpus, 0),
+    /*
+     * Default behavior is to automatically compute a valid index.
+     * FIXME: remove this property to keep it internal ?
+     */
+    DEFINE_PROP_INT32("cluster-id", CpusState, cluster_index, -1),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -30,6 +38,20 @@ static void cpus_reset(Object *obj)
     }
 }
 
+static void cpus_init(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+
+    /* may be overriden by subclasses or code to disable clustering */
+    s->is_cluster = true;
+}
+
+void cpus_disable_clustering(CpusState *s)
+{
+    assert(!DEVICE(s)->realized);
+    s->is_cluster = false;
+}
+
 static void cpus_create_cpus(CpusState *s, Error **errp)
 {
     Error *err = NULL;
@@ -44,6 +66,11 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
         object_property_add_child(OBJECT(s), "cpu[*]", OBJECT(cpu));
         object_unref(OBJECT(cpu));
 
+        /* set index in case of cluster */
+        if (s->is_cluster) {
+            cpu->cluster_index = s->cluster_index;
+        }
+
         /* let subclass configure the cpu */
         if (cgc->configure_cpu) {
             cgc->configure_cpu(s, cpu, i);
@@ -60,7 +87,7 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
 
 static void cpus_realize(DeviceState *dev, Error **errp)
 {
-    CpusState *s = CPUS(dev);
+    CpusState *item, *s = CPUS(dev);
     CpusClass *cgc = CPUS_GET_CLASS(s);
 
     /* if subclass defined a base type, let's check it */
@@ -77,6 +104,38 @@ static void cpus_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (s->is_cluster) {
+        if (s->cluster_index < 0) {
+            int32_t max = -1;
+            QLIST_FOREACH(item, &clusters, cluster_node) {
+                if (max < item->cluster_index) {
+                    max = item->cluster_index;
+                }
+            }
+            s->cluster_index = max + 1;
+        } else {
+            /*
+             * Check the index is not already taken.
+             */
+            QLIST_FOREACH(item, &clusters, cluster_node) {
+                if (s->cluster_index == item->cluster_index) {
+                    error_setg(errp, "cluster index %d already exists",
+                               s->cluster_index);
+                    return;
+                }
+            }
+        }
+
+        if (s->cluster_index >= MAX_CLUSTERS) {
+            error_setg(errp, "cluster index must be less than %d",
+                       MAX_CLUSTERS);
+            return;
+        }
+
+        /* Put the cpus in the inferior list */
+        QLIST_INSERT_HEAD(&clusters, s, cluster_node);
+    }
+
     /* create the cpus if needed */
     if (!cgc->skip_cpus_creation) {
         cpus_create_cpus(s, errp);
@@ -89,6 +148,9 @@ static void cpus_finalize(Object *obj)
     CpusState *s = CPUS(obj);
 
     g_free(s->cpus);
+
+    /* it may not be in the list */
+    QLIST_SAFE_REMOVE(s, cluster_node);
 }
 
 static void cpus_class_init(ObjectClass *klass, void *data)
@@ -114,6 +176,7 @@ static const TypeInfo cpus_type_info = {
     .parent            = TYPE_DEVICE,
     .abstract          = true,
     .instance_size     = sizeof(CpusState),
+    .instance_init     = cpus_init,
     .instance_finalize = cpus_finalize,
     .class_size        = sizeof(CpusClass),
     .class_init        = cpus_class_init,
-- 
2.35.1

[RFC PATCH 03/18] hw/cpu/cpus: prepare to handle cpu clusters

[Damien Hedde]

Some group of cpus need to form a cpu cluster to be exposed
in gdb as inferiors.

Note: 'is_cluster' field is required at least to
transition the riscv_hart_array (see following commits about that)

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h | 19 +++++++++++++
 hw/cpu/cpus.c         | 65 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
index c65f568ef8..295d7537e2 100644
--- a/include/hw/cpu/cpus.h
+++ b/include/hw/cpu/cpus.h
@@ -24,6 +24,10 @@
  * This is an abstract class, subclasses are supposed to be created on
  * per-architecture basis to handle the specifics of the cpu architecture.
  * Subclasses are meant to be user-creatable (for cold-plug).
+ *
+ * Optionnaly a group of cpus may correspond to a cpu cluster and be
+ * exposed as a gdbstub's inferior. In that case cpus must have the
+ * same memory view.
  */
 
 #define TYPE_CPUS "cpus"
@@ -37,10 +41,18 @@ OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
  *      order to eventually update this smoothly with a full
  *      CpuTopology structure in the future.
  * @cpus: Array of pointer to cpu objects.
+ * @cluster_node: node in the global cluster list.
+ * @is_cluster: true if the object corresponds to a cpu cluster. It can be
+ *      written before realize in order to enable/disable clustering.
+ * @cluster_index: The cluster ID. This value is for internal use only and
+ *      should not be exposed directly to the user or to the guest.
  */
 struct CpusState {
     /*< private >*/
     DeviceState parent_obj;
+    bool is_cluster;
+    int32_t cluster_index;
+    QLIST_ENTRY(CpusState) cluster_node;
 
     /*< public >*/
     char *cpu_type;
@@ -68,4 +80,11 @@ struct CpusClass {
     bool skip_cpus_creation;
 };
 
+/**
+ * cpus_disable_clustering:
+ * Disable clustering for this object.
+ * Has to be called before realize step.
+ */
+void cpus_disable_clustering(CpusState *s);
+
 #endif /* HW_CPU_CPUS_H */
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
index 5fad1de2c7..ed9402c100 100644
--- a/hw/cpu/cpus.c
+++ b/hw/cpu/cpus.c
@@ -16,9 +16,17 @@
 #include "hw/resettable.h"
 #include "sysemu/reset.h"
 
+static QLIST_HEAD(, CpusState) clusters =
+    QLIST_HEAD_INITIALIZER(clusters);
+
 static Property cpus_properties[] = {
     DEFINE_PROP_STRING("cpu-type", CpusState, cpu_type),
     DEFINE_PROP_UINT16("num-cpus", CpusState, topology.cpus, 0),
+    /*
+     * Default behavior is to automatically compute a valid index.
+     * FIXME: remove this property to keep it internal ?
+     */
+    DEFINE_PROP_INT32("cluster-id", CpusState, cluster_index, -1),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -30,6 +38,20 @@ static void cpus_reset(Object *obj)
     }
 }
 
+static void cpus_init(Object *obj)
+{
+    CpusState *s = CPUS(obj);
+
+    /* may be overriden by subclasses or code to disable clustering */
+    s->is_cluster = true;
+}
+
+void cpus_disable_clustering(CpusState *s)
+{
+    assert(!DEVICE(s)->realized);
+    s->is_cluster = false;
+}
+
 static void cpus_create_cpus(CpusState *s, Error **errp)
 {
     Error *err = NULL;
@@ -44,6 +66,11 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
         object_property_add_child(OBJECT(s), "cpu[*]", OBJECT(cpu));
         object_unref(OBJECT(cpu));
 
+        /* set index in case of cluster */
+        if (s->is_cluster) {
+            cpu->cluster_index = s->cluster_index;
+        }
+
         /* let subclass configure the cpu */
         if (cgc->configure_cpu) {
             cgc->configure_cpu(s, cpu, i);
@@ -60,7 +87,7 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
 
 static void cpus_realize(DeviceState *dev, Error **errp)
 {
-    CpusState *s = CPUS(dev);
+    CpusState *item, *s = CPUS(dev);
     CpusClass *cgc = CPUS_GET_CLASS(s);
 
     /* if subclass defined a base type, let's check it */
@@ -77,6 +104,38 @@ static void cpus_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (s->is_cluster) {
+        if (s->cluster_index < 0) {
+            int32_t max = -1;
+            QLIST_FOREACH(item, &clusters, cluster_node) {
+                if (max < item->cluster_index) {
+                    max = item->cluster_index;
+                }
+            }
+            s->cluster_index = max + 1;
+        } else {
+            /*
+             * Check the index is not already taken.
+             */
+            QLIST_FOREACH(item, &clusters, cluster_node) {
+                if (s->cluster_index == item->cluster_index) {
+                    error_setg(errp, "cluster index %d already exists",
+                               s->cluster_index);
+                    return;
+                }
+            }
+        }
+
+        if (s->cluster_index >= MAX_CLUSTERS) {
+            error_setg(errp, "cluster index must be less than %d",
+                       MAX_CLUSTERS);
+            return;
+        }
+
+        /* Put the cpus in the inferior list */
+        QLIST_INSERT_HEAD(&clusters, s, cluster_node);
+    }
+
     /* create the cpus if needed */
     if (!cgc->skip_cpus_creation) {
         cpus_create_cpus(s, errp);
@@ -89,6 +148,9 @@ static void cpus_finalize(Object *obj)
     CpusState *s = CPUS(obj);
 
     g_free(s->cpus);
+
+    /* it may not be in the list */
+    QLIST_SAFE_REMOVE(s, cluster_node);
 }
 
 static void cpus_class_init(ObjectClass *klass, void *data)
@@ -114,6 +176,7 @@ static const TypeInfo cpus_type_info = {
     .parent            = TYPE_DEVICE,
     .abstract          = true,
     .instance_size     = sizeof(CpusState),
+    .instance_init     = cpus_init,
     .instance_finalize = cpus_finalize,
     .class_size        = sizeof(CpusClass),
     .class_init        = cpus_class_init,
-- 
2.35.1

[RFC PATCH 04/18] hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_

[Damien Hedde]

Drop the cluster-id property as base class defines one.
cluster_id field is temporarily kept until gdbstub
is updated.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cluster.h | 17 ++++++++++--
 hw/cpu/cluster.c         | 58 +++++++++++++++++++++-------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 09a2b86832..2125765f21 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -22,6 +22,7 @@
 
 #include "hw/qdev-core.h"
 #include "qom/object.h"
+#include "hw/cpu/cpus.h"
 
 /*
  * CPU Cluster type
@@ -55,7 +56,7 @@
  */
 
 #define TYPE_CPU_CLUSTER "cpu-cluster"
-OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
+OBJECT_DECLARE_TYPE(CPUClusterState, CPUClusterClass, CPU_CLUSTER)
 
 /**
  * CPUClusterState:
@@ -66,10 +67,22 @@ OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
  */
 struct CPUClusterState {
     /*< private >*/
-    DeviceState parent_obj;
+    CpusState parent_obj;
 
     /*< public >*/
     uint32_t cluster_id;
 };
 
+/**
+ * CPUClusterClass:
+ * @parent_realize: to store base class realize method
+ */
+struct CPUClusterClass {
+    /*< private >*/
+    CpusClass parent_class;
+
+    /*< public >*/
+    DeviceRealize parent_realize;
+};
+
 #endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index e444b7c29d..3daf897bd9 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -20,50 +20,39 @@
 
 #include "qemu/osdep.h"
 #include "hw/cpu/cluster.h"
+#include "hw/cpu/cpus.h"
 #include "hw/qdev-properties.h"
 #include "hw/core/cpu.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/cutils.h"
 
-static Property cpu_cluster_properties[] = {
-    DEFINE_PROP_UINT32("cluster-id", CPUClusterState, cluster_id, 0),
-    DEFINE_PROP_END_OF_LIST()
-};
-
-typedef struct CallbackData {
-    CPUClusterState *cluster;
-    int cpu_count;
-} CallbackData;
-
 static int add_cpu_to_cluster(Object *obj, void *opaque)
 {
-    CallbackData *cbdata = opaque;
+    CpusState *base = CPUS(opaque);
     CPUState *cpu = (CPUState *)object_dynamic_cast(obj, TYPE_CPU);
 
     if (cpu) {
-        cpu->cluster_index = cbdata->cluster->cluster_id;
-        cbdata->cpu_count++;
+        cpu->cluster_index = base->cluster_index;
+        base->topology.cpus++;
     }
     return 0;
 }
 
 static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 {
-    /* Iterate through all our CPU children and set their cluster_index */
+    CPUClusterClass *ccc = CPU_CLUSTER_GET_CLASS(dev);
     CPUClusterState *cluster = CPU_CLUSTER(dev);
+    CpusState *base = CPUS(dev);
     Object *cluster_obj = OBJECT(dev);
-    CallbackData cbdata = {
-        .cluster = cluster,
-        .cpu_count = 0,
-    };
 
-    if (cluster->cluster_id >= MAX_CLUSTERS) {
-        error_setg(errp, "cluster-id must be less than %d", MAX_CLUSTERS);
-        return;
-    }
+    /* This is a special legacy case */
+    assert(base->topology.cpus == 0);
+    assert(base->cpu_type == NULL);
+    assert(base->is_cluster);
 
-    object_child_foreach_recursive(cluster_obj, add_cpu_to_cluster, &cbdata);
+    /* Iterate through all our CPU children and set their cluster_index */
+    object_child_foreach_recursive(cluster_obj, add_cpu_to_cluster, base);
 
     /*
      * A cluster with no CPUs is a bug in the board/SoC code that created it;
@@ -71,24 +60,39 @@ static void cpu_cluster_realize(DeviceState *dev, Error **errp)
      * created the CPUs and parented them into the cluster object before
      * realizing the cluster object.
      */
-    assert(cbdata.cpu_count > 0);
+    assert(base->topology.cpus > 0);
+
+    /* realize base class (will set cluster field to true) */
+    ccc->parent_realize(dev, errp);
+
+    /*
+     * Temporarily copy the cluster id from the base class as
+     * gdbstub still uses our field.
+     */
+    cluster->cluster_id = base->cluster_index;
 }
 
 static void cpu_cluster_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    CPUClusterClass *ccc = CPU_CLUSTER_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
 
-    device_class_set_props(dc, cpu_cluster_properties);
-    dc->realize = cpu_cluster_realize;
+    device_class_set_parent_realize(dc, cpu_cluster_realize,
+                                    &ccc->parent_realize);
 
     /* This is not directly for users, CPU children must be attached by code */
     dc->user_creatable = false;
+
+    /* Cpus are created by external code */
+    cc->skip_cpus_creation = true;
 }
 
 static const TypeInfo cpu_cluster_type_info = {
     .name = TYPE_CPU_CLUSTER,
-    .parent = TYPE_DEVICE,
+    .parent = TYPE_CPUS,
     .instance_size = sizeof(CPUClusterState),
+    .class_size = sizeof(CPUClusterClass),
     .class_init = cpu_cluster_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 04/18] hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_

[Damien Hedde]

Drop the cluster-id property as base class defines one.
cluster_id field is temporarily kept until gdbstub
is updated.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cluster.h | 17 ++++++++++--
 hw/cpu/cluster.c         | 58 +++++++++++++++++++++-------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 09a2b86832..2125765f21 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -22,6 +22,7 @@
 
 #include "hw/qdev-core.h"
 #include "qom/object.h"
+#include "hw/cpu/cpus.h"
 
 /*
  * CPU Cluster type
@@ -55,7 +56,7 @@
  */
 
 #define TYPE_CPU_CLUSTER "cpu-cluster"
-OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
+OBJECT_DECLARE_TYPE(CPUClusterState, CPUClusterClass, CPU_CLUSTER)
 
 /**
  * CPUClusterState:
@@ -66,10 +67,22 @@ OBJECT_DECLARE_SIMPLE_TYPE(CPUClusterState, CPU_CLUSTER)
  */
 struct CPUClusterState {
     /*< private >*/
-    DeviceState parent_obj;
+    CpusState parent_obj;
 
     /*< public >*/
     uint32_t cluster_id;
 };
 
+/**
+ * CPUClusterClass:
+ * @parent_realize: to store base class realize method
+ */
+struct CPUClusterClass {
+    /*< private >*/
+    CpusClass parent_class;
+
+    /*< public >*/
+    DeviceRealize parent_realize;
+};
+
 #endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index e444b7c29d..3daf897bd9 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -20,50 +20,39 @@
 
 #include "qemu/osdep.h"
 #include "hw/cpu/cluster.h"
+#include "hw/cpu/cpus.h"
 #include "hw/qdev-properties.h"
 #include "hw/core/cpu.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/cutils.h"
 
-static Property cpu_cluster_properties[] = {
-    DEFINE_PROP_UINT32("cluster-id", CPUClusterState, cluster_id, 0),
-    DEFINE_PROP_END_OF_LIST()
-};
-
-typedef struct CallbackData {
-    CPUClusterState *cluster;
-    int cpu_count;
-} CallbackData;
-
 static int add_cpu_to_cluster(Object *obj, void *opaque)
 {
-    CallbackData *cbdata = opaque;
+    CpusState *base = CPUS(opaque);
     CPUState *cpu = (CPUState *)object_dynamic_cast(obj, TYPE_CPU);
 
     if (cpu) {
-        cpu->cluster_index = cbdata->cluster->cluster_id;
-        cbdata->cpu_count++;
+        cpu->cluster_index = base->cluster_index;
+        base->topology.cpus++;
     }
     return 0;
 }
 
 static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 {
-    /* Iterate through all our CPU children and set their cluster_index */
+    CPUClusterClass *ccc = CPU_CLUSTER_GET_CLASS(dev);
     CPUClusterState *cluster = CPU_CLUSTER(dev);
+    CpusState *base = CPUS(dev);
     Object *cluster_obj = OBJECT(dev);
-    CallbackData cbdata = {
-        .cluster = cluster,
-        .cpu_count = 0,
-    };
 
-    if (cluster->cluster_id >= MAX_CLUSTERS) {
-        error_setg(errp, "cluster-id must be less than %d", MAX_CLUSTERS);
-        return;
-    }
+    /* This is a special legacy case */
+    assert(base->topology.cpus == 0);
+    assert(base->cpu_type == NULL);
+    assert(base->is_cluster);
 
-    object_child_foreach_recursive(cluster_obj, add_cpu_to_cluster, &cbdata);
+    /* Iterate through all our CPU children and set their cluster_index */
+    object_child_foreach_recursive(cluster_obj, add_cpu_to_cluster, base);
 
     /*
      * A cluster with no CPUs is a bug in the board/SoC code that created it;
@@ -71,24 +60,39 @@ static void cpu_cluster_realize(DeviceState *dev, Error **errp)
      * created the CPUs and parented them into the cluster object before
      * realizing the cluster object.
      */
-    assert(cbdata.cpu_count > 0);
+    assert(base->topology.cpus > 0);
+
+    /* realize base class (will set cluster field to true) */
+    ccc->parent_realize(dev, errp);
+
+    /*
+     * Temporarily copy the cluster id from the base class as
+     * gdbstub still uses our field.
+     */
+    cluster->cluster_id = base->cluster_index;
 }
 
 static void cpu_cluster_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    CPUClusterClass *ccc = CPU_CLUSTER_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
 
-    device_class_set_props(dc, cpu_cluster_properties);
-    dc->realize = cpu_cluster_realize;
+    device_class_set_parent_realize(dc, cpu_cluster_realize,
+                                    &ccc->parent_realize);
 
     /* This is not directly for users, CPU children must be attached by code */
     dc->user_creatable = false;
+
+    /* Cpus are created by external code */
+    cc->skip_cpus_creation = true;
 }
 
 static const TypeInfo cpu_cluster_type_info = {
     .name = TYPE_CPU_CLUSTER,
-    .parent = TYPE_DEVICE,
+    .parent = TYPE_CPUS,
     .instance_size = sizeof(CPUClusterState),
+    .class_size = sizeof(CPUClusterClass),
     .class_init = cpu_cluster_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 05/18] gdbstub: deal with _cpus_ object instead of _cpu-cluster_

[Damien Hedde]

The gdbstub will only handle _cpus_ object having set
the _is_cluster_ flag.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 gdbstub.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c8375e3c3f..0b3e943f95 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -38,7 +38,7 @@
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
-#include "hw/cpu/cluster.h"
+#include "hw/cpu/cpus.h"
 #include "hw/boards.h"
 #endif
 
@@ -3458,9 +3458,10 @@ static const TypeInfo char_gdb_type_info = {
 
 static int find_cpu_clusters(Object *child, void *opaque)
 {
-    if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+    if (object_dynamic_cast(child, TYPE_CPUS) &&
+        CPUS(child)->is_cluster) {
         GDBState *s = (GDBState *) opaque;
-        CPUClusterState *cluster = CPU_CLUSTER(child);
+        CpusState *cluster = CPUS(child);
         GDBProcess *process;
 
         s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
@@ -3472,8 +3473,9 @@ static int find_cpu_clusters(Object *child, void *opaque)
          * runtime, we enforce here that the machine does not use a cluster ID
          * that would lead to PID 0.
          */
-        assert(cluster->cluster_id != UINT32_MAX);
-        process->pid = cluster->cluster_id + 1;
+        assert(cluster->cluster_index >= 0 &&
+               cluster->cluster_index < UINT32_MAX);
+        process->pid = cluster->cluster_index + 1;
         process->attached = false;
         process->target_xml[0] = '\0';
 
-- 
2.35.1

[RFC PATCH 05/18] gdbstub: deal with _cpus_ object instead of _cpu-cluster_

[Damien Hedde]

The gdbstub will only handle _cpus_ object having set
the _is_cluster_ flag.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 gdbstub.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c8375e3c3f..0b3e943f95 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -38,7 +38,7 @@
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
-#include "hw/cpu/cluster.h"
+#include "hw/cpu/cpus.h"
 #include "hw/boards.h"
 #endif
 
@@ -3458,9 +3458,10 @@ static const TypeInfo char_gdb_type_info = {
 
 static int find_cpu_clusters(Object *child, void *opaque)
 {
-    if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+    if (object_dynamic_cast(child, TYPE_CPUS) &&
+        CPUS(child)->is_cluster) {
         GDBState *s = (GDBState *) opaque;
-        CPUClusterState *cluster = CPU_CLUSTER(child);
+        CpusState *cluster = CPUS(child);
         GDBProcess *process;
 
         s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
@@ -3472,8 +3473,9 @@ static int find_cpu_clusters(Object *child, void *opaque)
          * runtime, we enforce here that the machine does not use a cluster ID
          * that would lead to PID 0.
          */
-        assert(cluster->cluster_id != UINT32_MAX);
-        process->pid = cluster->cluster_id + 1;
+        assert(cluster->cluster_index >= 0 &&
+               cluster->cluster_index < UINT32_MAX);
+        process->pid = cluster->cluster_index + 1;
         process->attached = false;
         process->target_xml[0] = '\0';
 
-- 
2.35.1

[RFC PATCH 06/18] hw/cpu/cluster: remove cluster_id now that gdbstub is updated

[Damien Hedde]

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cluster.h | 7 -------
 hw/cpu/cluster.c         | 7 -------
 2 files changed, 14 deletions(-)

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 2125765f21..6704434cc0 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -60,17 +60,10 @@ OBJECT_DECLARE_TYPE(CPUClusterState, CPUClusterClass, CPU_CLUSTER)
 
 /**
  * CPUClusterState:
- * @cluster_id: The cluster ID. This value is for internal use only and should
- *   not be exposed directly to the user or to the guest.
- *
- * State of a CPU cluster.
  */
 struct CPUClusterState {
     /*< private >*/
     CpusState parent_obj;
-
-    /*< public >*/
-    uint32_t cluster_id;
 };
 
 /**
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index 3daf897bd9..51da6ce3a9 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -42,7 +42,6 @@ static int add_cpu_to_cluster(Object *obj, void *opaque)
 static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 {
     CPUClusterClass *ccc = CPU_CLUSTER_GET_CLASS(dev);
-    CPUClusterState *cluster = CPU_CLUSTER(dev);
     CpusState *base = CPUS(dev);
     Object *cluster_obj = OBJECT(dev);
 
@@ -64,12 +63,6 @@ static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 
     /* realize base class (will set cluster field to true) */
     ccc->parent_realize(dev, errp);
-
-    /*
-     * Temporarily copy the cluster id from the base class as
-     * gdbstub still uses our field.
-     */
-    cluster->cluster_id = base->cluster_index;
 }
 
 static void cpu_cluster_class_init(ObjectClass *klass, void *data)
-- 
2.35.1

[RFC PATCH 06/18] hw/cpu/cluster: remove cluster_id now that gdbstub is updated

[Damien Hedde]

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cluster.h | 7 -------
 hw/cpu/cluster.c         | 7 -------
 2 files changed, 14 deletions(-)

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
index 2125765f21..6704434cc0 100644
--- a/include/hw/cpu/cluster.h
+++ b/include/hw/cpu/cluster.h
@@ -60,17 +60,10 @@ OBJECT_DECLARE_TYPE(CPUClusterState, CPUClusterClass, CPU_CLUSTER)
 
 /**
  * CPUClusterState:
- * @cluster_id: The cluster ID. This value is for internal use only and should
- *   not be exposed directly to the user or to the guest.
- *
- * State of a CPU cluster.
  */
 struct CPUClusterState {
     /*< private >*/
     CpusState parent_obj;
-
-    /*< public >*/
-    uint32_t cluster_id;
 };
 
 /**
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index 3daf897bd9..51da6ce3a9 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -42,7 +42,6 @@ static int add_cpu_to_cluster(Object *obj, void *opaque)
 static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 {
     CPUClusterClass *ccc = CPU_CLUSTER_GET_CLASS(dev);
-    CPUClusterState *cluster = CPU_CLUSTER(dev);
     CpusState *base = CPUS(dev);
     Object *cluster_obj = OBJECT(dev);
 
@@ -64,12 +63,6 @@ static void cpu_cluster_realize(DeviceState *dev, Error **errp)
 
     /* realize base class (will set cluster field to true) */
     ccc->parent_realize(dev, errp);
-
-    /*
-     * Temporarily copy the cluster id from the base class as
-     * gdbstub still uses our field.
-     */
-    cluster->cluster_id = base->cluster_index;
 }
 
 static void cpu_cluster_class_init(ObjectClass *klass, void *data)
-- 
2.35.1

[RFC PATCH 07/18] hw/cpu/cpus: add a common start-powered-off property

[Damien Hedde]

Can be used to initialize the same property on all
cpus.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h | 3 +++
 hw/cpu/cpus.c         | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
index 295d7537e2..7e89a0d018 100644
--- a/include/hw/cpu/cpus.h
+++ b/include/hw/cpu/cpus.h
@@ -46,6 +46,8 @@ OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
  *      written before realize in order to enable/disable clustering.
  * @cluster_index: The cluster ID. This value is for internal use only and
  *      should not be exposed directly to the user or to the guest.
+ * @start_powered_off: Default start power state of all cpus
+ *     (can be modified on a per-cpu basis after realize).
  */
 struct CpusState {
     /*< private >*/
@@ -59,6 +61,7 @@ struct CpusState {
     struct {
         uint16_t cpus;
     } topology;
+    bool start_powered_off;
     CPUState **cpus;
 };
 
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
index ed9402c100..d1fe80f0ab 100644
--- a/hw/cpu/cpus.c
+++ b/hw/cpu/cpus.c
@@ -27,6 +27,7 @@ static Property cpus_properties[] = {
      * FIXME: remove this property to keep it internal ?
      */
     DEFINE_PROP_INT32("cluster-id", CpusState, cluster_index, -1),
+    DEFINE_PROP_BOOL("start-powered-off", CpusState, start_powered_off, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -71,6 +72,10 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
             cpu->cluster_index = s->cluster_index;
         }
 
+        /* set power start state */
+        qdev_prop_set_bit(DEVICE(cpu), "start-powered-off",
+                          s->start_powered_off);
+
         /* let subclass configure the cpu */
         if (cgc->configure_cpu) {
             cgc->configure_cpu(s, cpu, i);
-- 
2.35.1

[RFC PATCH 07/18] hw/cpu/cpus: add a common start-powered-off property

[Damien Hedde]

Can be used to initialize the same property on all
cpus.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/cpu/cpus.h | 3 +++
 hw/cpu/cpus.c         | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/cpu/cpus.h b/include/hw/cpu/cpus.h
index 295d7537e2..7e89a0d018 100644
--- a/include/hw/cpu/cpus.h
+++ b/include/hw/cpu/cpus.h
@@ -46,6 +46,8 @@ OBJECT_DECLARE_TYPE(CpusState, CpusClass, CPUS)
  *      written before realize in order to enable/disable clustering.
  * @cluster_index: The cluster ID. This value is for internal use only and
  *      should not be exposed directly to the user or to the guest.
+ * @start_powered_off: Default start power state of all cpus
+ *     (can be modified on a per-cpu basis after realize).
  */
 struct CpusState {
     /*< private >*/
@@ -59,6 +61,7 @@ struct CpusState {
     struct {
         uint16_t cpus;
     } topology;
+    bool start_powered_off;
     CPUState **cpus;
 };
 
diff --git a/hw/cpu/cpus.c b/hw/cpu/cpus.c
index ed9402c100..d1fe80f0ab 100644
--- a/hw/cpu/cpus.c
+++ b/hw/cpu/cpus.c
@@ -27,6 +27,7 @@ static Property cpus_properties[] = {
      * FIXME: remove this property to keep it internal ?
      */
     DEFINE_PROP_INT32("cluster-id", CpusState, cluster_index, -1),
+    DEFINE_PROP_BOOL("start-powered-off", CpusState, start_powered_off, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -71,6 +72,10 @@ static void cpus_create_cpus(CpusState *s, Error **errp)
             cpu->cluster_index = s->cluster_index;
         }
 
+        /* set power start state */
+        qdev_prop_set_bit(DEVICE(cpu), "start-powered-off",
+                          s->start_powered_off);
+
         /* let subclass configure the cpu */
         if (cgc->configure_cpu) {
             cgc->configure_cpu(s, cpu, i);
-- 
2.35.1

[RFC PATCH 08/18] hw/arm/arm_cpus: add arm_cpus device

[Damien Hedde]

This object can be used to create a group of homogeneous
arm cpus.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/arm/arm_cpus.h | 45 ++++++++++++++++++++++++++++
 hw/arm/arm_cpus.c         | 63 +++++++++++++++++++++++++++++++++++++++
 hw/arm/meson.build        |  1 +
 3 files changed, 109 insertions(+)
 create mode 100644 include/hw/arm/arm_cpus.h
 create mode 100644 hw/arm/arm_cpus.c

diff --git a/include/hw/arm/arm_cpus.h b/include/hw/arm/arm_cpus.h
new file mode 100644
index 0000000000..8c540d9853
--- /dev/null
+++ b/include/hw/arm/arm_cpus.h
@@ -0,0 +1,45 @@
+/*
+ * ARM CPUs
+ *
+ * Copyright (c) 2022 Greensocs
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_ARM_CPUS_H
+#define HW_ARM_CPUS_H
+
+#include "hw/cpu/cpus.h"
+#include "target/arm/cpu.h"
+
+#define TYPE_ARM_CPUS "arm-cpus"
+OBJECT_DECLARE_SIMPLE_TYPE(ArmCpusState, ARM_CPUS)
+
+/**
+ * ArmCpusState:
+ * @reset_hivecs: use to initialize cpu's reset-hivecs
+ * @has_el3: use to initialize cpu's has_el3
+ * @has_el2: use to initialize cpu's has_el2
+ * @reset_cbar: use to initialize cpu's reset-cbar
+ */
+struct ArmCpusState {
+    CpusState parent_obj;
+
+    bool reset_hivecs;
+    bool has_el3;
+    bool has_el2;
+    uint64_t reset_cbar;
+};
+
+/*
+ * arm_cpus_get_cpu:
+ * Helper to get an ArmCpu from the container.
+ */
+static inline ARMCPU *arm_cpus_get_cpu(ArmCpusState *s, unsigned i)
+{
+    return ARM_CPU(CPUS(s)->cpus[i]);
+}
+
+#endif /* HW_ARM_CPUS_H */
diff --git a/hw/arm/arm_cpus.c b/hw/arm/arm_cpus.c
new file mode 100644
index 0000000000..ed6483f45b
--- /dev/null
+++ b/hw/arm/arm_cpus.c
@@ -0,0 +1,63 @@
+/*
+ * ARM CPUs
+ *
+ * Copyright (c) 2022 Greensocs
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/arm/arm_cpus.h"
+#include "hw/cpu/cpus.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
+
+static Property arm_cpus_props[] = {
+    /* FIXME: get the default values from the arm cpu object */
+    DEFINE_PROP_BOOL("reset-hivecs", ArmCpusState, reset_hivecs, false),
+    DEFINE_PROP_BOOL("has_el3", ArmCpusState, has_el3, false),
+    DEFINE_PROP_BOOL("has_el2", ArmCpusState, has_el2, false),
+    DEFINE_PROP_UINT64("reset-cbar", ArmCpusState, reset_cbar, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void arm_cpus_configure_cpu(CpusState *base, CPUState *cpu,
+                                   unsigned i)
+{
+    ArmCpusState *s = ARM_CPUS(base);
+    DeviceState *cpudev = DEVICE(cpu);
+
+    qdev_prop_set_uint32(cpudev, "core-count", base->topology.cpus);
+    qdev_prop_set_bit(cpudev, "reset-hivecs", s->reset_hivecs);
+    qdev_prop_set_bit(cpudev, "has_el3", s->has_el3);
+    qdev_prop_set_bit(cpudev, "has_el2", s->has_el2);
+    qdev_prop_set_uint64(cpudev, "reset-cbar", s->reset_cbar);
+}
+
+static void arm_cpus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
+
+    device_class_set_props(dc, arm_cpus_props);
+
+    cc->configure_cpu = arm_cpus_configure_cpu;
+    cc->base_cpu_type = TYPE_ARM_CPU;
+}
+
+static const TypeInfo arm_cpus_info = {
+    .name              = TYPE_ARM_CPUS,
+    .parent            = TYPE_CPUS,
+    .instance_size     = sizeof(ArmCpusState),
+    .class_init        = arm_cpus_class_init,
+};
+
+static void arm_cpus_register_types(void)
+{
+    type_register_static(&arm_cpus_info);
+}
+
+type_init(arm_cpus_register_types)
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 721a8eb8be..feeb301c09 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -58,5 +58,6 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre.
 arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c', 'smmuv3.c'))
 arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c'))
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
+arm_ss.add(files('arm_cpus.c'))
 
 hw_arch += {'arm': arm_ss}
-- 
2.35.1

[RFC PATCH 08/18] hw/arm/arm_cpus: add arm_cpus device

[Damien Hedde]

This object can be used to create a group of homogeneous
arm cpus.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/arm/arm_cpus.h | 45 ++++++++++++++++++++++++++++
 hw/arm/arm_cpus.c         | 63 +++++++++++++++++++++++++++++++++++++++
 hw/arm/meson.build        |  1 +
 3 files changed, 109 insertions(+)
 create mode 100644 include/hw/arm/arm_cpus.h
 create mode 100644 hw/arm/arm_cpus.c

diff --git a/include/hw/arm/arm_cpus.h b/include/hw/arm/arm_cpus.h
new file mode 100644
index 0000000000..8c540d9853
--- /dev/null
+++ b/include/hw/arm/arm_cpus.h
@@ -0,0 +1,45 @@
+/*
+ * ARM CPUs
+ *
+ * Copyright (c) 2022 Greensocs
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_ARM_CPUS_H
+#define HW_ARM_CPUS_H
+
+#include "hw/cpu/cpus.h"
+#include "target/arm/cpu.h"
+
+#define TYPE_ARM_CPUS "arm-cpus"
+OBJECT_DECLARE_SIMPLE_TYPE(ArmCpusState, ARM_CPUS)
+
+/**
+ * ArmCpusState:
+ * @reset_hivecs: use to initialize cpu's reset-hivecs
+ * @has_el3: use to initialize cpu's has_el3
+ * @has_el2: use to initialize cpu's has_el2
+ * @reset_cbar: use to initialize cpu's reset-cbar
+ */
+struct ArmCpusState {
+    CpusState parent_obj;
+
+    bool reset_hivecs;
+    bool has_el3;
+    bool has_el2;
+    uint64_t reset_cbar;
+};
+
+/*
+ * arm_cpus_get_cpu:
+ * Helper to get an ArmCpu from the container.
+ */
+static inline ARMCPU *arm_cpus_get_cpu(ArmCpusState *s, unsigned i)
+{
+    return ARM_CPU(CPUS(s)->cpus[i]);
+}
+
+#endif /* HW_ARM_CPUS_H */
diff --git a/hw/arm/arm_cpus.c b/hw/arm/arm_cpus.c
new file mode 100644
index 0000000000..ed6483f45b
--- /dev/null
+++ b/hw/arm/arm_cpus.c
@@ -0,0 +1,63 @@
+/*
+ * ARM CPUs
+ *
+ * Copyright (c) 2022 Greensocs
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/arm/arm_cpus.h"
+#include "hw/cpu/cpus.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
+
+static Property arm_cpus_props[] = {
+    /* FIXME: get the default values from the arm cpu object */
+    DEFINE_PROP_BOOL("reset-hivecs", ArmCpusState, reset_hivecs, false),
+    DEFINE_PROP_BOOL("has_el3", ArmCpusState, has_el3, false),
+    DEFINE_PROP_BOOL("has_el2", ArmCpusState, has_el2, false),
+    DEFINE_PROP_UINT64("reset-cbar", ArmCpusState, reset_cbar, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void arm_cpus_configure_cpu(CpusState *base, CPUState *cpu,
+                                   unsigned i)
+{
+    ArmCpusState *s = ARM_CPUS(base);
+    DeviceState *cpudev = DEVICE(cpu);
+
+    qdev_prop_set_uint32(cpudev, "core-count", base->topology.cpus);
+    qdev_prop_set_bit(cpudev, "reset-hivecs", s->reset_hivecs);
+    qdev_prop_set_bit(cpudev, "has_el3", s->has_el3);
+    qdev_prop_set_bit(cpudev, "has_el2", s->has_el2);
+    qdev_prop_set_uint64(cpudev, "reset-cbar", s->reset_cbar);
+}
+
+static void arm_cpus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
+
+    device_class_set_props(dc, arm_cpus_props);
+
+    cc->configure_cpu = arm_cpus_configure_cpu;
+    cc->base_cpu_type = TYPE_ARM_CPU;
+}
+
+static const TypeInfo arm_cpus_info = {
+    .name              = TYPE_ARM_CPUS,
+    .parent            = TYPE_CPUS,
+    .instance_size     = sizeof(ArmCpusState),
+    .class_init        = arm_cpus_class_init,
+};
+
+static void arm_cpus_register_types(void)
+{
+    type_register_static(&arm_cpus_info);
+}
+
+type_init(arm_cpus_register_types)
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 721a8eb8be..feeb301c09 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -58,5 +58,6 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre.
 arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c', 'smmuv3.c'))
 arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c'))
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
+arm_ss.add(files('arm_cpus.c'))
 
 hw_arch += {'arm': arm_ss}
-- 
2.35.1

Re: [RFC PATCH 08/18] hw/arm/arm_cpus: add arm_cpus device

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> This object can be used to create a group of homogeneous
> arm cpus.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/arm/arm_cpus.h | 45 ++++++++++++++++++++++++++++
>   hw/arm/arm_cpus.c         | 63 +++++++++++++++++++++++++++++++++++++++
>   hw/arm/meson.build        |  1 +
>   3 files changed, 109 insertions(+)
>   create mode 100644 include/hw/arm/arm_cpus.h
>   create mode 100644 hw/arm/arm_cpus.c

> +/**
> + * ArmCpusState:
> + * @reset_hivecs: use to initialize cpu's reset-hivecs
> + * @has_el3: use to initialize cpu's has_el3
> + * @has_el2: use to initialize cpu's has_el2
> + * @reset_cbar: use to initialize cpu's reset-cbar
> + */
> +struct ArmCpusState {
> +    CpusState parent_obj;
> +
> +    bool reset_hivecs;
> +    bool has_el3;
> +    bool has_el2;
> +    uint64_t reset_cbar;
> +};
> +
> +/*
> + * arm_cpus_get_cpu:
> + * Helper to get an ArmCpu from the container.
> + */
> +static inline ARMCPU *arm_cpus_get_cpu(ArmCpusState *s, unsigned i)

Maybe we can avoid using 'State' suffix for CPU (still using Class 
suffixes) and name this:

   ARMCPU *arm_cpus_group_get_cpu(ArmCpusGroup *s, unsigned index);

Re: [RFC PATCH 08/18] hw/arm/arm_cpus: add arm_cpus device

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> This object can be used to create a group of homogeneous
> arm cpus.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/arm/arm_cpus.h | 45 ++++++++++++++++++++++++++++
>   hw/arm/arm_cpus.c         | 63 +++++++++++++++++++++++++++++++++++++++
>   hw/arm/meson.build        |  1 +
>   3 files changed, 109 insertions(+)
>   create mode 100644 include/hw/arm/arm_cpus.h
>   create mode 100644 hw/arm/arm_cpus.c

> +/**
> + * ArmCpusState:
> + * @reset_hivecs: use to initialize cpu's reset-hivecs
> + * @has_el3: use to initialize cpu's has_el3
> + * @has_el2: use to initialize cpu's has_el2
> + * @reset_cbar: use to initialize cpu's reset-cbar
> + */
> +struct ArmCpusState {
> +    CpusState parent_obj;
> +
> +    bool reset_hivecs;
> +    bool has_el3;
> +    bool has_el2;
> +    uint64_t reset_cbar;
> +};
> +
> +/*
> + * arm_cpus_get_cpu:
> + * Helper to get an ArmCpu from the container.
> + */
> +static inline ARMCPU *arm_cpus_get_cpu(ArmCpusState *s, unsigned i)

Maybe we can avoid using 'State' suffix for CPU (still using Class 
suffixes) and name this:

   ARMCPU *arm_cpus_group_get_cpu(ArmCpusGroup *s, unsigned index);

[RFC PATCH 09/18] hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus

[Damien Hedde]

qom-path of cpus are changed:
+ "apu-cluster/apu-cpu[n]" to "apu/cpu[n]"
+ "rpu-cluster/rpu-cpu[n]" to "rpu/cpu[n]"

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/arm/xlnx-zynqmp.h |   8 +--
 hw/arm/xlnx-zynqmp.c         | 121 +++++++++++++----------------------
 2 files changed, 48 insertions(+), 81 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 9d9a9d0bf9..1bcc3f9356 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -31,7 +31,7 @@
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/rtc/xlnx-zynqmp-rtc.h"
-#include "hw/cpu/cluster.h"
+#include "hw/arm/arm_cpus.h"
 #include "target/arm/cpu.h"
 #include "qom/object.h"
 #include "net/can_emu.h"
@@ -94,10 +94,8 @@ struct XlnxZynqMPState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState apu_cluster;
-    CPUClusterState rpu_cluster;
-    ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
-    ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
+    ArmCpusState apu;
+    ArmCpusState rpu;
     GICState gic;
     MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 5bfe285a19..fa0c093733 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -25,6 +25,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "kvm_arm.h"
+#include "hw/arm/arm_cpus.h"
 
 #define GIC_NUM_SPI_INTR 160
 
@@ -201,7 +202,7 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
 static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
                                    const char *boot_cpu, Error **errp)
 {
-    int i;
+    unsigned boot_idx;
     int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
                        XLNX_ZYNQMP_NUM_RPU_CPUS);
 
@@ -210,36 +211,21 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
         return;
     }
 
-    object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
-                            TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->rpu_cluster), "cluster-id", 1);
-
-    for (i = 0; i < num_rpus; i++) {
-        const char *name;
-
-        object_initialize_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
-                                &s->rpu_cpu[i],
-                                ARM_CPU_TYPE_NAME("cortex-r5f"));
-
-        name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
-        if (strcmp(name, boot_cpu)) {
-            /*
-             * Secondary CPUs start in powered-down state.
-             */
-            object_property_set_bool(OBJECT(&s->rpu_cpu[i]),
-                                     "start-powered-off", true, &error_abort);
-        } else {
-            s->boot_cpu_ptr = &s->rpu_cpu[i];
-        }
-
-        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), "reset-hivecs", true,
-                                 &error_abort);
-        if (!qdev_realize(DEVICE(&s->rpu_cpu[i]), NULL, errp)) {
-            return;
-        }
+    /* apus are already created, rpus will have cluster-id 1 */
+    object_initialize_child(OBJECT(s), "rpu", &s->rpu, TYPE_ARM_CPUS);
+    qdev_prop_set_uint32(DEVICE(&s->rpu), "num-cpus", num_rpus);
+    qdev_prop_set_string(DEVICE(&s->rpu), "cpu-type",
+                         ARM_CPU_TYPE_NAME("cortex-r5f"));
+    qdev_prop_set_bit(DEVICE(&s->rpu), "reset-hivecs", true);
+    qdev_prop_set_bit(DEVICE(&s->rpu), "start-powered-off", true);
+
+    qdev_realize(DEVICE(&s->rpu), NULL, &error_fatal);
+
+    if (sscanf(boot_cpu, "rpu-cpu[%u]", &boot_idx) && boot_idx < num_rpus) {
+        s->boot_cpu_ptr = arm_cpus_get_cpu(&s->rpu, boot_idx);
+        object_property_set_bool(OBJECT(s->boot_cpu_ptr), "start-powered-on",
+                                 true, &error_abort);
     }
-
-    qdev_realize(DEVICE(&s->rpu_cluster), NULL, &error_fatal);
 }
 
 static void xlnx_zynqmp_create_bbram(XlnxZynqMPState *s, qemu_irq *gic)
@@ -296,7 +282,8 @@ static void xlnx_zynqmp_create_apu_ctrl(XlnxZynqMPState *s, qemu_irq *gic)
         g_autofree gchar *name = g_strdup_printf("cpu%d", i);
 
         object_property_set_link(OBJECT(&s->apu_ctrl), name,
-                                 OBJECT(&s->apu_cpu[i]), &error_abort);
+                                 OBJECT(arm_cpus_get_cpu(&s->apu, i)),
+                                 &error_abort);
     }
 
     sysbus_realize(sbd, &error_fatal);
@@ -349,15 +336,10 @@ static void xlnx_zynqmp_init(Object *obj)
     int i;
     int num_apus = MIN(ms->smp.cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
-    object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
-                            TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->apu_cluster), "cluster-id", 0);
-
-    for (i = 0; i < num_apus; i++) {
-        object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
-                                &s->apu_cpu[i],
-                                ARM_CPU_TYPE_NAME("cortex-a53"));
-    }
+    object_initialize_child(obj, "apu", &s->apu, TYPE_ARM_CPUS);
+    qdev_prop_set_uint32(DEVICE(&s->apu), "num-cpus", num_apus);
+    qdev_prop_set_string(DEVICE(&s->apu), "cpu-type",
+                         ARM_CPU_TYPE_NAME("cortex-a53"));
 
     object_initialize_child(obj, "gic", &s->gic, gic_class_name());
 
@@ -416,6 +398,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     MemoryRegion *system_memory = get_system_memory();
     uint8_t i;
     uint64_t ram_size;
+    unsigned boot_idx;
     int num_apus = MIN(ms->smp.cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
     const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
     ram_addr_t ddr_low_size, ddr_high_size;
@@ -474,34 +457,19 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_bit(DEVICE(&s->gic),
                       "has-virtualization-extensions", s->virt);
 
-    qdev_realize(DEVICE(&s->apu_cluster), NULL, &error_fatal);
-
-    /* Realize APUs before realizing the GIC. KVM requires this.  */
-    for (i = 0; i < num_apus; i++) {
-        const char *name;
-
-        name = object_get_canonical_path_component(OBJECT(&s->apu_cpu[i]));
-        if (strcmp(name, boot_cpu)) {
-            /*
-             * Secondary CPUs start in powered-down state.
-             */
-            object_property_set_bool(OBJECT(&s->apu_cpu[i]),
-                                     "start-powered-off", true, &error_abort);
-        } else {
-            s->boot_cpu_ptr = &s->apu_cpu[i];
-        }
-
-        object_property_set_bool(OBJECT(&s->apu_cpu[i]), "has_el3", s->secure,
-                                 NULL);
-        object_property_set_bool(OBJECT(&s->apu_cpu[i]), "has_el2", s->virt,
-                                 NULL);
-        object_property_set_int(OBJECT(&s->apu_cpu[i]), "reset-cbar",
-                                GIC_BASE_ADDR, &error_abort);
-        object_property_set_int(OBJECT(&s->apu_cpu[i]), "core-count",
-                                num_apus, &error_abort);
-        if (!qdev_realize(DEVICE(&s->apu_cpu[i]), NULL, errp)) {
-            return;
-        }
+    /* Realize APUs before realizing the GIC. KVM requires this. */
+    qdev_prop_set_bit(DEVICE(&s->apu), "start-powered-off", true);
+    qdev_prop_set_bit(DEVICE(&s->apu), "has_el3", s->secure);
+    qdev_prop_set_bit(DEVICE(&s->apu), "has_el2", s->virt);
+    qdev_prop_set_uint64(DEVICE(&s->apu), "reset-cbar", GIC_BASE_ADDR);
+    if (!qdev_realize(DEVICE(&s->apu), NULL, errp)) {
+        return;
+    }
+    /* ensure boot cpu will start */
+    if (sscanf(boot_cpu, "apu-cpu[%u]", &boot_idx) && boot_idx < num_apus) {
+        s->boot_cpu_ptr = arm_cpus_get_cpu(&s->apu, boot_idx);
+        object_property_set_bool(OBJECT(s->boot_cpu_ptr), "start-powered-off",
+                                 false, &error_abort);
     }
 
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
@@ -533,32 +501,33 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     for (i = 0; i < num_apus; i++) {
+        ARMCPU *apu_cpu = arm_cpus_get_cpu(&s->apu, i);
         qemu_irq irq;
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_IRQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_FIQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 2,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_VIRQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 3,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_VFIQ));
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_PHYS, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_PHYS, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_VIRT, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_VIRT, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_HYP_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_HYP, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_HYP, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_SEC_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_SEC, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_SEC, irq);
 
         if (s->virt) {
             irq = qdev_get_gpio_in(DEVICE(&s->gic),
-- 
2.35.1

[RFC PATCH 09/18] hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus

[Damien Hedde]

qom-path of cpus are changed:
+ "apu-cluster/apu-cpu[n]" to "apu/cpu[n]"
+ "rpu-cluster/rpu-cpu[n]" to "rpu/cpu[n]"

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/arm/xlnx-zynqmp.h |   8 +--
 hw/arm/xlnx-zynqmp.c         | 121 +++++++++++++----------------------
 2 files changed, 48 insertions(+), 81 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 9d9a9d0bf9..1bcc3f9356 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -31,7 +31,7 @@
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/rtc/xlnx-zynqmp-rtc.h"
-#include "hw/cpu/cluster.h"
+#include "hw/arm/arm_cpus.h"
 #include "target/arm/cpu.h"
 #include "qom/object.h"
 #include "net/can_emu.h"
@@ -94,10 +94,8 @@ struct XlnxZynqMPState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState apu_cluster;
-    CPUClusterState rpu_cluster;
-    ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
-    ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
+    ArmCpusState apu;
+    ArmCpusState rpu;
     GICState gic;
     MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 5bfe285a19..fa0c093733 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -25,6 +25,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "kvm_arm.h"
+#include "hw/arm/arm_cpus.h"
 
 #define GIC_NUM_SPI_INTR 160
 
@@ -201,7 +202,7 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
 static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
                                    const char *boot_cpu, Error **errp)
 {
-    int i;
+    unsigned boot_idx;
     int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
                        XLNX_ZYNQMP_NUM_RPU_CPUS);
 
@@ -210,36 +211,21 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
         return;
     }
 
-    object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
-                            TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->rpu_cluster), "cluster-id", 1);
-
-    for (i = 0; i < num_rpus; i++) {
-        const char *name;
-
-        object_initialize_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
-                                &s->rpu_cpu[i],
-                                ARM_CPU_TYPE_NAME("cortex-r5f"));
-
-        name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
-        if (strcmp(name, boot_cpu)) {
-            /*
-             * Secondary CPUs start in powered-down state.
-             */
-            object_property_set_bool(OBJECT(&s->rpu_cpu[i]),
-                                     "start-powered-off", true, &error_abort);
-        } else {
-            s->boot_cpu_ptr = &s->rpu_cpu[i];
-        }
-
-        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), "reset-hivecs", true,
-                                 &error_abort);
-        if (!qdev_realize(DEVICE(&s->rpu_cpu[i]), NULL, errp)) {
-            return;
-        }
+    /* apus are already created, rpus will have cluster-id 1 */
+    object_initialize_child(OBJECT(s), "rpu", &s->rpu, TYPE_ARM_CPUS);
+    qdev_prop_set_uint32(DEVICE(&s->rpu), "num-cpus", num_rpus);
+    qdev_prop_set_string(DEVICE(&s->rpu), "cpu-type",
+                         ARM_CPU_TYPE_NAME("cortex-r5f"));
+    qdev_prop_set_bit(DEVICE(&s->rpu), "reset-hivecs", true);
+    qdev_prop_set_bit(DEVICE(&s->rpu), "start-powered-off", true);
+
+    qdev_realize(DEVICE(&s->rpu), NULL, &error_fatal);
+
+    if (sscanf(boot_cpu, "rpu-cpu[%u]", &boot_idx) && boot_idx < num_rpus) {
+        s->boot_cpu_ptr = arm_cpus_get_cpu(&s->rpu, boot_idx);
+        object_property_set_bool(OBJECT(s->boot_cpu_ptr), "start-powered-on",
+                                 true, &error_abort);
     }
-
-    qdev_realize(DEVICE(&s->rpu_cluster), NULL, &error_fatal);
 }
 
 static void xlnx_zynqmp_create_bbram(XlnxZynqMPState *s, qemu_irq *gic)
@@ -296,7 +282,8 @@ static void xlnx_zynqmp_create_apu_ctrl(XlnxZynqMPState *s, qemu_irq *gic)
         g_autofree gchar *name = g_strdup_printf("cpu%d", i);
 
         object_property_set_link(OBJECT(&s->apu_ctrl), name,
-                                 OBJECT(&s->apu_cpu[i]), &error_abort);
+                                 OBJECT(arm_cpus_get_cpu(&s->apu, i)),
+                                 &error_abort);
     }
 
     sysbus_realize(sbd, &error_fatal);
@@ -349,15 +336,10 @@ static void xlnx_zynqmp_init(Object *obj)
     int i;
     int num_apus = MIN(ms->smp.cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
-    object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
-                            TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->apu_cluster), "cluster-id", 0);
-
-    for (i = 0; i < num_apus; i++) {
-        object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
-                                &s->apu_cpu[i],
-                                ARM_CPU_TYPE_NAME("cortex-a53"));
-    }
+    object_initialize_child(obj, "apu", &s->apu, TYPE_ARM_CPUS);
+    qdev_prop_set_uint32(DEVICE(&s->apu), "num-cpus", num_apus);
+    qdev_prop_set_string(DEVICE(&s->apu), "cpu-type",
+                         ARM_CPU_TYPE_NAME("cortex-a53"));
 
     object_initialize_child(obj, "gic", &s->gic, gic_class_name());
 
@@ -416,6 +398,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     MemoryRegion *system_memory = get_system_memory();
     uint8_t i;
     uint64_t ram_size;
+    unsigned boot_idx;
     int num_apus = MIN(ms->smp.cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
     const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
     ram_addr_t ddr_low_size, ddr_high_size;
@@ -474,34 +457,19 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_bit(DEVICE(&s->gic),
                       "has-virtualization-extensions", s->virt);
 
-    qdev_realize(DEVICE(&s->apu_cluster), NULL, &error_fatal);
-
-    /* Realize APUs before realizing the GIC. KVM requires this.  */
-    for (i = 0; i < num_apus; i++) {
-        const char *name;
-
-        name = object_get_canonical_path_component(OBJECT(&s->apu_cpu[i]));
-        if (strcmp(name, boot_cpu)) {
-            /*
-             * Secondary CPUs start in powered-down state.
-             */
-            object_property_set_bool(OBJECT(&s->apu_cpu[i]),
-                                     "start-powered-off", true, &error_abort);
-        } else {
-            s->boot_cpu_ptr = &s->apu_cpu[i];
-        }
-
-        object_property_set_bool(OBJECT(&s->apu_cpu[i]), "has_el3", s->secure,
-                                 NULL);
-        object_property_set_bool(OBJECT(&s->apu_cpu[i]), "has_el2", s->virt,
-                                 NULL);
-        object_property_set_int(OBJECT(&s->apu_cpu[i]), "reset-cbar",
-                                GIC_BASE_ADDR, &error_abort);
-        object_property_set_int(OBJECT(&s->apu_cpu[i]), "core-count",
-                                num_apus, &error_abort);
-        if (!qdev_realize(DEVICE(&s->apu_cpu[i]), NULL, errp)) {
-            return;
-        }
+    /* Realize APUs before realizing the GIC. KVM requires this. */
+    qdev_prop_set_bit(DEVICE(&s->apu), "start-powered-off", true);
+    qdev_prop_set_bit(DEVICE(&s->apu), "has_el3", s->secure);
+    qdev_prop_set_bit(DEVICE(&s->apu), "has_el2", s->virt);
+    qdev_prop_set_uint64(DEVICE(&s->apu), "reset-cbar", GIC_BASE_ADDR);
+    if (!qdev_realize(DEVICE(&s->apu), NULL, errp)) {
+        return;
+    }
+    /* ensure boot cpu will start */
+    if (sscanf(boot_cpu, "apu-cpu[%u]", &boot_idx) && boot_idx < num_apus) {
+        s->boot_cpu_ptr = arm_cpus_get_cpu(&s->apu, boot_idx);
+        object_property_set_bool(OBJECT(s->boot_cpu_ptr), "start-powered-off",
+                                 false, &error_abort);
     }
 
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
@@ -533,32 +501,33 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     for (i = 0; i < num_apus; i++) {
+        ARMCPU *apu_cpu = arm_cpus_get_cpu(&s->apu, i);
         qemu_irq irq;
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_IRQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_FIQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 2,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_VIRQ));
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 3,
-                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                           qdev_get_gpio_in(DEVICE(apu_cpu),
                                             ARM_CPU_VFIQ));
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_PHYS, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_PHYS, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_VIRT, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_VIRT, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_HYP_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_HYP, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_HYP, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_SEC_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_SEC, irq);
+        qdev_connect_gpio_out(DEVICE(apu_cpu), GTIMER_SEC, irq);
 
         if (s->virt) {
             irq = qdev_get_gpio_in(DEVICE(&s->gic),
-- 
2.35.1

Re: [RFC PATCH 09/18] hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> qom-path of cpus are changed:
> + "apu-cluster/apu-cpu[n]" to "apu/cpu[n]"
> + "rpu-cluster/rpu-cpu[n]" to "rpu/cpu[n]"
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/arm/xlnx-zynqmp.h |   8 +--
>   hw/arm/xlnx-zynqmp.c         | 121 +++++++++++++----------------------
>   2 files changed, 48 insertions(+), 81 deletions(-)

>   static void xlnx_zynqmp_create_bbram(XlnxZynqMPState *s, qemu_irq *gic)
> @@ -296,7 +282,8 @@ static void xlnx_zynqmp_create_apu_ctrl(XlnxZynqMPState *s, qemu_irq *gic)
>           g_autofree gchar *name = g_strdup_printf("cpu%d", i);
>   
>           object_property_set_link(OBJECT(&s->apu_ctrl), name,
> -                                 OBJECT(&s->apu_cpu[i]), &error_abort);
> +                                 OBJECT(arm_cpus_get_cpu(&s->apu, i)),
> +                                 &error_abort);
>       }

Possible further improvement, XlnxZynqMPAPUCtrl can now take a link to
one ArmCpusGroup, instead of array of ARMCPU.

Re: [RFC PATCH 09/18] hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus

[Philippe Mathieu-Daudé]

On 30/3/22 14:56, Damien Hedde wrote:
> qom-path of cpus are changed:
> + "apu-cluster/apu-cpu[n]" to "apu/cpu[n]"
> + "rpu-cluster/rpu-cpu[n]" to "rpu/cpu[n]"
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   include/hw/arm/xlnx-zynqmp.h |   8 +--
>   hw/arm/xlnx-zynqmp.c         | 121 +++++++++++++----------------------
>   2 files changed, 48 insertions(+), 81 deletions(-)

>   static void xlnx_zynqmp_create_bbram(XlnxZynqMPState *s, qemu_irq *gic)
> @@ -296,7 +282,8 @@ static void xlnx_zynqmp_create_apu_ctrl(XlnxZynqMPState *s, qemu_irq *gic)
>           g_autofree gchar *name = g_strdup_printf("cpu%d", i);
>   
>           object_property_set_link(OBJECT(&s->apu_ctrl), name,
> -                                 OBJECT(&s->apu_cpu[i]), &error_abort);
> +                                 OBJECT(arm_cpus_get_cpu(&s->apu, i)),
> +                                 &error_abort);
>       }

Possible further improvement, XlnxZynqMPAPUCtrl can now take a link to
one ArmCpusGroup, instead of array of ARMCPU.

[RFC PATCH 10/18] hw/riscv/riscv_hart: prepare transition to cpus

[Damien Hedde]

riscv_hart_array does not need to be a sysbus device: it does not
have any mmio or sysbus irq.
We want to make it inherit the new cpus class so we need
a few tweaks:
+ a temporary helper realize so we can switch from sysbus_realize to
  qdev_realize (will be removed afer the transition is done).
+ a helper function to get an hart from the array (the current storage
  array field will be removed).
+ a helper function to get the number of harts in an array (the current
  field will be removed).

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/riscv/riscv_hart.h | 19 +++++++++++++++++++
 hw/riscv/riscv_hart.c         |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index bbc21cdc9a..71747bf37c 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -41,4 +41,23 @@ struct RISCVHartArrayState {
     RISCVCPU *harts;
 };
 
+/**
+ * riscv_array_get_hart:
+ */
+static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *harts, int i)
+{
+    return &harts->harts[i];
+}
+
+/**
+ * riscv_array_get_num_harts:
+ */
+static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *harts)
+{
+    return harts->num_harts;
+}
+
+/* Temporary function until we migrated the riscv hart array to simple device */
+void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp);
+
 #endif
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 613ea2aaa0..780fd3a59a 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -27,6 +27,11 @@
 #include "hw/qdev-properties.h"
 #include "hw/riscv/riscv_hart.h"
 
+void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
+{
+    sysbus_realize(SYS_BUS_DEVICE(state), errp);
+}
+
 static Property riscv_harts_props[] = {
     DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
-- 
2.35.1

[RFC PATCH 10/18] hw/riscv/riscv_hart: prepare transition to cpus

[Damien Hedde]

riscv_hart_array does not need to be a sysbus device: it does not
have any mmio or sysbus irq.
We want to make it inherit the new cpus class so we need
a few tweaks:
+ a temporary helper realize so we can switch from sysbus_realize to
  qdev_realize (will be removed afer the transition is done).
+ a helper function to get an hart from the array (the current storage
  array field will be removed).
+ a helper function to get the number of harts in an array (the current
  field will be removed).

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/riscv/riscv_hart.h | 19 +++++++++++++++++++
 hw/riscv/riscv_hart.c         |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index bbc21cdc9a..71747bf37c 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -41,4 +41,23 @@ struct RISCVHartArrayState {
     RISCVCPU *harts;
 };
 
+/**
+ * riscv_array_get_hart:
+ */
+static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *harts, int i)
+{
+    return &harts->harts[i];
+}
+
+/**
+ * riscv_array_get_num_harts:
+ */
+static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *harts)
+{
+    return harts->num_harts;
+}
+
+/* Temporary function until we migrated the riscv hart array to simple device */
+void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp);
+
 #endif
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 613ea2aaa0..780fd3a59a 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -27,6 +27,11 @@
 #include "hw/qdev-properties.h"
 #include "hw/riscv/riscv_hart.h"
 
+void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
+{
+    sysbus_realize(SYS_BUS_DEVICE(state), errp);
+}
+
 static Property riscv_harts_props[] = {
     DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
-- 
2.35.1

[RFC PATCH 11/18] hw/riscv: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

spike and virt machines will be handled separately in following
commits.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/boot.c            | 2 +-
 hw/riscv/microchip_pfsoc.c | 4 ++--
 hw/riscv/opentitan.c       | 2 +-
 hw/riscv/shakti_c.c        | 2 +-
 hw/riscv/sifive_e.c        | 2 +-
 hw/riscv/sifive_u.c        | 8 ++++----
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index cae74fcbcd..b21c8f6488 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,7 @@
 
 bool riscv_is_32bit(RISCVHartArrayState *harts)
 {
-    return harts->harts[0].env.misa_mxl_max == MXL_RV32;
+    return riscv_array_get_hart(harts, 0)->env.misa_mxl_max == MXL_RV32;
 }
 
 /*
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index cafd1fc9ae..82547a53e6 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -190,8 +190,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     NICInfo *nd;
     int i;
 
-    sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->u_cpus), &error_abort);
+    riscv_hart_array_realize(&s->e_cpus, &error_abort);
+    riscv_hart_array_realize(&s->u_cpus, &error_abort);
     /*
      * The cluster must be realized after the RISC-V hart array container,
      * as the container's CPU object is only created on realize, and the
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 833624d66c..2eb7454d8a 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -135,7 +135,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
     object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x8080, &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_abort);
+    riscv_hart_array_realize(&s->cpus, &error_abort);
 
     /* Boot ROM */
     memory_region_init_rom(&s->rom, OBJECT(dev_soc), "riscv.lowrisc.ibex.rom",
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 90e2cf609f..93e0c8dd68 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -108,7 +108,7 @@ static void shakti_c_soc_state_realize(DeviceState *dev, Error **errp)
     ShaktiCSoCState *sss = RISCV_SHAKTI_SOC(dev);
     MemoryRegion *system_memory = get_system_memory();
 
-    sysbus_realize(SYS_BUS_DEVICE(&sss->cpus), &error_abort);
+    riscv_hart_array_realize(&sss->cpus, &error_abort);
 
     sss->plic = sifive_plic_create(shakti_c_memmap[SHAKTI_C_PLIC].base,
         (char *)SHAKTI_C_PLIC_HART_CONFIG, ms->smp.cpus, 0,
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index dcb87b6cfd..25ba0a8c85 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -195,7 +195,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_abort);
+    riscv_hart_array_realize(&s->cpus, &error_abort);
 
     /* Mask ROM */
     memory_region_init_rom(&s->mask_rom, OBJECT(dev), "riscv.sifive.e.mrom",
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7fbc7dea42..c99e92a7eb 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -188,9 +188,9 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
             } else {
                 qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
             }
-            isa = riscv_isa_string(&s->soc.u_cpus.harts[cpu - 1]);
+            isa = riscv_isa_string(riscv_array_get_hart(&s->soc.u_cpus, cpu - 1));
         } else {
-            isa = riscv_isa_string(&s->soc.e_cpus.harts[0]);
+            isa = riscv_isa_string(riscv_array_get_hart(&s->soc.e_cpus, 0));
         }
         qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
         qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
@@ -830,8 +830,8 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", s->cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", 0x1004);
 
-    sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->u_cpus), &error_abort);
+    riscv_hart_array_realize(&s->e_cpus, &error_abort);
+    riscv_hart_array_realize(&s->u_cpus, &error_abort);
     /*
      * The cluster must be realized after the RISC-V hart array container,
      * as the container's CPU object is only created on realize, and the
-- 
2.35.1

[RFC PATCH 11/18] hw/riscv: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

spike and virt machines will be handled separately in following
commits.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/boot.c            | 2 +-
 hw/riscv/microchip_pfsoc.c | 4 ++--
 hw/riscv/opentitan.c       | 2 +-
 hw/riscv/shakti_c.c        | 2 +-
 hw/riscv/sifive_e.c        | 2 +-
 hw/riscv/sifive_u.c        | 8 ++++----
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index cae74fcbcd..b21c8f6488 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,7 @@
 
 bool riscv_is_32bit(RISCVHartArrayState *harts)
 {
-    return harts->harts[0].env.misa_mxl_max == MXL_RV32;
+    return riscv_array_get_hart(harts, 0)->env.misa_mxl_max == MXL_RV32;
 }
 
 /*
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index cafd1fc9ae..82547a53e6 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -190,8 +190,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     NICInfo *nd;
     int i;
 
-    sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->u_cpus), &error_abort);
+    riscv_hart_array_realize(&s->e_cpus, &error_abort);
+    riscv_hart_array_realize(&s->u_cpus, &error_abort);
     /*
      * The cluster must be realized after the RISC-V hart array container,
      * as the container's CPU object is only created on realize, and the
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 833624d66c..2eb7454d8a 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -135,7 +135,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
     object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x8080, &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_abort);
+    riscv_hart_array_realize(&s->cpus, &error_abort);
 
     /* Boot ROM */
     memory_region_init_rom(&s->rom, OBJECT(dev_soc), "riscv.lowrisc.ibex.rom",
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 90e2cf609f..93e0c8dd68 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -108,7 +108,7 @@ static void shakti_c_soc_state_realize(DeviceState *dev, Error **errp)
     ShaktiCSoCState *sss = RISCV_SHAKTI_SOC(dev);
     MemoryRegion *system_memory = get_system_memory();
 
-    sysbus_realize(SYS_BUS_DEVICE(&sss->cpus), &error_abort);
+    riscv_hart_array_realize(&sss->cpus, &error_abort);
 
     sss->plic = sifive_plic_create(shakti_c_memmap[SHAKTI_C_PLIC].base,
         (char *)SHAKTI_C_PLIC_HART_CONFIG, ms->smp.cpus, 0,
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index dcb87b6cfd..25ba0a8c85 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -195,7 +195,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_abort);
+    riscv_hart_array_realize(&s->cpus, &error_abort);
 
     /* Mask ROM */
     memory_region_init_rom(&s->mask_rom, OBJECT(dev), "riscv.sifive.e.mrom",
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7fbc7dea42..c99e92a7eb 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -188,9 +188,9 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
             } else {
                 qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
             }
-            isa = riscv_isa_string(&s->soc.u_cpus.harts[cpu - 1]);
+            isa = riscv_isa_string(riscv_array_get_hart(&s->soc.u_cpus, cpu - 1));
         } else {
-            isa = riscv_isa_string(&s->soc.e_cpus.harts[0]);
+            isa = riscv_isa_string(riscv_array_get_hart(&s->soc.e_cpus, 0));
         }
         qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
         qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
@@ -830,8 +830,8 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", s->cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", 0x1004);
 
-    sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort);
-    sysbus_realize(SYS_BUS_DEVICE(&s->u_cpus), &error_abort);
+    riscv_hart_array_realize(&s->e_cpus, &error_abort);
+    riscv_hart_array_realize(&s->u_cpus, &error_abort);
     /*
      * The cluster must be realized after the RISC-V hart array container,
      * as the container's CPU object is only created on realize, and the
-- 
2.35.1

[RFC PATCH 12/18] hw/riscv/virt: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_num_harts instead of accessing num_harts field.
+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/virt.c | 76 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index da50cbed43..12036aa95b 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -223,8 +223,11 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
     uint32_t cpu_phandle;
     MachineState *mc = MACHINE(s);
     char *name, *cpu_name, *core_name, *intc_name;
+    unsigned num_harts;
 
-    for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+
+    for (cpu = num_harts - 1; cpu >= 0; cpu--) {
         cpu_phandle = (*phandle)++;
 
         cpu_name = g_strdup_printf("/cpus/cpu@%d",
@@ -232,7 +235,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
         qemu_fdt_add_subnode(mc->fdt, cpu_name);
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
             (is_32_bit) ? "riscv,sv32" : "riscv,sv48");
-        name = riscv_isa_string(&s->soc[socket].harts[cpu]);
+        name = riscv_isa_string(riscv_array_get_hart(&s->soc[socket], cpu));
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
         g_free(name);
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "compatible", "riscv");
@@ -249,7 +252,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
         qemu_fdt_add_subnode(mc->fdt, intc_name);
         qemu_fdt_setprop_cell(mc->fdt, intc_name, "phandle",
             intc_phandles[cpu]);
-        if (riscv_feature(&s->soc[socket].harts[cpu].env,
+        if (riscv_feature(&riscv_array_get_hart(&s->soc[socket], cpu)->env,
                           RISCV_FEATURE_AIA)) {
             static const char * const compat[2] = {
                 "riscv,cpu-intc-aia", "riscv,cpu-intc"
@@ -299,14 +302,16 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     char *clint_name;
     uint32_t *clint_cells;
     unsigned long clint_addr;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     static const char * const clint_compat[2] = {
         "sifive,clint0", "riscv,clint0"
     };
 
-    clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    clint_cells = g_new0(uint32_t, num_harts * 4);
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         clint_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
         clint_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_SOFT);
         clint_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
@@ -322,7 +327,7 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
         0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
     qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
-        clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+        clint_cells, num_harts * sizeof(uint32_t) * 4);
     riscv_socket_fdt_write_id(mc, mc->fdt, clint_name, socket);
     g_free(clint_name);
 
@@ -340,13 +345,15 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     uint32_t *aclint_mswi_cells;
     uint32_t *aclint_sswi_cells;
     uint32_t *aclint_mtimer_cells;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
 
-    aclint_mswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
-    aclint_mtimer_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
-    aclint_sswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    aclint_mswi_cells = g_new0(uint32_t, num_harts * 2);
+    aclint_mtimer_cells = g_new0(uint32_t, num_harts * 2);
+    aclint_sswi_cells = g_new0(uint32_t, num_harts * 2);
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aclint_mswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aclint_mswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_SOFT);
         aclint_mtimer_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
@@ -354,7 +361,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
         aclint_sswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aclint_sswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_SOFT);
     }
-    aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2;
+    aclint_cells_size = num_harts * sizeof(uint32_t) * 2;
 
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
         addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
@@ -426,18 +433,20 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     char *plic_name;
     uint32_t *plic_cells;
     unsigned long plic_addr;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     static const char * const plic_compat[2] = {
         "sifive,plic-1.0.0", "riscv,plic0"
     };
 
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
     if (kvm_enabled()) {
-        plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+        plic_cells = g_new0(uint32_t, num_harts * 2);
     } else {
-        plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
+        plic_cells = g_new0(uint32_t, num_harts * 4);
     }
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         if (kvm_enabled()) {
             plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
             plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
@@ -460,7 +469,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
                                   ARRAY_SIZE(plic_compat));
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
-        plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+        plic_cells, num_harts * sizeof(uint32_t) * 4);
     qemu_fdt_setprop_cells(mc->fdt, plic_name, "reg",
         0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
     qemu_fdt_setprop_cell(mc->fdt, plic_name, "riscv,ndev", VIRTIO_NDEV);
@@ -492,6 +501,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t imsic_max_hart_per_socket, imsic_guest_bits;
     uint32_t *imsic_cells, *imsic_regs, imsic_addr, imsic_size;
+    unsigned num_harts;
 
     *msi_m_phandle = (*phandle)++;
     *msi_s_phandle = (*phandle)++;
@@ -505,15 +515,16 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     }
     imsic_max_hart_per_socket = 0;
     for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
         imsic_addr = memmap[VIRT_IMSIC_M].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
-        imsic_size = IMSIC_HART_SIZE(0) * s->soc[socket].num_harts;
+        imsic_size = IMSIC_HART_SIZE(0) * num_harts;
         imsic_regs[socket * 4 + 0] = 0;
         imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
         imsic_regs[socket * 4 + 2] = 0;
         imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
-        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
-            imsic_max_hart_per_socket = s->soc[socket].num_harts;
+        if (imsic_max_hart_per_socket < num_harts) {
+            imsic_max_hart_per_socket = num_harts;
         }
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
@@ -554,16 +565,16 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     imsic_guest_bits = imsic_num_bits(s->aia_guests + 1);
     imsic_max_hart_per_socket = 0;
     for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
         imsic_addr = memmap[VIRT_IMSIC_S].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
-        imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) *
-                     s->soc[socket].num_harts;
+        imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) * num_harts;
         imsic_regs[socket * 4 + 0] = 0;
         imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
         imsic_regs[socket * 4 + 2] = 0;
         imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
-        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
-            imsic_max_hart_per_socket = s->soc[socket].num_harts;
+        if (imsic_max_hart_per_socket < num_harts) {
+            imsic_max_hart_per_socket = num_harts;
         }
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
@@ -618,13 +629,15 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     unsigned long aplic_addr;
     MachineState *mc = MACHINE(s);
     uint32_t aplic_m_phandle, aplic_s_phandle;
+    unsigned num_harts;
 
     aplic_m_phandle = (*phandle)++;
     aplic_s_phandle = (*phandle)++;
-    aplic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    aplic_cells = g_new0(uint32_t, num_harts * 2);
 
     /* M-level APLIC node */
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
     }
@@ -638,7 +651,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
         qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
-            aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
+            aplic_cells, num_harts * sizeof(uint32_t) * 2);
     } else {
         qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
             msi_m_phandle);
@@ -656,7 +669,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     g_free(aplic_name);
 
     /* S-level APLIC node */
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
     }
@@ -670,7 +683,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
         qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
-            aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
+            aplic_cells, num_harts * sizeof(uint32_t) * 2);
     } else {
         qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
             msi_s_phandle);
@@ -699,6 +712,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t msi_m_phandle = 0, msi_s_phandle = 0;
     uint32_t *intc_phandles, xplic_phandles[MAX_NODES];
+    unsigned num_harts;
 
     qemu_fdt_add_subnode(mc->fdt, "/cpus");
     qemu_fdt_setprop_cell(mc->fdt, "/cpus", "timebase-frequency",
@@ -711,7 +725,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     phandle_pos = mc->smp.cpus;
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
-        phandle_pos -= s->soc[socket].num_harts;
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        phandle_pos -= num_harts;
 
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
         qemu_fdt_add_subnode(mc->fdt, clust_name);
@@ -742,7 +757,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     phandle_pos = mc->smp.cpus;
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
-        phandle_pos -= s->soc[socket].num_harts;
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        phandle_pos -= num_harts;
 
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
             create_fdt_socket_plic(s, memmap, socket, phandle,
@@ -1207,7 +1223,7 @@ static void virt_machine_init(MachineState *machine)
                                 base_hartid, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
                                 hart_count, &error_abort);
-        sysbus_realize(SYS_BUS_DEVICE(&s->soc[i]), &error_abort);
+        riscv_hart_array_realize(&s->soc[i], &error_abort);
 
         if (!kvm_enabled()) {
             if (s->have_aclint) {
-- 
2.35.1

[RFC PATCH 12/18] hw/riscv/virt: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_num_harts instead of accessing num_harts field.
+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/virt.c | 76 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index da50cbed43..12036aa95b 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -223,8 +223,11 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
     uint32_t cpu_phandle;
     MachineState *mc = MACHINE(s);
     char *name, *cpu_name, *core_name, *intc_name;
+    unsigned num_harts;
 
-    for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+
+    for (cpu = num_harts - 1; cpu >= 0; cpu--) {
         cpu_phandle = (*phandle)++;
 
         cpu_name = g_strdup_printf("/cpus/cpu@%d",
@@ -232,7 +235,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
         qemu_fdt_add_subnode(mc->fdt, cpu_name);
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
             (is_32_bit) ? "riscv,sv32" : "riscv,sv48");
-        name = riscv_isa_string(&s->soc[socket].harts[cpu]);
+        name = riscv_isa_string(riscv_array_get_hart(&s->soc[socket], cpu));
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
         g_free(name);
         qemu_fdt_setprop_string(mc->fdt, cpu_name, "compatible", "riscv");
@@ -249,7 +252,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
         qemu_fdt_add_subnode(mc->fdt, intc_name);
         qemu_fdt_setprop_cell(mc->fdt, intc_name, "phandle",
             intc_phandles[cpu]);
-        if (riscv_feature(&s->soc[socket].harts[cpu].env,
+        if (riscv_feature(&riscv_array_get_hart(&s->soc[socket], cpu)->env,
                           RISCV_FEATURE_AIA)) {
             static const char * const compat[2] = {
                 "riscv,cpu-intc-aia", "riscv,cpu-intc"
@@ -299,14 +302,16 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     char *clint_name;
     uint32_t *clint_cells;
     unsigned long clint_addr;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     static const char * const clint_compat[2] = {
         "sifive,clint0", "riscv,clint0"
     };
 
-    clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    clint_cells = g_new0(uint32_t, num_harts * 4);
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         clint_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
         clint_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_SOFT);
         clint_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
@@ -322,7 +327,7 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
         0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
     qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
-        clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+        clint_cells, num_harts * sizeof(uint32_t) * 4);
     riscv_socket_fdt_write_id(mc, mc->fdt, clint_name, socket);
     g_free(clint_name);
 
@@ -340,13 +345,15 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     uint32_t *aclint_mswi_cells;
     uint32_t *aclint_sswi_cells;
     uint32_t *aclint_mtimer_cells;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
 
-    aclint_mswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
-    aclint_mtimer_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
-    aclint_sswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    aclint_mswi_cells = g_new0(uint32_t, num_harts * 2);
+    aclint_mtimer_cells = g_new0(uint32_t, num_harts * 2);
+    aclint_sswi_cells = g_new0(uint32_t, num_harts * 2);
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aclint_mswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aclint_mswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_SOFT);
         aclint_mtimer_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
@@ -354,7 +361,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
         aclint_sswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aclint_sswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_SOFT);
     }
-    aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2;
+    aclint_cells_size = num_harts * sizeof(uint32_t) * 2;
 
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
         addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
@@ -426,18 +433,20 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     char *plic_name;
     uint32_t *plic_cells;
     unsigned long plic_addr;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     static const char * const plic_compat[2] = {
         "sifive,plic-1.0.0", "riscv,plic0"
     };
 
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
     if (kvm_enabled()) {
-        plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+        plic_cells = g_new0(uint32_t, num_harts * 2);
     } else {
-        plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
+        plic_cells = g_new0(uint32_t, num_harts * 4);
     }
 
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         if (kvm_enabled()) {
             plic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
             plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
@@ -460,7 +469,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
                                   ARRAY_SIZE(plic_compat));
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
-        plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+        plic_cells, num_harts * sizeof(uint32_t) * 4);
     qemu_fdt_setprop_cells(mc->fdt, plic_name, "reg",
         0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
     qemu_fdt_setprop_cell(mc->fdt, plic_name, "riscv,ndev", VIRTIO_NDEV);
@@ -492,6 +501,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t imsic_max_hart_per_socket, imsic_guest_bits;
     uint32_t *imsic_cells, *imsic_regs, imsic_addr, imsic_size;
+    unsigned num_harts;
 
     *msi_m_phandle = (*phandle)++;
     *msi_s_phandle = (*phandle)++;
@@ -505,15 +515,16 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     }
     imsic_max_hart_per_socket = 0;
     for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
         imsic_addr = memmap[VIRT_IMSIC_M].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
-        imsic_size = IMSIC_HART_SIZE(0) * s->soc[socket].num_harts;
+        imsic_size = IMSIC_HART_SIZE(0) * num_harts;
         imsic_regs[socket * 4 + 0] = 0;
         imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
         imsic_regs[socket * 4 + 2] = 0;
         imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
-        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
-            imsic_max_hart_per_socket = s->soc[socket].num_harts;
+        if (imsic_max_hart_per_socket < num_harts) {
+            imsic_max_hart_per_socket = num_harts;
         }
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
@@ -554,16 +565,16 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     imsic_guest_bits = imsic_num_bits(s->aia_guests + 1);
     imsic_max_hart_per_socket = 0;
     for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
         imsic_addr = memmap[VIRT_IMSIC_S].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
-        imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) *
-                     s->soc[socket].num_harts;
+        imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) * num_harts;
         imsic_regs[socket * 4 + 0] = 0;
         imsic_regs[socket * 4 + 1] = cpu_to_be32(imsic_addr);
         imsic_regs[socket * 4 + 2] = 0;
         imsic_regs[socket * 4 + 3] = cpu_to_be32(imsic_size);
-        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
-            imsic_max_hart_per_socket = s->soc[socket].num_harts;
+        if (imsic_max_hart_per_socket < num_harts) {
+            imsic_max_hart_per_socket = num_harts;
         }
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
@@ -618,13 +629,15 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     unsigned long aplic_addr;
     MachineState *mc = MACHINE(s);
     uint32_t aplic_m_phandle, aplic_s_phandle;
+    unsigned num_harts;
 
     aplic_m_phandle = (*phandle)++;
     aplic_s_phandle = (*phandle)++;
-    aplic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+    num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+    aplic_cells = g_new0(uint32_t, num_harts * 2);
 
     /* M-level APLIC node */
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
     }
@@ -638,7 +651,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
         qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
-            aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
+            aplic_cells, num_harts * sizeof(uint32_t) * 2);
     } else {
         qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
             msi_m_phandle);
@@ -656,7 +669,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     g_free(aplic_name);
 
     /* S-level APLIC node */
-    for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+    for (cpu = 0; cpu < num_harts; cpu++) {
         aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         aplic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
     }
@@ -670,7 +683,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
         qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
-            aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
+            aplic_cells, num_harts * sizeof(uint32_t) * 2);
     } else {
         qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
             msi_s_phandle);
@@ -699,6 +712,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t msi_m_phandle = 0, msi_s_phandle = 0;
     uint32_t *intc_phandles, xplic_phandles[MAX_NODES];
+    unsigned num_harts;
 
     qemu_fdt_add_subnode(mc->fdt, "/cpus");
     qemu_fdt_setprop_cell(mc->fdt, "/cpus", "timebase-frequency",
@@ -711,7 +725,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     phandle_pos = mc->smp.cpus;
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
-        phandle_pos -= s->soc[socket].num_harts;
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        phandle_pos -= num_harts;
 
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
         qemu_fdt_add_subnode(mc->fdt, clust_name);
@@ -742,7 +757,8 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     phandle_pos = mc->smp.cpus;
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
-        phandle_pos -= s->soc[socket].num_harts;
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        phandle_pos -= num_harts;
 
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
             create_fdt_socket_plic(s, memmap, socket, phandle,
@@ -1207,7 +1223,7 @@ static void virt_machine_init(MachineState *machine)
                                 base_hartid, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
                                 hart_count, &error_abort);
-        sysbus_realize(SYS_BUS_DEVICE(&s->soc[i]), &error_abort);
+        riscv_hart_array_realize(&s->soc[i], &error_abort);
 
         if (!kvm_enabled()) {
             if (s->have_aclint) {
-- 
2.35.1

[RFC PATCH 13/18] hw/riscv/spike: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_num_harts instead of accessing num_harts field.
+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/spike.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d059a67f9b..b75e3656e1 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -54,6 +54,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     uint64_t addr, size;
     unsigned long clint_addr;
     int cpu, socket;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     uint32_t *clint_cells;
     uint32_t cpu_phandle, intc_phandle, phandle = 1;
@@ -97,10 +98,10 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
         qemu_fdt_add_subnode(fdt, clust_name);
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        clint_cells =  g_new0(uint32_t, num_harts * 4);
 
-        clint_cells =  g_new0(uint32_t, s->soc[socket].num_harts * 4);
-
-        for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
+        for (cpu = num_harts - 1; cpu >= 0; cpu--) {
             cpu_phandle = phandle++;
 
             cpu_name = g_strdup_printf("/cpus/cpu@%d",
@@ -111,7 +112,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             } else {
                 qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
             }
-            name = riscv_isa_string(&s->soc[socket].harts[cpu]);
+            name = riscv_isa_string(riscv_array_get_hart(&s->soc[socket], cpu));
             qemu_fdt_setprop_string(fdt, cpu_name, "riscv,isa", name);
             g_free(name);
             qemu_fdt_setprop_string(fdt, cpu_name, "compatible", "riscv");
@@ -164,7 +165,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
         qemu_fdt_setprop_cells(fdt, clint_name, "reg",
             0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
         qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
-            clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+            clint_cells, num_harts * sizeof(uint32_t) * 4);
         riscv_socket_fdt_write_id(mc, fdt, clint_name, socket);
 
         g_free(clint_name);
@@ -229,7 +230,7 @@ static void spike_board_init(MachineState *machine)
                                 base_hartid, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
                                 hart_count, &error_abort);
-        sysbus_realize(SYS_BUS_DEVICE(&s->soc[i]), &error_abort);
+        riscv_hart_array_realize(&s->soc[i], &error_abort);
 
         /* Core Local Interruptor (timer and IPI) for each socket */
         riscv_aclint_swi_create(
@@ -311,7 +312,7 @@ static void spike_board_init(MachineState *machine)
 
     /* initialize HTIF using symbols found in load_kernel */
     htif_mm_init(system_memory, mask_rom,
-                 &s->soc[0].harts[0].env, serial_hd(0),
+                 &riscv_array_get_hart(&s->soc[0], 0)->env, serial_hd(0),
                  memmap[SPIKE_HTIF].base);
 }
 
-- 
2.35.1

[RFC PATCH 13/18] hw/riscv/spike: prepare riscv_hart transition to cpus

[Damien Hedde]

+ Use riscv_array_get_num_harts instead of accessing num_harts field.
+ Use riscv_array_get_hart instead of accessing harts field.
+ Use riscv_hart_array_realize instead of sysbus_realize.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/spike.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d059a67f9b..b75e3656e1 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -54,6 +54,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     uint64_t addr, size;
     unsigned long clint_addr;
     int cpu, socket;
+    unsigned num_harts;
     MachineState *mc = MACHINE(s);
     uint32_t *clint_cells;
     uint32_t cpu_phandle, intc_phandle, phandle = 1;
@@ -97,10 +98,10 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
         qemu_fdt_add_subnode(fdt, clust_name);
+        num_harts = riscv_array_get_num_harts(&s->soc[socket]);
+        clint_cells =  g_new0(uint32_t, num_harts * 4);
 
-        clint_cells =  g_new0(uint32_t, s->soc[socket].num_harts * 4);
-
-        for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
+        for (cpu = num_harts - 1; cpu >= 0; cpu--) {
             cpu_phandle = phandle++;
 
             cpu_name = g_strdup_printf("/cpus/cpu@%d",
@@ -111,7 +112,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             } else {
                 qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
             }
-            name = riscv_isa_string(&s->soc[socket].harts[cpu]);
+            name = riscv_isa_string(riscv_array_get_hart(&s->soc[socket], cpu));
             qemu_fdt_setprop_string(fdt, cpu_name, "riscv,isa", name);
             g_free(name);
             qemu_fdt_setprop_string(fdt, cpu_name, "compatible", "riscv");
@@ -164,7 +165,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
         qemu_fdt_setprop_cells(fdt, clint_name, "reg",
             0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
         qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
-            clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
+            clint_cells, num_harts * sizeof(uint32_t) * 4);
         riscv_socket_fdt_write_id(mc, fdt, clint_name, socket);
 
         g_free(clint_name);
@@ -229,7 +230,7 @@ static void spike_board_init(MachineState *machine)
                                 base_hartid, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
                                 hart_count, &error_abort);
-        sysbus_realize(SYS_BUS_DEVICE(&s->soc[i]), &error_abort);
+        riscv_hart_array_realize(&s->soc[i], &error_abort);
 
         /* Core Local Interruptor (timer and IPI) for each socket */
         riscv_aclint_swi_create(
@@ -311,7 +312,7 @@ static void spike_board_init(MachineState *machine)
 
     /* initialize HTIF using symbols found in load_kernel */
     htif_mm_init(system_memory, mask_rom,
-                 &s->soc[0].harts[0].env, serial_hd(0),
+                 &riscv_array_get_hart(&s->soc[0], 0)->env, serial_hd(0),
                  memmap[SPIKE_HTIF].base);
 }
 
-- 
2.35.1

[RFC PATCH 14/18] hw/riscv/riscv_hart: use cpus as base class

[Damien Hedde]

This is a drastic update: qom-path of riscv harts are changed
by this patch from "/path/to/array/hart[n]" to "/path/to/array/cpu[n]".

This object is now not anymore a SYS_BUS_DEVICE.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

I expect it is an issue regarding migration of riscv machines.
It is possible to keep an "hart[n]" alias to "cpu[n]" or even
to be able to configure the cpu child basename to "hart".

I'm not sure what we need to keep migration working: Does qom-path
has to stay identical ? Or having aliases is ok ?

Any ideas or comments ?

Any of these changes could be fixed by adding support in the base class
(child name, sysbus device, ...) if needed.
---
 include/hw/riscv/riscv_hart.h | 17 ++++++------
 hw/riscv/riscv_hart.c         | 49 ++++++++++++++---------------------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 71747bf37c..65ac0d2bc4 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -21,7 +21,7 @@
 #ifndef HW_RISCV_HART_H
 #define HW_RISCV_HART_H
 
-#include "hw/sysbus.h"
+#include "hw/cpu/cpus.h"
 #include "target/riscv/cpu.h"
 #include "qom/object.h"
 
@@ -31,30 +31,29 @@ OBJECT_DECLARE_SIMPLE_TYPE(RISCVHartArrayState, RISCV_HART_ARRAY)
 
 struct RISCVHartArrayState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    CpusState parent_obj;
 
     /*< public >*/
-    uint32_t num_harts;
     uint32_t hartid_base;
-    char *cpu_type;
     uint64_t resetvec;
-    RISCVCPU *harts;
 };
 
 /**
  * riscv_array_get_hart:
+ * Helper to get an hart from the container.
  */
-static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *harts, int i)
+static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *s, int i)
 {
-    return &harts->harts[i];
+    return RISCV_CPU(CPUS(s)->cpus[i]);
 }
 
 /**
  * riscv_array_get_num_harts:
+ * Helper to get the number of harts in the container.
  */
-static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *harts)
+static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *s)
 {
-    return harts->num_harts;
+    return CPUS(s)->topology.cpus;
 }
 
 /* Temporary function until we migrated the riscv hart array to simple device */
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 780fd3a59a..1b4ff7e3c6 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -22,67 +22,58 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "sysemu/reset.h"
-#include "hw/sysbus.h"
 #include "target/riscv/cpu.h"
 #include "hw/qdev-properties.h"
 #include "hw/riscv/riscv_hart.h"
+#include "hw/cpu/cpus.h"
 
 void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
 {
-    sysbus_realize(SYS_BUS_DEVICE(state), errp);
+    /* disable the clustering */
+    cpus_disable_clustering(CPUS(state));
+    qdev_realize(DEVICE(state), NULL, errp);
 }
 
 static Property riscv_harts_props[] = {
-    DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
-    DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
                        DEFAULT_RSTVEC),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void riscv_harts_cpu_reset(void *opaque)
+static void riscv_harts_configure_cpu(CpusState *base, CPUState *cpu,
+                                      unsigned i)
 {
-    RISCVCPU *cpu = opaque;
-    cpu_reset(CPU(cpu));
-}
+    RISCVHartArrayState *s = RISCV_HART_ARRAY(base);
+    DeviceState *cpudev = DEVICE(cpu);
+    CPURISCVState *cpuenv = cpu->env_ptr;
 
-static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
-                               char *cpu_type, Error **errp)
-{
-    object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
-    qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
-    s->harts[idx].env.mhartid = s->hartid_base + idx;
-    qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
-    return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
+    qdev_prop_set_uint64(cpudev, "resetvec", s->resetvec);
+    cpuenv->mhartid = s->hartid_base + i;
 }
 
-static void riscv_harts_realize(DeviceState *dev, Error **errp)
+static void riscv_harts_init(Object *obj)
 {
-    RISCVHartArrayState *s = RISCV_HART_ARRAY(dev);
-    int n;
-
-    s->harts = g_new0(RISCVCPU, s->num_harts);
-
-    for (n = 0; n < s->num_harts; n++) {
-        if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
-            return;
-        }
-    }
+    /* add a temporary property to keep num-harts */
+    object_property_add_alias(obj, "num-harts", obj, "num-cpus");
 }
 
 static void riscv_harts_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
 
     device_class_set_props(dc, riscv_harts_props);
-    dc->realize = riscv_harts_realize;
+
+    cc->base_cpu_type = TYPE_RISCV_CPU;
+    cc->configure_cpu = riscv_harts_configure_cpu;
 }
 
 static const TypeInfo riscv_harts_info = {
     .name          = TYPE_RISCV_HART_ARRAY,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_CPUS,
     .instance_size = sizeof(RISCVHartArrayState),
+    .instance_init = riscv_harts_init,
     .class_init    = riscv_harts_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 14/18] hw/riscv/riscv_hart: use cpus as base class

[Damien Hedde]

This is a drastic update: qom-path of riscv harts are changed
by this patch from "/path/to/array/hart[n]" to "/path/to/array/cpu[n]".

This object is now not anymore a SYS_BUS_DEVICE.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

I expect it is an issue regarding migration of riscv machines.
It is possible to keep an "hart[n]" alias to "cpu[n]" or even
to be able to configure the cpu child basename to "hart".

I'm not sure what we need to keep migration working: Does qom-path
has to stay identical ? Or having aliases is ok ?

Any ideas or comments ?

Any of these changes could be fixed by adding support in the base class
(child name, sysbus device, ...) if needed.
---
 include/hw/riscv/riscv_hart.h | 17 ++++++------
 hw/riscv/riscv_hart.c         | 49 ++++++++++++++---------------------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 71747bf37c..65ac0d2bc4 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -21,7 +21,7 @@
 #ifndef HW_RISCV_HART_H
 #define HW_RISCV_HART_H
 
-#include "hw/sysbus.h"
+#include "hw/cpu/cpus.h"
 #include "target/riscv/cpu.h"
 #include "qom/object.h"
 
@@ -31,30 +31,29 @@ OBJECT_DECLARE_SIMPLE_TYPE(RISCVHartArrayState, RISCV_HART_ARRAY)
 
 struct RISCVHartArrayState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    CpusState parent_obj;
 
     /*< public >*/
-    uint32_t num_harts;
     uint32_t hartid_base;
-    char *cpu_type;
     uint64_t resetvec;
-    RISCVCPU *harts;
 };
 
 /**
  * riscv_array_get_hart:
+ * Helper to get an hart from the container.
  */
-static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *harts, int i)
+static inline RISCVCPU *riscv_array_get_hart(RISCVHartArrayState *s, int i)
 {
-    return &harts->harts[i];
+    return RISCV_CPU(CPUS(s)->cpus[i]);
 }
 
 /**
  * riscv_array_get_num_harts:
+ * Helper to get the number of harts in the container.
  */
-static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *harts)
+static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *s)
 {
-    return harts->num_harts;
+    return CPUS(s)->topology.cpus;
 }
 
 /* Temporary function until we migrated the riscv hart array to simple device */
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 780fd3a59a..1b4ff7e3c6 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -22,67 +22,58 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "sysemu/reset.h"
-#include "hw/sysbus.h"
 #include "target/riscv/cpu.h"
 #include "hw/qdev-properties.h"
 #include "hw/riscv/riscv_hart.h"
+#include "hw/cpu/cpus.h"
 
 void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
 {
-    sysbus_realize(SYS_BUS_DEVICE(state), errp);
+    /* disable the clustering */
+    cpus_disable_clustering(CPUS(state));
+    qdev_realize(DEVICE(state), NULL, errp);
 }
 
 static Property riscv_harts_props[] = {
-    DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
-    DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
                        DEFAULT_RSTVEC),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void riscv_harts_cpu_reset(void *opaque)
+static void riscv_harts_configure_cpu(CpusState *base, CPUState *cpu,
+                                      unsigned i)
 {
-    RISCVCPU *cpu = opaque;
-    cpu_reset(CPU(cpu));
-}
+    RISCVHartArrayState *s = RISCV_HART_ARRAY(base);
+    DeviceState *cpudev = DEVICE(cpu);
+    CPURISCVState *cpuenv = cpu->env_ptr;
 
-static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
-                               char *cpu_type, Error **errp)
-{
-    object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
-    qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
-    s->harts[idx].env.mhartid = s->hartid_base + idx;
-    qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
-    return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
+    qdev_prop_set_uint64(cpudev, "resetvec", s->resetvec);
+    cpuenv->mhartid = s->hartid_base + i;
 }
 
-static void riscv_harts_realize(DeviceState *dev, Error **errp)
+static void riscv_harts_init(Object *obj)
 {
-    RISCVHartArrayState *s = RISCV_HART_ARRAY(dev);
-    int n;
-
-    s->harts = g_new0(RISCVCPU, s->num_harts);
-
-    for (n = 0; n < s->num_harts; n++) {
-        if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
-            return;
-        }
-    }
+    /* add a temporary property to keep num-harts */
+    object_property_add_alias(obj, "num-harts", obj, "num-cpus");
 }
 
 static void riscv_harts_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    CpusClass *cc = CPUS_CLASS(klass);
 
     device_class_set_props(dc, riscv_harts_props);
-    dc->realize = riscv_harts_realize;
+
+    cc->base_cpu_type = TYPE_RISCV_CPU;
+    cc->configure_cpu = riscv_harts_configure_cpu;
 }
 
 static const TypeInfo riscv_harts_info = {
     .name          = TYPE_RISCV_HART_ARRAY,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_CPUS,
     .instance_size = sizeof(RISCVHartArrayState),
+    .instance_init = riscv_harts_init,
     .class_init    = riscv_harts_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 15/18] hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update

[Damien Hedde]

These machines were creating 2 explicit clusters of different cpus
(a cluster of 1 cpu and a cluster of N cpus). These are removed as
they are now embedded in the riscv array.

Note: The qom-path of the riscv hart arrays are changed, the cluster
level is removed:
+ "/path/to/e-cluster/e-cpus" to "/path/to/e-cpus"
+ "/path/to/u-cluster/u-cpus" to "/path/to/u-cpus"

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

If keeping the qom-paths is necessary we can add a container as
a replacement of the cluster.
---
 include/hw/riscv/microchip_pfsoc.h |  2 --
 include/hw/riscv/sifive_u.h        |  2 --
 hw/riscv/microchip_pfsoc.c         | 28 ++++++----------------------
 hw/riscv/sifive_u.c                | 27 ++++++---------------------
 4 files changed, 12 insertions(+), 47 deletions(-)

diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index a0673f5f59..9101c94978 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -35,8 +35,6 @@ typedef struct MicrochipPFSoCState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState e_cluster;
-    CPUClusterState u_cluster;
     RISCVHartArrayState e_cpus;
     RISCVHartArrayState u_cpus;
     DeviceState *plic;
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 8f63a183c4..5439e0d0c3 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -38,8 +38,6 @@ typedef struct SiFiveUSoCState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState e_cluster;
-    CPUClusterState u_cluster;
     RISCVHartArrayState e_cpus;
     RISCVHartArrayState u_cpus;
     DeviceState *plic;
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 82547a53e6..f4b1400ba5 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -133,23 +133,15 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
     MachineState *ms = MACHINE(qdev_get_machine());
     MicrochipPFSoCState *s = MICROCHIP_PFSOC(obj);
 
-    object_initialize_child(obj, "e-cluster", &s->e_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->e_cluster), "cluster-id", 0);
-
-    object_initialize_child(OBJECT(&s->e_cluster), "e-cpus", &s->e_cpus,
-                            TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-harts", 1);
+    object_initialize_child(obj, "e-cpus", &s->e_cpus, TYPE_RISCV_HART_ARRAY);
+    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-cpus", 1);
     qdev_prop_set_uint32(DEVICE(&s->e_cpus), "hartid-base", 0);
     qdev_prop_set_string(DEVICE(&s->e_cpus), "cpu-type",
                          TYPE_RISCV_CPU_SIFIVE_E51);
     qdev_prop_set_uint64(DEVICE(&s->e_cpus), "resetvec", RESET_VECTOR);
 
-    object_initialize_child(obj, "u-cluster", &s->u_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->u_cluster), "cluster-id", 1);
-
-    object_initialize_child(OBJECT(&s->u_cluster), "u-cpus", &s->u_cpus,
-                            TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-harts", ms->smp.cpus - 1);
+    object_initialize_child(obj, "u-cpus", &s->u_cpus, TYPE_RISCV_HART_ARRAY);
+    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-cpus", ms->smp.cpus - 1);
     qdev_prop_set_uint32(DEVICE(&s->u_cpus), "hartid-base", 1);
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type",
                          TYPE_RISCV_CPU_SIFIVE_U54);
@@ -190,16 +182,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     NICInfo *nd;
     int i;
 
-    riscv_hart_array_realize(&s->e_cpus, &error_abort);
-    riscv_hart_array_realize(&s->u_cpus, &error_abort);
-    /*
-     * The cluster must be realized after the RISC-V hart array container,
-     * as the container's CPU object is only created on realize, and the
-     * CPU must exist and have been parented into the cluster before the
-     * cluster is realized.
-     */
-    qdev_realize(DEVICE(&s->e_cluster), NULL, &error_abort);
-    qdev_realize(DEVICE(&s->u_cluster), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->e_cpus), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->u_cpus), NULL, &error_abort);
 
     /* Reserved Memory at address 0 */
     memory_region_init_ram(rsvd0_mem, NULL, "microchip.pfsoc.rsvd0_mem",
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index c99e92a7eb..1d9a7c5bf1 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -44,7 +44,6 @@
 #include "hw/loader.h"
 #include "hw/sysbus.h"
 #include "hw/char/serial.h"
-#include "hw/cpu/cluster.h"
 #include "hw/misc/unimp.h"
 #include "hw/sd/sd.h"
 #include "hw/ssi/ssi.h"
@@ -786,20 +785,14 @@ static void sifive_u_soc_instance_init(Object *obj)
 {
     SiFiveUSoCState *s = RISCV_U_SOC(obj);
 
-    object_initialize_child(obj, "e-cluster", &s->e_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->e_cluster), "cluster-id", 0);
-
-    object_initialize_child(OBJECT(&s->e_cluster), "e-cpus", &s->e_cpus,
+    object_initialize_child(obj, "e-cpus", &s->e_cpus,
                             TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-harts", 1);
+    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-cpus", 1);
     qdev_prop_set_uint32(DEVICE(&s->e_cpus), "hartid-base", 0);
     qdev_prop_set_string(DEVICE(&s->e_cpus), "cpu-type", SIFIVE_E_CPU);
     qdev_prop_set_uint64(DEVICE(&s->e_cpus), "resetvec", 0x1004);
 
-    object_initialize_child(obj, "u-cluster", &s->u_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->u_cluster), "cluster-id", 1);
-
-    object_initialize_child(OBJECT(&s->u_cluster), "u-cpus", &s->u_cpus,
+    object_initialize_child(obj, "u-cpus", &s->u_cpus,
                             TYPE_RISCV_HART_ARRAY);
 
     object_initialize_child(obj, "prci", &s->prci, TYPE_SIFIVE_U_PRCI);
@@ -825,21 +818,13 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     int i, j;
     NICInfo *nd = &nd_table[0];
 
-    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-harts", ms->smp.cpus - 1);
+    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-cpus", ms->smp.cpus - 1);
     qdev_prop_set_uint32(DEVICE(&s->u_cpus), "hartid-base", 1);
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", s->cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", 0x1004);
 
-    riscv_hart_array_realize(&s->e_cpus, &error_abort);
-    riscv_hart_array_realize(&s->u_cpus, &error_abort);
-    /*
-     * The cluster must be realized after the RISC-V hart array container,
-     * as the container's CPU object is only created on realize, and the
-     * CPU must exist and have been parented into the cluster before the
-     * cluster is realized.
-     */
-    qdev_realize(DEVICE(&s->e_cluster), NULL, &error_abort);
-    qdev_realize(DEVICE(&s->u_cluster), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->e_cpus), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->u_cpus), NULL, &error_abort);
 
     /* boot rom */
     memory_region_init_rom(mask_rom, OBJECT(dev), "riscv.sifive.u.mrom",
-- 
2.35.1

[RFC PATCH 15/18] hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update

[Damien Hedde]

These machines were creating 2 explicit clusters of different cpus
(a cluster of 1 cpu and a cluster of N cpus). These are removed as
they are now embedded in the riscv array.

Note: The qom-path of the riscv hart arrays are changed, the cluster
level is removed:
+ "/path/to/e-cluster/e-cpus" to "/path/to/e-cpus"
+ "/path/to/u-cluster/u-cpus" to "/path/to/u-cpus"

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

If keeping the qom-paths is necessary we can add a container as
a replacement of the cluster.
---
 include/hw/riscv/microchip_pfsoc.h |  2 --
 include/hw/riscv/sifive_u.h        |  2 --
 hw/riscv/microchip_pfsoc.c         | 28 ++++++----------------------
 hw/riscv/sifive_u.c                | 27 ++++++---------------------
 4 files changed, 12 insertions(+), 47 deletions(-)

diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index a0673f5f59..9101c94978 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -35,8 +35,6 @@ typedef struct MicrochipPFSoCState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState e_cluster;
-    CPUClusterState u_cluster;
     RISCVHartArrayState e_cpus;
     RISCVHartArrayState u_cpus;
     DeviceState *plic;
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 8f63a183c4..5439e0d0c3 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -38,8 +38,6 @@ typedef struct SiFiveUSoCState {
     DeviceState parent_obj;
 
     /*< public >*/
-    CPUClusterState e_cluster;
-    CPUClusterState u_cluster;
     RISCVHartArrayState e_cpus;
     RISCVHartArrayState u_cpus;
     DeviceState *plic;
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 82547a53e6..f4b1400ba5 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -133,23 +133,15 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
     MachineState *ms = MACHINE(qdev_get_machine());
     MicrochipPFSoCState *s = MICROCHIP_PFSOC(obj);
 
-    object_initialize_child(obj, "e-cluster", &s->e_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->e_cluster), "cluster-id", 0);
-
-    object_initialize_child(OBJECT(&s->e_cluster), "e-cpus", &s->e_cpus,
-                            TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-harts", 1);
+    object_initialize_child(obj, "e-cpus", &s->e_cpus, TYPE_RISCV_HART_ARRAY);
+    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-cpus", 1);
     qdev_prop_set_uint32(DEVICE(&s->e_cpus), "hartid-base", 0);
     qdev_prop_set_string(DEVICE(&s->e_cpus), "cpu-type",
                          TYPE_RISCV_CPU_SIFIVE_E51);
     qdev_prop_set_uint64(DEVICE(&s->e_cpus), "resetvec", RESET_VECTOR);
 
-    object_initialize_child(obj, "u-cluster", &s->u_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->u_cluster), "cluster-id", 1);
-
-    object_initialize_child(OBJECT(&s->u_cluster), "u-cpus", &s->u_cpus,
-                            TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-harts", ms->smp.cpus - 1);
+    object_initialize_child(obj, "u-cpus", &s->u_cpus, TYPE_RISCV_HART_ARRAY);
+    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-cpus", ms->smp.cpus - 1);
     qdev_prop_set_uint32(DEVICE(&s->u_cpus), "hartid-base", 1);
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type",
                          TYPE_RISCV_CPU_SIFIVE_U54);
@@ -190,16 +182,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     NICInfo *nd;
     int i;
 
-    riscv_hart_array_realize(&s->e_cpus, &error_abort);
-    riscv_hart_array_realize(&s->u_cpus, &error_abort);
-    /*
-     * The cluster must be realized after the RISC-V hart array container,
-     * as the container's CPU object is only created on realize, and the
-     * CPU must exist and have been parented into the cluster before the
-     * cluster is realized.
-     */
-    qdev_realize(DEVICE(&s->e_cluster), NULL, &error_abort);
-    qdev_realize(DEVICE(&s->u_cluster), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->e_cpus), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->u_cpus), NULL, &error_abort);
 
     /* Reserved Memory at address 0 */
     memory_region_init_ram(rsvd0_mem, NULL, "microchip.pfsoc.rsvd0_mem",
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index c99e92a7eb..1d9a7c5bf1 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -44,7 +44,6 @@
 #include "hw/loader.h"
 #include "hw/sysbus.h"
 #include "hw/char/serial.h"
-#include "hw/cpu/cluster.h"
 #include "hw/misc/unimp.h"
 #include "hw/sd/sd.h"
 #include "hw/ssi/ssi.h"
@@ -786,20 +785,14 @@ static void sifive_u_soc_instance_init(Object *obj)
 {
     SiFiveUSoCState *s = RISCV_U_SOC(obj);
 
-    object_initialize_child(obj, "e-cluster", &s->e_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->e_cluster), "cluster-id", 0);
-
-    object_initialize_child(OBJECT(&s->e_cluster), "e-cpus", &s->e_cpus,
+    object_initialize_child(obj, "e-cpus", &s->e_cpus,
                             TYPE_RISCV_HART_ARRAY);
-    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-harts", 1);
+    qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-cpus", 1);
     qdev_prop_set_uint32(DEVICE(&s->e_cpus), "hartid-base", 0);
     qdev_prop_set_string(DEVICE(&s->e_cpus), "cpu-type", SIFIVE_E_CPU);
     qdev_prop_set_uint64(DEVICE(&s->e_cpus), "resetvec", 0x1004);
 
-    object_initialize_child(obj, "u-cluster", &s->u_cluster, TYPE_CPU_CLUSTER);
-    qdev_prop_set_uint32(DEVICE(&s->u_cluster), "cluster-id", 1);
-
-    object_initialize_child(OBJECT(&s->u_cluster), "u-cpus", &s->u_cpus,
+    object_initialize_child(obj, "u-cpus", &s->u_cpus,
                             TYPE_RISCV_HART_ARRAY);
 
     object_initialize_child(obj, "prci", &s->prci, TYPE_SIFIVE_U_PRCI);
@@ -825,21 +818,13 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     int i, j;
     NICInfo *nd = &nd_table[0];
 
-    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-harts", ms->smp.cpus - 1);
+    qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-cpus", ms->smp.cpus - 1);
     qdev_prop_set_uint32(DEVICE(&s->u_cpus), "hartid-base", 1);
     qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", s->cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", 0x1004);
 
-    riscv_hart_array_realize(&s->e_cpus, &error_abort);
-    riscv_hart_array_realize(&s->u_cpus, &error_abort);
-    /*
-     * The cluster must be realized after the RISC-V hart array container,
-     * as the container's CPU object is only created on realize, and the
-     * CPU must exist and have been parented into the cluster before the
-     * cluster is realized.
-     */
-    qdev_realize(DEVICE(&s->e_cluster), NULL, &error_abort);
-    qdev_realize(DEVICE(&s->u_cluster), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->e_cpus), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->u_cpus), NULL, &error_abort);
 
     /* boot rom */
     memory_region_init_rom(mask_rom, OBJECT(dev), "riscv.sifive.u.mrom",
-- 
2.35.1

[RFC PATCH 16/18] hw/riscv: update remaining machines due to riscv_hart_array update

[Damien Hedde]

virt & spike machines have multiple sockets (but they still put
all cpus in the same default cluster). For these 2 machines
we disable the clustering in the array in order to keep the
behavior.

opentitan, sifive_e and shakti_c, were using the default cluster
too. But we can use the embedded cluster as it is equivalent.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/opentitan.c | 4 ++--
 hw/riscv/shakti_c.c  | 4 ++--
 hw/riscv/sifive_e.c  | 4 ++--
 hw/riscv/spike.c     | 5 +++--
 hw/riscv/virt.c      | 5 +++--
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 2eb7454d8a..4e90610f1d 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -132,10 +132,10 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
+    object_property_set_int(OBJECT(&s->cpus), "num-cpus", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x8080, &error_abort);
-    riscv_hart_array_realize(&s->cpus, &error_abort);
+    qdev_realize(DEVICE(&s->cpus), NULL, &error_abort);
 
     /* Boot ROM */
     memory_region_init_rom(&s->rom, OBJECT(dev_soc), "riscv.lowrisc.ibex.rom",
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 93e0c8dd68..5158ea6e8b 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -108,7 +108,7 @@ static void shakti_c_soc_state_realize(DeviceState *dev, Error **errp)
     ShaktiCSoCState *sss = RISCV_SHAKTI_SOC(dev);
     MemoryRegion *system_memory = get_system_memory();
 
-    riscv_hart_array_realize(&sss->cpus, &error_abort);
+    qdev_realize(DEVICE(&sss->cpus), NULL, &error_abort);
 
     sss->plic = sifive_plic_create(shakti_c_memmap[SHAKTI_C_PLIC].base,
         (char *)SHAKTI_C_PLIC_HART_CONFIG, ms->smp.cpus, 0,
@@ -171,7 +171,7 @@ static void shakti_c_soc_instance_init(Object *obj)
      */
     object_property_set_str(OBJECT(&sss->cpus), "cpu-type",
                             TYPE_RISCV_CPU_SHAKTI_C, &error_abort);
-    object_property_set_int(OBJECT(&sss->cpus), "num-harts", 1,
+    object_property_set_int(OBJECT(&sss->cpus), "num-cpus", 1,
                             &error_abort);
 }
 
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 25ba0a8c85..c1e67c0f78 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -179,7 +179,7 @@ static void sifive_e_soc_init(Object *obj)
     SiFiveESoCState *s = RISCV_E_SOC(obj);
 
     object_initialize_child(obj, "cpus", &s->cpus, TYPE_RISCV_HART_ARRAY);
-    object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
+    object_property_set_int(OBJECT(&s->cpus), "num-cpus", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x1004, &error_abort);
     object_initialize_child(obj, "riscv.sifive.e.gpio0", &s->gpio,
@@ -195,7 +195,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    riscv_hart_array_realize(&s->cpus, &error_abort);
+    qdev_realize(DEVICE(&s->cpus), NULL, &error_abort);
 
     /* Mask ROM */
     memory_region_init_rom(&s->mask_rom, OBJECT(dev), "riscv.sifive.e.mrom",
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index b75e3656e1..17bba3c7fa 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -228,9 +228,10 @@ static void spike_board_init(MachineState *machine)
                                 machine->cpu_type, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "hartid-base",
                                 base_hartid, &error_abort);
-        object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
+        object_property_set_int(OBJECT(&s->soc[i]), "num-cpus",
                                 hart_count, &error_abort);
-        riscv_hart_array_realize(&s->soc[i], &error_abort);
+        cpus_disable_clustering(CPUS(&s->soc[i]));
+        qdev_realize(DEVICE(&s->soc[i]), NULL, &error_abort);
 
         /* Core Local Interruptor (timer and IPI) for each socket */
         riscv_aclint_swi_create(
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 12036aa95b..c51b124330 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1221,9 +1221,10 @@ static void virt_machine_init(MachineState *machine)
                                 machine->cpu_type, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "hartid-base",
                                 base_hartid, &error_abort);
-        object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
+        object_property_set_int(OBJECT(&s->soc[i]), "num-cpus",
                                 hart_count, &error_abort);
-        riscv_hart_array_realize(&s->soc[i], &error_abort);
+        cpus_disable_clustering(CPUS(&s->soc[i]));
+        qdev_realize(DEVICE(&s->soc[i]), NULL, &error_abort);
 
         if (!kvm_enabled()) {
             if (s->have_aclint) {
-- 
2.35.1

[RFC PATCH 16/18] hw/riscv: update remaining machines due to riscv_hart_array update

[Damien Hedde]

virt & spike machines have multiple sockets (but they still put
all cpus in the same default cluster). For these 2 machines
we disable the clustering in the array in order to keep the
behavior.

opentitan, sifive_e and shakti_c, were using the default cluster
too. But we can use the embedded cluster as it is equivalent.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/riscv/opentitan.c | 4 ++--
 hw/riscv/shakti_c.c  | 4 ++--
 hw/riscv/sifive_e.c  | 4 ++--
 hw/riscv/spike.c     | 5 +++--
 hw/riscv/virt.c      | 5 +++--
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 2eb7454d8a..4e90610f1d 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -132,10 +132,10 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
+    object_property_set_int(OBJECT(&s->cpus), "num-cpus", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x8080, &error_abort);
-    riscv_hart_array_realize(&s->cpus, &error_abort);
+    qdev_realize(DEVICE(&s->cpus), NULL, &error_abort);
 
     /* Boot ROM */
     memory_region_init_rom(&s->rom, OBJECT(dev_soc), "riscv.lowrisc.ibex.rom",
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 93e0c8dd68..5158ea6e8b 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -108,7 +108,7 @@ static void shakti_c_soc_state_realize(DeviceState *dev, Error **errp)
     ShaktiCSoCState *sss = RISCV_SHAKTI_SOC(dev);
     MemoryRegion *system_memory = get_system_memory();
 
-    riscv_hart_array_realize(&sss->cpus, &error_abort);
+    qdev_realize(DEVICE(&sss->cpus), NULL, &error_abort);
 
     sss->plic = sifive_plic_create(shakti_c_memmap[SHAKTI_C_PLIC].base,
         (char *)SHAKTI_C_PLIC_HART_CONFIG, ms->smp.cpus, 0,
@@ -171,7 +171,7 @@ static void shakti_c_soc_instance_init(Object *obj)
      */
     object_property_set_str(OBJECT(&sss->cpus), "cpu-type",
                             TYPE_RISCV_CPU_SHAKTI_C, &error_abort);
-    object_property_set_int(OBJECT(&sss->cpus), "num-harts", 1,
+    object_property_set_int(OBJECT(&sss->cpus), "num-cpus", 1,
                             &error_abort);
 }
 
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 25ba0a8c85..c1e67c0f78 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -179,7 +179,7 @@ static void sifive_e_soc_init(Object *obj)
     SiFiveESoCState *s = RISCV_E_SOC(obj);
 
     object_initialize_child(obj, "cpus", &s->cpus, TYPE_RISCV_HART_ARRAY);
-    object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
+    object_property_set_int(OBJECT(&s->cpus), "num-cpus", ms->smp.cpus,
                             &error_abort);
     object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x1004, &error_abort);
     object_initialize_child(obj, "riscv.sifive.e.gpio0", &s->gpio,
@@ -195,7 +195,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
 
     object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
                             &error_abort);
-    riscv_hart_array_realize(&s->cpus, &error_abort);
+    qdev_realize(DEVICE(&s->cpus), NULL, &error_abort);
 
     /* Mask ROM */
     memory_region_init_rom(&s->mask_rom, OBJECT(dev), "riscv.sifive.e.mrom",
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index b75e3656e1..17bba3c7fa 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -228,9 +228,10 @@ static void spike_board_init(MachineState *machine)
                                 machine->cpu_type, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "hartid-base",
                                 base_hartid, &error_abort);
-        object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
+        object_property_set_int(OBJECT(&s->soc[i]), "num-cpus",
                                 hart_count, &error_abort);
-        riscv_hart_array_realize(&s->soc[i], &error_abort);
+        cpus_disable_clustering(CPUS(&s->soc[i]));
+        qdev_realize(DEVICE(&s->soc[i]), NULL, &error_abort);
 
         /* Core Local Interruptor (timer and IPI) for each socket */
         riscv_aclint_swi_create(
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 12036aa95b..c51b124330 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1221,9 +1221,10 @@ static void virt_machine_init(MachineState *machine)
                                 machine->cpu_type, &error_abort);
         object_property_set_int(OBJECT(&s->soc[i]), "hartid-base",
                                 base_hartid, &error_abort);
-        object_property_set_int(OBJECT(&s->soc[i]), "num-harts",
+        object_property_set_int(OBJECT(&s->soc[i]), "num-cpus",
                                 hart_count, &error_abort);
-        riscv_hart_array_realize(&s->soc[i], &error_abort);
+        cpus_disable_clustering(CPUS(&s->soc[i]));
+        qdev_realize(DEVICE(&s->soc[i]), NULL, &error_abort);
 
         if (!kvm_enabled()) {
             if (s->have_aclint) {
-- 
2.35.1

[RFC PATCH 17/18] hw/riscv/riscv_hart: remove temporary features

[Damien Hedde]

Now that we updated all riscv machines, we can remove
the temporary realize helper and the alias property.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/riscv/riscv_hart.h |  3 ---
 hw/riscv/riscv_hart.c         | 14 --------------
 2 files changed, 17 deletions(-)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 65ac0d2bc4..acf5ee8575 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -56,7 +56,4 @@ static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *s)
     return CPUS(s)->topology.cpus;
 }
 
-/* Temporary function until we migrated the riscv hart array to simple device */
-void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp);
-
 #endif
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 1b4ff7e3c6..ea798de5d5 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -27,13 +27,6 @@
 #include "hw/riscv/riscv_hart.h"
 #include "hw/cpu/cpus.h"
 
-void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
-{
-    /* disable the clustering */
-    cpus_disable_clustering(CPUS(state));
-    qdev_realize(DEVICE(state), NULL, errp);
-}
-
 static Property riscv_harts_props[] = {
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
@@ -52,12 +45,6 @@ static void riscv_harts_configure_cpu(CpusState *base, CPUState *cpu,
     cpuenv->mhartid = s->hartid_base + i;
 }
 
-static void riscv_harts_init(Object *obj)
-{
-    /* add a temporary property to keep num-harts */
-    object_property_add_alias(obj, "num-harts", obj, "num-cpus");
-}
-
 static void riscv_harts_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -73,7 +60,6 @@ static const TypeInfo riscv_harts_info = {
     .name          = TYPE_RISCV_HART_ARRAY,
     .parent        = TYPE_CPUS,
     .instance_size = sizeof(RISCVHartArrayState),
-    .instance_init = riscv_harts_init,
     .class_init    = riscv_harts_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 17/18] hw/riscv/riscv_hart: remove temporary features

[Damien Hedde]

Now that we updated all riscv machines, we can remove
the temporary realize helper and the alias property.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/riscv/riscv_hart.h |  3 ---
 hw/riscv/riscv_hart.c         | 14 --------------
 2 files changed, 17 deletions(-)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 65ac0d2bc4..acf5ee8575 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -56,7 +56,4 @@ static inline unsigned riscv_array_get_num_harts(RISCVHartArrayState *s)
     return CPUS(s)->topology.cpus;
 }
 
-/* Temporary function until we migrated the riscv hart array to simple device */
-void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp);
-
 #endif
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 1b4ff7e3c6..ea798de5d5 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -27,13 +27,6 @@
 #include "hw/riscv/riscv_hart.h"
 #include "hw/cpu/cpus.h"
 
-void riscv_hart_array_realize(RISCVHartArrayState *state, Error **errp)
-{
-    /* disable the clustering */
-    cpus_disable_clustering(CPUS(state));
-    qdev_realize(DEVICE(state), NULL, errp);
-}
-
 static Property riscv_harts_props[] = {
     DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
@@ -52,12 +45,6 @@ static void riscv_harts_configure_cpu(CpusState *base, CPUState *cpu,
     cpuenv->mhartid = s->hartid_base + i;
 }
 
-static void riscv_harts_init(Object *obj)
-{
-    /* add a temporary property to keep num-harts */
-    object_property_add_alias(obj, "num-harts", obj, "num-cpus");
-}
-
 static void riscv_harts_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -73,7 +60,6 @@ static const TypeInfo riscv_harts_info = {
     .name          = TYPE_RISCV_HART_ARRAY,
     .parent        = TYPE_CPUS,
     .instance_size = sizeof(RISCVHartArrayState),
-    .instance_init = riscv_harts_init,
     .class_init    = riscv_harts_class_init,
 };
 
-- 
2.35.1

[RFC PATCH 18/18] add myself as reviewer of the newly added _cpus_

[Damien Hedde]

Add cpus into the "machine core" subset along with cpu
cluster and add myself as reviewer.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

Should we put all cpu groups in the same subset ?
hw/cpu/cluster
hw/cpu/cpus
hw/arm/arm_cpus
hw/riscv/riscv_array
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cc364afef7..80b9331d02 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1678,6 +1678,7 @@ M: Eduardo Habkost <eduardo@habkost.net>
 M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
 R: Philippe Mathieu-Daudé <f4bug@amsat.org>
 R: Yanan Wang <wangyanan55@huawei.com>
+R: Damien Hedde <damien.hedde@greensocs.com>
 S: Supported
 F: cpu.c
 F: hw/core/cpu.c
@@ -1687,11 +1688,13 @@ F: hw/core/machine-smp.c
 F: hw/core/null-machine.c
 F: hw/core/numa.c
 F: hw/cpu/cluster.c
+F: hw/cpu/cpus.c
 F: qapi/machine.json
 F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
+F: include/hw/cpu/cpus.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
 T: git https://gitlab.com/ehabkost/qemu.git machine-next
-- 
2.35.1

[RFC PATCH 18/18] add myself as reviewer of the newly added _cpus_

[Damien Hedde]

Add cpus into the "machine core" subset along with cpu
cluster and add myself as reviewer.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

Should we put all cpu groups in the same subset ?
hw/cpu/cluster
hw/cpu/cpus
hw/arm/arm_cpus
hw/riscv/riscv_array
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cc364afef7..80b9331d02 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1678,6 +1678,7 @@ M: Eduardo Habkost <eduardo@habkost.net>
 M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
 R: Philippe Mathieu-Daudé <f4bug@amsat.org>
 R: Yanan Wang <wangyanan55@huawei.com>
+R: Damien Hedde <damien.hedde@greensocs.com>
 S: Supported
 F: cpu.c
 F: hw/core/cpu.c
@@ -1687,11 +1688,13 @@ F: hw/core/machine-smp.c
 F: hw/core/null-machine.c
 F: hw/core/numa.c
 F: hw/cpu/cluster.c
+F: hw/cpu/cpus.c
 F: qapi/machine.json
 F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
+F: include/hw/cpu/cpus.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
 T: git https://gitlab.com/ehabkost/qemu.git machine-next
-- 
2.35.1

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

Ping !

It would be nice to have some rough feedback about this series.
I plan to submit it in 2 sub-series (riscv-array in a separate part) but 
would like to know first if this direction looks ok to you ?

Note that I'm a bit confused by the terminology. Should a "cluster" as 
in "machine topology's cluster" be a 1-1 match with a gdb inferior ?

Thanks,
Damien

On 3/30/22 14:56, Damien Hedde wrote:
> Hi,
> 
> This series add devices to be able to user-create (coldplug) cpu
> clusters. The existing cpu cluster dictates how cpus are exposed
> in gdb, but it does not handle the cpu objects creation. This series
> adds a new device to handle both issues and adds support for two
> architectures: arm and riscv.
> 
> Please look at patches 2 and 3 for more details about the new device.
> 
> Last part concerning the riscv is rfc as I do non-backward compatible
> updates. I'm not sure what migration (or other) constraints we have
> on these machines and I probably need to make some changes to cope
> with them.
> 
> This series almost deprecates the cpu-cluster type as all uses
> but one are replaced.
> 
> It is organized as follows:
> 
> + Patches 1 to 7 adds a new base device to replace cpu-cluster
> 
> + Patches 8 and 9 adds an arm specific version and replace existing
>    clusters in the xlnx-zynqmp machine.
> 
> + patches 10 to 17 updates the riscv_array. It was already used to
>    create cpus but was not a cpu cluster.
> 
> Thanks for any comments,
> --
> Damien
> 
> Damien Hedde (18):
>    define MAX_CLUSTERS in cpu.h instead of cluster.h
>    hw/cpu/cpus: introduce _cpus_ device
>    hw/cpu/cpus: prepare to handle cpu clusters
>    hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_
>    gdbstub: deal with _cpus_ object instead of _cpu-cluster_
>    hw/cpu/cluster: remove cluster_id now that gdbstub is updated
>    hw/cpu/cpus: add a common start-powered-off property
>    hw/arm/arm_cpus: add arm_cpus device
>    hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus
>    hw/riscv/riscv_hart: prepare transition to cpus
>    hw/riscv: prepare riscv_hart transition to cpus
>    hw/riscv/virt: prepare riscv_hart transition to cpus
>    hw/riscv/spike: prepare riscv_hart transition to cpus
>    hw/riscv/riscv_hart: use cpus as base class
>    hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update
>    hw/riscv: update remaining machines due to riscv_hart_array update
>    hw/riscv/riscv_hart: remove temporary features
>    add myself as reviewer of the newly added _cpus_
> 
>   include/hw/arm/arm_cpus.h          |  45 +++++++
>   include/hw/arm/xlnx-zynqmp.h       |   8 +-
>   include/hw/core/cpu.h              |   6 +
>   include/hw/cpu/cluster.h           |  26 ++--
>   include/hw/cpu/cpus.h              |  93 ++++++++++++++
>   include/hw/riscv/microchip_pfsoc.h |   2 -
>   include/hw/riscv/riscv_hart.h      |  25 +++-
>   include/hw/riscv/sifive_u.h        |   2 -
>   gdbstub.c                          |  12 +-
>   hw/arm/arm_cpus.c                  |  63 ++++++++++
>   hw/arm/xlnx-zynqmp.c               | 121 +++++++-----------
>   hw/cpu/cluster.c                   |  53 ++++----
>   hw/cpu/cpus.c                      | 195 +++++++++++++++++++++++++++++
>   hw/riscv/boot.c                    |   2 +-
>   hw/riscv/microchip_pfsoc.c         |  28 +----
>   hw/riscv/opentitan.c               |   4 +-
>   hw/riscv/riscv_hart.c              |  44 ++-----
>   hw/riscv/shakti_c.c                |   4 +-
>   hw/riscv/sifive_e.c                |   4 +-
>   hw/riscv/sifive_u.c                |  31 ++---
>   hw/riscv/spike.c                   |  18 +--
>   hw/riscv/virt.c                    |  79 +++++++-----
>   MAINTAINERS                        |   3 +
>   hw/arm/meson.build                 |   1 +
>   hw/cpu/meson.build                 |   2 +-
>   25 files changed, 612 insertions(+), 259 deletions(-)
>   create mode 100644 include/hw/arm/arm_cpus.h
>   create mode 100644 include/hw/cpu/cpus.h
>   create mode 100644 hw/arm/arm_cpus.c
>   create mode 100644 hw/cpu/cpus.c
> 

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

Ping !

It would be nice to have some rough feedback about this series.
I plan to submit it in 2 sub-series (riscv-array in a separate part) but 
would like to know first if this direction looks ok to you ?

Note that I'm a bit confused by the terminology. Should a "cluster" as 
in "machine topology's cluster" be a 1-1 match with a gdb inferior ?

Thanks,
Damien

On 3/30/22 14:56, Damien Hedde wrote:
> Hi,
> 
> This series add devices to be able to user-create (coldplug) cpu
> clusters. The existing cpu cluster dictates how cpus are exposed
> in gdb, but it does not handle the cpu objects creation. This series
> adds a new device to handle both issues and adds support for two
> architectures: arm and riscv.
> 
> Please look at patches 2 and 3 for more details about the new device.
> 
> Last part concerning the riscv is rfc as I do non-backward compatible
> updates. I'm not sure what migration (or other) constraints we have
> on these machines and I probably need to make some changes to cope
> with them.
> 
> This series almost deprecates the cpu-cluster type as all uses
> but one are replaced.
> 
> It is organized as follows:
> 
> + Patches 1 to 7 adds a new base device to replace cpu-cluster
> 
> + Patches 8 and 9 adds an arm specific version and replace existing
>    clusters in the xlnx-zynqmp machine.
> 
> + patches 10 to 17 updates the riscv_array. It was already used to
>    create cpus but was not a cpu cluster.
> 
> Thanks for any comments,
> --
> Damien
> 
> Damien Hedde (18):
>    define MAX_CLUSTERS in cpu.h instead of cluster.h
>    hw/cpu/cpus: introduce _cpus_ device
>    hw/cpu/cpus: prepare to handle cpu clusters
>    hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_
>    gdbstub: deal with _cpus_ object instead of _cpu-cluster_
>    hw/cpu/cluster: remove cluster_id now that gdbstub is updated
>    hw/cpu/cpus: add a common start-powered-off property
>    hw/arm/arm_cpus: add arm_cpus device
>    hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus
>    hw/riscv/riscv_hart: prepare transition to cpus
>    hw/riscv: prepare riscv_hart transition to cpus
>    hw/riscv/virt: prepare riscv_hart transition to cpus
>    hw/riscv/spike: prepare riscv_hart transition to cpus
>    hw/riscv/riscv_hart: use cpus as base class
>    hw/riscv/sifive_u&microchip_pfsoc: apply riscv_hart_array update
>    hw/riscv: update remaining machines due to riscv_hart_array update
>    hw/riscv/riscv_hart: remove temporary features
>    add myself as reviewer of the newly added _cpus_
> 
>   include/hw/arm/arm_cpus.h          |  45 +++++++
>   include/hw/arm/xlnx-zynqmp.h       |   8 +-
>   include/hw/core/cpu.h              |   6 +
>   include/hw/cpu/cluster.h           |  26 ++--
>   include/hw/cpu/cpus.h              |  93 ++++++++++++++
>   include/hw/riscv/microchip_pfsoc.h |   2 -
>   include/hw/riscv/riscv_hart.h      |  25 +++-
>   include/hw/riscv/sifive_u.h        |   2 -
>   gdbstub.c                          |  12 +-
>   hw/arm/arm_cpus.c                  |  63 ++++++++++
>   hw/arm/xlnx-zynqmp.c               | 121 +++++++-----------
>   hw/cpu/cluster.c                   |  53 ++++----
>   hw/cpu/cpus.c                      | 195 +++++++++++++++++++++++++++++
>   hw/riscv/boot.c                    |   2 +-
>   hw/riscv/microchip_pfsoc.c         |  28 +----
>   hw/riscv/opentitan.c               |   4 +-
>   hw/riscv/riscv_hart.c              |  44 ++-----
>   hw/riscv/shakti_c.c                |   4 +-
>   hw/riscv/sifive_e.c                |   4 +-
>   hw/riscv/sifive_u.c                |  31 ++---
>   hw/riscv/spike.c                   |  18 +--
>   hw/riscv/virt.c                    |  79 +++++++-----
>   MAINTAINERS                        |   3 +
>   hw/arm/meson.build                 |   1 +
>   hw/cpu/meson.build                 |   2 +-
>   25 files changed, 612 insertions(+), 259 deletions(-)
>   create mode 100644 include/hw/arm/arm_cpus.h
>   create mode 100644 include/hw/cpu/cpus.h
>   create mode 100644 hw/arm/arm_cpus.c
>   create mode 100644 hw/cpu/cpus.c
> 

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Peter Maydell]

On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Hi,
>
> This series add devices to be able to user-create (coldplug) cpu
> clusters. The existing cpu cluster dictates how cpus are exposed
> in gdb, but it does not handle the cpu objects creation. This series
> adds a new device to handle both issues and adds support for two
> architectures: arm and riscv.
>
> Please look at patches 2 and 3 for more details about the new device.
>
> Last part concerning the riscv is rfc as I do non-backward compatible
> updates. I'm not sure what migration (or other) constraints we have
> on these machines and I probably need to make some changes to cope
> with them.
>
> This series almost deprecates the cpu-cluster type as all uses
> but one are replaced.

I don't think we should have two different concepts which
are "a group of CPUs". It means we wind up with two different
ways to do something (which we have too many examples of
already), only one of which gets to use the nicer, more obvious
name.

The stated motivation is to allow user creation of CPU clusters,
but I'm not sure how this would work -- CPUs need a lot of
things wiring up like interrupt lines and memory regions, which
you can't do on the command line anyway. Do you have an example
of what the new code lets you do?

thanks
-- PMM

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Peter Maydell]

On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Hi,
>
> This series add devices to be able to user-create (coldplug) cpu
> clusters. The existing cpu cluster dictates how cpus are exposed
> in gdb, but it does not handle the cpu objects creation. This series
> adds a new device to handle both issues and adds support for two
> architectures: arm and riscv.
>
> Please look at patches 2 and 3 for more details about the new device.
>
> Last part concerning the riscv is rfc as I do non-backward compatible
> updates. I'm not sure what migration (or other) constraints we have
> on these machines and I probably need to make some changes to cope
> with them.
>
> This series almost deprecates the cpu-cluster type as all uses
> but one are replaced.

I don't think we should have two different concepts which
are "a group of CPUs". It means we wind up with two different
ways to do something (which we have too many examples of
already), only one of which gets to use the nicer, more obvious
name.

The stated motivation is to allow user creation of CPU clusters,
but I'm not sure how this would work -- CPUs need a lot of
things wiring up like interrupt lines and memory regions, which
you can't do on the command line anyway. Do you have an example
of what the new code lets you do?

thanks
-- PMM

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

On 4/21/22 17:51, Peter Maydell wrote:
> On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Hi,
>>
>> This series add devices to be able to user-create (coldplug) cpu
>> clusters. The existing cpu cluster dictates how cpus are exposed
>> in gdb, but it does not handle the cpu objects creation. This series
>> adds a new device to handle both issues and adds support for two
>> architectures: arm and riscv.
>>
>> Please look at patches 2 and 3 for more details about the new device.
>>
>> Last part concerning the riscv is rfc as I do non-backward compatible
>> updates. I'm not sure what migration (or other) constraints we have
>> on these machines and I probably need to make some changes to cope
>> with them.
>>
>> This series almost deprecates the cpu-cluster type as all uses
>> but one are replaced.
> 
> I don't think we should have two different concepts which
> are "a group of CPUs". It means we wind up with two different
> ways to do something (which we have too many examples of
> already), only one of which gets to use the nicer, more obvious
> name.

I was not sure that I could merge the cpu-cluster into the new object 
and agree having both is not ideal. I can
1. delete the last cpu-cluster usecase and the object type
2. manage to update in place the cpu-cluster instead of adding the new type.

In the end, the object will be the same.

I am also confused by the "cluster" naming.

The original cpu-cluster is used only to set cluster_index of cpus which 
is used in the gdbstub to group cpus in inferiors. So it is just a 
container to expose cpus in a specific gdb inferior.

But in most machines (which don't use explicit cpu-clusters) this 
cluster[_index] has nothing to do with the topology's cluster.

In practice, topology's cluster is not a 1-1 match of gdb inferior in 
gsbtub ("cpu-cluster") and I'm not sure what to do with that...

I'm happy to keep the "cpu-cluster" if everyone feel the same.

> 
> The stated motivation is to allow user creation of CPU clusters,
> but I'm not sure how this would work -- CPUs need a lot of
> things wiring up like interrupt lines and memory regions, which
> you can't do on the command line anyway. Do you have an example
> of what the new code lets you do?

This simplify the place where cpu-cluster was used (see patch 9 for 
example) as you don't need to create both the cpus and the cluster.

But mostly the goal is to provide cpu cold plug functionality with the 
qapi interface (not the cli interface). With this series + my other 
series adding commands to cold-plug devices and memory-map sysbus 
devices, I reproduced the arm virt board starting from the none machine.

Interrupts are wired using qom_set.

Thanks,
--
Damien

Re: [RFC PATCH 00/18] user-creatable cpu clusters

[Damien Hedde]

On 4/21/22 17:51, Peter Maydell wrote:
> On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Hi,
>>
>> This series add devices to be able to user-create (coldplug) cpu
>> clusters. The existing cpu cluster dictates how cpus are exposed
>> in gdb, but it does not handle the cpu objects creation. This series
>> adds a new device to handle both issues and adds support for two
>> architectures: arm and riscv.
>>
>> Please look at patches 2 and 3 for more details about the new device.
>>
>> Last part concerning the riscv is rfc as I do non-backward compatible
>> updates. I'm not sure what migration (or other) constraints we have
>> on these machines and I probably need to make some changes to cope
>> with them.
>>
>> This series almost deprecates the cpu-cluster type as all uses
>> but one are replaced.
> 
> I don't think we should have two different concepts which
> are "a group of CPUs". It means we wind up with two different
> ways to do something (which we have too many examples of
> already), only one of which gets to use the nicer, more obvious
> name.

I was not sure that I could merge the cpu-cluster into the new object 
and agree having both is not ideal. I can
1. delete the last cpu-cluster usecase and the object type
2. manage to update in place the cpu-cluster instead of adding the new type.

In the end, the object will be the same.

I am also confused by the "cluster" naming.

The original cpu-cluster is used only to set cluster_index of cpus which 
is used in the gdbstub to group cpus in inferiors. So it is just a 
container to expose cpus in a specific gdb inferior.

But in most machines (which don't use explicit cpu-clusters) this 
cluster[_index] has nothing to do with the topology's cluster.

In practice, topology's cluster is not a 1-1 match of gdb inferior in 
gsbtub ("cpu-cluster") and I'm not sure what to do with that...

I'm happy to keep the "cpu-cluster" if everyone feel the same.

> 
> The stated motivation is to allow user creation of CPU clusters,
> but I'm not sure how this would work -- CPUs need a lot of
> things wiring up like interrupt lines and memory regions, which
> you can't do on the command line anyway. Do you have an example
> of what the new code lets you do?

This simplify the place where cpu-cluster was used (see patch 9 for 
example) as you don't need to create both the cpus and the cluster.

But mostly the goal is to provide cpu cold plug functionality with the 
qapi interface (not the cli interface). With this series + my other 
series adding commands to cold-plug devices and memory-map sysbus 
devices, I reproduced the arm virt board starting from the none machine.

Interrupts are wired using qom_set.

Thanks,
--
Damien