On 10/30/23 11:39, Philippe Mathieu-Daudé wrote:
Following the discussion with Peter on my "cpus: Step toward
removing global 'first_cpu'" series [*], we now pass the array
of CPUs via property. Since we can not pass array of "link"
properties, we pass the QOM path of each CPU as a QList(String).
Tagged as RFC to discuss the idea of using qdev_prop_set_array
with qlist_append_str(object_get_canonical_path). Personally I
find it super simple.
I probably misunderstood the concept/problem but "super simple" is not the first
thing that came to my mind in patch 5 hehe
I didn't follow the whole thread, just the [*] message marked and a couple
of replies, but are we planning to deprecate qemu_get_cpu()? Patch 5 mentions
"Devices should avoid calling qemu_get_cpu() because this call doesn't work
as expected with heterogeneous machines". I'll take your word for it. But
e500 isn't an heterogeneous machine - we're creating ppc cpus only. So I'm
a bit confused here. Are you using e500 just as a simple PoC?
Regardless of the reason to use e500 in this RFC, I believe we would benefit
from having common helpers/magic sauce macros to add all this apparently
boilerplate code to replace qemu_get_cpu().
I mean, we're changing this:
- cpu = qemu_get_cpu(env_idx);
- if (cpu == NULL) {
- /* Unknown CPU */
- return;
- }
-
For a lot of QOM stuff like this:
+ cpu_qompath = object_get_canonical_path(OBJECT(cs));
+ qlist_append_str(spin_cpu_list, cpu_qompath);
+ qdev_prop_set_array(dev, "cpus-qom-path", spin_cpu_list);
+ if (s->cpu_count == 0) {
+ error_setg(errp, "'cpus-qom-path' property array must be set");
+ return;
+ } else if (s->cpu_count > MAX_CPUS) {
+ error_setg(errp, "at most %d CPUs are supported", MAX_CPUS);
+ return;
+ }
+
+ s->cpu = g_new(CPUState *, s->cpu_count);
+ for (unsigned i = 0; i < s->cpu_count; i++) {
+ bool ambiguous;
+ Object *obj;
+
+ obj = object_resolve_path(s->cpu_canonical_path[i], &ambiguous);
+ assert(!ambiguous);
+ s->cpu[i] = CPU(obj);
+ }
+
+static Property ppce500_spin_properties[] = {
+ DEFINE_PROP_ARRAY("cpus-qom-path", SpinState, cpu_count,
+ cpu_canonical_path, qdev_prop_string, char *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
So anything that makes the QOM stuff more palatable to deal with would be
really appreciated. Thanks,
Daniel
Regards,
Phil.
[*]
https://lore.kernel.org/qemu-devel/cafeaca9ft+qmyqslceljd7tefaos9jazmkywqe++s1amf7n...@mail.gmail.com/
Kevin Wolf (1):
qdev: Add qdev_prop_set_array()
Philippe Mathieu-Daudé (4):
hw/ppc/e500: Declare CPU QOM types using DEFINE_TYPES() macro
hw/ppc/e500: QOM-attach CPUs to the machine container
hw/ppc/e500: Inline sysbus_create_simple(E500_SPIN)
hw/ppc/e500: Pass array of CPUs as array of canonical QOM paths
include/hw/qdev-properties.h | 3 ++
hw/core/qdev-properties.c | 21 +++++++++++
hw/ppc/e500.c | 11 +++++-
hw/ppc/ppce500_spin.c | 69 ++++++++++++++++++++++++++----------
4 files changed, 84 insertions(+), 20 deletions(-)
