[kvm-unit-tests PATCH v3 0/4] More skey instr. emulation test

[kvm-unit-tests PATCH v3 0/4] More skey instr. emulation test

[Janis Schoetterl-Glausch]

Add test cases similar to those testing the effect of storage keys on
instructions emulated by KVM, but test instructions emulated by user
space/qemu instead.
Test that DIAG 308 is not subject to key protection.
Additionally, check the transaction exception identification on
protection exceptions.

This series is based on s390x: Rework TEID decoding and usage .
https://lore.kernel.org/kvm/20220520190850.3445768-1-scgl@linux.ibm.com/

v2 -> v3
 * move sclp patch and part of TEID test to series
       s390x: Rework TEID decoding and usage
 * make use of reworked TEID union in skey TEID test
 * get rid of pointer to array for diag 308 test
 * use lowcore symbol and mem_all
 * don't reset intparm when expecting exception in msch test

v1 -> v2
 * don't mixup sclp fix with new bits for the TEID patch
 * address feedback
       * cosmetic changes, i.e. shortening identifiers
       * remove unconditional report_info
 * add DIAG 308 test

Janis Schoetterl-Glausch (3):
  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/skey.c        | 376 +++++++++++++++++++++++++++++++++++++++++++-
 s390x/unittests.cfg |   1 +
 2 files changed, 371 insertions(+), 6 deletions(-)

Range-diff against v2:
1:  3c03bfba ! 1:  073ffb3c s390x: Test TEID values in storage key test
    @@ Commit message
     
         Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
     
    - ## lib/s390x/asm/facility.h ##
    -@@
    - #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[];
    -@@ lib/s390x/asm/facility.h: 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
    -
    - ## lib/s390x/sclp.h ##
    -@@ lib/s390x/sclp.h: 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;
    -@@ lib/s390x/sclp.h: 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
    -
    - ## lib/s390x/sclp.c ##
    -@@ lib/s390x/sclp.c: 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);
    -
      ## s390x/skey.c ##
     @@
       *  Janosch Frank <frankja@linux.vnet.ibm.com>
       */
      #include <libcflat.h>
    -+#include <bitops.h>
    ++#include <asm/arch_def.h>
      #include <asm/asm-offsets.h>
      #include <asm/interrupt.h>
      #include <vmalloc.h>
    @@ s390x/skey.c: static void test_test_protection(void)
     +
     +static void check_key_prot_exc(enum access access, enum protection prot)
     +{
    -+	struct lowcore *lc = 0;
     +	union teid teid;
    ++	int access_code;
     +
     +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
     +	report_prefix_push("TEID");
    -+	teid.val = lc->trans_exc_id;
    ++	teid.val = lowcore.trans_exc_id;
     +	switch (get_supp_on_prot_facility()) {
     +	case SOP_NONE:
     +	case SOP_BASIC:
     +		break;
     +	case SOP_ENHANCED_1:
    -+		if ((teid.val & (BIT(63 - 61))) == 0)
    -+			report_pass("key-controlled protection");
    ++		report(!teid.esop1_acc_list_or_dat, "valid protection code");
     +		break;
     +	case SOP_ENHANCED_2:
    -+		if ((teid.val & (BIT(63 - 56) | BIT(63 - 61))) == 0) {
    -+			report_pass("key-controlled protection");
    -+			if (teid.val & BIT(63 - 60)) {
    -+				int access_code = teid.fetch << 1 | teid.store;
    ++		switch (teid_esop2_prot_code(teid)) {
    ++		case PROT_KEY:
    ++			access_code = teid.acc_exc_f_s;
     +
    -+				if (access_code == 2)
    -+					report((access & 2) && (prot & 2),
    -+					       "exception due to fetch");
    -+				if (access_code == 1)
    -+					report((access & 1) && (prot & 1),
    -+					       "exception due to store");
    -+				/* no relevant information if code is 0 or 3 */
    ++			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_LAP:
    ++			report_pass("valid protection code");
    ++			break;
    ++		default:
    ++			report_fail("valid protection code");
     +		}
     +		break;
     +	}
    @@ s390x/skey.c: static void test_set_prefix(void)
      
     @@ s390x/skey.c: static void test_set_prefix(void)
      	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
    - 	set_prefix_key_1((uint32_t *)2048);
    + 	set_prefix_key_1((uint32_t *)&mem_all[2048]);
      	install_page(root, 0, 0);
     -	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
     +	check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
2:  0b7b0e57 ! 2:  9f300b87 s390x: Test effect of storage keys on some more instructions
    @@ s390x/skey.c: static void test_set_prefix(void)
     +{
     +	uint32_t program_mask;
     +
    -+/*
    -+ * gcc 12.0.1 warns if schib is < 4k.
    -+ * We need such addresses to test fetch protection override.
    -+ */
    -+#pragma GCC diagnostic push
    -+#pragma GCC diagnostic ignored "-Warray-bounds"
     +	asm volatile (
     +		"lr %%r1,%[sid]\n\t"
     +		"spka	0x10\n\t"
    @@ s390x/skey.c: static void test_set_prefix(void)
     +		  [schib] "Q" (*schib)
     +		: "%r1"
     +	);
    -+#pragma GCC diagnostic pop
     +	return program_mask >> 28;
     +}
     +
    @@ s390x/skey.c: static void test_set_prefix(void)
     +		expect_pgm_int();
     +		modify_subchannel_key_1(test_device_sid, schib);
     +		check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
    -+		WRITE_ONCE(schib->pmcw.intparm, 0);
     +		cc = stsch(test_device_sid, schib);
     +		report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
     +		report_prefix_pop();
    @@ s390x/skey.c: static void test_set_prefix(void)
     +		modify_subchannel_key_1(test_device_sid, (struct schib *)0);
     +		install_page(root, 0, 0);
     +		check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
    -+		WRITE_ONCE(schib->pmcw.intparm, 0);
     +		cc = stsch(test_device_sid, schib);
     +		report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
     +		report_prefix_pop();
    @@ s390x/skey.c: static void test_set_prefix(void)
     +		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 *)2048);
    ++		modify_subchannel_key_1(test_device_sid, (struct schib *)&mem_all[2048]);
     +		install_page(root, 0, 0);
     +		check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE);
    -+		WRITE_ONCE(schib->pmcw.intparm, 0);
     +		cc = stsch(test_device_sid, schib);
     +		report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel");
     +		report_prefix_pop();
3:  7fb70993 ! 3:  c4ca0619 s390x: Test effect of storage keys on diag 308
    @@ s390x/skey.c: static void test_store_cpu_address(void)
     +static void test_diag_308(void)
     +{
     +	uint16_t response;
    -+	uint32_t (*ipib)[1024] = (void *)pagebuf;
    ++	uint32_t *ipib = (uint32_t *)pagebuf;
     +
     +	report_prefix_push("DIAG 308");
    -+	(*ipib)[0] = 0; /* Invalid length */
    ++	WRITE_ONCE(ipib[0], 0); /* Invalid length */
     +	set_storage_key(ipib, 0x28, 0);
     +	/* key-controlled protection does not apply */
     +	asm volatile (

base-commit: 8719e8326101c1be8256617caf5835b57e819339
prerequisite-patch-id: aa682f50e4eba0e9b6cacd245d568f5bcca05e0f
prerequisite-patch-id: 55b90f625ada542f074cecb82cf63e2980205ce1
prerequisite-patch-id: bebbc71ca3cc8d085e36a049466dba5a420c9c75
prerequisite-patch-id: d38a4fc7bc1fa6e352502f294cb9413f0b738b99
prerequisite-patch-id: 16ccb9380be55e33fc96639bf69570a9f8327697
-- 
2.33.1

[kvm-unit-tests PATCH v3 1/3] s390x: Test TEID values in storage key test

[Janis Schoetterl-Glausch]

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>
---
 s390x/skey.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/s390x/skey.c b/s390x/skey.c
index 42bf598c..5e234cde 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,68 @@ 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:
+		break;
+	case SOP_ENHANCED_1:
+		report(!teid.esop1_acc_list_or_dat, "valid protection code");
+		break;
+	case SOP_ENHANCED_2:
+		switch (teid_esop2_prot_code(teid)) {
+		case PROT_KEY:
+			access_code = teid.acc_exc_f_s;
+
+			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_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 +262,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 +273,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 +291,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 +377,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 +390,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 +414,7 @@ static void test_set_prefix(void)
 	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
 	set_prefix_key_1((uint32_t *)&mem_all[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.33.1

[kvm-unit-tests PATCH v3 2/3] s390x: Test effect of storage keys on some more instructions

[Janis Schoetterl-Glausch]

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>
---
 s390x/skey.c        | 275 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   1 +
 2 files changed, 276 insertions(+)

diff --git a/s390x/skey.c b/s390x/skey.c
index 5e234cde..a55034b5 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>
@@ -299,6 +300,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.
@@ -425,6 +535,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, (struct schib *)&mem_all[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");
@@ -439,9 +712,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 b456b288..1280ff0f 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.33.1

[kvm-unit-tests PATCH v3 3/3] s390x: Test effect of storage keys on diag 308

[Janis Schoetterl-Glausch]

Test that key-controlled protection does not apply to diag 308.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 s390x/skey.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/s390x/skey.c b/s390x/skey.c
index a55034b5..074667e2 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -300,6 +300,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.
@@ -712,6 +737,7 @@ int main(void)
 	test_chg();
 	test_test_protection();
 	test_store_cpu_address();
+	test_diag_308();
 	test_channel_subsystem_call();
 
 	setup_vm();
-- 
2.33.1

Re: [kvm-unit-tests PATCH v3 3/3] s390x: Test effect of storage keys on diag 308

[Claudio Imbrenda]

On Mon, 23 May 2022 15:24:06 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

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

> ---
>  s390x/skey.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/s390x/skey.c b/s390x/skey.c
> index a55034b5..074667e2 100644
> --- a/s390x/skey.c
> +++ b/s390x/skey.c
> @@ -300,6 +300,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.
> @@ -712,6 +737,7 @@ int main(void)
>  	test_chg();
>  	test_test_protection();
>  	test_store_cpu_address();
> +	test_diag_308();
>  	test_channel_subsystem_call();
>  
>  	setup_vm();

Re: [kvm-unit-tests PATCH v3 1/3] s390x: Test TEID values in storage key test

[Claudio Imbrenda]

On Mon, 23 May 2022 15:24:04 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> 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>
> ---
>  s390x/skey.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/s390x/skey.c b/s390x/skey.c
> index 42bf598c..5e234cde 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,68 @@ 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:
> +		break;

for basic you should check for sop_teid_predictable and sop_acc_list

> +	case SOP_ENHANCED_1:
> +		report(!teid.esop1_acc_list_or_dat, "valid protection code");

actually, both values of esop1_acc_list_or_dat are wrong, since we're
expecting neither an access list nor a dat exception.

you need to check for esop1_teid_predictable instead (which you need to
add, see comment in that patchseries)

> +		break;
> +	case SOP_ENHANCED_2:
> +		switch (teid_esop2_prot_code(teid)) {
> +		case PROT_KEY:
> +			access_code = teid.acc_exc_f_s;

is the f/s feature guaranteed to be present when we have esop2?

can the f/s feature be present with esop1 or basic sop?

> +
> +			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_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 +262,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 +273,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 +291,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 +377,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 +390,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 +414,7 @@ static void test_set_prefix(void)
>  	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>  	set_prefix_key_1((uint32_t *)&mem_all[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();
>  

Re: [kvm-unit-tests PATCH v3 2/3] s390x: Test effect of storage keys on some more instructions

[Claudio Imbrenda]

On Mon, 23 May 2022 15:24:05 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

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

> ---
>  s390x/skey.c        | 275 ++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |   1 +
>  2 files changed, 276 insertions(+)
> 
> diff --git a/s390x/skey.c b/s390x/skey.c
> index 5e234cde..a55034b5 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>
> @@ -299,6 +300,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.
> @@ -425,6 +535,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, (struct schib *)&mem_all[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");
> @@ -439,9 +712,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 b456b288..1280ff0f 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

Re: [kvm-unit-tests PATCH v3 1/3] s390x: Test TEID values in storage key test

[Janis Schoetterl-Glausch]

On Tue, 2022-05-24 at 17:09 +0200, Claudio Imbrenda wrote:
> On Mon, 23 May 2022 15:24:04 +0200
> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
> 
> > 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>
> > ---
> >  s390x/skey.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 69 insertions(+), 6 deletions(-)
> > 
> > diff --git a/s390x/skey.c b/s390x/skey.c
> > index 42bf598c..5e234cde 100644
> > --- a/s390x/skey.c
> > +++ b/s390x/skey.c
> > @@ -8,6 +8,7 @@

[...]

> > +		break;
> > +	case SOP_ENHANCED_2:
> > +		switch (teid_esop2_prot_code(teid)) {
> > +		case PROT_KEY:
> > +			access_code = teid.acc_exc_f_s;
> 
> is the f/s feature guaranteed to be present when we have esop2?

That's how I understand it. For esop1 the PoP explicitly states that
the facility is a prerequisite, for esop2 it doesn't.
> 
> can the f/s feature be present with esop1 or basic sop?

esop1: yes, basic: no.
The way I read it, in the case of esop1 the bits are only meaningful
for DAT and access list exceptions, i.e. when the TEID is not
unpredictable.
> 
> > +
> > +			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_LAP:
> > +			report_pass("valid protection code");
> > +			break;
> > +		default:
> > +			report_fail("valid protection code");
> > +		}
> > +		break;
> > +	}
> > +	report_prefix_pop();
> > +}
> > +

[...]

Re: [kvm-unit-tests PATCH v3 1/3] s390x: Test TEID values in storage key test

[Claudio Imbrenda]

On Wed, 08 Jun 2022 19:03:23 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

[...]

> > > +		break;
> > > +	case SOP_ENHANCED_2:
> > > +		switch (teid_esop2_prot_code(teid)) {
> > > +		case PROT_KEY:
> > > +			access_code = teid.acc_exc_f_s;  
> > 
> > is the f/s feature guaranteed to be present when we have esop2?  
> 
> That's how I understand it. For esop1 the PoP explicitly states that
> the facility is a prerequisite, for esop2 it doesn't.
> > 
> > can the f/s feature be present with esop1 or basic sop?  
> 
> esop1: yes, basic: no.
> The way I read it, in the case of esop1 the bits are only meaningful
> for DAT and access list exceptions, i.e. when the TEID is not
> unpredictable.

I see, makes sense

maybe add a comment :)

> >   
> > > +
> > > +			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_LAP:
> > > +			report_pass("valid protection code");
> > > +			break;
> > > +		default:
> > > +			report_fail("valid protection code");
> > > +		}
> > > +		break;
> > > +	}
> > > +	report_prefix_pop();
> > > +}
> > > +  
> 
> [...]