[kvm-unit-tests PATCH v2 0/3] s390x: cpumodel: Add sclp checks

[kvm-unit-tests PATCH v2 0/3] s390x: cpumodel: Add sclp checks

[Janosch Frank]

SCLP facilities have been a bit overlooked in cpumodel tests and have
recently caused some headaches. So let's extend the tests and the
library with a bit of sclp feature checking.

Based on the uv_host branch / patches.

v2:
	* Check bit instead of mask
	* Squashed fmt 2 and 4 patches into one

Janosch Frank (3):
  s390x: sclp: Only fetch read info byte 134 if cpu entries are above it
  lib: s390x: sclp: Extend feature probing
  s390x: cpumodel: FMT2 and FMT4 SCLP test

 lib/s390x/sclp.c | 23 +++++++++++++++-
 lib/s390x/sclp.h | 38 ++++++++++++++++++++++++--
 s390x/cpumodel.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 127 insertions(+), 5 deletions(-)

-- 
2.30.2

[kvm-unit-tests PATCH v2 1/3] s390x: sclp: Only fetch read info byte 134 if cpu entries are above it

[Janosch Frank]

The cpu offset tells us where the cpu entries are in the sclp read
info structure.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 lib/s390x/sclp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index 7a9b2c52..f11c2035 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -138,7 +138,8 @@ void sclp_facilities_setup(void)
 	assert(read_info);
 
 	cpu = sclp_get_cpu_entries();
-	sclp_facilities.has_diag318 = read_info->byte_134_diag318;
+	if (read_info->offset_cpu > 134)
+		sclp_facilities.has_diag318 = read_info->byte_134_diag318;
 	for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
 		/*
 		 * The logic for only reading the facilities from the
-- 
2.30.2

[kvm-unit-tests PATCH v2 2/3] lib: s390x: sclp: Extend feature probing

[Janosch Frank]

Lets grab more of the feature bits from SCLP read info so we can use
them in the cpumodel tests.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/sclp.c | 20 ++++++++++++++++++++
 lib/s390x/sclp.h | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index f11c2035..291924b0 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -129,6 +129,13 @@ CPUEntry *sclp_get_cpu_entries(void)
 	return (CPUEntry *)(_read_info + read_info->offset_cpu);
 }
 
+static bool sclp_feat_check(int byte, int bit)
+{
+	uint8_t *rib = (uint8_t *)read_info;
+
+	return !!(rib[byte] & (0x80 >> bit));
+}
+
 void sclp_facilities_setup(void)
 {
 	unsigned short cpu0_addr = stap();
@@ -140,6 +147,14 @@ void sclp_facilities_setup(void)
 	cpu = sclp_get_cpu_entries();
 	if (read_info->offset_cpu > 134)
 		sclp_facilities.has_diag318 = read_info->byte_134_diag318;
+	sclp_facilities.has_gsls = sclp_feat_check(85, SCLP_FEAT_85_BIT_GSLS);
+	sclp_facilities.has_kss = sclp_feat_check(98, SCLP_FEAT_98_BIT_KSS);
+	sclp_facilities.has_cmma = sclp_feat_check(116, SCLP_FEAT_116_BIT_CMMA);
+	sclp_facilities.has_64bscao = sclp_feat_check(116, SCLP_FEAT_116_BIT_64BSCAO);
+	sclp_facilities.has_esca = sclp_feat_check(116, SCLP_FEAT_116_BIT_ESCA);
+	sclp_facilities.has_ibs = sclp_feat_check(117, SCLP_FEAT_117_BIT_IBS);
+	sclp_facilities.has_pfmfi = sclp_feat_check(117, SCLP_FEAT_117_BIT_PFMFI);
+
 	for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
 		/*
 		 * The logic for only reading the facilities from the
@@ -150,6 +165,11 @@ void sclp_facilities_setup(void)
 		 */
 		if (cpu->address == cpu0_addr) {
 			sclp_facilities.has_sief2 = cpu->feat_sief2;
+			sclp_facilities.has_skeyi = cpu->feat_skeyi;
+			sclp_facilities.has_siif = cpu->feat_siif;
+			sclp_facilities.has_sigpif = cpu->feat_sigpif;
+			sclp_facilities.has_ib = cpu->feat_ib;
+			sclp_facilities.has_cei = cpu->feat_cei;
 			break;
 		}
 	}
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 85231333..115bd2fa 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -94,9 +94,19 @@ typedef struct CPUEntry {
 	uint8_t reserved0;
 	uint8_t : 4;
 	uint8_t feat_sief2 : 1;
+	uint8_t feat_skeyi : 1;
+	uint8_t : 2;
+	uint8_t : 2;
+	uint8_t feat_gpere : 1;
+	uint8_t feat_siif : 1;
+	uint8_t feat_sigpif : 1;
 	uint8_t : 3;
-	uint8_t features_res2 [SCCB_CPU_FEATURE_LEN - 1];
-	uint8_t reserved2[6];
+	uint8_t reserved2[3];
+	uint8_t : 2;
+	uint8_t feat_ib : 1;
+	uint8_t feat_cei : 1;
+	uint8_t : 4;
+	uint8_t reserved3[6];
 	uint8_t type;
 	uint8_t reserved1;
 } __attribute__((packed)) CPUEntry;
@@ -105,10 +115,32 @@ extern struct sclp_facilities sclp_facilities;
 
 struct sclp_facilities {
 	uint64_t has_sief2 : 1;
+	uint64_t has_skeyi : 1;
+	uint64_t has_gpere : 1;
+	uint64_t has_siif : 1;
+	uint64_t has_sigpif : 1;
+	uint64_t has_ib : 1;
+	uint64_t has_cei : 1;
+
 	uint64_t has_diag318 : 1;
-	uint64_t : 62;
+	uint64_t has_gsls : 1;
+	uint64_t has_cmma : 1;
+	uint64_t has_64bscao : 1;
+	uint64_t has_esca : 1;
+	uint64_t has_kss : 1;
+	uint64_t has_pfmfi : 1;
+	uint64_t has_ibs : 1;
+	uint64_t : 64 - 15;
 };
 
+#define SCLP_FEAT_85_BIT_GSLS		7
+#define SCLP_FEAT_98_BIT_KSS		0
+#define SCLP_FEAT_116_BIT_64BSCAO	7
+#define SCLP_FEAT_116_BIT_CMMA		6
+#define SCLP_FEAT_116_BIT_ESCA		3
+#define SCLP_FEAT_117_BIT_PFMFI		6
+#define SCLP_FEAT_117_BIT_IBS		5
+
 typedef struct ReadInfo {
 	SCCBHeader h;
 	uint16_t rnmax;
-- 
2.30.2

[kvm-unit-tests PATCH v2 3/3] s390x: cpumodel: FMT2 and FMT4 SCLP test

[Janosch Frank]

SCLP is also part of the cpumodel, so we need to make sure that the
features indicated via read info / read cpu info are correct.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: David Hildenbrand <david@redhat.com>
---
 s390x/cpumodel.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/s390x/cpumodel.c b/s390x/cpumodel.c
index 4dd8b96f..67bb6543 100644
--- a/s390x/cpumodel.c
+++ b/s390x/cpumodel.c
@@ -2,14 +2,81 @@
 /*
  * Test the known dependencies for facilities
  *
- * Copyright 2019 IBM Corp.
+ * Copyright 2019, 2021 IBM Corp.
  *
  * Authors:
  *    Christian Borntraeger <borntraeger@de.ibm.com>
+ *    Janosch Frank <frankja@linux.ibm.com>
  */
 
 #include <asm/facility.h>
 #include <vm.h>
+#include <sclp.h>
+#include <uv.h>
+#include <asm/uv.h>
+
+static void test_sclp_missing_sief2_implications(void)
+{
+	/* Virtualization related facilities */
+	report(!sclp_facilities.has_64bscao, "!64bscao");
+	report(!sclp_facilities.has_pfmfi, "!pfmfi");
+	report(!sclp_facilities.has_gsls, "!gsls");
+	report(!sclp_facilities.has_cmma, "!cmma");
+	report(!sclp_facilities.has_esca, "!esca");
+	report(!sclp_facilities.has_kss, "!kss");
+	report(!sclp_facilities.has_ibs, "!ibs");
+
+	/* Virtualization related facilities reported via CPU entries */
+	report(!sclp_facilities.has_sigpif, "!sigpif");
+	report(!sclp_facilities.has_sief2, "!sief2");
+	report(!sclp_facilities.has_skeyi, "!skeyi");
+	report(!sclp_facilities.has_siif, "!siif");
+	report(!sclp_facilities.has_cei, "!cei");
+	report(!sclp_facilities.has_ib, "!ib");
+}
+
+static void test_sclp_features_fmt4(void)
+{
+	/*
+	 * STFLE facilities are handled by the Ultravisor but SCLP
+	 * facilities are advertised by the hypervisor.
+	 */
+	report_prefix_push("PV guest implies");
+
+	/* General facilities */
+	report(!sclp_facilities.has_diag318, "!diag318");
+
+	/*
+	 * Virtualization related facilities, all of which are
+	 * unavailable because there's no virtualization support in a
+	 * protected guest.
+	 */
+	test_sclp_missing_sief2_implications();
+
+	report_prefix_pop();
+}
+
+static void test_sclp_features_fmt2(void)
+{
+	if (sclp_facilities.has_sief2)
+		return;
+
+	report_prefix_push("!sief2 implies");
+	test_sclp_missing_sief2_implications();
+	report_prefix_pop();
+}
+
+static void test_sclp_features(void)
+{
+	report_prefix_push("sclp");
+
+	if (uv_os_is_guest())
+		test_sclp_features_fmt4();
+	else
+		test_sclp_features_fmt2();
+
+	report_prefix_pop();
+}
 
 static struct {
 	int facility;
@@ -60,6 +127,8 @@ int main(void)
 	}
 	report_prefix_pop();
 
+	test_sclp_features();
+
 	report_prefix_pop();
 	return report_summary();
 }
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2 2/3] lib: s390x: sclp: Extend feature probing

[David Hildenbrand]

On 19.05.21 10:26, Janosch Frank wrote:
> Lets grab more of the feature bits from SCLP read info so we can use
> them in the cpumodel tests.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   lib/s390x/sclp.c | 20 ++++++++++++++++++++
>   lib/s390x/sclp.h | 38 +++++++++++++++++++++++++++++++++++---
>   2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index f11c2035..291924b0 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -129,6 +129,13 @@ CPUEntry *sclp_get_cpu_entries(void)
>   	return (CPUEntry *)(_read_info + read_info->offset_cpu);
>   }
>   
> +static bool sclp_feat_check(int byte, int bit)
> +{
> +	uint8_t *rib = (uint8_t *)read_info;
> +
> +	return !!(rib[byte] & (0x80 >> bit));
> +}
> +
>   void sclp_facilities_setup(void)
>   {
>   	unsigned short cpu0_addr = stap();
> @@ -140,6 +147,14 @@ void sclp_facilities_setup(void)
>   	cpu = sclp_get_cpu_entries();
>   	if (read_info->offset_cpu > 134)
>   		sclp_facilities.has_diag318 = read_info->byte_134_diag318;
> +	sclp_facilities.has_gsls = sclp_feat_check(85, SCLP_FEAT_85_BIT_GSLS);
> +	sclp_facilities.has_kss = sclp_feat_check(98, SCLP_FEAT_98_BIT_KSS);
> +	sclp_facilities.has_cmma = sclp_feat_check(116, SCLP_FEAT_116_BIT_CMMA);
> +	sclp_facilities.has_64bscao = sclp_feat_check(116, SCLP_FEAT_116_BIT_64BSCAO);
> +	sclp_facilities.has_esca = sclp_feat_check(116, SCLP_FEAT_116_BIT_ESCA);
> +	sclp_facilities.has_ibs = sclp_feat_check(117, SCLP_FEAT_117_BIT_IBS);
> +	sclp_facilities.has_pfmfi = sclp_feat_check(117, SCLP_FEAT_117_BIT_PFMFI);
> +
>   	for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
>   		/*
>   		 * The logic for only reading the facilities from the
> @@ -150,6 +165,11 @@ void sclp_facilities_setup(void)
>   		 */
>   		if (cpu->address == cpu0_addr) {
>   			sclp_facilities.has_sief2 = cpu->feat_sief2;
> +			sclp_facilities.has_skeyi = cpu->feat_skeyi;
> +			sclp_facilities.has_siif = cpu->feat_siif;
> +			sclp_facilities.has_sigpif = cpu->feat_sigpif;
> +			sclp_facilities.has_ib = cpu->feat_ib;
> +			sclp_facilities.has_cei = cpu->feat_cei;
>   			break;
>   		}
>   	}
> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> index 85231333..115bd2fa 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -94,9 +94,19 @@ typedef struct CPUEntry {
>   	uint8_t reserved0;
>   	uint8_t : 4;
>   	uint8_t feat_sief2 : 1;
> +	uint8_t feat_skeyi : 1;
> +	uint8_t : 2;
> +	uint8_t : 2;
> +	uint8_t feat_gpere : 1;
> +	uint8_t feat_siif : 1;
> +	uint8_t feat_sigpif : 1;
>   	uint8_t : 3;
> -	uint8_t features_res2 [SCCB_CPU_FEATURE_LEN - 1];
> -	uint8_t reserved2[6];
> +	uint8_t reserved2[3];
> +	uint8_t : 2;
> +	uint8_t feat_ib : 1;
> +	uint8_t feat_cei : 1;
> +	uint8_t : 4;
> +	uint8_t reserved3[6];
>   	uint8_t type;
>   	uint8_t reserved1;
>   } __attribute__((packed)) CPUEntry;
> @@ -105,10 +115,32 @@ extern struct sclp_facilities sclp_facilities;
>   
>   struct sclp_facilities {
>   	uint64_t has_sief2 : 1;
> +	uint64_t has_skeyi : 1;
> +	uint64_t has_gpere : 1;
> +	uint64_t has_siif : 1;
> +	uint64_t has_sigpif : 1;
> +	uint64_t has_ib : 1;
> +	uint64_t has_cei : 1;
> +
>   	uint64_t has_diag318 : 1;
> -	uint64_t : 62;
> +	uint64_t has_gsls : 1;
> +	uint64_t has_cmma : 1;
> +	uint64_t has_64bscao : 1;
> +	uint64_t has_esca : 1;
> +	uint64_t has_kss : 1;
> +	uint64_t has_pfmfi : 1;
> +	uint64_t has_ibs : 1;
> +	uint64_t : 64 - 15;
>   };
>   
> +#define SCLP_FEAT_85_BIT_GSLS		7
> +#define SCLP_FEAT_98_BIT_KSS		0
> +#define SCLP_FEAT_116_BIT_64BSCAO	7
> +#define SCLP_FEAT_116_BIT_CMMA		6
> +#define SCLP_FEAT_116_BIT_ESCA		3
> +#define SCLP_FEAT_117_BIT_PFMFI		6
> +#define SCLP_FEAT_117_BIT_IBS		5
> +
>   typedef struct ReadInfo {
>   	SCCBHeader h;
>   	uint16_t rnmax;
> 

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH v2 2/3] lib: s390x: sclp: Extend feature probing

[Cornelia Huck]

On Wed, 19 May 2021 08:26:47 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Lets grab more of the feature bits from SCLP read info so we can use

s/Lets/Let's/ :)

> them in the cpumodel tests.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  lib/s390x/sclp.c | 20 ++++++++++++++++++++
>  lib/s390x/sclp.h | 38 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 55 insertions(+), 3 deletions(-)

(...)

Maybe add

/* bit number within a certain byte */

> +#define SCLP_FEAT_85_BIT_GSLS		7
> +#define SCLP_FEAT_98_BIT_KSS		0
> +#define SCLP_FEAT_116_BIT_64BSCAO	7
> +#define SCLP_FEAT_116_BIT_CMMA		6
> +#define SCLP_FEAT_116_BIT_ESCA		3
> +#define SCLP_FEAT_117_BIT_PFMFI		6
> +#define SCLP_FEAT_117_BIT_IBS		5
> +
>  typedef struct ReadInfo {
>  	SCCBHeader h;
>  	uint16_t rnmax;

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

Re: [kvm-unit-tests PATCH v2 3/3] s390x: cpumodel: FMT2 and FMT4 SCLP test

[Cornelia Huck]

On Wed, 19 May 2021 08:26:48 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> SCLP is also part of the cpumodel, so we need to make sure that the
> features indicated via read info / read cpu info are correct.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> ---
>  s390x/cpumodel.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)

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

Re: [kvm-unit-tests PATCH v2 2/3] lib: s390x: sclp: Extend feature probing

[Janosch Frank]

On 5/19/21 12:17 PM, Cornelia Huck wrote:
> On Wed, 19 May 2021 08:26:47 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Lets grab more of the feature bits from SCLP read info so we can use
> 
> s/Lets/Let's/ :)

Sigh

> 
>> them in the cpumodel tests.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>  lib/s390x/sclp.c | 20 ++++++++++++++++++++
>>  lib/s390x/sclp.h | 38 +++++++++++++++++++++++++++++++++++---
>>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> (...)
> 
> Maybe add
> 
> /* bit number within a certain byte */

Sure

> 
>> +#define SCLP_FEAT_85_BIT_GSLS		7
>> +#define SCLP_FEAT_98_BIT_KSS		0
>> +#define SCLP_FEAT_116_BIT_64BSCAO	7
>> +#define SCLP_FEAT_116_BIT_CMMA		6
>> +#define SCLP_FEAT_116_BIT_ESCA		3
>> +#define SCLP_FEAT_117_BIT_PFMFI		6
>> +#define SCLP_FEAT_117_BIT_IBS		5
>> +
>>  typedef struct ReadInfo {
>>  	SCCBHeader h;
>>  	uint16_t rnmax;
> 
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> 

Thanks!