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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 16/11/23 11:34, Gavin Shan wrote:
> 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 ?".

Not sure this is a good example :)

   $ qemu-system-arm -cpu ?
   qemu-system-arm: No machine specified, and there is no default
   Use -machine help to list supported machines

> So I wouldn't connect to MachineClass::valid_cpu_types[] here if you agree.

Agreed.