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

[Pierre Morel <pmorel@linux.ibm.com>]

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