[prev in list] [next in list] [prev in thread] [next in thread]
List: qemu-ppc
Subject: Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()
From: Philippe_Mathieu-Daudé <philmd () linaro ! org>
Date: 2024-03-29 13:03:48
Message-ID: 96bcb673-18dd-49bc-8bcb-281c7409b410 () linaro ! org
[Download RAW message or body]
Hi Markus,
On 26/3/24 14:28, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@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é <philmd@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/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;
>> +}
>
> The target-specific qmp_query_cpu_definitions() this is going to replace
> each execute the equivalent of *one* loop iteration: the one
> corresponding to their own arch bit.
>
> For the replacement to be faithful, as cpu_typename_by_arch_bit() must
> return non-null exactly once.
>
> This is the case for the qemu-system-TARGET. The solution feels
> overengineered there.
>
> I figure cpu_typename_by_arch_bit() will return non-null multiple times
> in a future single binary supporting heterogeneous machines.
>
> Such a single binary then can't serve as drop-in replacement for the
> qemu-system-TARGET, because query-cpu-definitions returns more.
>
> To get a drop-in replacement, we'll need additional logic to restrict
> the query for the homogeneous use case.
Can we ask the management layer to provide the current homogeneous
target via argument? Otherwise we can add a new query-cpu-definitions-v2
command requiring an explicit target argument, allowing 'all', and
deprecate the current query-cpu-definitions.
> I think this needs to be discussed in the commit message.
>
> Possibly easier: don't loop over the bits, relying on
> cpu_typename_by_arch_bit() to select the right one. Instead get the
> right bit from somewhere.
>
> We can switch to a loop when we need it for the heterogeneous case.
Alex suggested to consider heterogeneous emulation the new default,
and the current homogeneous use as a particular case. I'd rather not
plan on a "heterogeneous switch day" and get things integrated in
the way, otherwise we'll never get there...
Regards,
Phil.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic