Re: [PATCH v2 3/8] machine: Print supported CPU models instead of typenames

[Gavin Shan <gshan@redhat.com>]

On 7/28/23 00:27, Richard Henderson wrote:
> On 7/26/23 22:16, Gavin Shan wrote:
>>
>> On 7/27/23 09:08, Richard Henderson wrote:
>>> On 7/25/23 17:32, Gavin Shan wrote:
>>>> -static const char *q800_machine_valid_cpu_types[] = {
>>>> +static const char * const q800_machine_valid_cpu_types[] = {
>>>>       M68K_CPU_TYPE_NAME("m68040"),
>>>>       NULL
>>>>   };
>>>> +static const char * const q800_machine_valid_cpu_models[] = {
>>>> +    "m68040",
>>>> +    NULL
>>>> +};
>>>
>>> I really don't like this replication.
>>>
>>
>> Right, it's going to be lots of replications, but gives much flexibility.
>> There are 21 targets and we don't have fixed pattern for the mapping between
>> CPU model name and CPU typename. I'm summarizing the used patterns like below.
>>
>>    1 All CPU model names are mappinged to fixed CPU typename;
>>    2 CPU model name is same to CPU typename;
>>    3 CPU model name is alias to CPU typename;
>>    4 CPU model name is prefix of CPU typename;
>>
>>    Target         Categories    suffix-of-CPU-typename
>>    -------------------------------------------------------
>>    alpha          -234          -alpha-cpu
>>    arm            ---4          -arm-cpu
>>    avr            -2--
>>    cris           --34          -cris-cpu
>>    hexagon        ---4          -hexagon-cpu
>>    hppa           1---
>>    i386           ---4          -i386-cpu
>>    loongarch      -2-4          -loongarch-cpu
>>    m68k           ---4          -m68k-cpu
>>    microblaze     1---
>>    mips           ---4          -mips64-cpu  -mips-cpu
>>    nios2          1---
>>    openrisc       ---4          -or1k-cpu
>>    ppc            --34          -powerpc64-cpu  -powerpc-cpu
>>    riscv          ---4          -riscv-cpu
>>    rx             -2-4          -rx-cpu
>>    s390x          ---4          -s390x-cpu
>>    sh4            --34          -superh-cpu
>>    sparc          -2--
>>    tricore        ---4          -tricore-cpu
>>    xtensa         ---4          -xtensa-cpu
> 
> That is unfortunate, however...
> 
> 
>> There are several options as below. Please let me know which one or something
>> else is the best.
>>
>> (a) Keep what we have and use mc->valid_{cpu_types, cpu_models}[] to track
>> the valid CPU typenames and CPU model names.
>>
>> (b) Introduce CPUClass::model_name_by_typename(). Every target has their own
>> implementation to convert CPU typename to CPU model name. The CPU model name
>> is parsed from mc->valid_cpu_types[i].
>>
>>      char *CPUClass::model_by_typename(const char *typename);
>>
>> (c) As we discussed before, use mc->valid_cpu_type_suffix and mc->valid_cpu_models
>> because the CPU type check is currently needed by target arm/m68k/riscv where we
>> do have fixed pattern to convert CPU model names to CPU typenames. The CPU typename
>> is comprised of CPU model name and suffix. However, it won't be working when the CPU
>> type check is required by other target where we have patterns other than this.
> 
> (d) Merge the two arrays together and use macro expansion, e.g.
> 
> typedef struct {
>      const char *name;
>      const char *type;
> } Something;
> 
> #define ARM_SOMETHING(x)  { x, ARM_CPU_TYPE_NAME(x) }
> 
> static const Something valid[] = {
>      ARM_SOMETHING("cortex-a53"),
>      { NULL, NULL }
> };
> 
> where Something ought to be better named.
> 

Thanks, Richard. It's a nice idea, but not generalized enough. Igor suggested
to reuse the existing list_cpus() in another reply, and I suggested to add
CPUClass::type_to_model() to convert CPU type name to model name for every
target. Please take look and comment when you get a chance.

   https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00659.html

Thanks,
Gavin