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

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

[Nina Schoetterl-Glausch]

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

[kvm-unit-tests PATCH 01/10] s390x: topology: Introduce enums for polarization & cpu type

[Nina Schoetterl-Glausch]

Thereby get rid of magic values.

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 lib/s390x/stsi.h | 11 +++++++++++
 s390x/topology.c |  6 ++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
index 1351a6f3..2f6182c1 100644
--- a/lib/s390x/stsi.h
+++ b/lib/s390x/stsi.h
@@ -41,6 +41,17 @@ struct topology_core {
 	uint64_t mask;
 };
 
+enum topology_polarization {
+	POLARIZATION_HORIZONTAL = 0,
+	POLARIZATION_VERTICAL_LOW = 1,
+	POLARIZATION_VERTICAL_MEDIUM = 2,
+	POLARIZATION_VERTICAL_HIGH = 3,
+};
+
+enum cpu_type {
+	CPU_TYPE_IFL = 3,
+};
+
 #define CONTAINER_TLE_RES_BITS 0x00ffffffffffff00UL
 struct topology_container {
 	uint8_t nl;
diff --git a/s390x/topology.c b/s390x/topology.c
index 69558236..6ab8c8d4 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -261,7 +261,7 @@ static uint8_t *check_tle(void *tc)
 	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);
@@ -275,7 +275,9 @@ 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 == POLARIZATION_VERTICAL_HIGH ||
+		       cpus->pp == POLARIZATION_HORIZONTAL,
+		       "Dedicated CPUs are either vertically polarized or have high entitlement");
 
 	return tc + sizeof(*cpus);
 }
-- 
2.41.0

[kvm-unit-tests PATCH 02/10] s390x: topology: Fix report message

[Nina Schoetterl-Glausch]

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/topology.c b/s390x/topology.c
index 6ab8c8d4..1c4a86fe 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -277,7 +277,7 @@ static uint8_t *check_tle(void *tc)
 	else
 		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.41.0

[kvm-unit-tests PATCH 03/10] s390x: topology: Use function parameter in stsi_get_sysib

[Nina Schoetterl-Glausch]

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/topology.c b/s390x/topology.c
index 1c4a86fe..032e80dc 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -324,7 +324,7 @@ static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel2)
 
 	report_prefix_pushf("SYSIB");
 
-	ret = stsi(pagebuf, 15, 1, sel2);
+	ret = stsi(info, 15, 1, sel2);
 
 	if (max_nested_lvl >= sel2) {
 		report(!ret, "Valid instruction");
-- 
2.41.0

[kvm-unit-tests PATCH 04/10] s390x: topology: Fix parsing loop

[Nina Schoetterl-Glausch]

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/topology.c b/s390x/topology.c
index 032e80dc..c8ad4bcb 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -468,7 +468,7 @@ static void parse_topology_args(int argc, char **argv)
 		if (flag[0] != '-')
 			report_abort("Argument is expected to begin with '-'");
 		flag++;
-		for (level = 0; ARRAY_SIZE(levels); level++) {
+		for (level = 0; level < ARRAY_SIZE(levels); level++) {
 			if (!strcmp(levels[level], flag))
 				break;
 		}
-- 
2.41.0

[kvm-unit-tests PATCH 05/10] s390x: topology: Make some report messages unique

[Nina Schoetterl-Glausch]

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
run multiple times, possible with different PASS/FAIL values.

Convert some reports that don't actually test anything topology specific
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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/s390x/topology.c b/s390x/topology.c
index c8ad4bcb..03bc3d3b 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -114,7 +114,7 @@ static void check_polarization_change(void)
 	report_prefix_push("Polarization change");
 
 	/* We expect a clean state through reset */
-	report(diag308_load_reset(1), "load normal reset done");
+	assert(diag308_load_reset(1));
 
 	/*
 	 * Set vertical polarization to verify that RESET sets
@@ -123,7 +123,7 @@ static void check_polarization_change(void)
 	cc = ptf(PTF_REQ_VERTICAL, &rc);
 	report(cc == 0, "Set vertical polarization.");
 
-	report(diag308_load_reset(1), "load normal reset done");
+	assert(diag308_load_reset(1));
 
 	cc = ptf(PTF_CHECK, &rc);
 	report(cc == 0, "Reset should clear topology report");
@@ -137,25 +137,25 @@ static void check_polarization_change(void)
 	report(cc == 0, "Change to vertical");
 
 	cc = ptf(PTF_CHECK, &rc);
-	report(cc == 1, "Should report");
+	report(cc == 1, "Should report change after horizontal -> vertical");
 
 	cc = ptf(PTF_REQ_VERTICAL, &rc);
 	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, "Double change to vertical");
 
 	cc = ptf(PTF_CHECK, &rc);
-	report(cc == 0, "Should not report");
+	report(cc == 0, "Should not report change after vertical -> vertical");
 
 	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
 	report(cc == 0, "Change to horizontal");
 
 	cc = ptf(PTF_CHECK, &rc);
-	report(cc == 1, "Should Report");
+	report(cc == 1, "Should report change after vertical -> horizontal");
 
 	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
 	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED, "Double change to horizontal");
 
 	cc = ptf(PTF_CHECK, &rc);
-	report(cc == 0, "Should not report");
+	report(cc == 0, "Should not report change after horizontal -> horizontal");
 
 	report_prefix_pop();
 }
-- 
2.41.0

[kvm-unit-tests PATCH 06/10] s390x: topology: Refine stsi header test

[Nina Schoetterl-Glausch]

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 | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/s390x/topology.c b/s390x/topology.c
index 03bc3d3b..6a5f100e 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -187,18 +187,23 @@ static void stsi_check_maxcpus(struct sysinfo_15_1_x *info)
 }
 
 /*
- * stsi_check_mag
+ * stsi_check_header
  * @info: Pointer to the stsi information
+ * @sel2: stsi selector 2 value
  *
  * MAG field should match the architecture defined containers
  * when MNEST as returned by SCLP matches MNEST of the SYSIB.
  */
-static void stsi_check_mag(struct sysinfo_15_1_x *info)
+static void stsi_check_header(struct sysinfo_15_1_x *info, int sel2)
 {
 	int i;
 
-	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");
 	stsi_check_maxcpus(info);
 
 	/*
@@ -328,7 +333,6 @@ static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel2)
 
 	if (max_nested_lvl >= sel2) {
 		report(!ret, "Valid instruction");
-		report(sel2 == info->mnest, "Valid mnest");
 	} else {
 		report(ret, "Invalid instruction");
 	}
@@ -367,7 +371,7 @@ static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info, int sel2)
 		goto vertical;
 	}
 
-	stsi_check_mag(info);
+	stsi_check_header(info, sel2);
 	stsi_check_tle_coherency(info);
 
 vertical:
@@ -380,7 +384,7 @@ vertical:
 		goto end;
 	}
 
-	stsi_check_mag(info);
+	stsi_check_header(info, sel2);
 	stsi_check_tle_coherency(info);
 	report_prefix_pop();
 
-- 
2.41.0

[kvm-unit-tests PATCH 07/10] s390x: topology: Rename topology_core to topology_cpu

[Nina Schoetterl-Glausch]

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 | 4 ++--
 s390x/topology.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
index 2f6182c1..1e9d0958 100644
--- a/lib/s390x/stsi.h
+++ b/lib/s390x/stsi.h
@@ -30,7 +30,7 @@ struct sysinfo_3_2_2 {
 };
 
 #define CPUS_TLE_RES_BITS 0x00fffffff8000000UL
-struct topology_core {
+struct topology_cpu {
 	uint8_t nl;
 	uint8_t reserved1[3];
 	uint8_t reserved4:5;
@@ -61,7 +61,7 @@ struct topology_container {
 
 union topology_entry {
 	uint8_t nl;
-	struct topology_core cpu;
+	struct topology_cpu cpu;
 	struct topology_container container;
 };
 
diff --git a/s390x/topology.c b/s390x/topology.c
index 6a5f100e..df158aef 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -247,7 +247,7 @@ done:
 static uint8_t *check_tle(void *tc)
 {
 	struct topology_container *container = tc;
-	struct topology_core *cpus;
+	struct topology_cpu *cpus;
 	int n;
 
 	if (container->nl) {
-- 
2.41.0

[kvm-unit-tests PATCH 08/10] s390x: topology: Rewrite topology list test

[Nina Schoetterl-Glausch]

Rewrite recursion with separate functions for checking containers,
containers containing CPUs and CPUs.
This improves comprehension and allows for more tests.
We now also test for ordering of CPU TLEs and number of child entries.

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 lib/s390x/stsi.h |  36 +++++----
 s390x/topology.c | 201 ++++++++++++++++++++++++++++-------------------
 2 files changed, 142 insertions(+), 95 deletions(-)

diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
index 1e9d0958..f2290ca7 100644
--- a/lib/s390x/stsi.h
+++ b/lib/s390x/stsi.h
@@ -30,15 +30,18 @@ struct sysinfo_3_2_2 {
 };
 
 #define CPUS_TLE_RES_BITS 0x00fffffff8000000UL
-struct topology_cpu {
-	uint8_t nl;
-	uint8_t reserved1[3];
-	uint8_t reserved4:5;
-	uint8_t d:1;
-	uint8_t pp:2;
-	uint8_t type;
-	uint16_t origin;
-	uint64_t mask;
+union topology_cpu {
+	uint64_t raw[2];
+	struct {
+		uint8_t nl;
+		uint8_t reserved1[3];
+		uint8_t reserved4:5;
+		uint8_t d:1;
+		uint8_t pp:2;
+		uint8_t type;
+		uint16_t origin;
+		uint64_t mask;
+	};
 };
 
 enum topology_polarization {
@@ -53,16 +56,19 @@ enum cpu_type {
 };
 
 #define CONTAINER_TLE_RES_BITS 0x00ffffffffffff00UL
-struct topology_container {
-	uint8_t nl;
-	uint8_t reserved[6];
-	uint8_t id;
+union topology_container {
+	uint64_t raw;
+	struct {
+		uint8_t nl;
+		uint8_t reserved[6];
+		uint8_t id;
+	};
 };
 
 union topology_entry {
 	uint8_t nl;
-	struct topology_cpu cpu;
-	struct topology_container container;
+	union topology_cpu cpu;
+	union topology_container container;
 };
 
 #define CPU_TOPOLOGY_MAX_LEVEL 6
diff --git a/s390x/topology.c b/s390x/topology.c
index df158aef..037d22cd 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -2,7 +2,7 @@
 /*
  * CPU Topology
  *
- * Copyright IBM Corp. 2022
+ * Copyright IBM Corp. 2022, 2023
  *
  * Authors:
  *  Pierre Morel <pmorel@linux.ibm.com>
@@ -23,7 +23,6 @@ static uint8_t pagebuf[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
 
 static int max_nested_lvl;
 static int number_of_cpus;
-static int cpus_in_masks;
 static int max_cpus;
 
 /*
@@ -237,82 +236,6 @@ done:
 	report_prefix_pop();
 }
 
-/**
- * check_tle:
- * @tc: pointer to first TLE
- *
- * Recursively check the containers TLEs until we
- * find a CPU TLE.
- */
-static uint8_t *check_tle(void *tc)
-{
-	struct topology_container *container = tc;
-	struct topology_cpu *cpus;
-	int n;
-
-	if (container->nl) {
-		report_info("NL: %d id: %d", container->nl, container->id);
-
-		report(!(*(uint64_t *)tc & CONTAINER_TLE_RES_BITS),
-		       "reserved bits %016lx",
-		       *(uint64_t *)tc & CONTAINER_TLE_RES_BITS);
-
-		return check_tle(tc + sizeof(*container));
-	}
-
-	report_info("NL: %d", container->nl);
-	cpus = tc;
-
-	report(!(*(uint64_t *)tc & CPUS_TLE_RES_BITS), "reserved bits %016lx",
-	       *(uint64_t *)tc & CPUS_TLE_RES_BITS);
-
-	report(cpus->type == CPU_TYPE_IFL, "type IFL");
-
-	report_info("origin: %d", cpus->origin);
-	report_info("mask: %016lx", cpus->mask);
-	report_info("dedicated: %d entitlement: %d", cpus->d, cpus->pp);
-
-	n = __builtin_popcountl(cpus->mask);
-	report(n <= expected_topo_lvl[0], "CPUs per mask: %d out of max %d",
-	       n, expected_topo_lvl[0]);
-	cpus_in_masks += n;
-
-	if (!cpus->d)
-		report_skip("Not dedicated");
-	else
-		report(cpus->pp == POLARIZATION_VERTICAL_HIGH ||
-		       cpus->pp == POLARIZATION_HORIZONTAL,
-		       "Dedicated CPUs are either horizontally polarized or have high entitlement");
-
-	return tc + sizeof(*cpus);
-}
-
-/**
- * stsi_check_tle_coherency:
- * @info: Pointer to the stsi information
- *
- * We verify that we get the expected number of Topology List Entry
- * containers for a specific level.
- */
-static void stsi_check_tle_coherency(struct sysinfo_15_1_x *info)
-{
-	void *tc, *end;
-
-	report_prefix_push("TLE");
-	cpus_in_masks = 0;
-
-	tc = info->tle;
-	end = (void *)info + info->length;
-
-	while (tc < end)
-		tc = check_tle(tc);
-
-	report(cpus_in_masks == number_of_cpus, "CPUs in mask %d",
-	       cpus_in_masks);
-
-	report_prefix_pop();
-}
-
 /**
  * stsi_get_sysib:
  * @info: pointer to the STSI info structure
@@ -342,6 +265,124 @@ static int stsi_get_sysib(struct sysinfo_15_1_x *info, int sel2)
 	return ret;
 }
 
+static int check_cpu(union topology_cpu *cpu,
+		     union topology_container *parent)
+{
+	report_prefix_pushf("%d:%d:%d:%d", cpu->d, cpu->pp, cpu->type, cpu->origin);
+
+	report(!(cpu->raw[0] & CPUS_TLE_RES_BITS), "reserved bits %016lx",
+	       cpu->raw[0] & CPUS_TLE_RES_BITS);
+
+	report(cpu->type == CPU_TYPE_IFL, "type IFL");
+
+	if (cpu->d)
+		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");
+
+	report_prefix_pop();
+
+	return __builtin_popcountl(cpu->mask);
+}
+
+static union topology_container *check_child_cpus(struct sysinfo_15_1_x *info,
+						  union topology_container *cont,
+						  union topology_cpu *child,
+						  unsigned int *cpus_in_masks)
+{
+	void *last = ((void *)info) + info->length;
+	union topology_cpu *prev_cpu = NULL;
+	unsigned int cpus = 0;
+	int i;
+
+	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[i].type, "Correct ordering wrt type");
+			if (prev_cpu->type < child[i].type)
+				continue;
+			report(prev_cpu->pp >= child[i].pp, "Correct ordering wrt polarization");
+			if (prev_cpu->pp > child[i].pp)
+				continue;
+			report(prev_cpu->d || !child[i].d, "Correct ordering wrt dedication");
+			if (prev_cpu->d && !child[i].d)
+				continue;
+			report(prev_cpu->origin <= child[i].origin, "Correct ordering wrt origin");
+		}
+		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[i];
+}
+
+static union topology_container *check_container(struct sysinfo_15_1_x *info,
+						 union topology_container *cont,
+						 union topology_entry *child,
+						 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,
+							unsigned int *cpus_in_masks)
+{
+	void *last = ((void *)info) + info->length;
+	union topology_container *entry;
+	int i;
+
+	for (i = 0, entry = child; (void *)entry < last && entry->nl == cont->nl - 1; i++) {
+		entry = check_container(info, entry, (union topology_entry *)(entry + 1),
+					cpus_in_masks);
+	}
+	if (max_nested_lvl == info->mnest)
+		report(i <= expected_topo_lvl[cont->nl - 1], "%d children <= max of %d",
+		       i, expected_topo_lvl[cont->nl - 1]);
+
+	return entry;
+}
+
+static union topology_container *check_container(struct sysinfo_15_1_x *info,
+						 union topology_container *cont,
+						 union topology_entry *child,
+						 unsigned int *cpus_in_masks)
+{
+	union topology_container *entry;
+
+	report_prefix_pushf("%d", cont->id);
+
+	report(cont->nl - 1 == child->nl, "Level %d one above child level %d",
+	       cont->nl, child->nl);
+	report(!(cont->raw & CONTAINER_TLE_RES_BITS), "reserved bits %016lx",
+	       cont->raw & CONTAINER_TLE_RES_BITS);
+
+	if (cont->nl > 1)
+		entry = check_child_containers(info, cont, &child->container, cpus_in_masks);
+	else
+		entry = check_child_cpus(info, cont, &child->cpu, cpus_in_masks);
+
+	report_prefix_pop();
+	return entry;
+}
+
+static void check_topology_list(struct sysinfo_15_1_x *info, int sel2)
+{
+	union topology_container dummy = { .nl = sel2, .id = 0 };
+	unsigned int cpus_in_masks = 0;
+
+	report_prefix_push("TLE");
+
+	check_container(info, &dummy, info->tle, &cpus_in_masks);
+	report(cpus_in_masks == number_of_cpus,
+	       "Number of CPUs %d equals  %d CPUs in masks",
+	       number_of_cpus, cpus_in_masks);
+
+	report_prefix_pop();
+}
+
 /**
  * check_sysinfo_15_1_x:
  * @info: pointer to the STSI info structure
@@ -372,7 +413,7 @@ static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info, int sel2)
 	}
 
 	stsi_check_header(info, sel2);
-	stsi_check_tle_coherency(info);
+	check_topology_list(info, sel2);
 
 vertical:
 	report_prefix_pop();
@@ -385,7 +426,7 @@ vertical:
 	}
 
 	stsi_check_header(info, sel2);
-	stsi_check_tle_coherency(info);
+	check_topology_list(info, sel2);
 	report_prefix_pop();
 
 end:
-- 
2.41.0

[kvm-unit-tests PATCH 09/10] scripts: Implement multiline strings for extra_params

[Nina Schoetterl-Glausch]

Implement a rudimentary form only.
extra_params can get long when passing a lot of arguments to qemu.
Multiline strings help with readability of the .cfg file.
Multiline strings begin and end with """, which must occur on separate
lines.

For example:
extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
-append '-drawers 2 -books 2 -sockets 2 -cores 16' \
-device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""

The command string built with extra_params is eval'ed by the runtime
script, so the newlines need to be escaped with \.

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 scripts/common.bash  | 16 ++++++++++++++++
 scripts/runtime.bash |  4 ++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/scripts/common.bash b/scripts/common.bash
index 7b983f7d..b9413d68 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -36,6 +36,22 @@ function for_each_unittest()
 			kernel=$TEST_DIR/${BASH_REMATCH[1]}
 		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
 			smp=${BASH_REMATCH[1]}
+		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
+			opts=${BASH_REMATCH[1]}$'\n'
+			while read -r -u $fd; do
+				#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
+				else
+					opts+=$REPLY$'\n'
+				fi
+			done
 		elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
 			opts=${BASH_REMATCH[1]}
 		elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index ada8ffd7..fc156f2f 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -15,7 +15,7 @@ extract_summary()
 # We assume that QEMU is going to work if it tried to load the kernel
 premature_failure()
 {
-    local log="$(eval $(get_cmdline _NO_FILE_4Uhere_) 2>&1)"
+    local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
 
     echo "$log" | grep "_NO_FILE_4Uhere_" |
         grep -q -e "could not \(load\|open\) kernel" -e "error loading" &&
@@ -168,7 +168,7 @@ function run()
     # extra_params in the config file may contain backticks that need to be
     # expanded, so use eval to start qemu.  Use "> >(foo)" instead of a pipe to
     # preserve the exit status.
-    summary=$(eval $cmdline 2> >(RUNTIME_log_stderr $testname) \
+    summary=$(eval "$cmdline" 2> >(RUNTIME_log_stderr $testname) \
                              > >(tee >(RUNTIME_log_stdout $testname $kernel) | extract_summary))
     ret=$?
     [ "$KUT_STANDALONE" != "yes" ] && echo > >(RUNTIME_log_stdout $testname $kernel)
-- 
2.41.0

[kvm-unit-tests PATCH 10/10] s390x: topology: Add complex topology test

[Nina Schoetterl-Glausch]

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>
---
 s390x/unittests.cfg | 133 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 68e119e4..b08b0fb1 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -247,3 +247,136 @@ file = topology.elf
 [topology-2]
 file = topology.elf
 extra_params = -cpu max,ctop=on -smp sockets=31,cores=8,maxcpus=248  -append '-sockets 31 -cores 8'
+
+[topology-3]
+file = topology.elf
+extra_params = """-cpu max,ctop=on -smp cpus=1,drawers=2,books=2,sockets=2,cores=16,maxcpus=128 \
+-append '-drawers 2 -books 2 -sockets 2 -cores 16' \
+-device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=11,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=95,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=73,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=78,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=13,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=40,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=101,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=29,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=56,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=92,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=30,drawer-id=0,book-id=0,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=118,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=71,drawer-id=0,book-id=0,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=93,drawer-id=0,book-id=0,socket-id=0,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=16,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=5,drawer-id=0,book-id=0,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=42,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=98,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=44,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=23,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=65,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=102,drawer-id=0,book-id=0,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=57,drawer-id=0,book-id=0,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=125,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=127,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=82,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=14,drawer-id=0,book-id=0,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=91,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=12,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=8,drawer-id=0,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=112,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=109,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=19,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=96,drawer-id=0,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=67,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=80,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=108,drawer-id=0,book-id=1,socket-id=0,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=34,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=18,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=39,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=53,drawer-id=0,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=46,drawer-id=0,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=3,drawer-id=0,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=76,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=15,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=7,drawer-id=0,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=81,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=1,drawer-id=0,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=113,drawer-id=0,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=38,drawer-id=0,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=90,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=117,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=62,drawer-id=0,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=85,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=49,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=24,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=107,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=103,drawer-id=0,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=33,drawer-id=0,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=51,drawer-id=0,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=21,drawer-id=0,book-id=1,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=72,drawer-id=0,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=63,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=105,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=74,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=50,drawer-id=1,book-id=0,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=60,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=22,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=43,drawer-id=1,book-id=0,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=48,drawer-id=1,book-id=0,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=35,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=58,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=106,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=123,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=122,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=9,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=10,drawer-id=1,book-id=0,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=25,drawer-id=1,book-id=0,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=116,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=26,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=17,drawer-id=1,book-id=0,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=20,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=59,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=54,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=70,drawer-id=1,book-id=0,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=88,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=6,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=52,drawer-id=1,book-id=0,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=55,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=124,drawer-id=1,book-id=0,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=61,drawer-id=1,book-id=0,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=84,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=68,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=86,drawer-id=1,book-id=0,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=4,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=75,drawer-id=1,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=115,drawer-id=1,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=28,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=120,drawer-id=1,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=41,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=87,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=119,drawer-id=1,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=114,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=104,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=27,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=121,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=126,drawer-id=1,book-id=1,socket-id=0,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=37,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=32,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=94,drawer-id=1,book-id=1,socket-id=0,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=110,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=77,drawer-id=1,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=36,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=66,drawer-id=1,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=83,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=47,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=99,drawer-id=1,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=79,drawer-id=1,book-id=1,socket-id=1,entitlement=low,dedicated=false \
+-device max-s390x-cpu,core-id=100,drawer-id=1,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+-device max-s390x-cpu,core-id=89,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=false \
+-device max-s390x-cpu,core-id=2,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=45,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=69,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=64,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=97,drawer-id=1,book-id=1,socket-id=1,entitlement=high,dedicated=true \
+-device max-s390x-cpu,core-id=111,drawer-id=1,book-id=1,socket-id=1,entitlement=medium,dedicated=false \
+"""
-- 
2.41.0

Re: [kvm-unit-tests PATCH 01/10] s390x: topology: Introduce enums for polarization & cpu type

[Nico Boehr]

Quoting Nina Schoetterl-Glausch (2023-10-20 16:48:51)
> Thereby get rid of magic values.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH 01/10] s390x: topology: Introduce enums for polarization & cpu type

[Janosch Frank]

On 10/20/23 16:48, Nina Schoetterl-Glausch wrote:
> Thereby get rid of magic values.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [kvm-unit-tests PATCH 02/10] s390x: topology: Fix report message

[Janosch Frank]

On 10/20/23 16:48, Nina Schoetterl-Glausch wrote:
> 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>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [kvm-unit-tests PATCH 04/10] s390x: topology: Fix parsing loop

[Janosch Frank]

On 10/20/23 16:48, Nina Schoetterl-Glausch wrote:
> 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>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

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

[Nico Boehr]

Quoting Nina Schoetterl-Glausch (2023-10-20 16:48:50)
> 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.

Thanks, I've pushed this to our CI for coverage.

Re: [kvm-unit-tests PATCH 06/10] s390x: topology: Refine stsi header test

[Janosch Frank]

On 10/20/23 16:48, Nina Schoetterl-Glausch wrote:
> 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>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [kvm-unit-tests PATCH 09/10] scripts: Implement multiline strings for extra_params

[Thomas Huth]

On 20/10/2023 16.48, Nina Schoetterl-Glausch wrote:
> Implement a rudimentary form only.
> extra_params can get long when passing a lot of arguments to qemu.
> Multiline strings help with readability of the .cfg file.
> Multiline strings begin and end with """, which must occur on separate
> lines.
> 
> For example:
> extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
> -append '-drawers 2 -books 2 -sockets 2 -cores 16' \
> -device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""
> 
> The command string built with extra_params is eval'ed by the runtime
> script, so the newlines need to be escaped with \.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>   scripts/common.bash  | 16 ++++++++++++++++
>   scripts/runtime.bash |  4 ++--
>   2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/common.bash b/scripts/common.bash
> index 7b983f7d..b9413d68 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -36,6 +36,22 @@ function for_each_unittest()
>   			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>   		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
>   			smp=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> +			opts=${BASH_REMATCH[1]}$'\n'
> +			while read -r -u $fd; do
> +				#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
> +				else
> +					opts+=$REPLY$'\n'
> +				fi
> +			done

Phew, TIL that there is something like $'\n' in bash ...
Now with that knowledge, the regular expression make sense 8-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 09/10] scripts: Implement multiline strings for extra_params

[Nina Schoetterl-Glausch]

On Wed, 2023-10-25 at 19:50 +0200, Thomas Huth wrote:
> On 20/10/2023 16.48, Nina Schoetterl-Glausch wrote:
> > Implement a rudimentary form only.
> > extra_params can get long when passing a lot of arguments to qemu.
> > Multiline strings help with readability of the .cfg file.
> > Multiline strings begin and end with """, which must occur on separate
> > lines.
> > 
> > For example:
> > extra_params = """-cpu max,ctop=on -smp cpus=1,cores=16,maxcpus=128 \
> > -append '-drawers 2 -books 2 -sockets 2 -cores 16' \
> > -device max-s390x-cpu,core-id=31,drawer-id=0,book-id=0,socket-id=0"""
> > 
> > The command string built with extra_params is eval'ed by the runtime
> > script, so the newlines need to be escaped with \.
> > 
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> >   scripts/common.bash  | 16 ++++++++++++++++
> >   scripts/runtime.bash |  4 ++--
> >   2 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/common.bash b/scripts/common.bash
> > index 7b983f7d..b9413d68 100644
> > --- a/scripts/common.bash
> > +++ b/scripts/common.bash
> > @@ -36,6 +36,22 @@ function for_each_unittest()
> >   			kernel=$TEST_DIR/${BASH_REMATCH[1]}
> >   		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> >   			smp=${BASH_REMATCH[1]}
> > +		elif [[ $line =~ ^extra_params\ *=\ *'"""'(.*)$ ]]; then
> > +			opts=${BASH_REMATCH[1]}$'\n'
> > +			while read -r -u $fd; do
> > +				#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
> > +				else
> > +					opts+=$REPLY$'\n'
> > +				fi
> > +			done
> 
> Phew, TIL that there is something like $'\n' in bash ...
> Now with that knowledge, the regular expression make sense 8-)

Uh yeah, it's very write only, with backslash escaping, " inside ' and $'\n'.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
Thanks!

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

[Nico Boehr]

Quoting Nina Schoetterl-Glausch (2023-10-27 18:36:12)
> On Wed, 2023-10-25 at 14:21 +0200, Nico Boehr wrote:
> > Quoting Nina Schoetterl-Glausch (2023-10-20 16:48:50)
> > > 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.
> > 
> > Thanks, I've pushed this to our CI for coverage.
> 
> And it found some problems.
> Want me to resend the series or just fixup patches?

I think it would be best if you resend the whole series.