Philippe Mathieu-Daudé <phi...@linaro.org> writes:
> Each target use a common template for qmp_query_cpu_definitions().
>
> Extract it as generic_query_cpu_definitions(), keeping the
> target-specific implementations as the following SysemuCPUOps
> handlers:
> - cpu_list_compare()
> - add_definition()
> - add_alias_definitions()
>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
> MAINTAINERS | 2 +
> include/hw/core/sysemu-cpu-ops.h | 14 ++++++
> include/qapi/commands-target-compat.h | 14 ++++++
> system/cpu-qmp-cmds.c | 71 +++++++++++++++++++++++++++
> system/meson.build | 1 +
> 5 files changed, 102 insertions(+)
> create mode 100644 include/qapi/commands-target-compat.h
> create mode 100644 system/cpu-qmp-cmds.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af27490243..39d7c14d98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -148,6 +148,7 @@ M: Richard Henderson <richard.hender...@linaro.org>
> R: Paolo Bonzini <pbonz...@redhat.com>
> S: Maintained
> F: system/cpus.c
> +F: system/cpu-qmp-cmds.c
> F: system/cpu-qom-helpers.c
> F: system/watchpoint.c
> F: cpu-common.c
> @@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
> F: include/hw/boards.h
> F: include/hw/core/cpu.h
> F: include/hw/cpu/cluster.h
> +F: include/qapi/commands-target-compat.h
> F: include/sysemu/numa.h
> F: tests/unit/test-smp-parse.c
> T: git https://gitlab.com/ehabkost/qemu.git machine-next
> diff --git a/include/hw/core/sysemu-cpu-ops.h
> b/include/hw/core/sysemu-cpu-ops.h
> index 24d003fe04..2173226e97 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -11,6 +11,7 @@
> #define SYSEMU_CPU_OPS_H
>
> #include "hw/core/cpu.h"
> +#include "qapi/qapi-types-machine.h"
>
> /*
> * struct SysemuCPUOps: System operations specific to a CPU class
> @@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
> */
> bool (*virtio_is_big_endian)(CPUState *cpu);
>
> + /**
> + * @cpu_list_compare: Sort alphabetically by type name,
> + * respecting CPUClass::ordering.
> + */
> + gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer
> cpu_class_b);
Peeking ahead... This is used for sorting the subtypes of the CPU type
returned by cpu_typename_by_arch_bit(). Implementing the callback is
optional, and when absent, we don't sort, i.e. we get hash table order.
Worth mentioning it's optional?
> + /**
> + * @add_definition: Add the @cpu_class definition to @cpu_list.
> + */
> + void (*add_definition)(gpointer cpu_class, gpointer cpu_list);
This one appears to default to cpu_common_add_definition(). Worth
mentioning?
I despise Glib's pointless typedefs for ordinary C types.
> + /**
> + * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
> + */
> + void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
> /**
> * @legacy_vmsd: Legacy state for migration.
> * Do not use in new targets, use #DeviceClass::vmsd
> instead.
> diff --git a/include/qapi/commands-target-compat.h
> b/include/qapi/commands-target-compat.h
> new file mode 100644
> index 0000000000..86d45d8fcc
> --- /dev/null
> +++ b/include/qapi/commands-target-compat.h
Why "compat"?
> @@ -0,0 +1,14 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QAPI_COMPAT_TARGET_H
> +#define QAPI_COMPAT_TARGET_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
> +
> +#endif
> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
> new file mode 100644
> index 0000000000..daeb131159
> --- /dev/null
> +++ b/system/cpu-qmp-cmds.c
> @@ -0,0 +1,71 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qapi/commands-target-compat.h"
> +#include "sysemu/arch_init.h"
> +#include "hw/core/cpu.h"
> +#include "hw/core/sysemu-cpu-ops.h"
> +
> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
> +{
> + ObjectClass *oc = data;
> + CpuDefinitionInfoList **cpu_list = user_data;
> + CpuDefinitionInfo *info;
> + const char *typename;
> +
> + typename = object_class_get_name(oc);
> + info = g_malloc0(sizeof(*info));
> + info->name = cpu_model_from_type(typename);
> + info->q_typename = g_strdup(typename);
> +
> + QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
> + const char *cpu_typename)
> +{
> + ObjectClass *oc;
> + GSList *list;
> + const struct SysemuCPUOps *ops;
> +
> + oc = object_class_by_name(cpu_typename);
> + if (!oc) {
> + return;
> + }
> + ops = CPU_CLASS(oc)->sysemu_ops;
> +
> + list = object_class_get_list(cpu_typename, false);
> + if (ops->cpu_list_compare) {
> + list = g_slist_sort(list, ops->cpu_list_compare);
> + }
> + g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
> + cpu_list);
> + g_slist_free(list);
> +
> + if (ops->add_alias_definitions) {
> + ops->add_alias_definitions(cpu_list);
> + }
> +}
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
> +{
> + CpuDefinitionInfoList *cpu_list = NULL;
> +
> + for (unsigned i = 0; i <= QEMU_ARCH_BIT_LAST; i++) {
> + const char *cpu_typename;
> +
> + cpu_typename = cpu_typename_by_arch_bit(i);
> + if (!cpu_typename) {
> + continue;
> + }
> + arch_add_cpu_definitions(&cpu_list, cpu_typename);
> + }
> +
> + return cpu_list;
> +}
> diff --git a/system/meson.build b/system/meson.build
> index c6ee97e3b2..dd78caa9b7 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -10,6 +10,7 @@ system_ss.add(files(
> 'balloon.c',
> 'bootdevice.c',
> 'cpus.c',
> + 'cpu-qmp-cmds.c',
> 'cpu-qom-helpers.c',
> 'cpu-throttle.c',
> 'cpu-timers.c',
The commit message made me expect the complete refactoring in this
patch. In fact, it's just a first step: the new
generic_query_cpu_definitions() remains unused, and no target implements
the new callbacks. We can worry about this later.
Subsequent patches convert targets to generic_query_cpu_definitions()
one by one. To convince myself the replacement is faithful, I'll have
to refer back to this patch. That's fine.
