[kvm-unit-tests PATCH v2 0/4] S390x: CPU Topology Information

[kvm-unit-tests PATCH v2 0/4] S390x: CPU Topology Information

[Pierre Morel]

Hi,

second version of the series with corrections.

When facility 11 is available inside the S390x architecture, 2 new
instructions are available: PTF and STSI with function code 15.

Let's check their availability in QEMU/KVM and their coherence
with the CPU topology provided by the QEMU -smp parameter.

To run these tests successfully you will need the Linux and the QEMU
patches:
    https://lkml.org/lkml/2021/8/3/201

    https://lists.nongnu.org/archive/html/qemu-s390x/2021-07/msg00165.html

Regards,
Pierre

Pierre Morel (4):
  s390x: lib: Add SCLP toplogy nested level
  s390x: lib: Simplify stsi_get_fc and move it to library
  s390x: topology: Check the Perform Topology Function
  s390x: topology: Checking Configuration Topology Information

 lib/s390x/asm/arch_def.h |  16 ++
 lib/s390x/sclp.c         |   6 +
 lib/s390x/sclp.h         |   4 +-
 s390x/Makefile           |   1 +
 s390x/stsi.c             |  20 +--
 s390x/topology.c         | 307 +++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg      |   4 +
 7 files changed, 339 insertions(+), 19 deletions(-)
 create mode 100644 s390x/topology.c

-- 
2.25.1

Changelog:

From V1:

- Simplify the stsi_get_fc function when pushing it into lib
  (Janosch)

- Simplify PTF inline assembly as PTF instruction does not use RRE
  second argument
  (Claudio)

- Rename Test global name
  (Claudio, Janosch)

- readibility, naming for PTF_REQ_* and removed unused globals
  (Janosch)

- skipping tests which could fail when run on LPAR
  (Janosh)

- Missing prefix_pop
  (Janosch)

[kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Pierre Morel]

The maximum CPU Topology nested level is available with the SCLP
READ_INFO command inside the byte at offset 15 of the ReadInfo
structure.

Let's return this information to check the number of topology nested
information available with the STSI 15.1.x instruction.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/sclp.c | 6 ++++++
 lib/s390x/sclp.h | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index 9502d161..ee379ddf 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -123,6 +123,12 @@ int sclp_get_cpu_num(void)
 	return read_info->entries_cpu;
 }
 
+int sclp_get_stsi_parm(void)
+{
+	assert(read_info);
+	return read_info->stsi_parm;
+}
+
 CPUEntry *sclp_get_cpu_entries(void)
 {
 	assert(read_info);
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 28e526e2..1a365958 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -146,7 +146,8 @@ typedef struct ReadInfo {
 	SCCBHeader h;
 	uint16_t rnmax;
 	uint8_t rnsize;
-	uint8_t  _reserved1[16 - 11];       /* 11-15 */
+	uint8_t  _reserved1[15 - 11];       /* 11-14 */
+	uint8_t stsi_parm;
 	uint16_t entries_cpu;               /* 16-17 */
 	uint16_t offset_cpu;                /* 18-19 */
 	uint8_t  _reserved2[24 - 20];       /* 20-23 */
@@ -322,6 +323,7 @@ void sclp_console_setup(void);
 void sclp_print(const char *str);
 void sclp_read_info(void);
 int sclp_get_cpu_num(void);
+int sclp_get_stsi_parm(void);
 CPUEntry *sclp_get_cpu_entries(void);
 void sclp_facilities_setup(void);
 int sclp_service_call(unsigned int command, void *sccb);
-- 
2.25.1

[kvm-unit-tests PATCH v2 2/4] s390x: lib: Simplify stsi_get_fc and move it to library

[Pierre Morel]

stsi_get_fc is now needed in multiple tests.

As it does not need to store information but only returns
the machine level, suppress the address parameter.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
 s390x/stsi.c             | 20 ++------------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 15cf7d48..2f70d840 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -328,6 +328,22 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
 	return cc;
 }
 
+static inline unsigned long stsi_get_fc(void)
+{
+	register unsigned long r0 asm("0") = 0;
+	register unsigned long r1 asm("1") = 0;
+	int cc;
+
+	asm volatile("stsi	0\n"
+		     "ipm	%[cc]\n"
+		     "srl	%[cc],28\n"
+		     : "+d" (r0), [cc] "=d" (cc)
+		     : "d" (r1)
+		     : "cc", "memory");
+	assert(!cc);
+	return r0 >> 28;
+}
+
 static inline int servc(uint32_t command, unsigned long sccb)
 {
 	int cc;
diff --git a/s390x/stsi.c b/s390x/stsi.c
index 87d48047..391f8849 100644
--- a/s390x/stsi.c
+++ b/s390x/stsi.c
@@ -71,28 +71,12 @@ static void test_priv(void)
 	report_prefix_pop();
 }
 
-static inline unsigned long stsi_get_fc(void *addr)
-{
-	register unsigned long r0 asm("0") = 0;
-	register unsigned long r1 asm("1") = 0;
-	int cc;
-
-	asm volatile("stsi	0(%[addr])\n"
-		     "ipm	%[cc]\n"
-		     "srl	%[cc],28\n"
-		     : "+d" (r0), [cc] "=d" (cc)
-		     : "d" (r1), [addr] "a" (addr)
-		     : "cc", "memory");
-	assert(!cc);
-	return r0 >> 28;
-}
-
 static void test_fc(void)
 {
 	report(stsi(pagebuf, 7, 0, 0) == 3, "invalid fc");
 	report(stsi(pagebuf, 1, 0, 1) == 3, "invalid selector 1");
 	report(stsi(pagebuf, 1, 1, 0) == 3, "invalid selector 2");
-	report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2");
+	report(stsi_get_fc() >= 2, "query fc >= 2");
 }
 
 static void test_3_2_2(void)
@@ -112,7 +96,7 @@ static void test_3_2_2(void)
 	report_prefix_push("3.2.2");
 
 	/* Is the function code available at all? */
-	if (stsi_get_fc(pagebuf) < 3) {
+	if (stsi_get_fc() < 3) {
 		report_skip("Running under lpar, no level 3 to test.");
 		goto out;
 	}
-- 
2.25.1

[kvm-unit-tests PATCH v2 3/4] s390x: topology: Check the Perform Topology Function

[Pierre Morel]

We check the PTF instruction.

- We do not expect to support vertical polarization.

- We do not expect the Modified Topology Change Report to be
pending or not at the moment the first PTF instruction with
PTF_CHECK function code is done as some code already did run
a polarization change may have occur.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/Makefile      |  1 +
 s390x/topology.c    | 99 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  3 ++
 3 files changed, 103 insertions(+)
 create mode 100644 s390x/topology.c

diff --git a/s390x/Makefile b/s390x/Makefile
index 6565561b..c82b7dbf 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
 tests += $(TEST_DIR)/uv-host.elf
 tests += $(TEST_DIR)/edat.elf
 tests += $(TEST_DIR)/mvpg-sie.elf
+tests += $(TEST_DIR)/topology.elf
 
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 ifneq ($(HOST_KEY_DOCUMENT),)
diff --git a/s390x/topology.c b/s390x/topology.c
new file mode 100644
index 00000000..a0dc3b9e
--- /dev/null
+++ b/s390x/topology.c
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * CPU Topology
+ *
+ * Copyright (c) 2021 IBM Corp
+ *
+ * Authors:
+ *  Pierre Morel <pmorel@linux.ibm.com>
+ */
+
+#include <libcflat.h>
+#include <asm/page.h>
+#include <asm/asm-offsets.h>
+#include <asm/interrupt.h>
+#include <asm/facility.h>
+#include <smp.h>
+#include <sclp.h>
+
+static int machine_level;
+
+#define PTF_REQ_HORIZONTAL	0
+#define PTF_REQ_VERTICAL	1
+#define PTF_REQ_CHECK		2
+
+#define PTF_ERR_NO_REASON	0
+#define PTF_ERR_ALRDY_POLARIZED	1
+#define PTF_ERR_IN_PROGRESS	2
+
+static int ptf(unsigned long fc, unsigned long *rc)
+{
+	int cc;
+
+	asm volatile(
+		"       .insn   rre,0xb9a20000,%1,0\n"
+		"       ipm     %0\n"
+		"       srl     %0,28\n"
+		: "=d" (cc), "+d" (fc)
+		: "d" (fc)
+		: "cc");
+
+	*rc = fc >> 8;
+	return cc;
+}
+
+static void test_ptf(void)
+{
+	unsigned long rc;
+	int cc;
+
+	report_prefix_push("Topology Report pending");
+	/*
+	 * At this moment the topology may already have changed
+	 * since the VM has been started.
+	 * However, we can test if a second PTF instruction
+	 * reports that the topology did not change since the
+	 * preceding PFT instruction.
+	 */
+	ptf(PTF_REQ_CHECK, &rc);
+	cc = ptf(PTF_REQ_CHECK, &rc);
+	report(cc == 0, "PTF check clear");
+
+	/*
+	 * In the LPAR we can not assume the state of the polarizatiom
+	 * at this moment.
+	 * Let's skip the tests for LPAR.
+	 */
+	if (machine_level < 3)
+		goto end;
+
+	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
+	       "PTF horizontal already configured");
+
+	cc = ptf(PTF_REQ_VERTICAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_NO_REASON,
+	       "PTF vertical non possible");
+
+end:
+	report_prefix_pop();
+}
+
+int main(int argc, char *argv[])
+{
+	report_prefix_push("CPU Topology");
+
+	if (!test_facility(11)) {
+		report_skip("Topology facility not present");
+		goto end;
+	}
+
+	machine_level = stsi_get_fc();
+	report_info("Machine level %d", machine_level);
+
+	test_ptf();
+
+end:
+	report_prefix_pop();
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 9e1802fd..0f84d279 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -109,3 +109,6 @@ file = edat.elf
 
 [mvpg-sie]
 file = mvpg-sie.elf
+
+[topology]
+file = topology.elf
-- 
2.25.1

[kvm-unit-tests PATCH v2 4/4] s390x: topology: Checking Configuration Topology Information

[Pierre Morel]

STSI with function code 15 is used to store the CPU configuration
topology.

We check if the topology stored is coherent with the QEMU -smp
parameters.
The current check is done on the number of CPUs, the maximum number
of CPUs, the number of sockets and the number of cores per sockets.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 s390x/topology.c    | 208 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   1 +
 2 files changed, 209 insertions(+)

diff --git a/s390x/topology.c b/s390x/topology.c
index a0dc3b9e..be6f5abc 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -17,6 +17,53 @@
 #include <sclp.h>
 
 static int machine_level;
+static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+static int mnest;
+static long max_cpus;
+static long cores;
+static long sockets;
+static long books;
+static long drawers;
+static long nodes;
+static long ncpus;
+
+struct topology_core {
+	unsigned char nl;
+	unsigned char reserved0[3];
+	unsigned char :5;
+	unsigned char d:1;
+	unsigned char pp:2;
+	unsigned char type;
+	unsigned short origin;
+	unsigned long mask;
+};
+
+struct topology_container {
+	unsigned char nl;
+	unsigned char reserved[6];
+	unsigned char id;
+};
+
+union topology_entry {
+	unsigned char nl;
+	struct topology_core cpu;
+	struct topology_container container;
+};
+
+struct sysinfo_15_1_x {
+	unsigned char reserved0[2];
+	unsigned short length;
+	unsigned char mag6;
+	unsigned char mag5;
+	unsigned char mag4;
+	unsigned char mag3;
+	unsigned char mag2;
+	unsigned char mag1;
+	unsigned char reserved1;
+	unsigned char mnest;
+	unsigned char reserved2[4];
+	union topology_entry tle[0];
+};
 
 #define PTF_REQ_HORIZONTAL	0
 #define PTF_REQ_VERTICAL	1
@@ -79,10 +126,170 @@ end:
 	report_prefix_pop();
 }
 
+static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info)
+{
+	struct topology_container *tc, *end;
+	struct topology_core *cpus;
+	int nb_nl0 = 0, nb_nl1 = 0, nb_nl2 = 0, nb_nl3 = 0;
+
+	if (mnest > 5)
+		report(info->mag6 == 0, "topology level 6");
+	if (mnest > 4)
+		report(info->mag5 == nodes, "Maximum number of nodes");
+	if (mnest > 3)
+		report(info->mag4 == drawers, "Maximum number of drawers");
+	if (mnest > 2)
+		report(info->mag3 == books, "Maximum number of books");
+
+	/* Both levels 2 and 1 are always valid */
+	report(info->mag2 == sockets, "Maximum number of sockets");
+	report(info->mag1 == cores, "Maximum number of cores");
+
+	tc = (void *)&info->tle[0];
+	end = (struct topology_container *)((unsigned long)info + info->length);
+
+	while (tc < end) {
+		switch (tc->nl) {
+		case 3:
+			report_info("drawer: %d %d", tc->nl, tc->id);
+			nb_nl3++;
+			break;
+		case 2:
+			report_info("book  : %d %d", tc->nl, tc->id);
+			nb_nl2++;
+			break;
+		case 1:
+			report_info("socket: %d %d", tc->nl, tc->id);
+			nb_nl1++;
+			break;
+		case 0:
+			cpus = (struct topology_core *) tc;
+			report_info("cpu type %02x  d: %d pp: %d", cpus->type, cpus->d, cpus->pp);
+			report_info("origin : %04x mask %016lx", cpus->origin, cpus->mask);
+			tc++;
+			nb_nl0++;
+			break;
+		default:
+			report_abort("Unexpected TL Entry: tle->nl: %d", tc->nl);
+			return;
+		}
+		tc++;
+	}
+	/*
+	 * As we accept only 1 type of CPU, and only horizontal and dedicated CPUs
+	 * We expect max_cpus / cores CPU entries
+	 */
+	report(nb_nl0 ==  (1 + (ncpus - 1) / cores),
+			  "Check count of cores: %d %ld", nb_nl0, ncpus / cores);
+	/* We expect the same count of sockets and CPU entries */
+	report(nb_nl1 ==  nb_nl0, "Check count of sockets");
+	if (mnest > 2)
+		report(nb_nl2 == nb_nl1 / sockets, "Checks count of books");
+	if (mnest > 3)
+		report(nb_nl3 == nb_nl2 / books, "Checks count of drawers");
+}
+
+static void test_stsi(void)
+{
+	int ret;
+
+	mnest = sclp_get_stsi_parm();
+	/* If the STSI parm is 0, the maximum MNEST for STSI is 2 */
+	if (!mnest)
+		mnest = 2;
+	report_info("SCLP MNEST : %d", mnest);
+
+	ret = sclp_get_cpu_num();
+	report_info("SCLP nb CPU: %d", ret);
+
+	ret = stsi(pagebuf, 15, 1, 2);
+	report(!ret, "valid stsi 15.1.2");
+	if (!ret)
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf);
+	else
+		report_info(" ret: %d", ret);
+
+	if (mnest < 3) {
+		report(stsi(pagebuf, 15, 1, 3) == 3, "invalid stsi 15.1.3");
+	} else {
+		report(stsi(pagebuf, 15, 1, 3) == 0, "valid stsi 15.1.3");
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf);
+	}
+
+	if (mnest < 4) {
+		report(stsi(pagebuf, 15, 1, 4) == 3, "invalid stsi 15.1.4");
+	} else {
+		report(stsi(pagebuf, 15, 1, 4) == 0, "valid stsi 15.1.4");
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf);
+	}
+
+	if (mnest < 5) {
+		report(stsi(pagebuf, 15, 1, 5) == 3, "invalid stsi 15.1.5");
+	} else {
+		report(stsi(pagebuf, 15, 1, 5) == 0, "valid stsi 15.1.5");
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf);
+	}
+
+	if (mnest < 6) {
+		report(stsi(pagebuf, 15, 1, 6) == 3, "invalid stsi 15.1.6");
+	} else {
+		report(stsi(pagebuf, 15, 1, 6) == 0, "valid stsi 15.1.6");
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf);
+	}
+}
+
+static void parse_topology_args(int argc, char **argv)
+{
+	int i;
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp("-c", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-c (cores) needs a parameter");
+			cores = atol(argv[i]);
+		} else if (!strcmp("-s", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-s (sockets) needs a parameter");
+			sockets = atol(argv[i]);
+		} else if (!strcmp("-b", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-b (books) needs a parameter");
+			books = atol(argv[i]);
+		} else if (!strcmp("-d", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-d (drawers) needs a parameter");
+			drawers = atol(argv[i]);
+		} else if (!strcmp("-n", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-n (nodes) needs a parameter");
+			nodes = atol(argv[i]);
+		}
+	}
+	if (!cores)
+		cores = 1;
+	if (!sockets)
+		sockets = 1;
+	if (!books)
+		books = 1;
+	if (!drawers)
+		drawers = 1;
+	if (!nodes)
+		nodes = 1;
+	max_cpus = cores * sockets * books * drawers * nodes;
+	ncpus = smp_query_num_cpus();
+}
+
 int main(int argc, char *argv[])
 {
 	report_prefix_push("CPU Topology");
 
+	parse_topology_args(argc, argv);
+
 	if (!test_facility(11)) {
 		report_skip("Topology facility not present");
 		goto end;
@@ -92,6 +299,7 @@ int main(int argc, char *argv[])
 	report_info("Machine level %d", machine_level);
 
 	test_ptf();
+	test_stsi();
 
 end:
 	report_prefix_pop();
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 0f84d279..390e8398 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -112,3 +112,4 @@ file = mvpg-sie.elf
 
 [topology]
 file = topology.elf
+extra_params=-smp 5,sockets=4,cores=4,maxcpus=16 -append "-n 5 -s 4 -c 4 -m 16"
-- 
2.25.1

Re: [kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Janosch Frank]

On 8/10/21 6:22 PM, Pierre Morel wrote:
> The maximum CPU Topology nested level is available with the SCLP
> READ_INFO command inside the byte at offset 15 of the ReadInfo
> structure.
> 
> Let's return this information to check the number of topology nested
> information available with the STSI 15.1.x instruction.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

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

> ---
>  lib/s390x/sclp.c | 6 ++++++
>  lib/s390x/sclp.h | 4 +++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index 9502d161..ee379ddf 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -123,6 +123,12 @@ int sclp_get_cpu_num(void)
>  	return read_info->entries_cpu;
>  }
>  
> +int sclp_get_stsi_parm(void)
> +{
> +	assert(read_info);
> +	return read_info->stsi_parm;
> +}
> +
>  CPUEntry *sclp_get_cpu_entries(void)
>  {
>  	assert(read_info);
> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> index 28e526e2..1a365958 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -146,7 +146,8 @@ typedef struct ReadInfo {
>  	SCCBHeader h;
>  	uint16_t rnmax;
>  	uint8_t rnsize;
> -	uint8_t  _reserved1[16 - 11];       /* 11-15 */
> +	uint8_t  _reserved1[15 - 11];       /* 11-14 */
> +	uint8_t stsi_parm;
>  	uint16_t entries_cpu;               /* 16-17 */
>  	uint16_t offset_cpu;                /* 18-19 */
>  	uint8_t  _reserved2[24 - 20];       /* 20-23 */
> @@ -322,6 +323,7 @@ void sclp_console_setup(void);
>  void sclp_print(const char *str);
>  void sclp_read_info(void);
>  int sclp_get_cpu_num(void);
> +int sclp_get_stsi_parm(void);
>  CPUEntry *sclp_get_cpu_entries(void);
>  void sclp_facilities_setup(void);
>  int sclp_service_call(unsigned int command, void *sccb);
> 

Re: [kvm-unit-tests PATCH v2 2/4] s390x: lib: Simplify stsi_get_fc and move it to library

[Janosch Frank]

On 8/10/21 6:22 PM, Pierre Morel wrote:
> stsi_get_fc is now needed in multiple tests.
> 
> As it does not need to store information but only returns
> the machine level, suppress the address parameter.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Please push this one to devel for coverage:
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
>  s390x/stsi.c             | 20 ++------------------
>  2 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15cf7d48..2f70d840 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -328,6 +328,22 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
>  	return cc;
>  }
>  
> +static inline unsigned long stsi_get_fc(void)
> +{
> +	register unsigned long r0 asm("0") = 0;
> +	register unsigned long r1 asm("1") = 0;
> +	int cc;
> +
> +	asm volatile("stsi	0\n"
> +		     "ipm	%[cc]\n"
> +		     "srl	%[cc],28\n"
> +		     : "+d" (r0), [cc] "=d" (cc)
> +		     : "d" (r1)
> +		     : "cc", "memory");
> +	assert(!cc);
> +	return r0 >> 28;
> +}
> +
>  static inline int servc(uint32_t command, unsigned long sccb)
>  {
>  	int cc;
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 87d48047..391f8849 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -71,28 +71,12 @@ static void test_priv(void)
>  	report_prefix_pop();
>  }
>  
> -static inline unsigned long stsi_get_fc(void *addr)
> -{
> -	register unsigned long r0 asm("0") = 0;
> -	register unsigned long r1 asm("1") = 0;
> -	int cc;
> -
> -	asm volatile("stsi	0(%[addr])\n"
> -		     "ipm	%[cc]\n"
> -		     "srl	%[cc],28\n"
> -		     : "+d" (r0), [cc] "=d" (cc)
> -		     : "d" (r1), [addr] "a" (addr)
> -		     : "cc", "memory");
> -	assert(!cc);
> -	return r0 >> 28;
> -}
> -
>  static void test_fc(void)
>  {
>  	report(stsi(pagebuf, 7, 0, 0) == 3, "invalid fc");
>  	report(stsi(pagebuf, 1, 0, 1) == 3, "invalid selector 1");
>  	report(stsi(pagebuf, 1, 1, 0) == 3, "invalid selector 2");
> -	report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2");
> +	report(stsi_get_fc() >= 2, "query fc >= 2");
>  }
>  
>  static void test_3_2_2(void)
> @@ -112,7 +96,7 @@ static void test_3_2_2(void)
>  	report_prefix_push("3.2.2");
>  
>  	/* Is the function code available at all? */
> -	if (stsi_get_fc(pagebuf) < 3) {
> +	if (stsi_get_fc() < 3) {
>  		report_skip("Running under lpar, no level 3 to test.");
>  		goto out;
>  	}
> 

Re: [kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Pierre Morel]

On 8/11/21 4:59 PM, Janosch Frank wrote:
> On 8/10/21 6:22 PM, Pierre Morel wrote:
>> The maximum CPU Topology nested level is available with the SCLP
>> READ_INFO command inside the byte at offset 15 of the ReadInfo
>> structure.
>>
>> Let's return this information to check the number of topology nested
>> information available with the STSI 15.1.x instruction.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v2 2/4] s390x: lib: Simplify stsi_get_fc and move it to library

[Pierre Morel]

On 8/11/21 5:01 PM, Janosch Frank wrote:
> On 8/10/21 6:22 PM, Pierre Morel wrote:
>> stsi_get_fc is now needed in multiple tests.
>>
>> As it does not need to store information but only returns
>> the machine level, suppress the address parameter.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 
> Please push this one to devel for coverage:
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 

Thanks and done,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v2 3/4] s390x: topology: Check the Perform Topology Function

[Janosch Frank]

On 8/10/21 6:22 PM, Pierre Morel wrote:
> We check the PTF instruction.
> 
> - We do not expect to support vertical polarization.

KVM does not support vertical polarization and we don't expect it to be
added in the future?

> 
> - We do not expect the Modified Topology Change Report to be
> pending or not at the moment the first PTF instruction with
> PTF_CHECK function code is done as some code already did run
> a polarization change may have occur.

ENOPARSE

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  s390x/Makefile      |  1 +
>  s390x/topology.c    | 99 +++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |  3 ++
>  3 files changed, 103 insertions(+)
>  create mode 100644 s390x/topology.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 6565561b..c82b7dbf 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>  tests += $(TEST_DIR)/uv-host.elf
>  tests += $(TEST_DIR)/edat.elf
>  tests += $(TEST_DIR)/mvpg-sie.elf
> +tests += $(TEST_DIR)/topology.elf
>  
>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>  ifneq ($(HOST_KEY_DOCUMENT),)
> diff --git a/s390x/topology.c b/s390x/topology.c
> new file mode 100644
> index 00000000..a0dc3b9e
> --- /dev/null
> +++ b/s390x/topology.c
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * CPU Topology
> + *
> + * Copyright (c) 2021 IBM Corp
> + *
> + * Authors:
> + *  Pierre Morel <pmorel@linux.ibm.com>
> + */
> +
> +#include <libcflat.h>
> +#include <asm/page.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/interrupt.h>
> +#include <asm/facility.h>
> +#include <smp.h>
> +#include <sclp.h>
> +
> +static int machine_level;
> +
> +#define PTF_REQ_HORIZONTAL	0
> +#define PTF_REQ_VERTICAL	1
> +#define PTF_REQ_CHECK		2
> +
> +#define PTF_ERR_NO_REASON	0
> +#define PTF_ERR_ALRDY_POLARIZED	1
> +#define PTF_ERR_IN_PROGRESS	2
> +
> +static int ptf(unsigned long fc, unsigned long *rc)
> +{
> +	int cc;
> +
> +	asm volatile(
> +		"       .insn   rre,0xb9a20000,%1,0\n"
> +		"       ipm     %0\n"
> +		"       srl     %0,28\n"
> +		: "=d" (cc), "+d" (fc)
> +		: "d" (fc)
> +		: "cc");
> +
> +	*rc = fc >> 8;
> +	return cc;
> +}
> +
> +static void test_ptf(void)
> +{
> +	unsigned long rc;
> +	int cc;
> +
> +	report_prefix_push("Topology Report pending");
> +	/*
> +	 * At this moment the topology may already have changed
> +	 * since the VM has been started.
> +	 * However, we can test if a second PTF instruction
> +	 * reports that the topology did not change since the
> +	 * preceding PFT instruction.
> +	 */
> +	ptf(PTF_REQ_CHECK, &rc);
> +	cc = ptf(PTF_REQ_CHECK, &rc);
> +	report(cc == 0, "PTF check clear");
> +
> +	/*
> +	 * In the LPAR we can not assume the state of the polarizatiom

polarization

> +	 * at this moment.
> +	 * Let's skip the tests for LPAR.
> +	 */

Any idea what happens on z/VM?
We don't necessarily need to support z/VM but we at least need to skip
like we do on lpar :-)

Maybe also add a TODO, so we know we could improve the test?

> +	if (machine_level < 3)
> +		goto end;
> +

Add comments:
We're always horizontally polarized in KVM.

> +	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
> +	       "PTF horizontal already configured");
> +

KVM doesn't support vertical polarization.

> +	cc = ptf(PTF_REQ_VERTICAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
> +	       "PTF vertical non possible");

s/non/not/

> +
> +end:
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	report_prefix_push("CPU Topology");
> +
> +	if (!test_facility(11)) {
> +		report_skip("Topology facility not present");
> +		goto end;
> +	}
> +
> +	machine_level = stsi_get_fc();
> +	report_info("Machine level %d", machine_level);
> +
> +	test_ptf();
> +
> +end:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 9e1802fd..0f84d279 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -109,3 +109,6 @@ file = edat.elf
>  
>  [mvpg-sie]
>  file = mvpg-sie.elf
> +
> +[topology]
> +file = topology.elf
> 

Re: [kvm-unit-tests PATCH v2 3/4] s390x: topology: Check the Perform Topology Function

[Pierre Morel]

On 8/12/21 11:38 AM, Janosch Frank wrote:
> On 8/10/21 6:22 PM, Pierre Morel wrote:
>> We check the PTF instruction.
>>
>> - We do not expect to support vertical polarization.
> 
> KVM does not support vertical polarization and we don't expect it to be
> added in the future?

OK

> 
>>
>> - We do not expect the Modified Topology Change Report to be
>> pending or not at the moment the first PTF instruction with
>> PTF_CHECK function code is done as some code already did run
>> a polarization change may have occur.
> 
> ENOPARSE

OK I find another way to explain:

"
The Topology changes if the topology of the real CPUs backing the vCPUs 
changes.
This can happen between the initialization of the VM and the start of 
the guest.
As a consequence we can not expect the result of the first PTF instruction.
"
...
>> +	/*
>> +	 * In the LPAR we can not assume the state of the polarizatiom
> 
> polarization

yes

> 
>> +	 * at this moment.
>> +	 * Let's skip the tests for LPAR.
>> +	 */
> 
> Any idea what happens on z/VM?
> We don't necessarily need to support z/VM but we at least need to skip
> like we do on lpar :-)

No, I do not know.
Then OK we skip the test for zVM too

> 
> Maybe also add a TODO, so we know we could improve the test?
> 
>> +	if (machine_level < 3)
>> +		goto end;
>> +
> 
> Add comments:
> We're always horizontally polarized in KVM.

OK

> 
>> +	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
>> +	       "PTF horizontal already configured");
>> +
> 
> KVM doesn't support vertical polarization.

OK too

> 
>> +	cc = ptf(PTF_REQ_VERTICAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
>> +	       "PTF vertical non possible");
> 
> s/non/not/

yes, seems I forgot to change this.

...


Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Cornelia Huck]

On Tue, Aug 10 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:

> The maximum CPU Topology nested level is available with the SCLP
> READ_INFO command inside the byte at offset 15 of the ReadInfo
> structure.
>
> Let's return this information to check the number of topology nested
> information available with the STSI 15.1.x instruction.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  lib/s390x/sclp.c | 6 ++++++
>  lib/s390x/sclp.h | 4 +++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index 9502d161..ee379ddf 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -123,6 +123,12 @@ int sclp_get_cpu_num(void)
>  	return read_info->entries_cpu;
>  }
>  
> +int sclp_get_stsi_parm(void)
> +{
> +	assert(read_info);
> +	return read_info->stsi_parm;

Is this a generic "stsi parm", or always the concrete topology nested
level? IOW, is that name good, or too generic?

> +}
> +
>  CPUEntry *sclp_get_cpu_entries(void)
>  {
>  	assert(read_info);

Re: [kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Pierre Morel]

On 8/12/21 2:56 PM, Cornelia Huck wrote:
> On Tue, Aug 10 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> The maximum CPU Topology nested level is available with the SCLP
>> READ_INFO command inside the byte at offset 15 of the ReadInfo
>> structure.
>>
>> Let's return this information to check the number of topology nested
>> information available with the STSI 15.1.x instruction.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>   lib/s390x/sclp.c | 6 ++++++
>>   lib/s390x/sclp.h | 4 +++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>> index 9502d161..ee379ddf 100644
>> --- a/lib/s390x/sclp.c
>> +++ b/lib/s390x/sclp.c
>> @@ -123,6 +123,12 @@ int sclp_get_cpu_num(void)
>>   	return read_info->entries_cpu;
>>   }
>>   
>> +int sclp_get_stsi_parm(void)
>> +{
>> +	assert(read_info);
>> +	return read_info->stsi_parm;
> 
> Is this a generic "stsi parm", or always the concrete topology nested
> level? IOW, is that name good, or too generic?

It is the name used in the documentation, but for now only the 3 bits 
5-7 are used for the maximum value of the selector 2 of the STSI 
instruction allowed by the machine.


> 
>> +}
>> +
>>   CPUEntry *sclp_get_cpu_entries(void)
>>   {
>>   	assert(read_info);
> 

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v2 1/4] s390x: lib: Add SCLP toplogy nested level

[Cornelia Huck]

On Thu, Aug 12 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:

> On 8/12/21 2:56 PM, Cornelia Huck wrote:
>> On Tue, Aug 10 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:
>> 
>>> The maximum CPU Topology nested level is available with the SCLP
>>> READ_INFO command inside the byte at offset 15 of the ReadInfo
>>> structure.
>>>
>>> Let's return this information to check the number of topology nested
>>> information available with the STSI 15.1.x instruction.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>>> ---
>>>   lib/s390x/sclp.c | 6 ++++++
>>>   lib/s390x/sclp.h | 4 +++-
>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>>> index 9502d161..ee379ddf 100644
>>> --- a/lib/s390x/sclp.c
>>> +++ b/lib/s390x/sclp.c
>>> @@ -123,6 +123,12 @@ int sclp_get_cpu_num(void)
>>>   	return read_info->entries_cpu;
>>>   }
>>>   
>>> +int sclp_get_stsi_parm(void)
>>> +{
>>> +	assert(read_info);
>>> +	return read_info->stsi_parm;
>> 
>> Is this a generic "stsi parm", or always the concrete topology nested
>> level? IOW, is that name good, or too generic?
>
> It is the name used in the documentation, but for now only the 3 bits 
> 5-7 are used for the maximum value of the selector 2 of the STSI 
> instruction allowed by the machine.

Ok.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [kvm-unit-tests PATCH v2 2/4] s390x: lib: Simplify stsi_get_fc and move it to library

[Thomas Huth]

On 10/08/2021 18.22, Pierre Morel wrote:
> stsi_get_fc is now needed in multiple tests.
> 
> As it does not need to store information but only returns
> the machine level, suppress the address parameter.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
>   s390x/stsi.c             | 20 ++------------------
>   2 files changed, 18 insertions(+), 18 deletions(-)

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

Re: [kvm-unit-tests PATCH v2 2/4] s390x: lib: Simplify stsi_get_fc and move it to library

[Pierre Morel]

On 8/18/21 9:45 AM, Thomas Huth wrote:
> On 10/08/2021 18.22, Pierre Morel wrote:
>> stsi_get_fc is now needed in multiple tests.
>>
>> As it does not need to store information but only returns
>> the machine level, suppress the address parameter.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
>>   s390x/stsi.c             | 20 ++------------------
>>   2 files changed, 18 insertions(+), 18 deletions(-)
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen