On Wed, 2023-02-22 at 15:20 +0100, 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@xxxxxxxxxxxxx>
> ---
> include/hw/s390x/cpu-topology.h | 21 +++
> include/hw/s390x/sclp.h | 1 +
> target/s390x/cpu.h | 72 ++++++++
> hw/s390x/cpu-topology.c | 14 +-
> target/s390x/kvm/cpu_topology.c | 312 ++++++++++++++++++++++++++++++++
> target/s390x/kvm/kvm.c | 5 +-
> target/s390x/kvm/meson.build | 3 +-
> 7 files changed, 425 insertions(+), 3 deletions(-)
> create mode 100644 target/s390x/kvm/cpu_topology.c
>
> diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
> index fa7f885a9f..8dc42d2942 100644
> --- a/include/hw/s390x/cpu-topology.h
> +++ b/include/hw/s390x/cpu-topology.h
> @@ -16,8 +16,29 @@
>
> #define S390_TOPOLOGY_CPU_IFL 0x03
>
> +typedef union s390_topology_id {
> + uint64_t id;
> + struct {
> + uint8_t level5;
You could rename this to sentinel, since that's the only use case and
if there ever is another level the sentinel implementation might need
to be changed anyway.
> + uint8_t drawer;
> + uint8_t book;
> + uint8_t socket;
> + uint8_t dedicated;
> + uint8_t entitlement;
> + uint8_t type;
> + uint8_t origin;
> + };
> +} s390_topology_id;
> +
>
[...]
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index d654267a71..c899f4e04b 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -560,6 +560,25 @@ typedef struct SysIB_322 {
>
[...]
>
> +/*
> + * CPU Topology List provided by STSI with fc=15 provides a list
> + * of two different Topology List Entries (TLE) types to specify
> + * the topology hierarchy.
> + *
> + * - Container Topology List Entry
> + * Defines a container to contain other Topology List Entries
> + * of any type, nested containers or CPU.
> + * - CPU Topology List Entry
> + * Specifies the CPUs position, type, entitlement and polarization
> + * of the CPUs contained in the last Container TLE.
> + *
> + * There can be theoretically up to five levels of containers, QEMU
> + * uses only three levels, the drawer's, book's and socket's level.
> + *
> + * A container of with a nesting level (NL) greater than 1 can only
s/of//
> + * contain another container of nesting level NL-1.
> + *
> + * A container of nesting level 1 (socket), contains as many CPU TLE
> + * as needed to describe the position and qualities of all CPUs inside
> + * the container.
> + * The qualities of a CPU are polarization, entitlement and type.
> + *
> + * The CPU TLE defines the position of the CPUs of identical qualities
> + * using a 64bits mask which first bit has its offset defined by
> + * the CPU address orgin field of the CPU TLE like in:
> + * CPU address = origin * 64 + bit position within the mask
> + *
> + */
> +/* Container type Topology List Entry */
> +typedef struct SysIBTl_container {
> + uint8_t nl;
> + uint8_t reserved[6];
> + uint8_t id;
> +} QEMU_PACKED QEMU_ALIGNED(8) SysIBTl_container;
> +QEMU_BUILD_BUG_ON(sizeof(SysIBTl_container) != 8);
> +
[...]
> +
> +/**
> + * 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.polarization == S390_CPU_POLARIZATION_VERTICAL) {
> + /*
> + * Vertical polarization with dedicated CPU implies
> + * vertical high entitlement.
> + */
> + if (topology_id.dedicated) {
> + topology_id.entitlement = S390_CPU_ENTITLEMENT_HIGH;
> + } else {
> + topology_id.entitlement = cpu->env.entitlement;
> + }
I don't see why you need this if, it should already be correct.
> + }
I'd suggest the following:
* rename entitlement in s390_topology_id back to polarization, but keep entitlement everywhere else.
* remove horizontal/none from CpuS390Entitlement, this way the user cannot set it,
and it doesn't show up in the output of query-cpus-fast.
* this is where you convert between the two, so:
if horizontal, id.polarization = 0,
otherwise id.polarization = entitlement + 1, or a switch case.
* in patch 6 in s390_topology_set_cpus_entitlement you don't set the entitlement if the polarization
is horizontal, which is ok because of the conversion above.
> +
> + return topology_id;
> +}
> +
>
[...]
