On 1/16/23 14:11, Nina Schoetterl-Glausch wrote:
On Thu, 2023-01-05 at 15:53 +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 | 3 + include/hw/s390x/sclp.h | 1 + target/s390x/cpu.h | 78 ++++++++++++++++++ target/s390x/kvm/cpu_topology.c | 136 ++++++++++++++++++++++++++++++++ target/s390x/kvm/kvm.c | 5 +- target/s390x/kvm/meson.build | 3 +- 6 files changed, 224 insertions(+), 2 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 b3fd752d8d..9571aa70e5 100644 --- a/include/hw/s390x/cpu-topology.h +++ b/include/hw/s390x/cpu-topology.h @@ -41,6 +41,9 @@ typedef union s390_topology_id { }; } s390_topology_id; #define TOPO_CPU_MASK 0x000000000000003fUL +#define TOPO_SOCKET_MASK 0x0000ffffff000000UL +#define TOPO_BOOK_MASK 0x0000ffff00000000UL +#define TOPO_DRAWER_MASK 0x0000ff0000000000ULtypedef struct S390TopologyEntry {s390_topology_id id; diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h index d3ade40a5a..712fd68123 100644 --- a/include/hw/s390x/sclp.h +++ b/include/hw/s390x/sclp.h @@ -112,6 +112,7 @@ typedef struct CPUEntry { } QEMU_PACKED CPUEntry;#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128+#define SCLP_READ_SCP_INFO_MNEST 2 typedef struct ReadInfo { SCCBHeader h; uint16_t rnmax; diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 39ea63a416..78988048dd 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -561,6 +561,25 @@ typedef struct SysIB_322 { } SysIB_322; QEMU_BUILD_BUG_ON(sizeof(SysIB_322) != 4096);+#define S390_TOPOLOGY_MAG 6+#define S390_TOPOLOGY_MAG6 0 +#define S390_TOPOLOGY_MAG5 1 +#define S390_TOPOLOGY_MAG4 2 +#define S390_TOPOLOGY_MAG3 3 +#define S390_TOPOLOGY_MAG2 4 +#define S390_TOPOLOGY_MAG1 5 +/* Configuration topology */ +typedef struct SysIB_151x { + uint8_t reserved0[2]; + uint16_t length; + uint8_t mag[S390_TOPOLOGY_MAG]; + uint8_t reserved1; + uint8_t mnest; + uint32_t reserved2; + char tle[]; +} QEMU_PACKED QEMU_ALIGNED(8) SysIB_151x; +QEMU_BUILD_BUG_ON(sizeof(SysIB_151x) != 16); + typedef union SysIB { SysIB_111 sysib_111; SysIB_121 sysib_121; @@ -568,9 +587,68 @@ typedef union SysIB { SysIB_221 sysib_221; SysIB_222 sysib_222; SysIB_322 sysib_322; + SysIB_151x sysib_151x; } SysIB; QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);+/*+ * 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 one level, the socket level. + * + * A container of with a nesting level (NL) greater than 1 can only + * 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 */ +/* 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); + +/* CPU type Topology List Entry */ +typedef struct SysIBTl_cpu { + uint8_t nl; + uint8_t reserved0[3]; + uint8_t reserved1:5; + uint8_t dedicated:1; + uint8_t polarity:2; + uint8_t type; + uint16_t origin; + uint64_t mask; +} QEMU_PACKED QEMU_ALIGNED(8) SysIBTl_cpu; +QEMU_BUILD_BUG_ON(sizeof(SysIBTl_cpu) != 16); + +/* Max size of a SYSIB structure is when all CPU are alone in a container */ +#define S390_TOPOLOGY_SYSIB_SIZE (sizeof(SysIB_151x) + \ + S390_MAX_CPUS * (sizeof(SysIBTl_container) + \ + sizeof(SysIBTl_cpu)))I don't think this is accurate anymore, if you have drawers and books. In that case you could have 3 containers per 1 cpu. You could also use the maxcpus number at runtime instead of S390_MAX_CPUS. I also think you could do sizeof(SysIB) + sizeof(SysIBTl_cpu) if you check if the sysib overflows 4k while building it.
Right. And I think your other proposal here under is better
+ +void insert_stsi_15_1_x(S390CPU *cpu, int sel2, __u64 addr, uint8_t ar); + /* MMU defines */ #define ASCE_ORIGIN (~0xfffULL) /* segment table origin */ #define ASCE_SUBSPACE 0x200 /* subspace group control */ diff --git a/target/s390x/kvm/cpu_topology.c b/target/s390x/kvm/cpu_topology.c new file mode 100644 index 0000000000..3831a3264c --- /dev/null +++ b/target/s390x/kvm/cpu_topology.c @@ -0,0 +1,136 @@ +/* + * QEMU S390x CPU Topology + * + * Copyright IBM Corp. 2022 + * Author(s): Pierre Morel <pmorel@xxxxxxxxxxxxx> + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ +#include "qemu/osdep.h" +#include "cpu.h" +#include "hw/s390x/pv.h" +#include "hw/sysbus.h" +#include "hw/s390x/sclp.h" +#include "hw/s390x/cpu-topology.h" + +static char *fill_container(char *p, int level, int id) +{ + SysIBTl_container *tle = (SysIBTl_container *)p; + + tle->nl = level; + tle->id = id; + return p + sizeof(*tle); +} + +static char *fill_tle_cpu(char *p, S390TopologyEntry *entry) +{ + SysIBTl_cpu *tle = (SysIBTl_cpu *)p; + s390_topology_id topology_id = entry->id; + + tle->nl = 0; + tle->dedicated = topology_id.d; + tle->polarity = topology_id.p; + tle->type = topology_id.type; + tle->origin = topology_id.origin;You need to multiply that value by 64, no? And convert it to BE.
Yes right, I already had this error, I must have lost it in a rebase.
+ tle->mask = cpu_to_be64(entry->mask); + return p + sizeof(*tle); +} + +static char *s390_top_set_level(char *p, int level) +{ + S390TopologyEntry *entry; + uint64_t last_socket = -1UL; + uint64_t last_book = -1UL; + uint64_t last_drawer = -1UL;-1UL looks funny to me, but there is nothing wrong with it. But I don't see a reason not to use int and initialize it with -1.+ int drawer_cnt = 0; + int book_cnt = 0; + int socket_cnt = 0; + + QTAILQ_FOREACH(entry, &s390_topology.list, next) { + + if (level > 3 && (last_drawer != entry->id.drawer)) { + book_cnt = 0; + socket_cnt = 0; + p = fill_container(p, 3, drawer_cnt++); + last_drawer = entry->id.id & TOPO_DRAWER_MASK; + p = fill_container(p, 2, book_cnt++); + last_book = entry->id.id & TOPO_BOOK_MASK; + p = fill_container(p, 1, socket_cnt++); + last_socket = entry->id.id & TOPO_SOCKET_MASK; + p = fill_tle_cpu(p, entry); + } else if (level > 2 && (last_book != + (entry->id.id & TOPO_BOOK_MASK))) { + socket_cnt = 0; + p = fill_container(p, 2, book_cnt++); + last_book = entry->id.id & TOPO_BOOK_MASK; + p = fill_container(p, 1, socket_cnt++); + last_socket = entry->id.id & TOPO_SOCKET_MASK; + p = fill_tle_cpu(p, entry); + } else if (last_socket != (entry->id.id & TOPO_SOCKET_MASK)) { + p = fill_container(p, 1, socket_cnt++); + last_socket = entry->id.id & TOPO_SOCKET_MASK; + p = fill_tle_cpu(p, entry); + } else { + p = fill_tle_cpu(p, entry); + } + } + + return p; +}I think you can do this a bit more readable and reduce redundancy. Pseudo code: foreach entry: bool drawer_change = last_drawer != current_drawer bool book_change = drawer_change || last_book != current_book bool socket_change = book_change || last_socket != current_socket if (level > 3 && drawer_change) reset book id fill drawer container drawer id++ if (level > 2 && book_change) reset socket id fill book container book id++ if (socket_change) fill socket container socket id++ fill cpu entry update last_drawer, _book, _socket You can also check after after every fill if the buffer has been overflowed, that is if the function wrote more than sizeof(SysIB) - sizeof(SysIB_151x) bytes. Or you check it once at the end if you increase the size of the buffer a bit. Then you don't need to allocate the absolute maximum. I think you could also use global ids for the containers. So directly use the drawer id from the entry, use (drawer id * smp.books) + book id, and so on. If you update last_* after setting *_changed you don't need to maintain ids, you can just use last_*.
OK, seems better to me Thanks Regards, Pierre
[...]
-- Pierre Morel IBM Lab Boeblingen
