Re: [PATCH v18 01/17] s390x/cpu topology: add s390 specifics to CPU topology

[Pierre Morel]

On 3/27/23 23:34, Nina Schoetterl-Glausch wrote:
> On Wed, 2023-03-15 at 15:34 +0100, Pierre Morel wrote:
> > S390 adds two new SMP levels, drawers and books to the CPU
> > topology.
> > The S390 CPU have specific topology features like dedication
> > and entitlement to give to the guest indications on the host
> > vCPUs scheduling and help the guest take the best decisions
> > on the scheduling of threads on the vCPUs.
> >
> > Let us provide the SMP properties with books and drawers levels
> > and S390 CPU with dedication and entitlement,
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   qapi/machine-common.json        | 22 +++++++++++++++
> >   qapi/machine-target.json        | 12 +++++++++
> >   qapi/machine.json               | 17 +++++++++---
> >   include/hw/boards.h             | 10 ++++++-
> >   include/hw/s390x/cpu-topology.h | 15 +++++++++++
> >   target/s390x/cpu.h              |  6 +++++
> >   hw/core/machine-smp.c           | 48 ++++++++++++++++++++++++++++-----
> >   hw/core/machine.c               |  4 +++
> >   hw/s390x/s390-virtio-ccw.c      |  2 ++
> >   softmmu/vl.c                    |  6 +++++
> >   target/s390x/cpu.c              |  7 +++++
> >   qapi/meson.build                |  1 +
> >   qemu-options.hx                 |  7 +++--
> >   13 files changed, 144 insertions(+), 13 deletions(-)
> >   create mode 100644 qapi/machine-common.json
> >   create mode 100644 include/hw/s390x/cpu-topology.h
> [...]
> > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> > index c3dab007da..b8233df5a9 100644
> > --- a/hw/core/machine-smp.c
> > +++ b/hw/core/machine-smp.c
> > @@ -31,6 +31,14 @@ static char *cpu_hierarchy_to_string(MachineState *ms)
> >       MachineClass *mc = MACHINE_GET_CLASS(ms);
> >       GString *s = g_string_new(NULL);
> >   
> > +    if (mc->smp_props.drawers_supported) {
> > +        g_string_append_printf(s, " * drawers (%u)", ms->smp.drawers);
> > +    }
> > +
> > +    if (mc->smp_props.books_supported) {
> > +        g_string_append_printf(s, " * books (%u)", ms->smp.books);
> > +    }
> > +
> >       g_string_append_printf(s, "sockets (%u)", ms->smp.sockets);
> The output of this doesn't look great.
> How about:
>
> static char *cpu_hierarchy_to_string(MachineState *ms)
> {
>      MachineClass *mc = MACHINE_GET_CLASS(ms);
>      GString *s = g_string_new(NULL);
>      const char *multiply = " * ", *prefix = "";
>
>      if (mc->smp_props.drawers_supported) {
>          g_string_append_printf(s, "drawers (%u)", ms->smp.drawers);
>          prefix = multiply;
>      }
>
>      if (mc->smp_props.books_supported) {
>          g_string_append_printf(s, "%sbooks (%u)", prefix, ms->smp.books);
>          prefix = multiply;
>      }
>
>      g_string_append_printf(s, "%ssockets (%u)", prefix, ms->smp.sockets);
>
>      if (mc->smp_props.dies_supported) {
>          g_string_append_printf(s, " * dies (%u)", ms->smp.dies);
>      }
>
>      if (mc->smp_props.clusters_supported) {
>          g_string_append_printf(s, " * clusters (%u)", ms->smp.clusters);
>      }
>
>      g_string_append_printf(s, " * cores (%u)", ms->smp.cores);
>      g_string_append_printf(s, " * threads (%u)", ms->smp.threads);
>
>      return g_string_free(s, false);
> }
>
>
> [...]

Yes, looks better, thanks

regards,

Pierre