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

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

[Pierre Morel]

Hi,

new 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 to the QEMU -smp parameter and as
argument for the test.

To run these tests successfully you will need both the Linux and the
QEMU patches, at least the following or newer patches:

    https://lkml.org/lkml/2021/8/3/201

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

Then if you have the text you can run directly the unit test or directly
start QEMU with:

# ./s390x-run s390x/topology.elf \
	-smp 5,drawers=2,books=3,sockets=4,cores=4,maxcpus=96 \
	-append "-d 2 -b 3 -s 4 -c 4 -m 96"

If you do not have the patches you can still use the test but only with
the first two topology levels like in:

# ./s390x-run s390x/topology.elf \
	-smp 5,sockets=24,cores=4,maxcpus=96 \
	-append "-s 24 -c 4 -m 96"

Of course the declaration of the number of socket and core must be
coherent.

Regards,
Pierre

Pierre Morel (4):
  s390x: lib: Add SCLP toplogy nested level
  s390x: stsi: Define vm_is_kvm to be used in different tests
  s390x: topology: Check the Perform Topology Function
  s390x: topology: Checking Configuration Topology Information

 lib/s390x/sclp.c    |   6 +
 lib/s390x/sclp.h    |   4 +-
 lib/s390x/stsi.h    |  76 ++++++++++
 lib/s390x/vm.c      |  39 +++++
 lib/s390x/vm.h      |   1 +
 s390x/Makefile      |   1 +
 s390x/stsi.c        |  23 +--
 s390x/topology.c    | 346 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   4 +
 9 files changed, 478 insertions(+), 22 deletions(-)
 create mode 100644 lib/s390x/stsi.h
 create mode 100644 s390x/topology.c

-- 
2.27.0

Changelog:

From V2:

- Check if the test in running in KVM
  (Janosch)

- patch on "Simplify stsi_get_fc and move it to library"
  pushed separatly

- replace named level with abstracted topology levels
  to get rid of a possible naming controversy
  (Pierre)

- Better checks and new checks for STSI(15,1,x)
  (Pierre)

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 v3 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>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.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 33985eb4..e15b3b2c 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -128,6 +128,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 fead007a..541eb441 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 */
@@ -323,6 +324,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.27.0

[kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Pierre Morel]

We need in several tests to check if the VM we are running in
is KVM.
Let's add the test.

To check the VM type we use the STSI 3.2.2 instruction, let's
define it's response structure in a central header.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
 lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
 lib/s390x/vm.h   |  1 +
 s390x/stsi.c     | 23 ++---------------------
 4 files changed, 74 insertions(+), 21 deletions(-)
 create mode 100644 lib/s390x/stsi.h

diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
new file mode 100644
index 00000000..02cc94a6
--- /dev/null
+++ b/lib/s390x/stsi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Structures used to Store System Information
+ *
+ * Copyright (c) 2021 IBM Inc
+ */
+
+#ifndef _S390X_STSI_H_
+#define _S390X_STSI_H_
+
+struct sysinfo_3_2_2 {
+	uint8_t reserved[31];
+	uint8_t count;
+	struct {
+		uint8_t reserved2[4];
+		uint16_t total_cpus;
+		uint16_t conf_cpus;
+		uint16_t standby_cpus;
+		uint16_t reserved_cpus;
+		uint8_t name[8];
+		uint32_t caf;
+		uint8_t cpi[16];
+		uint8_t reserved5[3];
+		uint8_t ext_name_encoding;
+		uint32_t reserved3;
+		uint8_t uuid[16];
+	} vm[8];
+	uint8_t reserved4[1504];
+	uint8_t ext_names[8][256];
+};
+
+#endif  /* _S390X_STSI_H_ */
diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
index a5b92863..3e11401e 100644
--- a/lib/s390x/vm.c
+++ b/lib/s390x/vm.c
@@ -12,6 +12,7 @@
 #include <alloc_page.h>
 #include <asm/arch_def.h>
 #include "vm.h"
+#include "stsi.h"
 
 /**
  * Detect whether we are running with TCG (instead of KVM)
@@ -43,3 +44,41 @@ out:
 	free_page(buf);
 	return is_tcg;
 }
+
+/**
+ * Detect whether we are running with KVM
+ */
+
+bool vm_is_kvm(void)
+{
+	/* EBCDIC for "KVM/" */
+	const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
+	static bool initialized;
+	static bool is_kvm;
+	struct sysinfo_3_2_2 *stsi_322;
+
+	if (initialized)
+		return is_kvm;
+
+	if (stsi_get_fc() < 3) {
+		initialized = true;
+		return is_kvm;
+	}
+
+	stsi_322 = alloc_page();
+	if (!stsi_322)
+		return false;
+
+	if (stsi(stsi_322, 3, 2, 2))
+		goto out;
+
+	/*
+	 * If the manufacturer string is "KVM/" in EBCDIC, then we
+	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
+	 */
+	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));
+	initialized = true;
+out:
+	free_page(stsi_322);
+	return is_kvm;
+}
diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
index 7abba0cc..44097b4a 100644
--- a/lib/s390x/vm.h
+++ b/lib/s390x/vm.h
@@ -9,5 +9,6 @@
 #define _S390X_VM_H_
 
 bool vm_is_tcg(void);
+bool vm_is_kvm(void);
 
 #endif  /* _S390X_VM_H_ */
diff --git a/s390x/stsi.c b/s390x/stsi.c
index 391f8849..1ed045e2 100644
--- a/s390x/stsi.c
+++ b/s390x/stsi.c
@@ -13,27 +13,8 @@
 #include <asm/asm-offsets.h>
 #include <asm/interrupt.h>
 #include <smp.h>
+#include "stsi.h"
 
-struct stsi_322 {
-	uint8_t reserved[31];
-	uint8_t count;
-	struct {
-		uint8_t reserved2[4];
-		uint16_t total_cpus;
-		uint16_t conf_cpus;
-		uint16_t standby_cpus;
-		uint16_t reserved_cpus;
-		uint8_t name[8];
-		uint32_t caf;
-		uint8_t cpi[16];
-		uint8_t reserved5[3];
-		uint8_t ext_name_encoding;
-		uint32_t reserved3;
-		uint8_t uuid[16];
-	} vm[8];
-	uint8_t reserved4[1504];
-	uint8_t ext_names[8][256];
-};
 static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
 
 static void test_specs(void)
@@ -91,7 +72,7 @@ static void test_3_2_2(void)
 	/* EBCDIC for "KVM/" */
 	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
 	const char vm_name_ext[] = "kvm-unit-test";
-	struct stsi_322 *data = (void *)pagebuf;
+	struct sysinfo_3_2_2 *data = (void *)pagebuf;
 
 	report_prefix_push("3.2.2");
 
-- 
2.27.0

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Janosch Frank]

On 1/10/22 14:37, Pierre Morel wrote:
> We need in several tests to check if the VM we are running in
> is KVM.
> Let's add the test.
> 
> To check the VM type we use the STSI 3.2.2 instruction, let's
> define it's response structure in a central header.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>   lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>   lib/s390x/vm.h   |  1 +
>   s390x/stsi.c     | 23 ++---------------------
>   4 files changed, 74 insertions(+), 21 deletions(-)
>   create mode 100644 lib/s390x/stsi.h
> 
> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
> new file mode 100644
> index 00000000..02cc94a6
> --- /dev/null
> +++ b/lib/s390x/stsi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Structures used to Store System Information
> + *
> + * Copyright (c) 2021 IBM Inc
> + */
> +
> +#ifndef _S390X_STSI_H_
> +#define _S390X_STSI_H_
> +
> +struct sysinfo_3_2_2 {
> +	uint8_t reserved[31];
> +	uint8_t count;
> +	struct {
> +		uint8_t reserved2[4];
> +		uint16_t total_cpus;
> +		uint16_t conf_cpus;
> +		uint16_t standby_cpus;
> +		uint16_t reserved_cpus;
> +		uint8_t name[8];
> +		uint32_t caf;
> +		uint8_t cpi[16];
> +		uint8_t reserved5[3];
> +		uint8_t ext_name_encoding;
> +		uint32_t reserved3;
> +		uint8_t uuid[16];
> +	} vm[8];
> +	uint8_t reserved4[1504];
> +	uint8_t ext_names[8][256];
> +};
> +
> +#endif  /* _S390X_STSI_H_ */
> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
> index a5b92863..3e11401e 100644
> --- a/lib/s390x/vm.c
> +++ b/lib/s390x/vm.c
> @@ -12,6 +12,7 @@
>   #include <alloc_page.h>
>   #include <asm/arch_def.h>
>   #include "vm.h"
> +#include "stsi.h"
>   
>   /**
>    * Detect whether we are running with TCG (instead of KVM)

We could add a fc < 3 check to the vm_is_tcg() function and add a 
vm_is_lpar() which does a simple fc ==1 check.

> @@ -43,3 +44,41 @@ out:
>   	free_page(buf);
>   	return is_tcg;
>   }
> +
> +/**
> + * Detect whether we are running with KVM
> + */
> +
> +bool vm_is_kvm(void)
> +{
> +	/* EBCDIC for "KVM/" */
> +	const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
> +	static bool initialized;
> +	static bool is_kvm;
> +	struct sysinfo_3_2_2 *stsi_322;
> +
> +	if (initialized)
> +		return is_kvm;
> +
> +	if (stsi_get_fc() < 3) {
> +		initialized = true;
> +		return is_kvm;
> +	}
> +
> +	stsi_322 = alloc_page();
> +	if (!stsi_322)
> +		return false;
> +
> +	if (stsi(stsi_322, 3, 2, 2))
> +		goto out;
> +
> +	/*
> +	 * If the manufacturer string is "KVM/" in EBCDIC, then we
> +	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
> +	 */
> +	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));

So I had a look at this before Christmas and I think it's wrong.

QEMU will still set the cpi to KVM/LINUX if we are under tcg.
So we need to do add a !tcg check here and fix this comment.

I.e. we always have the KVM/LINUX cpi but if we're under TCG the 
manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and 
if we want to fix this at some point or not.

> +	initialized = true;
> +out:
> +	free_page(stsi_322);
> +	return is_kvm;
> +}
> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
> index 7abba0cc..44097b4a 100644
> --- a/lib/s390x/vm.h
> +++ b/lib/s390x/vm.h
> @@ -9,5 +9,6 @@
>   #define _S390X_VM_H_
>   
>   bool vm_is_tcg(void);
> +bool vm_is_kvm(void);
>   
>   #endif  /* _S390X_VM_H_ */
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 391f8849..1ed045e2 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -13,27 +13,8 @@
>   #include <asm/asm-offsets.h>
>   #include <asm/interrupt.h>
>   #include <smp.h>
> +#include "stsi.h"
>   
> -struct stsi_322 {
> -	uint8_t reserved[31];
> -	uint8_t count;
> -	struct {
> -		uint8_t reserved2[4];
> -		uint16_t total_cpus;
> -		uint16_t conf_cpus;
> -		uint16_t standby_cpus;
> -		uint16_t reserved_cpus;
> -		uint8_t name[8];
> -		uint32_t caf;
> -		uint8_t cpi[16];
> -		uint8_t reserved5[3];
> -		uint8_t ext_name_encoding;
> -		uint32_t reserved3;
> -		uint8_t uuid[16];
> -	} vm[8];
> -	uint8_t reserved4[1504];
> -	uint8_t ext_names[8][256];
> -};
>   static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>   
>   static void test_specs(void)
> @@ -91,7 +72,7 @@ static void test_3_2_2(void)
>   	/* EBCDIC for "KVM/" */
>   	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>   	const char vm_name_ext[] = "kvm-unit-test";
> -	struct stsi_322 *data = (void *)pagebuf;
> +	struct sysinfo_3_2_2 *data = (void *)pagebuf;
>   
>   	report_prefix_push("3.2.2");
>   
> 

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Pierre Morel]

On 1/11/22 13:27, Janosch Frank wrote:
> On 1/10/22 14:37, Pierre Morel wrote:
>> We need in several tests to check if the VM we are running in
>> is KVM.
>> Let's add the test.
>>
>> To check the VM type we use the STSI 3.2.2 instruction, let's
>> define it's response structure in a central header.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>>   lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/vm.h   |  1 +
>>   s390x/stsi.c     | 23 ++---------------------
>>   4 files changed, 74 insertions(+), 21 deletions(-)
>>   create mode 100644 lib/s390x/stsi.h
>>
>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
>> new file mode 100644
>> index 00000000..02cc94a6
>> --- /dev/null
>> +++ b/lib/s390x/stsi.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Structures used to Store System Information
>> + *
>> + * Copyright (c) 2021 IBM Inc
>> + */
>> +
>> +#ifndef _S390X_STSI_H_
>> +#define _S390X_STSI_H_
>> +
>> +struct sysinfo_3_2_2 {
>> +    uint8_t reserved[31];
>> +    uint8_t count;
>> +    struct {
>> +        uint8_t reserved2[4];
>> +        uint16_t total_cpus;
>> +        uint16_t conf_cpus;
>> +        uint16_t standby_cpus;
>> +        uint16_t reserved_cpus;
>> +        uint8_t name[8];
>> +        uint32_t caf;
>> +        uint8_t cpi[16];
>> +        uint8_t reserved5[3];
>> +        uint8_t ext_name_encoding;
>> +        uint32_t reserved3;
>> +        uint8_t uuid[16];
>> +    } vm[8];
>> +    uint8_t reserved4[1504];
>> +    uint8_t ext_names[8][256];
>> +};
>> +
>> +#endif  /* _S390X_STSI_H_ */
>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>> index a5b92863..3e11401e 100644
>> --- a/lib/s390x/vm.c
>> +++ b/lib/s390x/vm.c
>> @@ -12,6 +12,7 @@
>>   #include <alloc_page.h>
>>   #include <asm/arch_def.h>
>>   #include "vm.h"
>> +#include "stsi.h"
>>   /**
>>    * Detect whether we are running with TCG (instead of KVM)
> 
> We could add a fc < 3 check to the vm_is_tcg() function and add a 

OK

> vm_is_lpar() which does a simple fc ==1 check.

hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?

Do you have an experimental return on this?

> 
>> @@ -43,3 +44,41 @@ out:
>>       free_page(buf);
>>       return is_tcg;
>>   }
>> +
>> +/**
>> + * Detect whether we are running with KVM
>> + */
>> +
>> +bool vm_is_kvm(void)
>> +{
>> +    /* EBCDIC for "KVM/" */
>> +    const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>> +    static bool initialized;
>> +    static bool is_kvm;
>> +    struct sysinfo_3_2_2 *stsi_322;
>> +
>> +    if (initialized)
>> +        return is_kvm;
>> +
>> +    if (stsi_get_fc() < 3) {
>> +        initialized = true;
>> +        return is_kvm;
>> +    }
>> +
>> +    stsi_322 = alloc_page();
>> +    if (!stsi_322)
>> +        return false;
>> +
>> +    if (stsi(stsi_322, 3, 2, 2))
>> +        goto out;
>> +
>> +    /*
>> +     * If the manufacturer string is "KVM/" in EBCDIC, then we
>> +     * are on KVM (otherwise the string is "IBM" in EBCDIC)
>> +     */
>> +    is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, 
>> sizeof(kvm_ebcdic));
> 
> So I had a look at this before Christmas and I think it's wrong.
> 
> QEMU will still set the cpi to KVM/LINUX if we are under tcg.
> So we need to do add a !tcg check here and fix this comment.
> 
> I.e. we always have the KVM/LINUX cpi but if we're under TCG the 
> manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and 
> if we want to fix this at some point or not.

indeed I did not check this!!

> 
>> +    initialized = true;
>> +out:
>> +    free_page(stsi_322);
>> +    return is_kvm;
>> +}

...snip...

Thanks for the review, I make the changes.
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Janosch Frank]

On 1/17/22 15:57, Pierre Morel wrote:
> 
> 
> On 1/11/22 13:27, Janosch Frank wrote:
>> On 1/10/22 14:37, Pierre Morel wrote:
>>> We need in several tests to check if the VM we are running in
>>> is KVM.
>>> Let's add the test.
>>>
>>> To check the VM type we use the STSI 3.2.2 instruction, let's
>>> define it's response structure in a central header.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>    lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>>>    lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>>    lib/s390x/vm.h   |  1 +
>>>    s390x/stsi.c     | 23 ++---------------------
>>>    4 files changed, 74 insertions(+), 21 deletions(-)
>>>    create mode 100644 lib/s390x/stsi.h
>>>
>>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
>>> new file mode 100644
>>> index 00000000..02cc94a6
>>> --- /dev/null
>>> +++ b/lib/s390x/stsi.h
>>> @@ -0,0 +1,32 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +/*
>>> + * Structures used to Store System Information
>>> + *
>>> + * Copyright (c) 2021 IBM Inc
>>> + */
>>> +
>>> +#ifndef _S390X_STSI_H_
>>> +#define _S390X_STSI_H_
>>> +
>>> +struct sysinfo_3_2_2 {
>>> +    uint8_t reserved[31];
>>> +    uint8_t count;
>>> +    struct {
>>> +        uint8_t reserved2[4];
>>> +        uint16_t total_cpus;
>>> +        uint16_t conf_cpus;
>>> +        uint16_t standby_cpus;
>>> +        uint16_t reserved_cpus;
>>> +        uint8_t name[8];
>>> +        uint32_t caf;
>>> +        uint8_t cpi[16];
>>> +        uint8_t reserved5[3];
>>> +        uint8_t ext_name_encoding;
>>> +        uint32_t reserved3;
>>> +        uint8_t uuid[16];
>>> +    } vm[8];
>>> +    uint8_t reserved4[1504];
>>> +    uint8_t ext_names[8][256];
>>> +};
>>> +
>>> +#endif  /* _S390X_STSI_H_ */
>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>>> index a5b92863..3e11401e 100644
>>> --- a/lib/s390x/vm.c
>>> +++ b/lib/s390x/vm.c
>>> @@ -12,6 +12,7 @@
>>>    #include <alloc_page.h>
>>>    #include <asm/arch_def.h>
>>>    #include "vm.h"
>>> +#include "stsi.h"
>>>    /**
>>>     * Detect whether we are running with TCG (instead of KVM)
>>
>> We could add a fc < 3 check to the vm_is_tcg() function and add a
> 
> OK
> 
>> vm_is_lpar() which does a simple fc ==1 check.
> 
> hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
> do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?
> 

Right
I'll do some tests on the lpar stsi output and have a look what we get back.

> Do you have an experimental return on this?

ENOPARSE

> 
>>
>>> @@ -43,3 +44,41 @@ out:
>>>        free_page(buf);
>>>        return is_tcg;
>>>    }
>>> +
>>> +/**
>>> + * Detect whether we are running with KVM
>>> + */
>>> +
>>> +bool vm_is_kvm(void)
>>> +{
>>> +    /* EBCDIC for "KVM/" */
>>> +    const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>>> +    static bool initialized;
>>> +    static bool is_kvm;
>>> +    struct sysinfo_3_2_2 *stsi_322;
>>> +
>>> +    if (initialized)
>>> +        return is_kvm;
>>> +
>>> +    if (stsi_get_fc() < 3) {
>>> +        initialized = true;
>>> +        return is_kvm;
>>> +    }
>>> +
>>> +    stsi_322 = alloc_page();
>>> +    if (!stsi_322)
>>> +        return false;
>>> +
>>> +    if (stsi(stsi_322, 3, 2, 2))
>>> +        goto out;
>>> +
>>> +    /*
>>> +     * If the manufacturer string is "KVM/" in EBCDIC, then we
>>> +     * are on KVM (otherwise the string is "IBM" in EBCDIC)
>>> +     */
>>> +    is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic,
>>> sizeof(kvm_ebcdic));
>>
>> So I had a look at this before Christmas and I think it's wrong.
>>
>> QEMU will still set the cpi to KVM/LINUX if we are under tcg.
>> So we need to do add a !tcg check here and fix this comment.
>>
>> I.e. we always have the KVM/LINUX cpi but if we're under TCG the
>> manufacturer in fc == 1 is QEMU. I'm not sure if this is intentional and
>> if we want to fix this at some point or not.
> 
> indeed I did not check this!!
> 
>>
>>> +    initialized = true;
>>> +out:
>>> +    free_page(stsi_322);
>>> +    return is_kvm;
>>> +}
> 
> ...snip...
> 
> Thanks for the review, I make the changes.
> Pierre
> 

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Pierre Morel]

On 1/18/22 09:35, Janosch Frank wrote:
> On 1/17/22 15:57, Pierre Morel wrote:
>>
>>
>> On 1/11/22 13:27, Janosch Frank wrote:
>>> On 1/10/22 14:37, Pierre Morel wrote:
>>>> We need in several tests to check if the VM we are running in
>>>> is KVM.
>>>> Let's add the test.
>>>>
>>>> To check the VM type we use the STSI 3.2.2 instruction, let's
>>>> define it's response structure in a central header.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>>>>    lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>>>    lib/s390x/vm.h   |  1 +
>>>>    s390x/stsi.c     | 23 ++---------------------
>>>>    4 files changed, 74 insertions(+), 21 deletions(-)
>>>>    create mode 100644 lib/s390x/stsi.h
>>>>
>>>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
>>>> new file mode 100644
>>>> index 00000000..02cc94a6
>>>> --- /dev/null
>>>> +++ b/lib/s390x/stsi.h
>>>> @@ -0,0 +1,32 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Structures used to Store System Information
>>>> + *
>>>> + * Copyright (c) 2021 IBM Inc
>>>> + */
>>>> +
>>>> +#ifndef _S390X_STSI_H_
>>>> +#define _S390X_STSI_H_
>>>> +
>>>> +struct sysinfo_3_2_2 {
>>>> +    uint8_t reserved[31];
>>>> +    uint8_t count;
>>>> +    struct {
>>>> +        uint8_t reserved2[4];
>>>> +        uint16_t total_cpus;
>>>> +        uint16_t conf_cpus;
>>>> +        uint16_t standby_cpus;
>>>> +        uint16_t reserved_cpus;
>>>> +        uint8_t name[8];
>>>> +        uint32_t caf;
>>>> +        uint8_t cpi[16];
>>>> +        uint8_t reserved5[3];
>>>> +        uint8_t ext_name_encoding;
>>>> +        uint32_t reserved3;
>>>> +        uint8_t uuid[16];
>>>> +    } vm[8];
>>>> +    uint8_t reserved4[1504];
>>>> +    uint8_t ext_names[8][256];
>>>> +};
>>>> +
>>>> +#endif  /* _S390X_STSI_H_ */
>>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c
>>>> index a5b92863..3e11401e 100644
>>>> --- a/lib/s390x/vm.c
>>>> +++ b/lib/s390x/vm.c
>>>> @@ -12,6 +12,7 @@
>>>>    #include <alloc_page.h>
>>>>    #include <asm/arch_def.h>
>>>>    #include "vm.h"
>>>> +#include "stsi.h"
>>>>    /**
>>>>     * Detect whether we are running with TCG (instead of KVM)
>>>
>>> We could add a fc < 3 check to the vm_is_tcg() function and add a
>>
>> OK
>>
>>> vm_is_lpar() which does a simple fc ==1 check.
>>
>> hum, the doc says 1 is basic, 2 is lpar, 3 is vm, shouldn't we
>> do a check on fc == 2 or have a vm_is_vm checking fc < 3 ?
>>
> 
> Right
> I'll do some tests on the lpar stsi output and have a look what we get 
> back.
> 
>> Do you have an experimental return on this?
> 
> ENOPARSE

:) you just answered.
I wanted to ask if you did tests which gave you "1" as result.


-- 
Pierre Morel
IBM Lab Boeblingen

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Claudio Imbrenda]

On Mon, 10 Jan 2022 14:37:53 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> We need in several tests to check if the VM we are running in
> is KVM.
> Let's add the test.
> 
> To check the VM type we use the STSI 3.2.2 instruction, let's
> define it's response structure in a central header.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>  lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>  lib/s390x/vm.h   |  1 +
>  s390x/stsi.c     | 23 ++---------------------
>  4 files changed, 74 insertions(+), 21 deletions(-)
>  create mode 100644 lib/s390x/stsi.h
> 
> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
> new file mode 100644
> index 00000000..02cc94a6
> --- /dev/null
> +++ b/lib/s390x/stsi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Structures used to Store System Information
> + *
> + * Copyright (c) 2021 IBM Inc

Copyright IBM Corp. 2021

> + */
> +
> +#ifndef _S390X_STSI_H_
> +#define _S390X_STSI_H_

[...]

> +
> +/**
> + * Detect whether we are running with KVM
> + */
> +
> +bool vm_is_kvm(void)
> +{
> +	/* EBCDIC for "KVM/" */
> +	const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
> +	static bool initialized;
> +	static bool is_kvm;
> +	struct sysinfo_3_2_2 *stsi_322;
> +
> +	if (initialized)
> +		return is_kvm;
> +
> +	if (stsi_get_fc() < 3) {
> +		initialized = true;
> +		return is_kvm;
> +	}
> +
> +	stsi_322 = alloc_page();
> +	if (!stsi_322)
> +		return false;

I don't like returning false if the allocation fails.
The allocation should not fail: assert(stsi_322);

> +
> +	if (stsi(stsi_322, 3, 2, 2))
> +		goto out;
> +
> +	/*
> +	 * If the manufacturer string is "KVM/" in EBCDIC, then we
> +	 * are on KVM (otherwise the string is "IBM" in EBCDIC)
> +	 */
> +	is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic));
> +	initialized = true;
> +out:
> +	free_page(stsi_322);
> +	return is_kvm;
> +}
> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h
> index 7abba0cc..44097b4a 100644
> --- a/lib/s390x/vm.h
> +++ b/lib/s390x/vm.h
> @@ -9,5 +9,6 @@
>  #define _S390X_VM_H_
>  
>  bool vm_is_tcg(void);
> +bool vm_is_kvm(void);
>  
>  #endif  /* _S390X_VM_H_ */
> diff --git a/s390x/stsi.c b/s390x/stsi.c
> index 391f8849..1ed045e2 100644
> --- a/s390x/stsi.c
> +++ b/s390x/stsi.c
> @@ -13,27 +13,8 @@
>  #include <asm/asm-offsets.h>
>  #include <asm/interrupt.h>
>  #include <smp.h>
> +#include "stsi.h"
>  
> -struct stsi_322 {
> -	uint8_t reserved[31];
> -	uint8_t count;
> -	struct {
> -		uint8_t reserved2[4];
> -		uint16_t total_cpus;
> -		uint16_t conf_cpus;
> -		uint16_t standby_cpus;
> -		uint16_t reserved_cpus;
> -		uint8_t name[8];
> -		uint32_t caf;
> -		uint8_t cpi[16];
> -		uint8_t reserved5[3];
> -		uint8_t ext_name_encoding;
> -		uint32_t reserved3;
> -		uint8_t uuid[16];
> -	} vm[8];
> -	uint8_t reserved4[1504];
> -	uint8_t ext_names[8][256];
> -};
>  static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>  
>  static void test_specs(void)
> @@ -91,7 +72,7 @@ static void test_3_2_2(void)
>  	/* EBCDIC for "KVM/" */
>  	const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>  	const char vm_name_ext[] = "kvm-unit-test";
> -	struct stsi_322 *data = (void *)pagebuf;
> +	struct sysinfo_3_2_2 *data = (void *)pagebuf;
>  
>  	report_prefix_push("3.2.2");
>  

Re: [kvm-unit-tests PATCH v3 2/4] s390x: stsi: Define vm_is_kvm to be used in different tests

[Pierre Morel]

On 1/11/22 14:08, Claudio Imbrenda wrote:
> On Mon, 10 Jan 2022 14:37:53 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> We need in several tests to check if the VM we are running in
>> is KVM.
>> Let's add the test.
>>
>> To check the VM type we use the STSI 3.2.2 instruction, let's
>> define it's response structure in a central header.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++++++
>>   lib/s390x/vm.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/vm.h   |  1 +
>>   s390x/stsi.c     | 23 ++---------------------
>>   4 files changed, 74 insertions(+), 21 deletions(-)
>>   create mode 100644 lib/s390x/stsi.h
>>
>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
>> new file mode 100644
>> index 00000000..02cc94a6
>> --- /dev/null
>> +++ b/lib/s390x/stsi.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Structures used to Store System Information
>> + *
>> + * Copyright (c) 2021 IBM Inc
> 
> Copyright IBM Corp. 2021

OK

> 
>> + */
>> +
>> +#ifndef _S390X_STSI_H_
>> +#define _S390X_STSI_H_
> 
> [...]
> 
>> +
>> +/**
>> + * Detect whether we are running with KVM
>> + */
>> +
>> +bool vm_is_kvm(void)
>> +{
>> +	/* EBCDIC for "KVM/" */
>> +	const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 };
>> +	static bool initialized;
>> +	static bool is_kvm;
>> +	struct sysinfo_3_2_2 *stsi_322;
>> +
>> +	if (initialized)
>> +		return is_kvm;
>> +
>> +	if (stsi_get_fc() < 3) {
>> +		initialized = true;
>> +		return is_kvm;
>> +	}
>> +
>> +	stsi_322 = alloc_page();
>> +	if (!stsi_322)
>> +		return false;
> 
> I don't like returning false if the allocation fails.
> The allocation should not fail: assert(stsi_322);

OK, right.

> 
>> +
>> +	if (stsi(stsi_322, 3, 2, 2))
>> +		goto out;
>> +

Thanks for the review,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 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    | 115 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   3 ++
 3 files changed, 119 insertions(+)
 create mode 100644 s390x/topology.c

diff --git a/s390x/Makefile b/s390x/Makefile
index 1e567c11..fa21a882 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -26,6 +26,7 @@ tests += $(TEST_DIR)/edat.elf
 tests += $(TEST_DIR)/mvpg-sie.elf
 tests += $(TEST_DIR)/spec_ex-sie.elf
 tests += $(TEST_DIR)/firq.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..a227555e
--- /dev/null
+++ b/s390x/topology.c
@@ -0,0 +1,115 @@
+/* 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>
+#include <s390x/vm.h>
+
+#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 should clear topology report");
+
+	report_prefix_pop();
+
+	report_prefix_push("Topology polarisation check");
+	/*
+	 * We can not assume the state of the polarization for
+	 * any Virtual Machine but KVM.
+	 * Let's skip the polarisation tests for other VMs.
+	 */
+	if (!vm_is_kvm()) {
+		report_skip("Topology polarisation check is done for KVM only");
+		goto end;
+	}
+
+	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
+	       "KVM always provides horizontal polarization");
+
+	cc = ptf(PTF_REQ_VERTICAL, &rc);
+	report(cc == 2 && rc == PTF_ERR_NO_REASON,
+	       "KVM doesn't support vertical polarization.");
+
+end:
+	report_prefix_pop();
+}
+
+static struct {
+	const char *name;
+	void (*func)(void);
+} tests[] = {
+	{ "PTF", test_ptf},
+	{ NULL, NULL }
+};
+
+int main(int argc, char *argv[])
+{
+	int i;
+
+	report_prefix_push("CPU Topology");
+
+	if (!test_facility(11)) {
+		report_skip("Topology facility not present");
+		goto end;
+	}
+
+	report_info("Machine level %ld", stsi_get_fc());
+
+	for (i = 0; tests[i].name; i++) {
+		report_prefix_push(tests[i].name);
+		tests[i].func();
+		report_prefix_pop();
+	}
+end:
+	report_prefix_pop();
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 054560c2..e2d3e6a5 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -122,3 +122,6 @@ extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=1 -devi
 file = firq.elf
 timeout = 20
 extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=2 -device qemu-s390x-cpu,core-id=1
+
+[topology]
+file = topology.elf
-- 
2.27.0

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

[Claudio Imbrenda]

On Mon, 10 Jan 2022 14:37:54 +0100
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    | 115 ++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |   3 ++
>  3 files changed, 119 insertions(+)
>  create mode 100644 s390x/topology.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 1e567c11..fa21a882 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -26,6 +26,7 @@ tests += $(TEST_DIR)/edat.elf
>  tests += $(TEST_DIR)/mvpg-sie.elf
>  tests += $(TEST_DIR)/spec_ex-sie.elf
>  tests += $(TEST_DIR)/firq.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..a227555e
> --- /dev/null
> +++ b/s390x/topology.c
> @@ -0,0 +1,115 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * CPU Topology
> + *
> + * Copyright (c) 2021 IBM Corp

Copyright IBM Corp. 2021

> + *
> + * 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>
> +#include <s390x/vm.h>
> +
> +#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 should clear topology report");
> +
> +	report_prefix_pop();
> +
> +	report_prefix_push("Topology polarisation check");
> +	/*
> +	 * We can not assume the state of the polarization for
> +	 * any Virtual Machine but KVM.
> +	 * Let's skip the polarisation tests for other VMs.
> +	 */
> +	if (!vm_is_kvm()) {
> +		report_skip("Topology polarisation check is done for KVM only");
> +		goto end;
> +	}
> +
> +	cc = ptf(PTF_REQ_HORIZONTAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_ALRDY_POLARIZED,
> +	       "KVM always provides horizontal polarization");
> +
> +	cc = ptf(PTF_REQ_VERTICAL, &rc);
> +	report(cc == 2 && rc == PTF_ERR_NO_REASON,
> +	       "KVM doesn't support vertical polarization.");
> +
> +end:
> +	report_prefix_pop();
> +}
> +
> +static struct {
> +	const char *name;
> +	void (*func)(void);
> +} tests[] = {
> +	{ "PTF", test_ptf},
> +	{ NULL, NULL }
> +};
> +
> +int main(int argc, char *argv[])
> +{
> +	int i;
> +
> +	report_prefix_push("CPU Topology");
> +
> +	if (!test_facility(11)) {
> +		report_skip("Topology facility not present");
> +		goto end;
> +	}
> +
> +	report_info("Machine level %ld", stsi_get_fc());
> +
> +	for (i = 0; tests[i].name; i++) {
> +		report_prefix_push(tests[i].name);
> +		tests[i].func();
> +		report_prefix_pop();
> +	}
> +end:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 054560c2..e2d3e6a5 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -122,3 +122,6 @@ extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=1 -devi
>  file = firq.elf
>  timeout = 20
>  extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=2 -device qemu-s390x-cpu,core-id=1
> +
> +[topology]
> +file = topology.elf

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

[Pierre Morel]

On 1/11/22 12:25, Claudio Imbrenda wrote:
> On Mon, 10 Jan 2022 14:37:54 +0100
> 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    | 115 ++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |   3 ++
>>   3 files changed, 119 insertions(+)
>>   create mode 100644 s390x/topology.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 1e567c11..fa21a882 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -26,6 +26,7 @@ tests += $(TEST_DIR)/edat.elf
>>   tests += $(TEST_DIR)/mvpg-sie.elf
>>   tests += $(TEST_DIR)/spec_ex-sie.elf
>>   tests += $(TEST_DIR)/firq.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..a227555e
>> --- /dev/null
>> +++ b/s390x/topology.c
>> @@ -0,0 +1,115 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * CPU Topology
>> + *
>> + * Copyright (c) 2021 IBM Corp
> 
> Copyright IBM Corp. 2021

Yes thanks
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

[kvm-unit-tests PATCH v3 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 between the QEMU -smp
  parameters and kernel parameters.
- the number of CPUs
- the maximum number of CPUs
- the number of containers of each levels for every STSI(15.1.x)
  instruction allowed by the machine.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 lib/s390x/stsi.h    |  44 +++++++++
 s390x/topology.c    | 231 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   1 +
 3 files changed, 276 insertions(+)

diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
index 02cc94a6..e3fc7ac0 100644
--- a/lib/s390x/stsi.h
+++ b/lib/s390x/stsi.h
@@ -29,4 +29,48 @@ struct sysinfo_3_2_2 {
 	uint8_t ext_names[8][256];
 };
 
+struct topology_core {
+	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;
+};
+
+struct topology_container {
+	uint8_t nl;
+	uint8_t reserved[6];
+	uint8_t id;
+};
+
+union topology_entry {
+	uint8_t nl;
+	struct topology_core cpu;
+	struct topology_container container;
+};
+
+#define CPU_TOPOLOGY_MAX_LEVEL 6
+struct sysinfo_15_1_x {
+	uint8_t reserved0[2];
+	uint16_t length;
+	uint8_t mag[CPU_TOPOLOGY_MAX_LEVEL];
+	uint8_t reserved10;
+	uint8_t mnest;
+	uint8_t reserved12[4];
+	union topology_entry tle[0];
+};
+
+static inline int cpus_in_tle_mask(uint64_t val)
+{
+	int i, n;
+
+	for (i = 0, n = 0; i < 64; i++, val >>= 1)
+		if (val & 0x01)
+			n++;
+	return n;
+}
+
 #endif  /* _S390X_STSI_H_ */
diff --git a/s390x/topology.c b/s390x/topology.c
index a227555e..d06e7c4d 100644
--- a/s390x/topology.c
+++ b/s390x/topology.c
@@ -16,6 +16,15 @@
 #include <smp.h>
 #include <sclp.h>
 #include <s390x/vm.h>
+#include <s390x/stsi.h>
+
+static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
+
+static int max_nested_lvl;
+static int number_of_cpus;
+static int max_cpus = 1;
+static int arch_topo_lvl[CPU_TOPOLOGY_MAX_LEVEL];	/* Topology level defined by architecture */
+static int stsi_nested_lvl[CPU_TOPOLOGY_MAX_LEVEL];	/* Topology nested level reported in STSI */
 
 #define PTF_REQ_HORIZONTAL	0
 #define PTF_REQ_VERTICAL	1
@@ -83,11 +92,230 @@ end:
 	report_prefix_pop();
 }
 
+/*
+ * stsi_check_maxcpus
+ * @info: Pointer to the stsi information
+ *
+ * The product of the numbers of containers per level
+ * is the maximum number of CPU allowed by the machine.
+ */
+static void stsi_check_maxcpus(struct sysinfo_15_1_x *info)
+{
+	int n, i;
+
+	report_prefix_push("maximum cpus");
+
+	for (i = 0, n = 1; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
+		report_info("Mag%d: %d", CPU_TOPOLOGY_MAX_LEVEL - i, info->mag[i]);
+		n *= info->mag[i] ? info->mag[i] : 1;
+	}
+	report(n == max_cpus, "Maximum CPUs %d expected %d", n, max_cpus);
+
+	report_prefix_pop();
+}
+
+/*
+ * stsi_check_tle_coherency
+ * @info: Pointer to the stsi information
+ * @sel2: Topology level to check.
+ *
+ * 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, int sel2)
+{
+	struct topology_container *tc, *end;
+	struct topology_core *cpus;
+	int n = 0;
+	int i;
+
+	report_prefix_push("TLE coherency");
+
+	tc = (void *)&info->tle[0];
+	end = (struct topology_container *)((unsigned long)info + info->length);
+
+	for (i = 0; i < CPU_TOPOLOGY_MAX_LEVEL; i++)
+		stsi_nested_lvl[i] = 0;
+
+	while (tc < end) {
+		if (tc->nl > 5) {
+			report_abort("Unexpected TL Entry: tle->nl: %d", tc->nl);
+			return;
+		}
+		if (tc->nl == 0) {
+			cpus = (struct topology_core *)tc;
+			n += cpus_in_tle_mask(cpus->mask);
+			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);
+		}
+
+		stsi_nested_lvl[tc->nl]++;
+		report_info("level %d: lvl: %d id: %d cnt: %d",
+			    tc->nl, tc->nl, tc->id, stsi_nested_lvl[tc->nl]);
+
+		/* trick: CPU TLEs are twice the size of containers TLE */
+		if (tc->nl == 0)
+			tc++;
+		tc++;
+	}
+	report(n == number_of_cpus, "Number of CPUs  : %d expect %d", n, number_of_cpus);
+	/*
+	 * For KVM we accept
+	 * - only 1 type of CPU
+	 * - only horizontal topology
+	 * - only dedicated CPUs
+	 * This leads to expect the number of entries of level 0 CPU
+	 * Topology Level Entry (TLE) to be:
+	 * 1 + (number_of_cpus - 1)  / arch_topo_lvl[0]
+	 *
+	 * For z/VM or LPAR this number can only be greater if different
+	 * polarity, CPU types because there may be a nested level 0 CPU TLE
+	 * for each of the CPU/polarity/sharing types in a level 1 container TLE.
+	 */
+	n =  (number_of_cpus - 1)  / arch_topo_lvl[0];
+	report(stsi_nested_lvl[0] >=  n + 1,
+	       "CPU Type TLE    : %d expect %d", stsi_nested_lvl[0], n + 1);
+
+	/* For each level found in STSI */
+	for (i = 1; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
+		/*
+		 * For non QEMU/KVM hypervizor the concatanation of the levels
+		 * above level 1 are architecture dependent.
+		 * Skip these checks.
+		 */
+		if (!vm_is_kvm() && sel2 != 2)
+			continue;
+
+		/* For QEMU/KVM we expect a simple calculation */
+		if (sel2 > i) {
+			report(stsi_nested_lvl[i] ==  n + 1,
+			       "Container TLE  %d: %d expect %d", i, stsi_nested_lvl[i], n + 1);
+			n /= arch_topo_lvl[i];
+		}
+	}
+
+	report_prefix_pop();
+}
+
+/*
+ * check_sysinfo_15_1_x
+ * @info: pointer to the STSI info structure
+ * @sel2: the selector giving the topology level to check
+ *
+ * Check if the validity of the STSI instruction and then
+ * calls specific checks on the information buffer.
+ */
+static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info, int sel2)
+{
+	int ret;
+
+	report_prefix_pushf("15_1_%d", sel2);
+
+	ret = stsi(pagebuf, 15, 1, sel2);
+	if (max_nested_lvl >= sel2)
+		report(!ret, "Valid stsi instruction");
+	else
+		report(ret, "Invalid stsi instruction");
+	if (ret)
+		goto end;
+
+	stsi_check_maxcpus(info);
+	stsi_check_tle_coherency(info, sel2);
+
+end:
+	report_prefix_pop();
+}
+
+/*
+ * test_stsi
+ *
+ * Retrieves the maximum nested topology level supported by the architecture
+ * and the number of CPUs.
+ * Calls the checking for the STSI instruction in sel2 reverse level order
+ * from 6 (CPU_TOPOLOGY_MAX_LEVEL) to 2 to have the most interesting level,
+ * the one triggering a topology-change-report-pending condition, level 2,
+ * at the end of the report.
+ *
+ */
+static void test_stsi(void)
+{
+	int sel2;
+
+	max_nested_lvl = sclp_get_stsi_parm();
+	/* If the STSI parm is 0, the Maximum Nesting for STSI is 2 */
+	if (!max_nested_lvl)
+		max_nested_lvl = 2;
+	report_info("SCLP maximum nested level : %d", max_nested_lvl);
+
+	number_of_cpus = sclp_get_cpu_num();
+	report_info("SCLP number of CPU: %d", number_of_cpus);
+
+	/* STSI selector 2 can takes values between 2 and 6 */
+	for (sel2 = 6; sel2 >= 2; sel2--)
+		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf, sel2);
+}
+
+/*
+ * parse_topology_args
+ * @argc: number of arguments
+ * @argv: argument array
+ *
+ * This function initialize the architecture topology levels
+ * which should be the same as the one provided by the hypervisor.
+ *
+ * We use the current names found in IBM/Z literature, Linux and QEMU:
+ * cores, sockets/packages, books, drawers and nodes to facilitate the
+ * human machine interface but store the result in a machine abstract
+ * array of architecture topology levels.
+ * Note that when QEMU uses socket as a name for the topology level 1
+ * Linux uses package or physical_package.
+ */
+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");
+			arch_topo_lvl[0] = atol(argv[i]);
+		} else if (!strcmp("-s", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-s (sockets) needs a parameter");
+			arch_topo_lvl[1] = atol(argv[i]);
+		} else if (!strcmp("-b", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-b (books) needs a parameter");
+			arch_topo_lvl[2] = atol(argv[i]);
+		} else if (!strcmp("-d", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-d (drawers) needs a parameter");
+			arch_topo_lvl[3] = atol(argv[i]);
+		} else if (!strcmp("-n", argv[i])) {
+			i++;
+			if (i >= argc)
+				report_abort("-n (nodes) needs a parameter");
+			arch_topo_lvl[4] = atol(argv[i]);
+		}
+	}
+
+	for (i = 0; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
+		if (!arch_topo_lvl[i])
+			arch_topo_lvl[i] = 1;
+		max_cpus *= arch_topo_lvl[i];
+	}
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
 } tests[] = {
 	{ "PTF", test_ptf},
+	{ "STSI", test_stsi},
 	{ NULL, NULL }
 };
 
@@ -97,6 +325,8 @@ 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;
@@ -109,6 +339,7 @@ int main(int argc, char *argv[])
 		tests[i].func();
 		report_prefix_pop();
 	}
+
 end:
 	report_prefix_pop();
 	return report_summary();
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index e2d3e6a5..bc19b5b6 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -125,3 +125,4 @@ extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=2 -devi
 
 [topology]
 file = topology.elf
+extra_params=-smp 5,drawers=2,books=3,sockets=4,cores=4,maxcpus=16 -append "-d 2 -b 3 -s 4 -c 4"
-- 
2.27.0

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

[Janosch Frank]

On 1/10/22 14:37, Pierre Morel wrote:
> STSI with function code 15 is used to store the CPU configuration
> topology.
> 
> We check :
> - if the topology stored is coherent between the QEMU -smp
>    parameters and kernel parameters.
> - the number of CPUs
> - the maximum number of CPUs
> - the number of containers of each levels for every STSI(15.1.x)
>    instruction allowed by the machine.

The full review of this will take some time.

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/stsi.h    |  44 +++++++++
>   s390x/topology.c    | 231 ++++++++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg |   1 +
>   3 files changed, 276 insertions(+)
> 
> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
> index 02cc94a6..e3fc7ac0 100644
> --- a/lib/s390x/stsi.h
> +++ b/lib/s390x/stsi.h
> @@ -29,4 +29,48 @@ struct sysinfo_3_2_2 {
>   	uint8_t ext_names[8][256];
>   };
>   
> +struct topology_core {
> +	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;
> +};
> +
> +struct topology_container {
> +	uint8_t nl;
> +	uint8_t reserved[6];
> +	uint8_t id;
> +};
> +
> +union topology_entry {
> +	uint8_t nl;
> +	struct topology_core cpu;
> +	struct topology_container container;
> +};
> +
> +#define CPU_TOPOLOGY_MAX_LEVEL 6
> +struct sysinfo_15_1_x {
> +	uint8_t reserved0[2];
> +	uint16_t length;
> +	uint8_t mag[CPU_TOPOLOGY_MAX_LEVEL];
> +	uint8_t reserved10;

reserved0a?

> +	uint8_t mnest;
> +	uint8_t reserved12[4];

reserved0c?

> +	union topology_entry tle[0];
> +};
> +
> +static inline int cpus_in_tle_mask(uint64_t val)
> +{
> +	int i, n;
> +
> +	for (i = 0, n = 0; i < 64; i++, val >>= 1)
> +		if (val & 0x01)
> +			n++;
> +	return n;
> +}
> +
>   #endif  /* _S390X_STSI_H_ */
> diff --git a/s390x/topology.c b/s390x/topology.c
> index a227555e..d06e7c4d 100644
> --- a/s390x/topology.c
> +++ b/s390x/topology.c
> @@ -16,6 +16,15 @@
>   #include <smp.h>
>   #include <sclp.h>
>   #include <s390x/vm.h>
> +#include <s390x/stsi.h>
> +
> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
> +
> +static int max_nested_lvl;
> +static int number_of_cpus;
> +static int max_cpus = 1;
> +static int arch_topo_lvl[CPU_TOPOLOGY_MAX_LEVEL];	/* Topology level defined by architecture */
> +static int stsi_nested_lvl[CPU_TOPOLOGY_MAX_LEVEL];	/* Topology nested level reported in STSI */
>   
>   #define PTF_REQ_HORIZONTAL	0
>   #define PTF_REQ_VERTICAL	1
> @@ -83,11 +92,230 @@ end:
>   	report_prefix_pop();
>   }
>   
> +/*
> + * stsi_check_maxcpus
> + * @info: Pointer to the stsi information
> + *
> + * The product of the numbers of containers per level
> + * is the maximum number of CPU allowed by the machine.
> + */
> +static void stsi_check_maxcpus(struct sysinfo_15_1_x *info)
> +{
> +	int n, i;
> +
> +	report_prefix_push("maximum cpus");
> +
> +	for (i = 0, n = 1; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
> +		report_info("Mag%d: %d", CPU_TOPOLOGY_MAX_LEVEL - i, info->mag[i]);
> +		n *= info->mag[i] ? info->mag[i] : 1;
> +	}
> +	report(n == max_cpus, "Maximum CPUs %d expected %d", n, max_cpus);
> +
> +	report_prefix_pop();
> +}
> +
> +/*
> + * stsi_check_tle_coherency
> + * @info: Pointer to the stsi information
> + * @sel2: Topology level to check.
> + *
> + * 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, int sel2)
> +{
> +	struct topology_container *tc, *end;
> +	struct topology_core *cpus;
> +	int n = 0;
> +	int i;
> +
> +	report_prefix_push("TLE coherency");
> +
> +	tc = (void *)&info->tle[0];

tc = &info->tle[0].container ?

> +	end = (struct topology_container *)((unsigned long)info + info->length);
> +
> +	for (i = 0; i < CPU_TOPOLOGY_MAX_LEVEL; i++)
> +		stsi_nested_lvl[i] = 0;
> +
> +	while (tc < end) {
> +		if (tc->nl > 5) {
> +			report_abort("Unexpected TL Entry: tle->nl: %d", tc->nl);
> +			return;
> +		}
> +		if (tc->nl == 0) {
> +			cpus = (struct topology_core *)tc;
> +			n += cpus_in_tle_mask(cpus->mask);
> +			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);
> +		}
> +
> +		stsi_nested_lvl[tc->nl]++;
> +		report_info("level %d: lvl: %d id: %d cnt: %d",
> +			    tc->nl, tc->nl, tc->id, stsi_nested_lvl[tc->nl]);
> +
> +		/* trick: CPU TLEs are twice the size of containers TLE */
> +		if (tc->nl == 0)
> +			tc++;
> +		tc++;
> +	}
> +	report(n == number_of_cpus, "Number of CPUs  : %d expect %d", n, number_of_cpus);
> +	/*
> +	 * For KVM we accept
> +	 * - only 1 type of CPU
> +	 * - only horizontal topology
> +	 * - only dedicated CPUs
> +	 * This leads to expect the number of entries of level 0 CPU
> +	 * Topology Level Entry (TLE) to be:
> +	 * 1 + (number_of_cpus - 1)  / arch_topo_lvl[0]
> +	 *
> +	 * For z/VM or LPAR this number can only be greater if different
> +	 * polarity, CPU types because there may be a nested level 0 CPU TLE
> +	 * for each of the CPU/polarity/sharing types in a level 1 container TLE.
> +	 */
> +	n =  (number_of_cpus - 1)  / arch_topo_lvl[0];
> +	report(stsi_nested_lvl[0] >=  n + 1,
> +	       "CPU Type TLE    : %d expect %d", stsi_nested_lvl[0], n + 1);
> +
> +	/* For each level found in STSI */
> +	for (i = 1; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
> +		/*
> +		 * For non QEMU/KVM hypervizor the concatanation of the levels

hypervisor

concatenation

> +		 * above level 1 are architecture dependent.
> +		 * Skip these checks.
> +		 */
> +		if (!vm_is_kvm() && sel2 != 2)
> +			continue;
> +
> +		/* For QEMU/KVM we expect a simple calculation */
> +		if (sel2 > i) {
> +			report(stsi_nested_lvl[i] ==  n + 1,
> +			       "Container TLE  %d: %d expect %d", i, stsi_nested_lvl[i], n + 1);
> +			n /= arch_topo_lvl[i];
> +		}
> +	}
> +
> +	report_prefix_pop();
> +}
> +
> +/*
> + * check_sysinfo_15_1_x
> + * @info: pointer to the STSI info structure
> + * @sel2: the selector giving the topology level to check
> + *
> + * Check if the validity of the STSI instruction and then
> + * calls specific checks on the information buffer.
> + */
> +static void check_sysinfo_15_1_x(struct sysinfo_15_1_x *info, int sel2)
> +{
> +	int ret;
> +
> +	report_prefix_pushf("15_1_%d", sel2);
> +
> +	ret = stsi(pagebuf, 15, 1, sel2);
> +	if (max_nested_lvl >= sel2)
> +		report(!ret, "Valid stsi instruction");
> +	else
> +		report(ret, "Invalid stsi instruction");
> +	if (ret)
> +		goto end;
> +
> +	stsi_check_maxcpus(info);
> +	stsi_check_tle_coherency(info, sel2);
> +
> +end:
> +	report_prefix_pop();
> +}
> +
> +/*
> + * test_stsi
> + *
> + * Retrieves the maximum nested topology level supported by the architecture
> + * and the number of CPUs.
> + * Calls the checking for the STSI instruction in sel2 reverse level order
> + * from 6 (CPU_TOPOLOGY_MAX_LEVEL) to 2 to have the most interesting level,
> + * the one triggering a topology-change-report-pending condition, level 2,
> + * at the end of the report.
> + *
> + */
> +static void test_stsi(void)
> +{
> +	int sel2;
> +
> +	max_nested_lvl = sclp_get_stsi_parm();
> +	/* If the STSI parm is 0, the Maximum Nesting for STSI is 2 */
> +	if (!max_nested_lvl)
> +		max_nested_lvl = 2;
> +	report_info("SCLP maximum nested level : %d", max_nested_lvl);
> +
> +	number_of_cpus = sclp_get_cpu_num();
> +	report_info("SCLP number of CPU: %d", number_of_cpus);
> +
> +	/* STSI selector 2 can takes values between 2 and 6 */
> +	for (sel2 = 6; sel2 >= 2; sel2--)
> +		check_sysinfo_15_1_x((struct sysinfo_15_1_x *)pagebuf, sel2);
> +}
> +
> +/*
> + * parse_topology_args
> + * @argc: number of arguments
> + * @argv: argument array
> + *
> + * This function initialize the architecture topology levels
> + * which should be the same as the one provided by the hypervisor.
> + *
> + * We use the current names found in IBM/Z literature, Linux and QEMU:
> + * cores, sockets/packages, books, drawers and nodes to facilitate the
> + * human machine interface but store the result in a machine abstract
> + * array of architecture topology levels.
> + * Note that when QEMU uses socket as a name for the topology level 1
> + * Linux uses package or physical_package.
> + */
> +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");
> +			arch_topo_lvl[0] = atol(argv[i]);
> +		} else if (!strcmp("-s", argv[i])) {
> +			i++;
> +			if (i >= argc)
> +				report_abort("-s (sockets) needs a parameter");
> +			arch_topo_lvl[1] = atol(argv[i]);
> +		} else if (!strcmp("-b", argv[i])) {
> +			i++;
> +			if (i >= argc)
> +				report_abort("-b (books) needs a parameter");
> +			arch_topo_lvl[2] = atol(argv[i]);
> +		} else if (!strcmp("-d", argv[i])) {
> +			i++;
> +			if (i >= argc)
> +				report_abort("-d (drawers) needs a parameter");
> +			arch_topo_lvl[3] = atol(argv[i]);
> +		} else if (!strcmp("-n", argv[i])) {
> +			i++;
> +			if (i >= argc)
> +				report_abort("-n (nodes) needs a parameter");
> +			arch_topo_lvl[4] = atol(argv[i]);
> +		}
> +	}
> +
> +	for (i = 0; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
> +		if (!arch_topo_lvl[i])
> +			arch_topo_lvl[i] = 1;
> +		max_cpus *= arch_topo_lvl[i];
> +	}
> +}
> +
>   static struct {
>   	const char *name;
>   	void (*func)(void);
>   } tests[] = {
>   	{ "PTF", test_ptf},
> +	{ "STSI", test_stsi},
>   	{ NULL, NULL }
>   };
>   
> @@ -97,6 +325,8 @@ 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;
> @@ -109,6 +339,7 @@ int main(int argc, char *argv[])
>   		tests[i].func();
>   		report_prefix_pop();
>   	}
> +
>   end:
>   	report_prefix_pop();
>   	return report_summary();
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index e2d3e6a5..bc19b5b6 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -125,3 +125,4 @@ extra_params = -smp 1,maxcpus=3 -cpu qemu -device qemu-s390x-cpu,core-id=2 -devi
>   
>   [topology]
>   file = topology.elf
> +extra_params=-smp 5,drawers=2,books=3,sockets=4,cores=4,maxcpus=16 -append "-d 2 -b 3 -s 4 -c 4"
> 

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

[Pierre Morel]

On 1/11/22 14:30, Janosch Frank wrote:
> On 1/10/22 14:37, Pierre Morel wrote:
>> STSI with function code 15 is used to store the CPU configuration
>> topology.
>>
>> We check :
>> - if the topology stored is coherent between the QEMU -smp
>>    parameters and kernel parameters.
>> - the number of CPUs
>> - the maximum number of CPUs
>> - the number of containers of each levels for every STSI(15.1.x)
>>    instruction allowed by the machine.
> 
> The full review of this will take some time.
> 
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/stsi.h    |  44 +++++++++
>>   s390x/topology.c    | 231 ++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg |   1 +
>>   3 files changed, 276 insertions(+)
>>
>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h
>> index 02cc94a6..e3fc7ac0 100644
>> --- a/lib/s390x/stsi.h
>> +++ b/lib/s390x/stsi.h
>> @@ -29,4 +29,48 @@ struct sysinfo_3_2_2 {
>>       uint8_t ext_names[8][256];
>>   };
>> +struct topology_core {
>> +    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;
>> +};
>> +
>> +struct topology_container {
>> +    uint8_t nl;
>> +    uint8_t reserved[6];
>> +    uint8_t id;
>> +};
>> +
>> +union topology_entry {
>> +    uint8_t nl;
>> +    struct topology_core cpu;
>> +    struct topology_container container;
>> +};
>> +
>> +#define CPU_TOPOLOGY_MAX_LEVEL 6
>> +struct sysinfo_15_1_x {
>> +    uint8_t reserved0[2];
>> +    uint16_t length;
>> +    uint8_t mag[CPU_TOPOLOGY_MAX_LEVEL];
>> +    uint8_t reserved10;
> 
> reserved0a?

OK

> 
>> +    uint8_t mnest;
>> +    uint8_t reserved12[4];
> 
> reserved0c?

OK

> 
>> +    union topology_entry tle[0];

...snip...

>> +static void stsi_check_tle_coherency(struct sysinfo_15_1_x *info, int 
>> sel2)
>> +{
>> +    struct topology_container *tc, *end;
>> +    struct topology_core *cpus;
>> +    int n = 0;
>> +    int i;
>> +
>> +    report_prefix_push("TLE coherency");
>> +
>> +    tc = (void *)&info->tle[0];
> 
> tc = &info->tle[0].container ?

Yes, clearly better than a cast.

> 
>> +    end = (struct topology_container *)((unsigned long)info + 
...snip...
>> +    /* For each level found in STSI */
>> +    for (i = 1; i < CPU_TOPOLOGY_MAX_LEVEL; i++) {
>> +        /*
>> +         * For non QEMU/KVM hypervizor the concatanation of the levels
> 
> hypervisor
> 
> concatenation

Yes, thanks.

> 
>> +         * above level 1 are architecture dependent.

...snip...

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen