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

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

[Pierre Morel]

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: Move stsi_get_fc 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             |  16 ---
 s390x/topology.c         | 294 +++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg      |   4 +
 7 files changed, 324 insertions(+), 17 deletions(-)
 create mode 100644 s390x/topology.c

-- 
2.25.1

[kvm-unit-tests PATCH v1 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>
---
 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 v1 2/4] s390x: lib: Move stsi_get_fc to library

[Pierre Morel]

It's needed in multiple tests now.

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

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 15cf7d48..57d7ddac 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 *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 inline int servc(uint32_t command, unsigned long sccb)
 {
 	int cc;
diff --git a/s390x/stsi.c b/s390x/stsi.c
index 87d48047..11986d13 100644
--- a/s390x/stsi.c
+++ b/s390x/stsi.c
@@ -71,22 +71,6 @@ 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");
-- 
2.25.1

[kvm-unit-tests PATCH v1 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    | 87 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |  3 ++
 3 files changed, 91 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..4146189a
--- /dev/null
+++ b/s390x/topology.c
@@ -0,0 +1,87 @@
+/* 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 uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+int machine_level;
+int mnest;
+
+#define PTF_HORIZONTAL	0
+#define PTF_VERTICAL	1
+#define PTF_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,%1\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_CHECK, &rc);
+	cc = ptf(PTF_CHECK, &rc);
+	report(cc == 0, "PTF check clear");
+	cc = ptf(PTF_HORIZONTAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
+	       "PTF horizontal already configured");
+	cc = ptf(PTF_VERTICAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_NO_REASON,
+	       "PTF vertical non possible");
+
+	report_prefix_pop();
+}
+
+int main(int argc, char *argv[])
+{
+	report_prefix_push("stsi");
+
+	if (!test_facility(11)) {
+		report_skip("Topology facility not present");
+		goto end;
+	}
+
+	report_info("Machine level %ld", stsi_get_fc(pagebuf));
+
+	test_ptf();
+end:
+	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 v1 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    | 207 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   1 +
 2 files changed, 208 insertions(+)

diff --git a/s390x/topology.c b/s390x/topology.c
index 4146189a..1eb463fd 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -19,6 +19,51 @@
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 int machine_level;
 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_HORIZONTAL	0
 #define PTF_VERTICAL	1
@@ -70,9 +115,170 @@ static void test_ptf(void)
 	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 book");
+
+	/* 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;
+
+	report_info("VM Level: %ld", stsi_get_fc(pagebuf));
+
+	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 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("stsi");
+	parse_topology_args(argc, argv);
 
 	if (!test_facility(11)) {
 		report_skip("Topology facility not present");
@@ -82,6 +288,7 @@ int main(int argc, char *argv[])
 	report_info("Machine level %ld", stsi_get_fc(pagebuf));
 
 	test_ptf();
+	test_stsi();
 end:
 	return report_summary();
 }
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 v1 1/4] s390x: lib: Add SCLP toplogy nested level

[Claudio Imbrenda]

On Mon,  9 Aug 2021 10:48:51 +0200
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;
> +}
> +
>  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 v1 2/4] s390x: lib: Move stsi_get_fc to library

[Claudio Imbrenda]

On Mon,  9 Aug 2021 10:48:52 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> It's needed in multiple tests now.
> 
> 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             | 16 ----------------
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15cf7d48..57d7ddac 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 *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 inline int servc(uint32_t command, unsigned long sccb)
>  {
>  	int cc;
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 87d48047..11986d13 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -71,22 +71,6 @@ 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");

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

[Claudio Imbrenda]

On Mon,  9 Aug 2021 10:48:53 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> 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    | 87
> +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg |
> 3 ++ 3 files changed, 91 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..4146189a
> --- /dev/null
> +++ b/s390x/topology.c
> @@ -0,0 +1,87 @@
> +/* 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 uint8_t pagebuf[PAGE_SIZE * 2]
> __attribute__((aligned(PAGE_SIZE * 2))); +int machine_level;
> +int mnest;
> +
> +#define PTF_HORIZONTAL	0
> +#define PTF_VERTICAL	1
> +#define PTF_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,%1\n"

I know you copied this from the kernel, but the second argument is not
really there according to the PoP, so maybe it's better to have this
instead?

	.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_CHECK, &rc);
> +	cc = ptf(PTF_CHECK, &rc);
> +	report(cc == 0, "PTF check clear");
> +	cc = ptf(PTF_HORIZONTAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
> +	       "PTF horizontal already configured");
> +	cc = ptf(PTF_VERTICAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
> +	       "PTF vertical non possible");

*not possible

> +
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	report_prefix_push("stsi");

should this really be "stsi" ?

> +
> +	if (!test_facility(11)) {
> +		report_skip("Topology facility not present");
> +		goto end;
> +	}
> +
> +	report_info("Machine level %ld", stsi_get_fc(pagebuf));
> +
> +	test_ptf();
> +end:
> +	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 v1 3/4] s390x: topology: check the Perform Topology Function

[Janosch Frank]

On 8/9/21 10:48 AM, Pierre Morel wrote:
> 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    | 87 +++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |  3 ++
>  3 files changed, 91 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..4146189a
> --- /dev/null
> +++ b/s390x/topology.c
> @@ -0,0 +1,87 @@
> +/* 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 uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));

We don't actually need that I made a mistake in stsi_get_fc().
I'll comment in the other patch.

> +int machine_level;
> +int mnest;
> +
> +#define PTF_HORIZONTAL	0
> +#define PTF_VERTICAL	1

PTF_REQ_*

> +#define PTF_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,%1\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_CHECK, &rc);
> +	cc = ptf(PTF_CHECK, &rc)> +	report(cc == 0, "PTF check clear");

Please leave a \n after a report for readability.

> +	cc = ptf(PTF_HORIZONTAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
> +	       "PTF horizontal already configured");
> +	cc = ptf(PTF_VERTICAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
> +	       "PTF vertical non possible");

I've yet to look into your KVM/qemu code so I don't really understand
what you're testing here and why we can expect to get those results.

Maybe add a comment?
Also what will happen if we start this test under LPAR or z/VM, will it
fail?

> +
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	report_prefix_push("stsi");

Where did you copy that test from? :-)

> +
> +	if (!test_facility(11)) {
> +		report_skip("Topology facility not present");
> +		goto end;
> +	}
> +
> +	report_info("Machine level %ld", stsi_get_fc(pagebuf));
> +
> +	test_ptf();
> +end:

report_prefix_pop is missing here

> +	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 v1 2/4] s390x: lib: Move stsi_get_fc to library

[Janosch Frank]

On 8/9/21 10:48 AM, Pierre Morel wrote:
> It's needed in multiple tests now.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
>  s390x/stsi.c             | 16 ----------------
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 15cf7d48..57d7ddac 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 *addr)

We don't need an address for fc == 0. You can remove that and fix the
s390x/stsi.c call.

> +{
> +	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 inline int servc(uint32_t command, unsigned long sccb)
>  {
>  	int cc;
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 87d48047..11986d13 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -71,22 +71,6 @@ 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");
> 

Re: [kvm-unit-tests PATCH v1 4/4] s390x: Topology: checking Configuration Topology Information

[Claudio Imbrenda]

On Mon,  9 Aug 2021 10:48:54 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> 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    | 207
> ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg |
> 1 + 2 files changed, 208 insertions(+)
> 
> diff --git a/s390x/topology.c b/s390x/topology.c
> index 4146189a..1eb463fd 100644
> --- a/s390x/topology.c
> +++ b/s390x/topology.c
> @@ -19,6 +19,51 @@
>  static uint8_t pagebuf[PAGE_SIZE * 2]
> __attribute__((aligned(PAGE_SIZE * 2))); int machine_level;
>  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_HORIZONTAL	0
>  #define PTF_VERTICAL	1
> @@ -70,9 +115,170 @@ static void test_ptf(void)
>  	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
> book");

* 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;
> +
> +	report_info("VM Level: %ld", stsi_get_fc(pagebuf));
> +
> +	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 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("stsi");
> +	parse_topology_args(argc, argv);
>  
>  	if (!test_facility(11)) {
>  		report_skip("Topology facility not present");
> @@ -82,6 +288,7 @@ int main(int argc, char *argv[])
>  	report_info("Machine level %ld", stsi_get_fc(pagebuf));
>  
>  	test_ptf();
> +	test_stsi();
>  end:
>  	return report_summary();
>  }
> 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"

Re: [kvm-unit-tests PATCH v1 2/4] s390x: lib: Move stsi_get_fc to library

[Pierre Morel]

On 8/9/21 12:16 PM, Janosch Frank wrote:
> On 8/9/21 10:48 AM, Pierre Morel wrote:
>> It's needed in multiple tests now.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/asm/arch_def.h | 16 ++++++++++++++++
>>   s390x/stsi.c             | 16 ----------------
>>   2 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 15cf7d48..57d7ddac 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 *addr)
> 
> We don't need an address for fc == 0. You can remove that and fix the
> s390x/stsi.c call.

OK, I do that.
Thanks,

Pierre


-- 
Pierre Morel
IBM Lab Boeblingen

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

[Pierre Morel]

On 8/9/21 11:53 AM, Claudio Imbrenda wrote:
> On Mon,  9 Aug 2021 10:48:51 +0200
> 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>

Thanks,
Pierre


-- 
Pierre Morel
IBM Lab Boeblingen

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

[Pierre Morel]

On 8/9/21 12:03 PM, Claudio Imbrenda wrote:
> On Mon,  9 Aug 2021 10:48:53 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> 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    | 87
>> +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg |
>> 3 ++ 3 files changed, 91 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..4146189a
>> --- /dev/null
>> +++ b/s390x/topology.c
>> @@ -0,0 +1,87 @@
>> +/* 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 uint8_t pagebuf[PAGE_SIZE * 2]
>> __attribute__((aligned(PAGE_SIZE * 2))); +int machine_level;
>> +int mnest;
>> +
>> +#define PTF_HORIZONTAL	0
>> +#define PTF_VERTICAL	1
>> +#define PTF_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,%1\n"
> 
> I know you copied this from the kernel, but the second argument is not
> really there according to the PoP, so maybe it's better to have this
> instead?
> 
> 	.insn   rre,0xb9a20000,%1,0\n

OK, thanks.

> 
>> +		"       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_CHECK, &rc);
>> +	cc = ptf(PTF_CHECK, &rc);
>> +	report(cc == 0, "PTF check clear");
>> +	cc = ptf(PTF_HORIZONTAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
>> +	       "PTF horizontal already configured");
>> +	cc = ptf(PTF_VERTICAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
>> +	       "PTF vertical non possible");
> 
> *not possible

Oh yes :)

> 
>> +
>> +	report_prefix_pop();
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	report_prefix_push("stsi");
> 
> should this really be "stsi" ?

No, I think CPU-Topology should be better.




-- 
Pierre Morel
IBM Lab Boeblingen

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

[Pierre Morel]

On 8/9/21 12:12 PM, Janosch Frank wrote:
> On 8/9/21 10:48 AM, Pierre Morel wrote:
>> 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    | 87 +++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |  3 ++
>>   3 files changed, 91 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..4146189a
>> --- /dev/null
>> +++ b/s390x/topology.c
>> @@ -0,0 +1,87 @@
>> +/* 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 uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
> 
> We don't actually need that I made a mistake in stsi_get_fc().
> I'll comment in the other patch.

OK I saw.
thx

> 
>> +int machine_level;
>> +int mnest;
>> +
>> +#define PTF_HORIZONTAL	0
>> +#define PTF_VERTICAL	1
> 
> PTF_REQ_*

OK

> 
>> +#define PTF_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,%1\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_CHECK, &rc);
>> +	cc = ptf(PTF_CHECK, &rc)> +	report(cc == 0, "PTF check clear");
> 
> Please leave a \n after a report for readability.
OK

> 
>> +	cc = ptf(PTF_HORIZONTAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
>> +	       "PTF horizontal already configured");
>> +	cc = ptf(PTF_VERTICAL, &rc);
>> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
>> +	       "PTF vertical non possible");
> 
> I've yet to look into your KVM/qemu code so I don't really understand
> what you're testing here and why we can expect to get those results.

In KVM please ignore the stupid patch 3 commented by Heiko and that will 
disappear.

It changes nothing to the first two patches.

OK, I will add some comments to explain what we await and why.

> 
> Maybe add a comment?
> Also what will happen if we start this test under LPAR or z/VM, will it
> fail?

The last one may fail, PTF would succeed AFAIU under LPAR.
For z/VM, no idea.


> 
>> +
>> +	report_prefix_pop();
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	report_prefix_push("stsi");
> 
> Where did you copy that test from? :-)

:) yes cut and paste, I will change trhe prefix to "CPU Topology"

> 
>> +
>> +	if (!test_facility(11)) {
>> +		report_skip("Topology facility not present");
>> +		goto end;
>> +	}
>> +
>> +	report_info("Machine level %ld", stsi_get_fc(pagebuf));
>> +
>> +	test_ptf();
>> +end:
> 
> report_prefix_pop is missing here

Right.
will add.

> 
>> +	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
>>
> 

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v1 4/4] s390x: Topology: checking Configuration Topology Information

[Pierre Morel]

On 8/9/21 12:22 PM, Claudio Imbrenda wrote:
> On Mon,  9 Aug 2021 10:48:54 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> STSI with function code 15 is used to store the CPU configuration
>> topology.
>>
...
>>   
>> +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
>> book");
> 
> * books

right, thanks

...

regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen