* [kvm-unit-tests GIT PULL 01/12] s390x: Fix sclp facility bit numbers
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 02/12] s390x: lib: SOP facility query function Claudio Imbrenda
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
sclp_feat_check takes care of adjusting the bit numbering such that they
can be defined as they are in the documentation.
Fixes: 4dd649c8 ("lib: s390x: sclp: Extend feature probing")
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Message-Id: <20220621143015.748290-2-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/sclp.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index e48a5a3d..3488f4d2 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -134,13 +134,13 @@ struct sclp_facilities {
};
/* 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
+#define SCLP_FEAT_85_BIT_GSLS 0
+#define SCLP_FEAT_98_BIT_KSS 7
+#define SCLP_FEAT_116_BIT_64BSCAO 0
+#define SCLP_FEAT_116_BIT_CMMA 1
+#define SCLP_FEAT_116_BIT_ESCA 4
+#define SCLP_FEAT_117_BIT_PFMFI 1
+#define SCLP_FEAT_117_BIT_IBS 2
typedef struct ReadInfo {
SCCBHeader h;
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 02/12] s390x: lib: SOP facility query function
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 01/12] s390x: Fix sclp facility bit numbers Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 03/12] s390x: Rework TEID decoding and usage Claudio Imbrenda
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Add function returning which suppression-on-protection facility is
installed.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-Id: <20220621143015.748290-3-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/facility.h | 21 +++++++++++++++++++++
lib/s390x/sclp.h | 4 ++++
lib/s390x/sclp.c | 2 ++
3 files changed, 27 insertions(+)
diff --git a/lib/s390x/asm/facility.h b/lib/s390x/asm/facility.h
index 49380203..a66fe56a 100644
--- a/lib/s390x/asm/facility.h
+++ b/lib/s390x/asm/facility.h
@@ -12,6 +12,7 @@
#include <asm/facility.h>
#include <asm/arch_def.h>
#include <bitops.h>
+#include <sclp.h>
#define NB_STFL_DOUBLEWORDS 32
extern uint64_t stfl_doublewords[];
@@ -42,4 +43,24 @@ static inline void setup_facilities(void)
stfle(stfl_doublewords, NB_STFL_DOUBLEWORDS);
}
+enum supp_on_prot_facility {
+ SOP_NONE,
+ SOP_BASIC,
+ SOP_ENHANCED_1,
+ SOP_ENHANCED_2,
+};
+
+static inline enum supp_on_prot_facility get_supp_on_prot_facility(void)
+{
+ if (sclp_facilities.has_esop) {
+ if (test_facility(131)) /* side-effect-access facility */
+ return SOP_ENHANCED_2;
+ else
+ return SOP_ENHANCED_1;
+ }
+ if (sclp_facilities.has_sop)
+ return SOP_BASIC;
+ return SOP_NONE;
+}
+
#endif
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 3488f4d2..853529bf 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -123,7 +123,9 @@ struct sclp_facilities {
uint64_t has_cei : 1;
uint64_t has_diag318 : 1;
+ uint64_t has_sop : 1;
uint64_t has_gsls : 1;
+ uint64_t has_esop : 1;
uint64_t has_cmma : 1;
uint64_t has_64bscao : 1;
uint64_t has_esca : 1;
@@ -134,7 +136,9 @@ struct sclp_facilities {
};
/* bit number within a certain byte */
+#define SCLP_FEAT_80_BIT_SOP 2
#define SCLP_FEAT_85_BIT_GSLS 0
+#define SCLP_FEAT_85_BIT_ESOP 6
#define SCLP_FEAT_98_BIT_KSS 7
#define SCLP_FEAT_116_BIT_64BSCAO 0
#define SCLP_FEAT_116_BIT_CMMA 1
diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index b8204c5f..e6017f64 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -152,7 +152,9 @@ 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_sop = sclp_feat_check(80, SCLP_FEAT_80_BIT_SOP);
sclp_facilities.has_gsls = sclp_feat_check(85, SCLP_FEAT_85_BIT_GSLS);
+ sclp_facilities.has_esop = sclp_feat_check(85, SCLP_FEAT_85_BIT_ESOP);
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);
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 03/12] s390x: Rework TEID decoding and usage
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 01/12] s390x: Fix sclp facility bit numbers Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 02/12] s390x: lib: SOP facility query function Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 04/12] s390x: Test TEID values in storage key test Claudio Imbrenda
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
The translation-exception identification (TEID) contains information to
identify the cause of certain program exceptions, including translation
exceptions occurring during dynamic address translation, as well as
protection exceptions.
The meaning of fields in the TEID is complex, depending on the exception
occurring and various potentially installed facilities.
Rework the type describing the TEID, in order to ease decoding.
Change the existing code interpreting the TEID and extend it to take the
installed suppression-on-protection facility into account.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Message-Id: <20220621143015.748290-4-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/interrupt.h | 62 ++++++++++++++++++++++++++++++---------
lib/s390x/fault.h | 30 +++++--------------
lib/s390x/fault.c | 58 +++++++++++++++++++++++-------------
lib/s390x/interrupt.c | 2 +-
s390x/edat.c | 25 +++++++++-------
5 files changed, 108 insertions(+), 69 deletions(-)
diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index d9ab0bd7..fc66a925 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -20,23 +20,57 @@
union teid {
unsigned long val;
- struct {
- unsigned long addr:52;
- unsigned long fetch:1;
- unsigned long store:1;
- unsigned long reserved:6;
- unsigned long acc_list_prot:1;
- /*
- * depending on the exception and the installed facilities,
- * the m field can indicate several different things,
- * including whether the exception was triggered by a MVPG
- * instruction, or whether the addr field is meaningful
- */
- unsigned long m:1;
- unsigned long asce_id:2;
+ union {
+ /* common fields DAT exc & protection exc */
+ struct {
+ uint64_t addr : 52 - 0;
+ uint64_t acc_exc_fetch_store : 54 - 52;
+ uint64_t side_effect_acc : 55 - 54;
+ uint64_t /* reserved */ : 62 - 55;
+ uint64_t asce_id : 64 - 62;
+ };
+ /* DAT exc */
+ struct {
+ uint64_t /* pad */ : 61 - 0;
+ uint64_t dat_move_page : 62 - 61;
+ };
+ /* suppression on protection */
+ struct {
+ uint64_t /* pad */ : 60 - 0;
+ uint64_t sop_acc_list : 61 - 60;
+ uint64_t sop_teid_predictable : 62 - 61;
+ };
+ /* enhanced suppression on protection 2 */
+ struct {
+ uint64_t /* pad */ : 56 - 0;
+ uint64_t esop2_prot_code_0 : 57 - 56;
+ uint64_t /* pad */ : 60 - 57;
+ uint64_t esop2_prot_code_1 : 61 - 60;
+ uint64_t esop2_prot_code_2 : 62 - 61;
+ };
};
};
+enum prot_code {
+ PROT_KEY_OR_LAP,
+ PROT_DAT,
+ PROT_KEY,
+ PROT_ACC_LIST,
+ PROT_LAP,
+ PROT_IEP,
+ PROT_NUM_CODES /* Must always be last */
+};
+
+static inline enum prot_code teid_esop2_prot_code(union teid teid)
+{
+ int code = (teid.esop2_prot_code_0 << 2 |
+ teid.esop2_prot_code_1 << 1 |
+ teid.esop2_prot_code_2);
+
+ assert(code < PROT_NUM_CODES);
+ return (enum prot_code)code;
+}
+
void register_pgm_cleanup_func(void (*f)(void));
void handle_pgm_int(struct stack_frame_int *stack);
void handle_ext_int(struct stack_frame_int *stack);
diff --git a/lib/s390x/fault.h b/lib/s390x/fault.h
index 726da2f0..867997f2 100644
--- a/lib/s390x/fault.h
+++ b/lib/s390x/fault.h
@@ -11,32 +11,16 @@
#define _S390X_FAULT_H_
#include <bitops.h>
+#include <asm/facility.h>
+#include <asm/interrupt.h>
/* Instruction execution prevention, i.e. no-execute, 101 */
-static inline bool prot_is_iep(uint64_t teid)
+static inline bool prot_is_iep(union teid teid)
{
- if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
- return true;
-
- return false;
-}
-
-/* Standard DAT exception, 001 */
-static inline bool prot_is_datp(uint64_t teid)
-{
- if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
- return true;
-
- return false;
-}
-
-/* Low-address protection exception, 100 */
-static inline bool prot_is_lap(uint64_t teid)
-{
- if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
- return true;
-
- return false;
+ if (!test_facility(130))
+ return false;
+ /* IEP installed -> ESOP2 installed */
+ return teid_esop2_prot_code(teid) == PROT_IEP;
}
void print_decode_teid(uint64_t teid);
diff --git a/lib/s390x/fault.c b/lib/s390x/fault.c
index efa62fcb..1cd6e265 100644
--- a/lib/s390x/fault.c
+++ b/lib/s390x/fault.c
@@ -13,35 +13,51 @@
#include <asm/page.h>
#include <fault.h>
-/* Decodes the protection exceptions we'll most likely see */
-static void print_decode_pgm_prot(uint64_t teid)
-{
- if (prot_is_lap(teid)) {
- printf("Type: LAP\n");
- return;
- }
- if (prot_is_iep(teid)) {
- printf("Type: IEP\n");
- return;
- }
+static void print_decode_pgm_prot(union teid teid)
+{
+ switch (get_supp_on_prot_facility()) {
+ case SOP_NONE:
+ case SOP_BASIC:
+ printf("Type: ?\n"); /* modern/relevant machines have ESOP */
+ break;
+ case SOP_ENHANCED_1:
+ if (teid.sop_teid_predictable) {/* implies access list or DAT */
+ if (teid.sop_acc_list)
+ printf("Type: ACC\n");
+ else
+ printf("Type: DAT\n");
+ } else {
+ printf("Type: KEY or LAP\n");
+ }
+ break;
+ case SOP_ENHANCED_2: {
+ static const char * const prot_str[] = {
+ "KEY or LAP",
+ "DAT",
+ "KEY",
+ "ACC",
+ "LAP",
+ "IEP",
+ };
+ _Static_assert(ARRAY_SIZE(prot_str) == PROT_NUM_CODES);
+ int prot_code = teid_esop2_prot_code(teid);
- if (prot_is_datp(teid)) {
- printf("Type: DAT\n");
- return;
+ printf("Type: %s\n", prot_str[prot_code]);
+ }
}
}
-void print_decode_teid(uint64_t teid)
+void print_decode_teid(uint64_t raw_teid)
{
- int asce_id = teid & 3;
+ union teid teid = { .val = raw_teid };
bool dat = lowcore.pgm_old_psw.mask & PSW_MASK_DAT;
printf("Memory exception information:\n");
printf("DAT: %s\n", dat ? "on" : "off");
printf("AS: ");
- switch (asce_id) {
+ switch (teid.asce_id) {
case AS_PRIM:
printf("Primary\n");
break;
@@ -65,10 +81,10 @@ void print_decode_teid(uint64_t teid)
*/
if ((lowcore.pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
lowcore.pgm_int_code == PGM_INT_CODE_SECURE_STOR_VIOLATION) &&
- !test_bit_inv(61, &teid)) {
- printf("Address: %lx, unpredictable\n ", teid & PAGE_MASK);
+ !teid.sop_teid_predictable) {
+ printf("Address: %lx, unpredictable\n ", raw_teid & PAGE_MASK);
return;
}
- printf("TEID: %lx\n", teid);
- printf("Address: %lx\n\n", teid & PAGE_MASK);
+ printf("TEID: %lx\n", raw_teid);
+ printf("Address: %lx\n\n", raw_teid & PAGE_MASK);
}
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 6da20c44..ac3d1ecd 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -77,7 +77,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
break;
case PGM_INT_CODE_PROTECTION:
/* Handling for iep.c test case. */
- if (prot_is_iep(lowcore.trans_exc_id))
+ if (prot_is_iep((union teid) { .val = lowcore.trans_exc_id }))
/*
* We branched to the instruction that caused
* the exception so we can use the return
diff --git a/s390x/edat.c b/s390x/edat.c
index c6c25042..16138397 100644
--- a/s390x/edat.c
+++ b/s390x/edat.c
@@ -26,8 +26,8 @@ static void *root, *mem, *m;
volatile unsigned int *p;
/*
- * Check if a non-access-list protection exception happened for the given
- * address, in the primary address space.
+ * Check if the exception is consistent with DAT protection and has the correct
+ * address and primary address space.
*/
static bool check_pgm_prot(void *ptr)
{
@@ -37,14 +37,19 @@ static bool check_pgm_prot(void *ptr)
return false;
teid.val = lowcore.trans_exc_id;
-
- /*
- * depending on the presence of the ESOP feature, the rest of the
- * field might or might not be meaningful when the m field is 0.
- */
- if (!teid.m)
- return true;
- return (!teid.acc_list_prot && !teid.asce_id &&
+ switch (get_supp_on_prot_facility()) {
+ case SOP_NONE:
+ case SOP_BASIC:
+ assert(false); /* let's ignore ancient/irrelevant machines */
+ case SOP_ENHANCED_1:
+ if (!teid.sop_teid_predictable) /* implies key or low addr */
+ return false;
+ break;
+ case SOP_ENHANCED_2:
+ if (teid_esop2_prot_code(teid) != PROT_DAT)
+ return false;
+ }
+ return (!teid.sop_acc_list && !teid.asce_id &&
(teid.addr == ((unsigned long)ptr >> PAGE_SHIFT)));
}
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 04/12] s390x: Test TEID values in storage key test
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (2 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 03/12] s390x: Rework TEID decoding and usage Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 05/12] s390x: Test effect of storage keys on some more instructions Claudio Imbrenda
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
On a protection exception, test that the Translation-Exception
Identification (TEID) values are correct given the circumstances of the
particular test.
The meaning of the TEID values is dependent on the installed
suppression-on-protection facility.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Message-Id: <20220621143609.753452-2-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/skey.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 72 insertions(+), 6 deletions(-)
diff --git a/s390x/skey.c b/s390x/skey.c
index 445476a0..efce1fc3 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -8,6 +8,7 @@
* Janosch Frank <frankja@linux.vnet.ibm.com>
*/
#include <libcflat.h>
+#include <asm/arch_def.h>
#include <asm/asm-offsets.h>
#include <asm/interrupt.h>
#include <vmalloc.h>
@@ -158,6 +159,71 @@ static void test_test_protection(void)
report_prefix_pop();
}
+enum access {
+ ACC_STORE = 1,
+ ACC_FETCH = 2,
+ ACC_UPDATE = 3,
+};
+
+enum protection {
+ PROT_STORE = 1,
+ PROT_FETCH_STORE = 3,
+};
+
+static void check_key_prot_exc(enum access access, enum protection prot)
+{
+ union teid teid;
+ int access_code;
+
+ check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ report_prefix_push("TEID");
+ teid.val = lowcore.trans_exc_id;
+ switch (get_supp_on_prot_facility()) {
+ case SOP_NONE:
+ case SOP_BASIC:
+ /* let's ignore ancient/irrelevant machines */
+ break;
+ case SOP_ENHANCED_1:
+ report(!teid.sop_teid_predictable, "valid protection code");
+ /* no access code in case of key protection */
+ break;
+ case SOP_ENHANCED_2:
+ switch (teid_esop2_prot_code(teid)) {
+ case PROT_KEY:
+ /* ESOP-2: no need to check facility */
+ access_code = teid.acc_exc_fetch_store;
+
+ switch (access_code) {
+ case 0:
+ report_pass("valid access code");
+ break;
+ case 1:
+ case 2:
+ report((access & access_code) && (prot & access_code),
+ "valid access code");
+ break;
+ case 3:
+ /*
+ * This is incorrect in that reserved values
+ * should be ignored, but kvm should not return
+ * a reserved value and having a test for that
+ * is more valuable.
+ */
+ report_fail("valid access code");
+ break;
+ }
+ /* fallthrough */
+ case PROT_KEY_OR_LAP:
+ report_pass("valid protection code");
+ break;
+ default:
+ report_fail("valid protection code");
+ }
+ break;
+ }
+ report_prefix_pop();
+}
+
/*
* Perform STORE CPU ADDRESS (STAP) instruction while temporarily executing
* with access key 1.
@@ -199,7 +265,7 @@ static void test_store_cpu_address(void)
expect_pgm_int();
*out = 0xbeef;
store_cpu_address_key_1(out);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_STORE, PROT_STORE);
report(*out == 0xbeef, "no store occurred");
report_prefix_pop();
@@ -210,7 +276,7 @@ static void test_store_cpu_address(void)
expect_pgm_int();
*out = 0xbeef;
store_cpu_address_key_1(out);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_STORE, PROT_STORE);
report(*out == 0xbeef, "no store occurred");
report_prefix_pop();
@@ -228,7 +294,7 @@ static void test_store_cpu_address(void)
expect_pgm_int();
*out = 0xbeef;
store_cpu_address_key_1(out);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_STORE, PROT_STORE);
report(*out == 0xbeef, "no store occurred");
report_prefix_pop();
@@ -314,7 +380,7 @@ static void test_set_prefix(void)
set_storage_key(pagebuf, 0x28, 0);
expect_pgm_int();
set_prefix_key_1(prefix_ptr);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
report(get_prefix() == old_prefix, "did not set prefix");
report_prefix_pop();
@@ -327,7 +393,7 @@ static void test_set_prefix(void)
install_page(root, virt_to_pte_phys(root, pagebuf), 0);
set_prefix_key_1((uint32_t *)0);
install_page(root, 0, 0);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
report(get_prefix() == old_prefix, "did not set prefix");
report_prefix_pop();
@@ -351,7 +417,7 @@ static void test_set_prefix(void)
install_page(root, virt_to_pte_phys(root, pagebuf), 0);
set_prefix_key_1(OPAQUE_PTR(2048));
install_page(root, 0, 0);
- check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
report(get_prefix() == old_prefix, "did not set prefix");
report_prefix_pop();
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 05/12] s390x: Test effect of storage keys on some more instructions
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (3 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 04/12] s390x: Test TEID values in storage key test Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 06/12] s390x: Test effect of storage keys on diag 308 Claudio Imbrenda
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Test correctness of some instructions handled by user space instead of
KVM with regards to storage keys.
Test success and error conditions, including coverage of storage and
fetch protection override.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-Id: <20220621143609.753452-3-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/skey.c | 275 ++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 1 +
2 files changed, 276 insertions(+)
diff --git a/s390x/skey.c b/s390x/skey.c
index efce1fc3..aa9b4dcd 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -12,6 +12,7 @@
#include <asm/asm-offsets.h>
#include <asm/interrupt.h>
#include <vmalloc.h>
+#include <css.h>
#include <asm/page.h>
#include <asm/facility.h>
#include <asm/mem.h>
@@ -302,6 +303,115 @@ static void test_store_cpu_address(void)
report_prefix_pop();
}
+/*
+ * Perform CHANNEL SUBSYSTEM CALL (CHSC) instruction while temporarily executing
+ * with access key 1.
+ */
+static unsigned int chsc_key_1(void *comm_block)
+{
+ uint32_t program_mask;
+
+ asm volatile (
+ "spka 0x10\n\t"
+ ".insn rre,0xb25f0000,%[comm_block],0\n\t"
+ "spka 0\n\t"
+ "ipm %[program_mask]\n"
+ : [program_mask] "=d" (program_mask)
+ : [comm_block] "d" (comm_block)
+ : "memory"
+ );
+ return program_mask >> 28;
+}
+
+static const char chsc_msg[] = "Performed store-channel-subsystem-characteristics";
+static void init_comm_block(uint16_t *comm_block)
+{
+ memset(comm_block, 0, PAGE_SIZE);
+ /* store-channel-subsystem-characteristics command */
+ comm_block[0] = 0x10;
+ comm_block[1] = 0x10;
+ comm_block[9] = 0;
+}
+
+static void test_channel_subsystem_call(void)
+{
+ uint16_t *comm_block = (uint16_t *)&pagebuf;
+ unsigned int cc;
+
+ report_prefix_push("CHANNEL SUBSYSTEM CALL");
+
+ report_prefix_push("zero key");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x10, 0);
+ asm volatile (
+ ".insn rre,0xb25f0000,%[comm_block],0\n\t"
+ "ipm %[cc]\n"
+ : [cc] "=d" (cc)
+ : [comm_block] "d" (comm_block)
+ : "memory"
+ );
+ cc = cc >> 28;
+ report(cc == 0 && comm_block[9], chsc_msg);
+ report_prefix_pop();
+
+ report_prefix_push("matching key");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x10, 0);
+ cc = chsc_key_1(comm_block);
+ report(cc == 0 && comm_block[9], chsc_msg);
+ report_prefix_pop();
+
+ report_prefix_push("mismatching key");
+
+ report_prefix_push("no fetch protection");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x20, 0);
+ expect_pgm_int();
+ chsc_key_1(comm_block);
+ check_key_prot_exc(ACC_UPDATE, PROT_STORE);
+ report_prefix_pop();
+
+ report_prefix_push("fetch protection");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x28, 0);
+ expect_pgm_int();
+ chsc_key_1(comm_block);
+ check_key_prot_exc(ACC_UPDATE, PROT_FETCH_STORE);
+ report_prefix_pop();
+
+ ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+
+ report_prefix_push("storage-protection override, invalid key");
+ set_storage_key(comm_block, 0x20, 0);
+ init_comm_block(comm_block);
+ expect_pgm_int();
+ chsc_key_1(comm_block);
+ check_key_prot_exc(ACC_UPDATE, PROT_STORE);
+ report_prefix_pop();
+
+ report_prefix_push("storage-protection override, override key");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x90, 0);
+ cc = chsc_key_1(comm_block);
+ report(cc == 0 && comm_block[9], chsc_msg);
+ report_prefix_pop();
+
+ ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+
+ report_prefix_push("storage-protection override disabled, override key");
+ init_comm_block(comm_block);
+ set_storage_key(comm_block, 0x90, 0);
+ expect_pgm_int();
+ chsc_key_1(comm_block);
+ check_key_prot_exc(ACC_UPDATE, PROT_STORE);
+ report_prefix_pop();
+
+ report_prefix_pop();
+
+ set_storage_key(comm_block, 0x00, 0);
+ report_prefix_pop();
+}
+
/*
* Perform SET PREFIX (SPX) instruction while temporarily executing
* with access key 1.
@@ -428,6 +538,169 @@ static void test_set_prefix(void)
report_prefix_pop();
}
+/*
+ * Perform MODIFY SUBCHANNEL (MSCH) instruction while temporarily executing
+ * with access key 1.
+ */
+static uint32_t modify_subchannel_key_1(uint32_t sid, struct schib *schib)
+{
+ uint32_t program_mask;
+
+ asm volatile (
+ "lr %%r1,%[sid]\n\t"
+ "spka 0x10\n\t"
+ "msch %[schib]\n\t"
+ "spka 0\n\t"
+ "ipm %[program_mask]\n"
+ : [program_mask] "=d" (program_mask)
+ : [sid] "d" (sid),
+ [schib] "Q" (*schib)
+ : "%r1"
+ );
+ return program_mask >> 28;
+}
+
+static void test_msch(void)
+{
+ struct schib *schib = (struct schib *)pagebuf;
+ struct schib *no_override_schib;
+ int test_device_sid;
+ pgd_t *root;
+ int cc;
+
+ report_prefix_push("MSCH");
+ root = (pgd_t *)(stctg(1) & PAGE_MASK);
+ test_device_sid = css_enumerate();
+
+ if (!(test_device_sid & SCHID_ONE)) {
+ report_fail("no I/O device found");
+ return;
+ }
+
+ cc = stsch(test_device_sid, schib);
+ if (cc) {
+ report_fail("could not store SCHIB");
+ return;
+ }
+
+ report_prefix_push("zero key");
+ schib->pmcw.intparm = 100;
+ set_storage_key(schib, 0x28, 0);
+ cc = msch(test_device_sid, schib);
+ if (!cc) {
+ WRITE_ONCE(schib->pmcw.intparm, 0);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 100, "fetched from SCHIB");
+ } else {
+ report_fail("MSCH cc != 0");
+ }
+ report_prefix_pop();
+
+ report_prefix_push("matching key");
+ schib->pmcw.intparm = 200;
+ set_storage_key(schib, 0x18, 0);
+ cc = modify_subchannel_key_1(test_device_sid, schib);
+ if (!cc) {
+ WRITE_ONCE(schib->pmcw.intparm, 0);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 200, "fetched from SCHIB");
+ } else {
+ report_fail("MSCH cc != 0");
+ }
+ report_prefix_pop();
+
+ report_prefix_push("mismatching key");
+
+ report_prefix_push("no fetch protection");
+ schib->pmcw.intparm = 300;
+ set_storage_key(schib, 0x20, 0);
+ cc = modify_subchannel_key_1(test_device_sid, schib);
+ if (!cc) {
+ WRITE_ONCE(schib->pmcw.intparm, 0);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 300, "fetched from SCHIB");
+ } else {
+ report_fail("MSCH cc != 0");
+ }
+ report_prefix_pop();
+
+ schib->pmcw.intparm = 0;
+ if (!msch(test_device_sid, schib)) {
+ report_prefix_push("fetch protection");
+ schib->pmcw.intparm = 400;
+ set_storage_key(schib, 0x28, 0);
+ expect_pgm_int();
+ modify_subchannel_key_1(test_device_sid, schib);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
+ report_prefix_pop();
+ } else {
+ report_fail("could not reset SCHIB");
+ }
+
+ register_pgm_cleanup_func(dat_fixup_pgm_int);
+
+ schib->pmcw.intparm = 0;
+ if (!msch(test_device_sid, schib)) {
+ report_prefix_push("remapped page, fetch protection");
+ schib->pmcw.intparm = 500;
+ set_storage_key(pagebuf, 0x28, 0);
+ expect_pgm_int();
+ install_page(root, virt_to_pte_phys(root, pagebuf), 0);
+ modify_subchannel_key_1(test_device_sid, (struct schib *)0);
+ install_page(root, 0, 0);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
+ report_prefix_pop();
+ } else {
+ report_fail("could not reset SCHIB");
+ }
+
+ ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
+
+ report_prefix_push("fetch-protection override applies");
+ schib->pmcw.intparm = 600;
+ set_storage_key(pagebuf, 0x28, 0);
+ install_page(root, virt_to_pte_phys(root, pagebuf), 0);
+ cc = modify_subchannel_key_1(test_device_sid, (struct schib *)0);
+ install_page(root, 0, 0);
+ if (!cc) {
+ WRITE_ONCE(schib->pmcw.intparm, 0);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 600, "fetched from SCHIB");
+ } else {
+ report_fail("MSCH cc != 0");
+ }
+ report_prefix_pop();
+
+ schib->pmcw.intparm = 0;
+ if (!msch(test_device_sid, schib)) {
+ report_prefix_push("fetch-protection override does not apply");
+ schib->pmcw.intparm = 700;
+ no_override_schib = (struct schib *)(pagebuf + 2048);
+ memcpy(no_override_schib, schib, sizeof(struct schib));
+ set_storage_key(pagebuf, 0x28, 0);
+ expect_pgm_int();
+ install_page(root, virt_to_pte_phys(root, pagebuf), 0);
+ modify_subchannel_key_1(test_device_sid, OPAQUE_PTR(2048));
+ install_page(root, 0, 0);
+ check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
+ cc = stsch(test_device_sid, schib);
+ report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
+ report_prefix_pop();
+ } else {
+ report_fail("could not reset SCHIB");
+ }
+
+ ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
+ register_pgm_cleanup_func(NULL);
+ report_prefix_pop();
+ set_storage_key(schib, 0x00, 0);
+ report_prefix_pop();
+}
+
int main(void)
{
report_prefix_push("skey");
@@ -442,9 +715,11 @@ int main(void)
test_chg();
test_test_protection();
test_store_cpu_address();
+ test_channel_subsystem_call();
setup_vm();
test_set_prefix();
+ test_msch();
done:
report_prefix_pop();
return report_summary();
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 8e52f560..f7b1fc3d 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -41,6 +41,7 @@ file = sthyi.elf
[skey]
file = skey.elf
+extra_params = -device virtio-net-ccw
[diag10]
file = diag10.elf
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 06/12] s390x: Test effect of storage keys on diag 308
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (4 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 05/12] s390x: Test effect of storage keys on some more instructions Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 07/12] s390x/intercept: Test invalid prefix argument to SET PREFIX Claudio Imbrenda
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Test that key-controlled protection does not apply to diag 308.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-Id: <20220621143609.753452-4-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/skey.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/s390x/skey.c b/s390x/skey.c
index aa9b4dcd..7e85f97d 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -303,6 +303,31 @@ static void test_store_cpu_address(void)
report_prefix_pop();
}
+static void test_diag_308(void)
+{
+ uint16_t response;
+ uint32_t *ipib = (uint32_t *)pagebuf;
+
+ report_prefix_push("DIAG 308");
+ WRITE_ONCE(ipib[0], 0); /* Invalid length */
+ set_storage_key(ipib, 0x28, 0);
+ /* key-controlled protection does not apply */
+ asm volatile (
+ "lr %%r2,%[ipib]\n\t"
+ "spka 0x10\n\t"
+ "diag %%r2,%[code],0x308\n\t"
+ "spka 0\n\t"
+ "lr %[response],%%r3\n"
+ : [response] "=d" (response)
+ : [ipib] "d" (ipib),
+ [code] "d" (5)
+ : "%r2", "%r3"
+ );
+ report(response == 0x402, "no exception on fetch, response: invalid IPIB");
+ set_storage_key(ipib, 0x00, 0);
+ report_prefix_pop();
+}
+
/*
* Perform CHANNEL SUBSYSTEM CALL (CHSC) instruction while temporarily executing
* with access key 1.
@@ -715,6 +740,7 @@ int main(void)
test_chg();
test_test_protection();
test_store_cpu_address();
+ test_diag_308();
test_channel_subsystem_call();
setup_vm();
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 07/12] s390x/intercept: Test invalid prefix argument to SET PREFIX
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (5 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 06/12] s390x: Test effect of storage keys on diag 308 Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 08/12] lib: s390x: add functions to set and clear PSW bits Claudio Imbrenda
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja, Janis Schoetterl-Glausch
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
According to the architecture, SET PREFIX must try to access the new
prefix area and recognize an addressing exception if the area is not
accessible.
Test that the exception occurs when we try to set a prefix higher
than the available memory.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Message-Id: <20220627152412.2243255-1-scgl@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/intercept.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 86e57e11..54bed5a4 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -74,6 +74,22 @@ static void test_spx(void)
expect_pgm_int();
asm volatile(" spx 0(%0) " : : "r"(-8L));
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
+
+ new_prefix = get_ram_size() & 0x7fffe000;
+ if (get_ram_size() - new_prefix < 2 * PAGE_SIZE) {
+ expect_pgm_int();
+ asm volatile("spx %0 " : : "Q"(new_prefix));
+ check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
+
+ /*
+ * Cannot test inaccessibility of the second page the same way.
+ * If we try to use the last page as first half of the prefix
+ * area and our ram size is a multiple of 8k, after SPX aligns
+ * the address to 8k we have a completely accessible area.
+ */
+ } else {
+ report_skip("inaccessible prefix area");
+ }
}
/* Test the STORE CPU ADDRESS instruction */
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 08/12] lib: s390x: add functions to set and clear PSW bits
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (6 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 07/12] s390x/intercept: Test invalid prefix argument to SET PREFIX Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 09/12] s390x: skey.c: rework the interrupt handler Claudio Imbrenda
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja
Add some functions to set and/or clear bits in the PSW.
Also introduce PSW_MASK_KEY and re-order the PSW_MASK_* constants so
they are descending in value.
This should improve code readability.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 58 +++++++++++++++++++++++++++++++++++-----
lib/s390x/asm/pgtable.h | 2 --
lib/s390x/mmu.c | 14 +---------
lib/s390x/sclp.c | 7 +----
s390x/diag288.c | 6 ++---
s390x/selftest.c | 4 +--
s390x/skrf.c | 12 +++------
s390x/smp.c | 18 +++----------
8 files changed, 63 insertions(+), 58 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 78b257b7..358ef82e 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -46,9 +46,10 @@ struct psw {
#define AS_SECN 2
#define AS_HOME 3
-#define PSW_MASK_EXT 0x0100000000000000UL
-#define PSW_MASK_IO 0x0200000000000000UL
#define PSW_MASK_DAT 0x0400000000000000UL
+#define PSW_MASK_IO 0x0200000000000000UL
+#define PSW_MASK_EXT 0x0100000000000000UL
+#define PSW_MASK_KEY 0x00F0000000000000UL
#define PSW_MASK_WAIT 0x0002000000000000UL
#define PSW_MASK_PSTATE 0x0001000000000000UL
#define PSW_MASK_EA 0x0000000100000000UL
@@ -313,6 +314,53 @@ static inline void load_psw_mask(uint64_t mask)
: "+r" (tmp) : "a" (&psw) : "memory", "cc" );
}
+/**
+ * psw_mask_clear_bits - clears bits from the current PSW mask
+ * @clear: bitmask of bits that will be cleared
+ */
+static inline void psw_mask_clear_bits(uint64_t clear)
+{
+ load_psw_mask(extract_psw_mask() & ~clear);
+}
+
+/**
+ * psw_mask_set_bits - sets bits on the current PSW mask
+ * @set: bitmask of bits that will be set
+ */
+static inline void psw_mask_set_bits(uint64_t set)
+{
+ load_psw_mask(extract_psw_mask() | set);
+}
+
+/**
+ * psw_mask_clear_and_set_bits - clears and sets bits on the current PSW mask
+ * @clear: bitmask of bits that will be cleared
+ * @set: bitmask of bits that will be set
+ *
+ * The bits in the @clear mask will be cleared, then the bits in the @set mask
+ * will be set.
+ */
+static inline void psw_mask_clear_and_set_bits(uint64_t clear, uint64_t set)
+{
+ load_psw_mask((extract_psw_mask() & ~clear) | set);
+}
+
+/**
+ * enable_dat - enable the DAT bit in the current PSW
+ */
+static inline void enable_dat(void)
+{
+ psw_mask_set_bits(PSW_MASK_DAT);
+}
+
+/**
+ * disable_dat - disable the DAT bit in the current PSW
+ */
+static inline void disable_dat(void)
+{
+ psw_mask_clear_bits(PSW_MASK_DAT);
+}
+
static inline void wait_for_interrupt(uint64_t irq_mask)
{
uint64_t psw_mask = extract_psw_mask();
@@ -327,11 +375,7 @@ static inline void wait_for_interrupt(uint64_t irq_mask)
static inline void enter_pstate(void)
{
- uint64_t mask;
-
- mask = extract_psw_mask();
- mask |= PSW_MASK_PSTATE;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_PSTATE);
}
static inline void leave_pstate(void)
diff --git a/lib/s390x/asm/pgtable.h b/lib/s390x/asm/pgtable.h
index f166dcc6..7b556ad9 100644
--- a/lib/s390x/asm/pgtable.h
+++ b/lib/s390x/asm/pgtable.h
@@ -247,6 +247,4 @@ static inline void idte_pgdp(unsigned long vaddr, pgdval_t *pgdp)
idte((unsigned long)(pgdp - pgd_index(vaddr)) | ASCE_DT_REGION1, vaddr);
}
-void configure_dat(int enable);
-
#endif /* _ASMS390X_PGTABLE_H_ */
diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
index c9f8754c..b474d702 100644
--- a/lib/s390x/mmu.c
+++ b/lib/s390x/mmu.c
@@ -29,18 +29,6 @@
static pgd_t *table_root;
-void configure_dat(int enable)
-{
- uint64_t mask;
-
- if (enable)
- mask = extract_psw_mask() | PSW_MASK_DAT;
- else
- mask = extract_psw_mask() & ~PSW_MASK_DAT;
-
- load_psw_mask(mask);
-}
-
static void mmu_enable(pgd_t *pgtable)
{
const uint64_t asce = __pa(pgtable) | ASCE_DT_REGION1 |
@@ -51,7 +39,7 @@ static void mmu_enable(pgd_t *pgtable)
assert(stctg(1) == asce);
/* enable dat (primary == 0 set as default) */
- configure_dat(1);
+ enable_dat();
/* we can now also use DAT unconditionally in our PGM handler */
lowcore.pgm_new_psw.mask |= PSW_MASK_DAT;
diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index e6017f64..390fde70 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -48,13 +48,8 @@ static void mem_init(phys_addr_t mem_end)
void sclp_setup_int(void)
{
- uint64_t mask;
-
ctl_set_bit(0, CTL0_SERVICE_SIGNAL);
-
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
}
void sclp_handle_ext(void)
diff --git a/s390x/diag288.c b/s390x/diag288.c
index e414865b..46dc0ed8 100644
--- a/s390x/diag288.c
+++ b/s390x/diag288.c
@@ -78,16 +78,14 @@ static void test_priv(void)
static void test_bite(void)
{
- uint64_t mask, time;
+ uint64_t time;
/* If watchdog doesn't bite, the cpu timer does */
asm volatile("stck %0" : "=Q" (time) : : "cc");
time += (uint64_t)(16000 * 1000) << 12;
asm volatile("sckc %0" : : "Q" (time));
ctl_set_bit(0, CTL0_CLOCK_COMPARATOR);
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
/* Arm watchdog */
lowcore.restart_new_psw.mask = extract_psw_mask() & ~PSW_MASK_EXT;
diff --git a/s390x/selftest.c b/s390x/selftest.c
index 239bc5e3..13fd36bc 100644
--- a/s390x/selftest.c
+++ b/s390x/selftest.c
@@ -64,9 +64,9 @@ static void test_malloc(void)
report(tmp != tmp2, "allocated memory addresses differ");
expect_pgm_int();
- configure_dat(0);
+ disable_dat();
*tmp = 987654321;
- configure_dat(1);
+ enable_dat();
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
free(tmp);
diff --git a/s390x/skrf.c b/s390x/skrf.c
index 1a811894..26f70b4e 100644
--- a/s390x/skrf.c
+++ b/s390x/skrf.c
@@ -63,11 +63,9 @@ static void test_pfmf(void)
static void test_psw_key(void)
{
- uint64_t psw_mask = extract_psw_mask() | 0xF0000000000000UL;
-
report_prefix_push("psw key");
expect_pgm_int();
- load_psw_mask(psw_mask);
+ psw_mask_set_bits(PSW_MASK_KEY);
check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
report_prefix_pop();
}
@@ -140,17 +138,13 @@ static void ecall_cleanup(void)
/* Set a key into the external new psw mask and open external call masks */
static void ecall_setup(void)
{
- uint64_t mask;
-
register_pgm_cleanup_func(ecall_cleanup);
expect_pgm_int();
/* Put a skey into the ext new psw */
- lowcore.ext_new_psw.mask = 0x00F0000000000000UL | PSW_MASK_64;
+ lowcore.ext_new_psw.mask = PSW_MASK_KEY | PSW_MASK_64;
/* Open up ext masks */
ctl_set_bit(0, CTL0_EXTERNAL_CALL);
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
/* Tell cpu 0 that we're ready */
set_flag(1);
}
diff --git a/s390x/smp.c b/s390x/smp.c
index 6d474d0d..0df4751f 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -288,13 +288,9 @@ static void test_set_prefix(void)
static void ecall(void)
{
- unsigned long mask;
-
expect_ext_int();
ctl_set_bit(0, CTL0_EXTERNAL_CALL);
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
set_flag(1);
while (lowcore.ext_int_code != 0x1202) { mb(); }
report_pass("received");
@@ -321,13 +317,9 @@ static void test_ecall(void)
static void emcall(void)
{
- unsigned long mask;
-
expect_ext_int();
ctl_set_bit(0, CTL0_EMERGENCY_SIGNAL);
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
set_flag(1);
while (lowcore.ext_int_code != 0x1201) { mb(); }
report_pass("received");
@@ -466,14 +458,10 @@ static void test_reset_initial(void)
static void test_local_ints(void)
{
- unsigned long mask;
-
/* Open masks for ecall and emcall */
ctl_set_bit(0, CTL0_EXTERNAL_CALL);
ctl_set_bit(0, CTL0_EMERGENCY_SIGNAL);
- mask = extract_psw_mask();
- mask |= PSW_MASK_EXT;
- load_psw_mask(mask);
+ psw_mask_set_bits(PSW_MASK_EXT);
set_flag(1);
}
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 09/12] s390x: skey.c: rework the interrupt handler
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (7 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 08/12] lib: s390x: add functions to set and clear PSW bits Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 10/12] lib: s390x: better smp interrupt checks Claudio Imbrenda
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja
The skey test currently uses a cleanup function to work around the
issues that arise when the lowcore is not mapped, since the interrupt
handler needs to access it.
Instead of a cleanup function, simply disable DAT for the interrupt
handler for the tests that remap page 0. This is needed in preparation
of and upcoming patch that will cause the interrupt handler to read
from lowcore before calling the cleanup function.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/skey.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/s390x/skey.c b/s390x/skey.c
index 7e85f97d..1167e4d3 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -451,19 +451,6 @@ static void set_prefix_key_1(uint32_t *prefix_ptr)
);
}
-/*
- * We remapped page 0, making the lowcore inaccessible, which breaks the normal
- * handler and breaks skipping the faulting instruction.
- * Just disable dynamic address translation to make things work.
- */
-static void dat_fixup_pgm_int(void)
-{
- uint64_t psw_mask = extract_psw_mask();
-
- psw_mask &= ~PSW_MASK_DAT;
- load_psw_mask(psw_mask);
-}
-
#define PREFIX_AREA_SIZE (PAGE_SIZE * 2)
static char lowcore_tmp[PREFIX_AREA_SIZE] __attribute__((aligned(PREFIX_AREA_SIZE)));
@@ -519,7 +506,13 @@ static void test_set_prefix(void)
report(get_prefix() == old_prefix, "did not set prefix");
report_prefix_pop();
- register_pgm_cleanup_func(dat_fixup_pgm_int);
+ /*
+ * Page 0 will be remapped, making the lowcore inaccessible, which
+ * breaks the normal handler and breaks skipping the faulting
+ * instruction. Disable dynamic address translation for the
+ * interrupt handler to make things work.
+ */
+ lowcore.pgm_new_psw.mask &= ~PSW_MASK_DAT;
report_prefix_push("remapped page, fetch protection");
set_prefix(old_prefix);
@@ -557,7 +550,7 @@ static void test_set_prefix(void)
report_prefix_pop();
ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
- register_pgm_cleanup_func(NULL);
+ lowcore.pgm_new_psw.mask |= PSW_MASK_DAT;
report_prefix_pop();
set_storage_key(pagebuf, 0x00, 0);
report_prefix_pop();
@@ -664,7 +657,13 @@ static void test_msch(void)
report_fail("could not reset SCHIB");
}
- register_pgm_cleanup_func(dat_fixup_pgm_int);
+ /*
+ * Page 0 will be remapped, making the lowcore inaccessible, which
+ * breaks the normal handler and breaks skipping the faulting
+ * instruction. Disable dynamic address translation for the
+ * interrupt handler to make things work.
+ */
+ lowcore.pgm_new_psw.mask &= ~PSW_MASK_DAT;
schib->pmcw.intparm = 0;
if (!msch(test_device_sid, schib)) {
@@ -720,7 +719,7 @@ static void test_msch(void)
}
ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
- register_pgm_cleanup_func(NULL);
+ lowcore.pgm_new_psw.mask |= PSW_MASK_DAT;
report_prefix_pop();
set_storage_key(schib, 0x00, 0);
report_prefix_pop();
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 10/12] lib: s390x: better smp interrupt checks
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (8 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 09/12] s390x: skey.c: rework the interrupt handler Claudio Imbrenda
@ 2022-07-21 14:06 ` Claudio Imbrenda
2022-07-21 14:07 ` [kvm-unit-tests GIT PULL 11/12] s390x: intercept: fence one test when using TCG Claudio Imbrenda
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja
Use per-CPU flags and callbacks for Program and Extern interrupts,
instead of global variables.
This allows for more accurate error handling; a CPU waiting for an
interrupt will not have it "stolen" by a different CPU that was not
supposed to wait for one, and now two CPUs can wait for interrupts at
the same time.
This will significantly improve error reporting and debugging when
things go wrong.
Both program interrupts and external interrupts are now CPU-bound, even
though some external interrupts are floating (notably, the SCLP
interrupt). In those cases, the testcases should mask interrupts and/or
expect them appropriately according to need.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 17 ++++++++-
lib/s390x/asm/interrupt.h | 3 +-
lib/s390x/smp.h | 8 +---
lib/s390x/interrupt.c | 77 +++++++++++++++++++++++++++++++--------
lib/s390x/smp.c | 11 ++++++
s390x/skrf.c | 2 +-
6 files changed, 92 insertions(+), 26 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 358ef82e..e7ae454b 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -41,6 +41,18 @@ struct psw {
uint64_t addr;
};
+struct cpu {
+ struct lowcore *lowcore;
+ uint64_t *stack;
+ void (*pgm_cleanup_func)(struct stack_frame_int *);
+ void (*ext_cleanup_func)(struct stack_frame_int *);
+ uint16_t addr;
+ uint16_t idx;
+ bool active;
+ bool pgm_int_expected;
+ bool ext_int_expected;
+};
+
#define AS_PRIM 0
#define AS_ACCR 1
#define AS_SECN 2
@@ -125,7 +137,8 @@ struct lowcore {
uint8_t pad_0x0280[0x0308 - 0x0280]; /* 0x0280 */
uint64_t sw_int_crs[16]; /* 0x0308 */
struct psw sw_int_psw; /* 0x0388 */
- uint8_t pad_0x0310[0x11b0 - 0x0398]; /* 0x0398 */
+ struct cpu *this_cpu; /* 0x0398 */
+ uint8_t pad_0x03a0[0x11b0 - 0x03a0]; /* 0x03a0 */
uint64_t mcck_ext_sa_addr; /* 0x11b0 */
uint8_t pad_0x11b8[0x1200 - 0x11b8]; /* 0x11b8 */
uint64_t fprs_sa[16]; /* 0x1200 */
@@ -148,6 +161,8 @@ _Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");
extern struct lowcore lowcore;
+#define THIS_CPU (lowcore.this_cpu)
+
#define PGM_INT_CODE_OPERATION 0x01
#define PGM_INT_CODE_PRIVILEGED_OPERATION 0x02
#define PGM_INT_CODE_EXECUTE 0x03
diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index fc66a925..35c1145f 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -71,7 +71,8 @@ static inline enum prot_code teid_esop2_prot_code(union teid teid)
return (enum prot_code)code;
}
-void register_pgm_cleanup_func(void (*f)(void));
+void register_pgm_cleanup_func(void (*f)(struct stack_frame_int *));
+void register_ext_cleanup_func(void (*f)(struct stack_frame_int *));
void handle_pgm_int(struct stack_frame_int *stack);
void handle_ext_int(struct stack_frame_int *stack);
void handle_mcck_int(void);
diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
index df184cb8..f4ae973d 100644
--- a/lib/s390x/smp.h
+++ b/lib/s390x/smp.h
@@ -12,13 +12,6 @@
#include <asm/arch_def.h>
-struct cpu {
- struct lowcore *lowcore;
- uint64_t *stack;
- uint16_t addr;
- bool active;
-};
-
struct cpu_status {
uint64_t fprs[16]; /* 0x0000 */
uint64_t grs[16]; /* 0x0080 */
@@ -52,5 +45,6 @@ int smp_cpu_setup(uint16_t idx, struct psw psw);
void smp_teardown(void);
void smp_setup(void);
int smp_sigp(uint16_t idx, uint8_t order, unsigned long parm, uint32_t *status);
+struct lowcore *smp_get_lowcore(uint16_t idx);
#endif
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index ac3d1ecd..7cc2c5fb 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -15,25 +15,36 @@
#include <fault.h>
#include <asm/page.h>
-static bool pgm_int_expected;
-static bool ext_int_expected;
-static void (*pgm_cleanup_func)(void);
-
+/**
+ * expect_pgm_int - Expect a program interrupt on the current CPU.
+ */
void expect_pgm_int(void)
{
- pgm_int_expected = true;
+ THIS_CPU->pgm_int_expected = true;
lowcore.pgm_int_code = 0;
lowcore.trans_exc_id = 0;
mb();
}
+/**
+ * expect_ext_int - Expect an external interrupt on the current CPU.
+ */
void expect_ext_int(void)
{
- ext_int_expected = true;
+ THIS_CPU->ext_int_expected = true;
lowcore.ext_int_code = 0;
mb();
}
+/**
+ * clear_pgm_int - Clear program interrupt information
+ *
+ * Clear program interrupt information, including the expected program
+ * interrupt flag.
+ * No program interrupts are expected after calling this function.
+ *
+ * Return: the program interrupt code before clearing
+ */
uint16_t clear_pgm_int(void)
{
uint16_t code;
@@ -42,10 +53,17 @@ uint16_t clear_pgm_int(void)
code = lowcore.pgm_int_code;
lowcore.pgm_int_code = 0;
lowcore.trans_exc_id = 0;
- pgm_int_expected = false;
+ THIS_CPU->pgm_int_expected = false;
return code;
}
+/**
+ * check_pgm_int_code - Check the program interrupt code on the current CPU.
+ * @code the expected program interrupt code on the current CPU
+ *
+ * Check and report if the program interrupt on the current CPU matches the
+ * expected one.
+ */
void check_pgm_int_code(uint16_t code)
{
mb();
@@ -54,9 +72,34 @@ void check_pgm_int_code(uint16_t code)
lowcore.pgm_int_code);
}
-void register_pgm_cleanup_func(void (*f)(void))
+/**
+ * register_pgm_cleanup_func - Register a cleanup function for progam
+ * interrupts for the current CPU.
+ * @f the cleanup function to be registered on the current CPU
+ *
+ * Register a cleanup function to be called at the end of the normal
+ * interrupt handling for program interrupts for this CPU.
+ *
+ * Pass NULL to unregister a previously registered cleanup function.
+ */
+void register_pgm_cleanup_func(void (*f)(struct stack_frame_int *))
+{
+ THIS_CPU->pgm_cleanup_func = f;
+}
+
+/**
+ * register_ext_cleanup_func - Register a cleanup function for external
+ * interrupts for the current CPU.
+ * @f the cleanup function to be registered on the current CPU
+ *
+ * Register a cleanup function to be called at the end of the normal
+ * interrupt handling for external interrupts for this CPU.
+ *
+ * Pass NULL to unregister a previously registered cleanup function.
+ */
+void register_ext_cleanup_func(void (*f)(struct stack_frame_int *))
{
- pgm_cleanup_func = f;
+ THIS_CPU->ext_cleanup_func = f;
}
static void fixup_pgm_int(struct stack_frame_int *stack)
@@ -183,24 +226,23 @@ static void print_pgm_info(struct stack_frame_int *stack)
void handle_pgm_int(struct stack_frame_int *stack)
{
- if (!pgm_int_expected) {
+ if (!THIS_CPU->pgm_int_expected) {
/* Force sclp_busy to false, otherwise we will loop forever */
sclp_handle_ext();
print_pgm_info(stack);
}
- pgm_int_expected = false;
+ THIS_CPU->pgm_int_expected = false;
- if (pgm_cleanup_func)
- (*pgm_cleanup_func)();
+ if (THIS_CPU->pgm_cleanup_func)
+ THIS_CPU->pgm_cleanup_func(stack);
else
fixup_pgm_int(stack);
}
void handle_ext_int(struct stack_frame_int *stack)
{
- if (!ext_int_expected &&
- lowcore.ext_int_code != EXT_IRQ_SERVICE_SIG) {
+ if (!THIS_CPU->ext_int_expected && lowcore.ext_int_code != EXT_IRQ_SERVICE_SIG) {
report_abort("Unexpected external call interrupt (code %#x): on cpu %d at %#lx",
lowcore.ext_int_code, stap(), lowcore.ext_old_psw.addr);
return;
@@ -210,11 +252,14 @@ void handle_ext_int(struct stack_frame_int *stack)
stack->crs[0] &= ~(1UL << 9);
sclp_handle_ext();
} else {
- ext_int_expected = false;
+ THIS_CPU->ext_int_expected = false;
}
if (!(stack->crs[0] & CR0_EXTM_MASK))
lowcore.ext_old_psw.mask &= ~PSW_MASK_EXT;
+
+ if (THIS_CPU->ext_cleanup_func)
+ THIS_CPU->ext_cleanup_func(stack);
}
void handle_mcck_int(void)
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index a0495cd9..0d98c17d 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -39,6 +39,15 @@ int smp_query_num_cpus(void)
return sclp_get_cpu_num();
}
+struct lowcore *smp_get_lowcore(uint16_t idx)
+{
+ if (THIS_CPU->idx == idx)
+ return &lowcore;
+
+ check_idx(idx);
+ return cpus[idx].lowcore;
+}
+
int smp_sigp(uint16_t idx, uint8_t order, unsigned long parm, uint32_t *status)
{
check_idx(idx);
@@ -253,6 +262,7 @@ static int smp_cpu_setup_nolock(uint16_t idx, struct psw psw)
/* Copy all exception psws. */
memcpy(lc, cpus[0].lowcore, 512);
+ lc->this_cpu = &cpus[idx];
/* Setup stack */
cpus[idx].stack = (uint64_t *)alloc_pages(2);
@@ -325,6 +335,7 @@ void smp_setup(void)
for (i = 0; i < num; i++) {
cpus[i].addr = entry[i].address;
cpus[i].active = false;
+ cpus[i].idx = i;
/*
* Fill in the boot CPU. If the boot CPU is not at index 0,
* swap it with the one at index 0. This guarantees that the
diff --git a/s390x/skrf.c b/s390x/skrf.c
index 26f70b4e..4cb563c3 100644
--- a/s390x/skrf.c
+++ b/s390x/skrf.c
@@ -119,7 +119,7 @@ static void set_flag(int val)
mb();
}
-static void ecall_cleanup(void)
+static void ecall_cleanup(struct stack_frame_int *stack)
{
lowcore.ext_new_psw.mask = PSW_MASK_64;
lowcore.sw_int_crs[0] = BIT_ULL(CTL0_AFP);
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 11/12] s390x: intercept: fence one test when using TCG
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (9 preceding siblings ...)
2022-07-21 14:06 ` [kvm-unit-tests GIT PULL 10/12] lib: s390x: better smp interrupt checks Claudio Imbrenda
@ 2022-07-21 14:07 ` Claudio Imbrenda
2022-07-21 14:07 ` [kvm-unit-tests GIT PULL 12/12] s390x: intercept: make sure all output lines are unique Claudio Imbrenda
2022-07-21 14:42 ` [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:07 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja
Qemu commit f8333de2793 ("target/s390x/tcg: SPX: check validity of new prefix")
fixes a TCG bug discovered with a new testcase in the intercept test.
The gitlab pipeline for the KVM unit tests uses TCG and it will keep
failing every time as long as the pipeline uses a version of Qemu
without the aforementioned patch.
Fence the specific testcase for now. Once the pipeline is fixed, this
patch can safely be reverted.
This patch is meant to go on top this already queued patch from Janis:
"s390x/intercept: Test invalid prefix argument to SET PREFIX"
https://lore.kernel.org/all/20220627152412.2243255-1-scgl@linux.ibm.com/
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Link: https://lore.kernel.org/r/20220721133002.142897-2-imbrenda@linux.ibm.com
Message-Id: <20220721133002.142897-2-imbrenda@linux.ibm.com>
---
s390x/intercept.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 54bed5a4..656b8adb 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -14,6 +14,7 @@
#include <asm/page.h>
#include <asm/facility.h>
#include <asm/time.h>
+#include <hardware.h>
static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
@@ -76,7 +77,8 @@ static void test_spx(void)
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
new_prefix = get_ram_size() & 0x7fffe000;
- if (get_ram_size() - new_prefix < 2 * PAGE_SIZE) {
+ /* TODO: Remove host_is_tcg() checks once CIs are using QEMU >= 7.1 */
+ if (!host_is_tcg() && (get_ram_size() - new_prefix < 2 * PAGE_SIZE)) {
expect_pgm_int();
asm volatile("spx %0 " : : "Q"(new_prefix));
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
@@ -88,7 +90,10 @@ static void test_spx(void)
* the address to 8k we have a completely accessible area.
*/
} else {
- report_skip("inaccessible prefix area");
+ if (host_is_tcg())
+ report_skip("inaccessible prefix area (workaround for TCG bug)");
+ else
+ report_skip("inaccessible prefix area");
}
}
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [kvm-unit-tests GIT PULL 12/12] s390x: intercept: make sure all output lines are unique
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (10 preceding siblings ...)
2022-07-21 14:07 ` [kvm-unit-tests GIT PULL 11/12] s390x: intercept: fence one test when using TCG Claudio Imbrenda
@ 2022-07-21 14:07 ` Claudio Imbrenda
2022-07-21 14:42 ` [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2022-07-21 14:07 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, thuth, frankja
The intercept test has the same output line twice for two different
testcases.
Fix this by adding report_prefix_push() as appropriate.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Link: https://lore.kernel.org/r/20220721133002.142897-3-imbrenda@linux.ibm.com
Message-Id: <20220721133002.142897-3-imbrenda@linux.ibm.com>
---
s390x/intercept.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 656b8adb..9e826b6c 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -68,14 +68,19 @@ static void test_spx(void)
set_prefix(old_prefix);
report(pagebuf[GEN_LC_STFL] != 0, "stfl to new prefix");
+ report_prefix_push("operand not word aligned");
expect_pgm_int();
asm volatile(" spx 0(%0) " : : "r"(1));
check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+ report_prefix_pop();
+ report_prefix_push("operand outside memory");
expect_pgm_int();
asm volatile(" spx 0(%0) " : : "r"(-8L));
check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
+ report_prefix_pop();
+ report_prefix_push("new prefix outside memory");
new_prefix = get_ram_size() & 0x7fffe000;
/* TODO: Remove host_is_tcg() checks once CIs are using QEMU >= 7.1 */
if (!host_is_tcg() && (get_ram_size() - new_prefix < 2 * PAGE_SIZE)) {
@@ -95,6 +100,7 @@ static void test_spx(void)
else
report_skip("inaccessible prefix area");
}
+ report_prefix_pop();
}
/* Test the STORE CPU ADDRESS instruction */
--
2.36.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests
2022-07-21 14:06 [kvm-unit-tests GIT PULL 00/12] s390x: improve error reporting, more storage key tests Claudio Imbrenda
` (11 preceding siblings ...)
2022-07-21 14:07 ` [kvm-unit-tests GIT PULL 12/12] s390x: intercept: make sure all output lines are unique Claudio Imbrenda
@ 2022-07-21 14:42 ` Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2022-07-21 14:42 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, thuth, frankja
On 7/21/22 16:06, Claudio Imbrenda wrote:
> Hi Paolo,
>
> please merge the following changes:
> * new testcases to test storage keys functionality
> * improved parsing of interrupt parameters
> * readability improvements
> * CI fix to overcome a qemu bug exposed by the new tests
> * better error reporting for SMP tests
>
> MERGE:
> https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/merge_requests/34
>
> PIPELINE:
> https://gitlab.com/imbrenda/kvm-unit-tests/-/pipelines/593541216
>
> PULL:
> https://gitlab.com/imbrenda/kvm-unit-tests.git s390x-next-2022-07
>
>
> Claudio Imbrenda (5):
> lib: s390x: add functions to set and clear PSW bits
> s390x: skey.c: rework the interrupt handler
> lib: s390x: better smp interrupt checks
> s390x: intercept: fence one test when using TCG
> s390x: intercept: make sure all output lines are unique
>
> Janis Schoetterl-Glausch (7):
> s390x: Fix sclp facility bit numbers
> s390x: lib: SOP facility query function
> s390x: Rework TEID decoding and usage
> s390x: Test TEID values in storage key test
> s390x: Test effect of storage keys on some more instructions
> s390x: Test effect of storage keys on diag 308
> s390x/intercept: Test invalid prefix argument to SET PREFIX
>
> lib/s390x/asm/arch_def.h | 75 ++++++-
> lib/s390x/asm/facility.h | 21 ++
> lib/s390x/asm/interrupt.h | 65 ++++--
> lib/s390x/asm/pgtable.h | 2 -
> lib/s390x/fault.h | 30 +--
> lib/s390x/sclp.h | 18 +-
> lib/s390x/smp.h | 8 +-
> lib/s390x/fault.c | 58 ++++--
> lib/s390x/interrupt.c | 79 ++++++--
> lib/s390x/mmu.c | 14 +-
> lib/s390x/sclp.c | 9 +-
> lib/s390x/smp.c | 11 +
> s390x/diag288.c | 6 +-
> s390x/edat.c | 25 ++-
> s390x/intercept.c | 27 +++
> s390x/selftest.c | 4 +-
> s390x/skey.c | 408 ++++++++++++++++++++++++++++++++++++--
> s390x/skrf.c | 14 +-
> s390x/smp.c | 18 +-
> s390x/unittests.cfg | 1 +
> 20 files changed, 712 insertions(+), 181 deletions(-)
>
Merged, thanks!
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread