Re: [RFC PATCH-for-10.1 12/19] cpus: Move target-agnostic methods out of cpu-target.c

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/3/25 16:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   cpu-target.c         | 78 +-------------------------------------------
>   hw/core/cpu-common.c | 74 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 75 insertions(+), 77 deletions(-)
> 
> diff --git a/cpu-target.c b/cpu-target.c
> index 01b0064b91f..20db5ff3108 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -19,94 +19,18 @@
>   
>   #include "qemu/osdep.h"
>   #include "cpu.h"
> -#include "qapi/error.h"
> -#include "qemu/error-report.h"
> -#include "qemu/qemu-print.h"
> -#include "qemu/target_info.h"
>   #include "system/accel-ops.h"
>   #include "system/cpus.h"
>   #include "exec/cpu-common.h"
> -#include "exec/tswap.h"
>   #include "exec/replay-core.h"
>   #include "exec/log.h"
> -#include "accel/accel-cpu-target.h"
> +#include "hw/core/cpu.h"
>   #include "trace/trace-root.h"
>   
>   /* Validate correct placement of CPUArchState. */
>   QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>   QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>   
> -char *cpu_model_from_type(const char *typename)
> -{
> -    g_autofree char *suffix = g_strdup_printf("-%s", target_cpu_type());
> -
> -    if (!object_class_by_name(typename)) {
> -        return NULL;
> -    }
> -
> -    if (g_str_has_suffix(typename, suffix)) {
> -        return g_strndup(typename, strlen(typename) - strlen(suffix));
> -    }
> -
> -    return g_strdup(typename);
> -}
> -
> -const char *parse_cpu_option(const char *cpu_option)
> -{
> -    ObjectClass *oc;
> -    CPUClass *cc;
> -    gchar **model_pieces;
> -    const char *cpu_type;
> -
> -    model_pieces = g_strsplit(cpu_option, ",", 2);
> -    if (!model_pieces[0]) {
> -        error_report("-cpu option cannot be empty");
> -        exit(1);
> -    }
> -
> -    oc = cpu_class_by_name(target_cpu_type(), model_pieces[0]);
> -    if (oc == NULL) {
> -        error_report("unable to find CPU model '%s'", model_pieces[0]);
> -        g_strfreev(model_pieces);
> -        exit(EXIT_FAILURE);
> -    }
> -
> -    cpu_type = object_class_get_name(oc);
> -    cc = CPU_CLASS(oc);
> -    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> -    g_strfreev(model_pieces);
> -    return cpu_type;
> -}
> -
> -static void cpu_list_entry(gpointer data, gpointer user_data)
> -{
> -    CPUClass *cc = CPU_CLASS(OBJECT_CLASS(data));
> -    const char *typename = object_class_get_name(OBJECT_CLASS(data));
> -    g_autofree char *model = cpu_model_from_type(typename);
> -
> -    if (cc->deprecation_note) {
> -        qemu_printf("  %s (deprecated)\n", model);
> -    } else {
> -        qemu_printf("  %s\n", model);
> -    }
> -}
> -
> -void list_cpus(void)
> -{
> -    CPUClass *cc = CPU_CLASS(object_class_by_name(target_cpu_type()));
> -
> -    if (cc->list_cpus) {
> -        cc->list_cpus();
> -    } else {
> -        GSList *list;
> -
> -        list = object_class_get_list_sorted(TYPE_CPU, false);
> -        qemu_printf("Available CPUs:\n");
> -        g_slist_foreach(list, cpu_list_entry, NULL);
> -        g_slist_free(list);
> -    }
> -}
> -
>   /* enable or disable single step mode. EXCP_DEBUG is returned by the
>      CPU loop after each instruction */
>   void cpu_single_step(CPUState *cpu, int enabled)
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index 9064dd24f82..6d0788331c7 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -25,6 +25,9 @@
>   #include "qemu/log.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/lockcnt.h"
> +#include "qemu/error-report.h"
> +#include "qemu/qemu-print.h"
> +#include "qemu/target_info.h"
>   #include "exec/log.h"
>   #include "exec/gdbstub.h"
>   #include "system/tcg.h"
> @@ -152,6 +155,21 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>       return NULL;
>   }
>   
> +char *cpu_model_from_type(const char *typename)
> +{
> +    g_autofree char *suffix = g_strdup_printf("-%s", target_cpu_type());
> +
> +    if (!object_class_by_name(typename)) {
> +        return NULL;
> +    }
> +
> +    if (g_str_has_suffix(typename, suffix)) {
> +        return g_strndup(typename, strlen(typename) - strlen(suffix));
> +    }
> +
> +    return g_strdup(typename);
> +}
> +
>   static void cpu_common_parse_features(const char *typename, char *features,
>                                         Error **errp)
>   {
> @@ -183,6 +201,33 @@ static void cpu_common_parse_features(const char *typename, char *features,
>       }
>   }
>   
> +const char *parse_cpu_option(const char *cpu_option)
> +{
> +    ObjectClass *oc;
> +    CPUClass *cc;
> +    gchar **model_pieces;
> +    const char *cpu_type;
> +
> +    model_pieces = g_strsplit(cpu_option, ",", 2);
> +    if (!model_pieces[0]) {
> +        error_report("-cpu option cannot be empty");
> +        exit(1);
> +    }
> +
> +    oc = cpu_class_by_name(target_cpu_type(), model_pieces[0]);
> +    if (oc == NULL) {
> +        error_report("unable to find CPU model '%s'", model_pieces[0]);
> +        g_strfreev(model_pieces);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    cpu_type = object_class_get_name(oc);
> +    cc = CPU_CLASS(oc);
> +    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> +    g_strfreev(model_pieces);
> +    return cpu_type;
> +}
> +
>   bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
>   {
>       if (!accel_cpu_common_realize(cpu, errp)) {
> @@ -359,3 +404,32 @@ static void cpu_register_types(void)
>   }
>   
>   type_init(cpu_register_types)
> +
> +static void cpu_list_entry(gpointer data, gpointer user_data)
> +{
> +    CPUClass *cc = CPU_CLASS(OBJECT_CLASS(data));
> +    const char *typename = object_class_get_name(OBJECT_CLASS(data));
> +    g_autofree char *model = cpu_model_from_type(typename);
> +
> +    if (cc->deprecation_note) {
> +        qemu_printf("  %s (deprecated)\n", model);
> +    } else {
> +        qemu_printf("  %s\n", model);
> +    }
> +}
> +
> +void list_cpus(void)
> +{
> +    CPUClass *cc = CPU_CLASS(object_class_by_name(target_cpu_type()));
> +
> +    if (cc->list_cpus) {
> +        cc->list_cpus();
> +    } else {
> +        GSList *list;
> +
> +        list = object_class_get_list_sorted(TYPE_CPU, false);
> +        qemu_printf("Available CPUs:\n");
> +        g_slist_foreach(list, cpu_list_entry, NULL);
> +        g_slist_free(list);
> +    }
> +}

Sounds good,

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>