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

[Gavin Shan <gshan@redhat.com>]

Hi Phil,

On 11/16/23 17:51, Philippe Mathieu-Daudé wrote:
> On 16/11/23 08:39, Philippe Mathieu-Daudé wrote:
>> 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.
> 

When cpu_list() is called, it's possible the machine type option isn't
parsed yet. Besides, this function is usually used by "qemu-system-arm -cpu ?".
So I wouldn't connect to MachineClass::valid_cpu_types[] here if you agree.

Thanks,
Gavin

>> 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
>>
>>
>