Re: [PATCH v19 01/21] s390x/cpu topology: add s390 specifics to CPU topology

[Nina Schoetterl-Glausch]

On Mon, 2023-04-03 at 18:28 +0200, 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>
> ---
>  MAINTAINERS                     |  5 ++++
>  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 ++++++++++

Is hw/s390x the right path for cpu-topology?
I haven't understood the difference between hw/s390x and target/s390x
but target/s390x feels more correct, I could be mistaken though.

>  target/s390x/cpu.h              |  5 ++++
>  hw/core/machine-smp.c           | 53 ++++++++++++++++++++++++++++-----
>  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 +++--
>  14 files changed, 152 insertions(+), 14 deletions(-)
>  create mode 100644 qapi/machine-common.json
>  create mode 100644 include/hw/s390x/cpu-topology.h
> 
[...]

> diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> new file mode 100644
> index 0000000000..73ea38d976
> --- /dev/null
> +++ b/qapi/machine-common.json
> @@ -0,0 +1,22 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# = Machines S390 data types
> +##
> +
> +##
> +# @CpuS390Entitlement:
> +#
> +# An enumeration of cpu entitlements that can be assumed by a virtual
> +# S390 CPU
> +#
> +# Since: 8.1
> +##
> +{ 'enum': 'CpuS390Entitlement',
> +  'prefix': 'S390_CPU_ENTITLEMENT',
> +  'data': [ 'horizontal', 'low', 'medium', 'high' ] }

You can get rid of the horizontal value now that the entitlement is ignored if 
the
polarization is vertical.

[...]

> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index b10a8541ff..57165fa3a0 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -37,6 +37,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/reset.h"
>  #endif
> +#include "hw/s390x/cpu-topology.h"
>  
>  #define CR0_RESET       0xE0UL
>  #define CR14_RESET      0xC2000000UL;
> @@ -259,6 +260,12 @@ static gchar *s390_gdb_arch_name(CPUState *cs)
>  static Property s390x_cpu_properties[] = {
>  #if !defined(CONFIG_USER_ONLY)
>      DEFINE_PROP_UINT32("core-id", S390CPU, env.core_id, 0),
> +    DEFINE_PROP_INT32("socket-id", S390CPU, env.socket_id, -1),
> +    DEFINE_PROP_INT32("book-id", S390CPU, env.book_id, -1),
> +    DEFINE_PROP_INT32("drawer-id", S390CPU, env.drawer_id, -1),
> +    DEFINE_PROP_BOOL("dedicated", S390CPU, env.dedicated, false),
> +    DEFINE_PROP_UINT8("entitlement", S390CPU, env.entitlement,
> +                      S390_CPU_ENTITLEMENT__MAX),

I would define an entitlement PropertyInfo in qdev-properties-system.[ch],
then one can use e.g.

-device z14-s390x-cpu,core-id=11,entitlement=high

on the command line and cpu hotplug.

I think setting the default entitlement to medium here should be fine.

[...]