Re: [PATCH v5 06/31] cpu: Add generic cpu_list()

[Philippe Mathieu-Daudé]

On 16/11/23 08:39, Philippe Mathieu-Daudé wrote:
> Hi Gavin,
>
> On 15/11/23 00:56, Gavin Shan wrote:
> > Add generic cpu_list() to replace the individual target's implementation
> > in the subsequent commits. Currently, there are 3 targets with no 
> > cpu_list()
> > implementation: microblaze and nios2. With this applied, those two 
> > targets
> > switch to the generic cpu_list().
> >
> > [gshan@gshan q]$ ./build/qemu-system-microblaze -cpu ?
> > Available CPUs:
> >    microblaze-cpu
> >
> > [gshan@gshan q]$ ./build/qemu-system-nios2 -cpu ?
> > Available CPUs:
> >    nios2-cpu
> >
> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > ---
> >   bsd-user/main.c |  5 +----
> >   cpu-target.c    | 29 ++++++++++++++++++++++++++---
> >   2 files changed, 27 insertions(+), 7 deletions(-)
>
>
> > diff --git a/cpu-target.c b/cpu-target.c
> > index c078c0e91b..acfc654b95 100644
> > --- a/cpu-target.c
> > +++ b/cpu-target.c
> > @@ -24,6 +24,7 @@
> >   #include "hw/qdev-core.h"
> >   #include "hw/qdev-properties.h"
> >   #include "qemu/error-report.h"
> > +#include "qemu/qemu-print.h"
> >   #include "migration/vmstate.h"
> >   #ifdef CONFIG_USER_ONLY
> >   #include "qemu.h"
> > @@ -283,12 +284,34 @@ const char *parse_cpu_option(const char 
> > *cpu_option)
> >       return cpu_type;
> >   }
> > +#ifndef cpu_list
> > +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);
> > +    }
> > +}
> > +
> > +static void cpu_list(void)
> > +{
> > +    GSList *list;
> > +
> > +    list = object_class_get_list_sorted(TYPE_CPU, false);
> > +    qemu_printf("Available CPUs:\n");
>
> Since this output will likely be displayed a lot, IMHO it is worth
> doing a first pass to get the number of available CPUs. If it is 1,
> print using singular but even better smth like:
>
>         "This machine can only be used with the following CPU:"

Hmm I missed this code is common to user/system emulation.

System helper could be clever by using the intersection of cpu_list()
and MachineClass::valid_cpu_types[] sets.

> That said, this can be done later on top, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> > +    g_slist_foreach(list, cpu_list_entry, NULL);
> > +    g_slist_free(list);
> > +}
> > +#endif