Re: [PATCH v20 03/21] target/s390x/cpu topology: handle STSI(15) and build the SYSIB

[Nina Schoetterl-Glausch]

On Tue, 2023-04-25 at 18:14 +0200, Pierre Morel wrote:
> On interception of STSI(15.1.x) the System Information Block
> (SYSIB) is built from the list of pre-ordered topology entries.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  MAINTAINERS                     |   1 +
>  include/hw/s390x/cpu-topology.h |  24 +++
>  include/hw/s390x/sclp.h         |   1 +
>  target/s390x/cpu.h              |  72 ++++++++
>  hw/s390x/cpu-topology.c         |  13 +-
>  target/s390x/kvm/cpu_topology.c | 308 ++++++++++++++++++++++++++++++++
>  target/s390x/kvm/kvm.c          |   5 +-
>  target/s390x/kvm/meson.build    |   3 +-
>  8 files changed, 424 insertions(+), 3 deletions(-)
>  create mode 100644 target/s390x/kvm/cpu_topology.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bb7b34d0d8..de9052f753 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1659,6 +1659,7 @@ M: Pierre Morel <pmorel@linux.ibm.com>
>  S: Supported
>  F: include/hw/s390x/cpu-topology.h
>  F: hw/s390x/cpu-topology.c
> +F: target/s390x/kvm/cpu_topology.c
>  
>  X86 Machines
>  ------------
> diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
> index af36f634e0..87bfeb631e 100644
> --- a/include/hw/s390x/cpu-topology.h
> +++ b/include/hw/s390x/cpu-topology.h
> @@ -15,9 +15,33 @@
> 
[...]

> +typedef struct S390TopologyEntry {
> +    QTAILQ_ENTRY(S390TopologyEntry) next;
> +    s390_topology_id id;
> +    uint64_t mask;
> +} S390TopologyEntry;
> +
>  typedef struct S390Topology {
>      uint8_t *cores_per_socket;
> +    QTAILQ_HEAD(, S390TopologyEntry) list;

Since you recompute the list on every STSI, you no longer need this in here.
You can create it in insert_stsi_15_1_x.

>      CpuTopology *smp;
> +    bool vertical_polarization;
>  } S390Topology;

[...]

> +/*
> + * Macro to check that the size of data after increment
> + * will not get bigger than the size of the SysIB.
> + */
> +#define SYSIB_GUARD(data, x) do {       \
> +        data += x;                      \
> +        if (data  > sizeof(SysIB)) {    \
                    ^ two spaces

> +            return 0;                   \
> +        }                               \
> +    } while (0)
> +

[...]

> +/**
> + * s390_topology_from_cpu:
> + * @cpu: The S390CPU
> + *
> + * Initialize the topology id from the CPU environment.
> + */
> +static s390_topology_id s390_topology_from_cpu(S390CPU *cpu)
> +{
> +    s390_topology_id topology_id = {0};
> +
> +    topology_id.drawer = cpu->env.drawer_id;
> +    topology_id.book = cpu->env.book_id;
> +    topology_id.socket = cpu->env.socket_id;
> +    topology_id.origin = cpu->env.core_id / 64;
> +    topology_id.type = S390_TOPOLOGY_CPU_IFL;
> +    topology_id.dedicated = cpu->env.dedicated;
> +
> +    if (s390_topology.vertical_polarization) {
> +        /*
> +         * Vertical polarization with dedicated CPU implies
> +         * vertical high entitlement.
> +         */

This has already been adjusted or rejected when the entitlement was set.

> +        if (topology_id.dedicated) {
> +            topology_id.entitlement = S390_CPU_ENTITLEMENT_HIGH;
> +        } else {
> +            topology_id.entitlement = cpu->env.entitlement;

You only need this assignment.
> +        }
> +    }
> +
> +    return topology_id;

[...]