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

[Gavin Shan]

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