Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Gavin Shan <gshan@redhat.com>]

On 9/7/23 18:20, David Hildenbrand wrote:
> On 07.09.23 02:35, Gavin Shan wrote:
>> For target/s390x, the CPU type name is always the combination of the
>> CPU modle name and suffix. The CPU model names have been correctly
>> shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
>>
>> Use generic helper cpu_model_from_type() to show the CPU model names
>> in the above two functions. Besides, we need validate the CPU class
>> in s390_cpu_class_by_name(), as other targets do.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/s390x/cpu_models.c        | 18 +++++++++++-------
>>   target/s390x/cpu_models_sysemu.c |  9 ++++-----
>>   2 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 91ce896491..103e9072b8 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -338,7 +338,8 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>   {
>>       const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
>>       CPUClass *cc = CPU_CLASS(scc);
>> -    char *name = g_strdup(object_class_get_name((ObjectClass *)data));
>> +    const char *typename = object_class_get_name((ObjectClass *)data);
>> +    char *model = cpu_model_from_type(typename);
>>       g_autoptr(GString) details = g_string_new("");
>>       if (scc->is_static) {
>> @@ -355,14 +356,12 @@ static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
>>           g_string_truncate(details, details->len - 2);
>>       }
>> -    /* strip off the -s390x-cpu */
>> -    g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>>       if (details->len) {
>> -        qemu_printf("s390 %-15s %-35s (%s)\n", name, scc->desc, details->str);
>> +        qemu_printf("s390 %-15s %-35s (%s)\n", model, scc->desc, details->str);
>>       } else {
>> -        qemu_printf("s390 %-15s %-35s\n", name, scc->desc);
>> +        qemu_printf("s390 %-15s %-35s\n", model, scc->desc);
>>       }
>> -    g_free(name);
>> +    g_free(model);
>>   }
>>   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
>> @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char *name)
>>       oc = object_class_by_name(typename);
>>       g_free(typename);
>> -    return oc;
>> +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
>> +        !object_class_is_abstract(oc)) {
>> +        return oc;
>> +    }
>> +
>> +    return NULL;
> 
> Why is that change required?
> 

Good question. It's possible that other class's name conflicts with
CPU class's name. For example, class "abc-base-s390x-cpu" has been
registered for a misc class other than a CPU class. We don't want
s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
Almost all other target does similar check.

Thanks,
Gavin