[kvm-unit-tests PATCH 00/10] s390x: topology: Fixes and extension

Nina Schoetterl-Glausch
2023-10-20T14:58:23Z

v1 -> v2:
* patch 1, introducing enums (Janosch)
* add comment explaining 8 alignment of stsi block length
* unsigned cpu_in_masks, iteration (Nico)
* fix copy paste error when checking ordering (thanks Nina)
* don't escape newline when \\ at end of line in multiline string
* change commit messages (thanks Janosch, thanks Nico)
* pick up tags (thanks Janosch, thanks Nico)

Fix a number of issues as well as rewrite and extend the topology list
checking.
Add a test case with a complex topology configuration.
In order to keep the unittests.cfg file readable, implement multiline
strings for extra_params.

Nina Schoetterl-Glausch (10):
s390x: topology: Introduce enums for polarization & cpu type
s390x: topology: Fix report message
s390x: topology: Use function parameter in stsi_get_sysib
s390x: topology: Fix parsing loop
s390x: topology: Make some report messages unique
s390x: topology: Refine stsi header test
s390x: topology: Rename topology_core to topology_cpu
s390x: topology: Rewrite topology list test
scripts: Implement multiline strings for extra_params
s390x: topology: Add complex topology test

scripts/common.bash | 16 +++
scripts/runtime.bash | 4 +-
lib/s390x/stsi.h | 47 ++++++---
s390x/topology.c | 231 ++++++++++++++++++++++++++-----------------
s390x/unittests.cfg | 133 +++++++++++++++++++++++++
5 files changed, 322 insertions(+), 109 deletions(-)

Range-diff:
-: -------- > 1: 334fec11 s390x: topology: Introduce enums for polarization & cpu type
1: a5d45194 ! 2: e3fabae5 s390x: topology: Fix report message
@@ Commit message

A polarization value of 0 means horizontal polarization.

+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## s390x/topology.c ##
@@ s390x/topology.c: static uint8_t *check_tle(void *tc)
- if (!cpus->d)
- report_skip("Not dedicated");
else
-- report(cpus->pp == 3 || cpus->pp == 0, "Dedicated CPUs are either vertically polarized or have high entitlement");
-+ report(cpus->pp == 3 || cpus->pp == 0, "Dedicated CPUs are either horizontally polarized or have high entitlement");
+ report(cpus->pp == POLARIZATION_VERTICAL_HIGH ||
+ cpus->pp == POLARIZATION_HORIZONTAL,
+- "Dedicated CPUs are either vertically polarized or have high entitlement");
++ "Dedicated CPUs are either horizontally polarized or have high entitlement");

return tc + sizeof(*cpus);
}
2: 218cf7c1 ! 3: 95c652d4 s390x: topology: Use parameter in stsi_get_sysib
@@ Metadata
Author: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## Commit message ##
- s390x: topology: Use parameter in stsi_get_sysib
+ s390x: topology: Use function parameter in stsi_get_sysib

- Instead of accessing global pagebuf.
+ Actually use the function parameter we're give instead of a hardcoded
+ access to the static variable pagebuf.

+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## s390x/topology.c ##
3: c9723868 ! 4: 4b6cf1ea s390x: topology: Fix parsing loop
@@ Commit message

Without a comparison the loop is infinite.

+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## s390x/topology.c ##
4: 8813e81d ! 5: fc10ff22 s390x: topology: Don't use non unique message
@@ Metadata
Author: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## Commit message ##
- s390x: topology: Don't use non unique message
+ s390x: topology: Make some report messages unique

When we test something, i.e. do a report() we want unique messages,
otherwise, from the test output, it will appear as if the same test was
@@ Commit message
into asserts.
Refine the report message for others.

+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## s390x/topology.c ##
5: 71b82851 ! 6: 37ef7a6c s390x: topology: Refine stsi header test
@@ Commit message
Add checks for length field.
Also minor refactor.

+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## s390x/topology.c ##
@@ s390x/topology.c: static void stsi_check_maxcpus(struct sysinfo_15_1_x *info)
- report_prefix_push("MAG");
+ report_prefix_push("Header");

++ /* Header is 16 bytes, each TLE 8 or 16, therefore alignment must be 8 at least */
+ report(IS_ALIGNED(info->length, 8), "Length %d multiple of 8", info->length);
+ report(info->length < PAGE_SIZE, "Length %d in bounds", info->length);
+ report(sel2 == info->mnest, "Valid mnest");
6: db844035 ! 7: 3247ed4d s390x: topology: Rename topology_core to topology_cpu
@@ Commit message

This is more in line with the nomenclature in the PoP.

+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
+ Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

## lib/s390x/stsi.h ##
7: da81fd0c ! 8: 0fea753f s390x: topology: Rewrite topology list test
@@ lib/s390x/stsi.h: struct sysinfo_3_2_2 {
+ };
};

+ enum topology_polarization {
+@@ lib/s390x/stsi.h: enum cpu_type {
+ };
+
#define CONTAINER_TLE_RES_BITS 0x00ffffffffffff00UL
-struct topology_container {
- uint8_t nl;
@@ s390x/topology.c: done:
- report(!(*(uint64_t *)tc & CPUS_TLE_RES_BITS), "reserved bits %016lx",
- *(uint64_t *)tc & CPUS_TLE_RES_BITS);
-
-- report(cpus->type == 0x03, "type IFL");
+- report(cpus->type == CPU_TYPE_IFL, "type IFL");
-
- report_info("origin: %d", cpus->origin);
- report_info("mask: %016lx", cpus->mask);
@@ s390x/topology.c: done:
- if (!cpus->d)
- report_skip("Not dedicated");
- else
-- report(cpus->pp == 3 || cpus->pp == 0, "Dedicated CPUs are either horizontally polarized or have high entitlement");
+- report(cpus->pp == POLARIZATION_VERTICAL_HIGH ||
+- cpus->pp == POLARIZATION_HORIZONTAL,
+- "Dedicated CPUs are either horizontally polarized or have high entitlement");
-
- return tc + sizeof(*cpus);
-}
@@ s390x/topology.c: static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel
+ report(!(cpu->raw[0] & CPUS_TLE_RES_BITS), "reserved bits %016lx",
+ cpu->raw[0] & CPUS_TLE_RES_BITS);
+
-+ report(cpu->type == 0x03, "type IFL");
++ report(cpu->type == CPU_TYPE_IFL, "type IFL");
+
+ if (cpu->d)
-+ report(cpu->pp == 3 || cpu->pp == 0,
++ report(cpu->pp == POLARIZATION_VERTICAL_HIGH ||
++ cpu->pp == POLARIZATION_HORIZONTAL,
+ "Dedicated CPUs are either horizontally polarized or have high entitlement");
+ else
+ report_skip("Not dedicated");
@@ s390x/topology.c: static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel
+static union topology_container *check_child_cpus(struct sysinfo_15_1_x *info,
+ union topology_container *cont,
+ union topology_cpu *child,
-+ int *cpus_in_masks)
++ unsigned int *cpus_in_masks)
+{
+ void *last = ((void *)info) + info->length;
+ union topology_cpu *prev_cpu = NULL;
-+ int cpus = 0;
++ unsigned int cpus = 0;
++ int i;
+
-+ for (; (void *)child < last && child->nl == 0; child++) {
-+ cpus += check_cpu(child, cont);
++ for (i = 0; (void *)&child[i] < last && child[i].nl == 0; i++) {
++ cpus += check_cpu(&child[i], cont);
+ if (prev_cpu) {
-+ report(prev_cpu->type <= child->type, "Correct ordering wrt type");
-+ if (prev_cpu->type < child->type)
++ report(prev_cpu->type <= child[i].type, "Correct ordering wrt type");
++ if (prev_cpu->type < child[i].type)
+ continue;
-+ report(prev_cpu->pp >= child->pp, "Correct ordering wrt polarization");
-+ if (prev_cpu->type > child->type)
++ report(prev_cpu->pp >= child[i].pp, "Correct ordering wrt polarization");
++ if (prev_cpu->pp > child[i].pp)
+ continue;
-+ report(prev_cpu->d || !child->d, "Correct ordering wrt dedication");
-+ if (prev_cpu->d && !child->d)
++ report(prev_cpu->d || !child[i].d, "Correct ordering wrt dedication");
++ if (prev_cpu->d && !child[i].d)
+ continue;
-+ report(prev_cpu->origin <= child->origin, "Correct ordering wrt origin");
++ report(prev_cpu->origin <= child[i].origin, "Correct ordering wrt origin");
+ }
-+ prev_cpu = child;
++ prev_cpu = &child[i];
+ }
+ report(cpus <= expected_topo_lvl[0], "%d children <= max of %d",
+ cpus, expected_topo_lvl[0]);
+ *cpus_in_masks += cpus;
+
-+ return (union topology_container *)child;
++ return (union topology_container *)&child[i];
+}
+
+static union topology_container *check_container(struct sysinfo_15_1_x *info,
+ union topology_container *cont,
+ union topology_entry *child,
-+ int *cpus_in_masks);
++ unsigned int *cpus_in_masks);
+
+static union topology_container *check_child_containers(struct sysinfo_15_1_x *info,
+ union topology_container *cont,
+ union topology_container *child,
-+ int *cpus_in_masks)
++ unsigned int *cpus_in_masks)
+{
+ void *last = ((void *)info) + info->length;
+ union topology_container *entry;
@@ s390x/topology.c: static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel
+static union topology_container *check_container(struct sysinfo_15_1_x *info,
+ union topology_container *cont,
+ union topology_entry *child,
-+ int *cpus_in_masks)
++ unsigned int *cpus_in_masks)
+{
+ union topology_container *entry;
+
@@ s390x/topology.c: static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel
+static void check_topology_list(struct sysinfo_15_1_x *info, int sel2)
+{
+ union topology_container dummy = { .nl = sel2, .id = 0 };
-+ int cpus_in_masks = 0;
++ unsigned int cpus_in_masks = 0;
+
+ report_prefix_push("TLE");
+
8: 5ff58671 ! 9: 6ef31e52 scripts: Implement multiline strings for extra_params
@@ scripts/common.bash: function for_each_unittest()
+ elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
+ opts=${BASH_REMATCH[1]}$'\n'
+ while read -r -u $fd; do
-+ opts=${opts%\\$'\n'}
++ #escape backslash newline, but not double backslash
++ if [[ $opts =~ [^\\]*(\\*)$'\n'$ ]]; then
++ if (( ${#BASH_REMATCH[1]} % 2 == 1 )); then
++ opts=${opts%\\$'\n'}
++ fi
++ fi
+ if [[ "$REPLY" =~ ^(.*)'"""'[:blank:]*$ ]]; then
+ opts+=${BASH_REMATCH[1]}
+ break
9: 185e2752 ! 10: 7e5a9bb5 s390x: topology: Add complex topology test
@@ Commit message
s390x: topology: Add complex topology test

Run the topology test case with a complex topology configuration.
+ Randomly generated with:
+ python -c 'import random
+ ds=bs=ss=2
+ cs=16
+ cids=list(range(1,ds*bs*ss*cs))
+ random.shuffle(cids)
+ i = 0
+ for d in range(ds):
+ for b in range(bs):
+ for s in range(ss):
+ for c in range(cs):
+ if (d,b,s,c) != (0,0,0,0):
+ ded=["false","true"][random.randrange(0,2)]
+ ent="high" if ded == "true" else ["low", "medium", "high"][random.randrange(0,3)]
+ print(f"-device max-s390x-cpu,core-id={cids[i]},drawer-id={d},book-id={b},socket-id={s},entitlement={ent},dedicated={ded}")
+ i+=1'

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

base-commit: bfe5d7d0e14c8199d134df84d6ae8487a9772c48
--
2.41.0