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

[Nina Schoetterl-Glausch]

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);
}


[...]