All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions
@ 2022-04-21  9:04 Janis Schoetterl-Glausch
  2022-04-22 11:56 ` Claudio Imbrenda
  0 siblings, 1 reply; 3+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-04-21  9:04 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank, Claudio Imbrenda
  Cc: Janis Schoetterl-Glausch, David Hildenbrand, kvm, linux-s390

Some instructions are emulated by KVM. Test that KVM correctly emulates
storage key checking for two of those instructions (STORE CPU ADDRESS,
SET PREFIX).
Test success and error conditions, including coverage of storage and
fetch protection override.
Also add test for TEST PROTECTION, even if that instruction will not be
emulated by KVM under normal conditions.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
v2 -> v3:
 * fix asm for SET PREFIX zero key test: make input
 * implement Thomas' suggestions:
   https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@redhat.com/

v1 -> v2:
 * use install_page instead of manual page table entry manipulation
 * check that no store occurred if none is expected
 * try to check that no fetch occured if not expected, although in
   practice a fetch would probably cause the test to crash
 * reset storage key to 0 after test

Range-diff against v2:
1:  a2e076d3 ! 1:  dc4ae46f s390x: Test effect of storage keys on some instructions
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	unsigned long addr = (unsigned long)pagebuf;
     +
     +	report_prefix_push("TPROT");
    ++
     +	set_storage_key(pagebuf, 0x10, 0);
     +	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
     +	report(tprot(addr, 1) == 0, "access key matches -> no protection");
     +	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
    ++
     +	set_storage_key(pagebuf, 0x18, 0);
     +	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
    ++
    ++	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
    ++	set_storage_key(pagebuf, 0x90, 0);
    ++	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
    ++	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
    ++
    ++	set_storage_key(pagebuf, 0x00, 0);
     +	report_prefix_pop();
     +}
     +
    ++/*
    ++ * Perform STORE CPU ADDRESS (STAP) instruction while temporarily executing
    ++ * with access key 1.
    ++ */
     +static void store_cpu_address_key_1(uint16_t *out)
     +{
     +	asm volatile (
     +		"spka 0x10(0)\n\t"
     +		"stap %0\n\t"
     +		"spka 0(0)\n"
    -+	     : "+Q" (*out) /* exception: old value remains in out -> + constraint*/
    ++	     : "+Q" (*out) /* exception: old value remains in out -> + constraint */
     +	);
     +}
     +
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	uint16_t *out = (uint16_t *)pagebuf;
     +	uint16_t cpu_addr;
     +
    ++	report_prefix_push("STORE CPU ADDRESS");
     +	asm ("stap %0" : "=Q" (cpu_addr));
     +
    -+	report_prefix_push("STORE CPU ADDRESS, zero key");
    ++	report_prefix_push("zero key");
     +	set_storage_key(pagebuf, 0x20, 0);
    -+	*out = 0xbeef;
    ++	WRITE_ONCE(*out, 0xbeef);
     +	asm ("stap %0" : "=Q" (*out));
     +	report(*out == cpu_addr, "store occurred");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("STORE CPU ADDRESS, matching key");
    ++	report_prefix_push("matching key");
     +	set_storage_key(pagebuf, 0x10, 0);
     +	*out = 0xbeef;
     +	store_cpu_address_key_1(out);
     +	report(*out == cpu_addr, "store occurred");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("STORE CPU ADDRESS, mismatching key");
    ++	report_prefix_push("mismatching key");
     +	set_storage_key(pagebuf, 0x20, 0);
     +	expect_pgm_int();
     +	*out = 0xbeef;
    @@ s390x/skey.c: static void test_invalid_address(void)
     +
     +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
     +
    -+	report_prefix_push("STORE CPU ADDRESS, storage-protection override, invalid key");
    ++	report_prefix_push("storage-protection override, invalid key");
     +	set_storage_key(pagebuf, 0x20, 0);
     +	expect_pgm_int();
     +	*out = 0xbeef;
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	report(*out == 0xbeef, "no store occurred");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("STORE CPU ADDRESS, storage-protection override, override key");
    ++	report_prefix_push("storage-protection override, override key");
     +	set_storage_key(pagebuf, 0x90, 0);
     +	*out = 0xbeef;
     +	store_cpu_address_key_1(out);
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	report_prefix_pop();
     +
     +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
    ++
    ++	report_prefix_push("storage-protection override disabled, override key");
    ++	set_storage_key(pagebuf, 0x90, 0);
    ++	expect_pgm_int();
    ++	*out = 0xbeef;
    ++	store_cpu_address_key_1(out);
    ++	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
    ++	report(*out == 0xbeef, "no store occurred");
    ++	report_prefix_pop();
    ++
     +	set_storage_key(pagebuf, 0x00, 0);
    ++	report_prefix_pop();
     +}
     +
    ++/*
    ++ * Perform SET PREFIX (SPX) instruction while temporarily executing
    ++ * with access key 1.
    ++ */
     +static void set_prefix_key_1(uint32_t *out)
     +{
     +	asm volatile (
    @@ s390x/skey.c: static void test_invalid_address(void)
     +
     +/*
     + * We remapped page 0, making the lowcore inaccessible, which breaks the normal
    -+ * hanlder and breaks skipping the faulting instruction.
    ++ * handler and breaks skipping the faulting instruction.
     + * Just disable dynamic address translation to make things work.
     + */
     +static void dat_fixup_pgm_int(void)
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	uint32_t *out = (uint32_t *)pagebuf;
     +	pgd_t *root;
     +
    ++	report_prefix_push("SET PREFIX");
     +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
     +
     +	asm volatile("stpx	%0" : "=Q"(*out));
     +
    -+	report_prefix_push("SET PREFIX, zero key");
    ++	report_prefix_push("zero key");
     +	set_storage_key(pagebuf, 0x20, 0);
    -+	asm volatile("spx	%0" : "=Q" (*out));
    ++	asm volatile("spx	%0" :: "Q" (*out));
     +	report_pass("no exception");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("SET PREFIX, matching key");
    ++	report_prefix_push("matching key");
     +	set_storage_key(pagebuf, 0x10, 0);
     +	set_prefix_key_1(out);
     +	report_pass("no exception");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("SET PREFIX, mismatching key, no fetch protection");
    ++	report_prefix_push("mismatching key, no fetch protection");
     +	set_storage_key(pagebuf, 0x20, 0);
     +	set_prefix_key_1(out);
     +	report_pass("no exception");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection");
    ++	report_prefix_push("mismatching key, fetch protection");
     +	set_storage_key(pagebuf, 0x28, 0);
     +	expect_pgm_int();
     +	*out = 0xdeadbeef;
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	report_prefix_pop();
     +
     +	register_pgm_cleanup_func(dat_fixup_pgm_int);
    ++
    ++	report_prefix_push("mismatching key, remapped page, fetch protection");
    ++	set_storage_key(pagebuf, 0x28, 0);
    ++	expect_pgm_int();
    ++	WRITE_ONCE(*out, 0xdeadbeef);
    ++	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);
    ++	asm volatile("stpx	%0" : "=Q"(*out));
    ++	report(*out != 0xdeadbeef, "no fetch occurred");
    ++	report_prefix_pop();
    ++
     +	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
     +
    -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection override applies");
    ++	report_prefix_push("mismatching key, fetch protection override applies");
     +	set_storage_key(pagebuf, 0x28, 0);
     +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
    -+	set_prefix_key_1(0);
    ++	set_prefix_key_1((uint32_t *)0);
     +	install_page(root, 0, 0);
     +	report_pass("no exception");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection override does not apply");
    ++	report_prefix_push("mismatching key, fetch protection override does not apply");
     +	out = (uint32_t *)(pagebuf + 2048);
     +	set_storage_key(pagebuf, 0x28, 0);
     +	expect_pgm_int();
    -+	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
     +	WRITE_ONCE(*out, 0xdeadbeef);
    ++	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
     +	set_prefix_key_1((uint32_t *)2048);
     +	install_page(root, 0, 0);
     +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
     +	set_storage_key(pagebuf, 0x00, 0);
     +	register_pgm_cleanup_func(NULL);
    ++	report_prefix_pop();
     +}
     +
      int main(void)

 lib/s390x/asm/arch_def.h |  20 ++--
 s390x/skey.c             | 215 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+), 9 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index bab3c374..676a2753 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -55,15 +55,17 @@ struct psw {
 #define PSW_MASK_BA			0x0000000080000000UL
 #define PSW_MASK_64			(PSW_MASK_BA | PSW_MASK_EA)
 
-#define CTL0_LOW_ADDR_PROT		(63 - 35)
-#define CTL0_EDAT			(63 - 40)
-#define CTL0_IEP			(63 - 43)
-#define CTL0_AFP			(63 - 45)
-#define CTL0_VECTOR			(63 - 46)
-#define CTL0_EMERGENCY_SIGNAL		(63 - 49)
-#define CTL0_EXTERNAL_CALL		(63 - 50)
-#define CTL0_CLOCK_COMPARATOR		(63 - 52)
-#define CTL0_SERVICE_SIGNAL		(63 - 54)
+#define CTL0_LOW_ADDR_PROT			(63 - 35)
+#define CTL0_EDAT				(63 - 40)
+#define CTL0_FETCH_PROTECTION_OVERRIDE		(63 - 38)
+#define CTL0_STORAGE_PROTECTION_OVERRIDE	(63 - 39)
+#define CTL0_IEP				(63 - 43)
+#define CTL0_AFP				(63 - 45)
+#define CTL0_VECTOR				(63 - 46)
+#define CTL0_EMERGENCY_SIGNAL			(63 - 49)
+#define CTL0_EXTERNAL_CALL			(63 - 50)
+#define CTL0_CLOCK_COMPARATOR			(63 - 52)
+#define CTL0_SERVICE_SIGNAL			(63 - 54)
 #define CR0_EXTM_MASK			0x0000000000006200UL /* Combined external masks */
 
 #define CTL2_GUARDED_STORAGE		(63 - 59)
diff --git a/s390x/skey.c b/s390x/skey.c
index edad53e9..849d105f 100644
--- a/s390x/skey.c
+++ b/s390x/skey.c
@@ -10,6 +10,7 @@
 #include <libcflat.h>
 #include <asm/asm-offsets.h>
 #include <asm/interrupt.h>
+#include <vmalloc.h>
 #include <asm/page.h>
 #include <asm/facility.h>
 #include <asm/mem.h>
@@ -118,6 +119,215 @@ static void test_invalid_address(void)
 	report_prefix_pop();
 }
 
+static void test_test_protection(void)
+{
+	unsigned long addr = (unsigned long)pagebuf;
+
+	report_prefix_push("TPROT");
+
+	set_storage_key(pagebuf, 0x10, 0);
+	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
+	report(tprot(addr, 1) == 0, "access key matches -> no protection");
+	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
+
+	set_storage_key(pagebuf, 0x18, 0);
+	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
+
+	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+	set_storage_key(pagebuf, 0x90, 0);
+	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
+	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+
+	set_storage_key(pagebuf, 0x00, 0);
+	report_prefix_pop();
+}
+
+/*
+ * Perform STORE CPU ADDRESS (STAP) instruction while temporarily executing
+ * with access key 1.
+ */
+static void store_cpu_address_key_1(uint16_t *out)
+{
+	asm volatile (
+		"spka 0x10(0)\n\t"
+		"stap %0\n\t"
+		"spka 0(0)\n"
+	     : "+Q" (*out) /* exception: old value remains in out -> + constraint */
+	);
+}
+
+static void test_store_cpu_address(void)
+{
+	uint16_t *out = (uint16_t *)pagebuf;
+	uint16_t cpu_addr;
+
+	report_prefix_push("STORE CPU ADDRESS");
+	asm ("stap %0" : "=Q" (cpu_addr));
+
+	report_prefix_push("zero key");
+	set_storage_key(pagebuf, 0x20, 0);
+	WRITE_ONCE(*out, 0xbeef);
+	asm ("stap %0" : "=Q" (*out));
+	report(*out == cpu_addr, "store occurred");
+	report_prefix_pop();
+
+	report_prefix_push("matching key");
+	set_storage_key(pagebuf, 0x10, 0);
+	*out = 0xbeef;
+	store_cpu_address_key_1(out);
+	report(*out == cpu_addr, "store occurred");
+	report_prefix_pop();
+
+	report_prefix_push("mismatching key");
+	set_storage_key(pagebuf, 0x20, 0);
+	expect_pgm_int();
+	*out = 0xbeef;
+	store_cpu_address_key_1(out);
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	report(*out == 0xbeef, "no store occurred");
+	report_prefix_pop();
+
+	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+
+	report_prefix_push("storage-protection override, invalid key");
+	set_storage_key(pagebuf, 0x20, 0);
+	expect_pgm_int();
+	*out = 0xbeef;
+	store_cpu_address_key_1(out);
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	report(*out == 0xbeef, "no store occurred");
+	report_prefix_pop();
+
+	report_prefix_push("storage-protection override, override key");
+	set_storage_key(pagebuf, 0x90, 0);
+	*out = 0xbeef;
+	store_cpu_address_key_1(out);
+	report(*out == cpu_addr, "override occurred");
+	report_prefix_pop();
+
+	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
+
+	report_prefix_push("storage-protection override disabled, override key");
+	set_storage_key(pagebuf, 0x90, 0);
+	expect_pgm_int();
+	*out = 0xbeef;
+	store_cpu_address_key_1(out);
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	report(*out == 0xbeef, "no store occurred");
+	report_prefix_pop();
+
+	set_storage_key(pagebuf, 0x00, 0);
+	report_prefix_pop();
+}
+
+/*
+ * Perform SET PREFIX (SPX) instruction while temporarily executing
+ * with access key 1.
+ */
+static void set_prefix_key_1(uint32_t *out)
+{
+	asm volatile (
+		"spka 0x10(0)\n\t"
+		"spx	%0\n\t"
+		"spka 0(0)\n"
+	     :: "Q" (*out)
+	);
+}
+
+/*
+ * 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);
+}
+
+static void test_set_prefix(void)
+{
+	uint32_t *out = (uint32_t *)pagebuf;
+	pgd_t *root;
+
+	report_prefix_push("SET PREFIX");
+	root = (pgd_t *)(stctg(1) & PAGE_MASK);
+
+	asm volatile("stpx	%0" : "=Q"(*out));
+
+	report_prefix_push("zero key");
+	set_storage_key(pagebuf, 0x20, 0);
+	asm volatile("spx	%0" :: "Q" (*out));
+	report_pass("no exception");
+	report_prefix_pop();
+
+	report_prefix_push("matching key");
+	set_storage_key(pagebuf, 0x10, 0);
+	set_prefix_key_1(out);
+	report_pass("no exception");
+	report_prefix_pop();
+
+	report_prefix_push("mismatching key, no fetch protection");
+	set_storage_key(pagebuf, 0x20, 0);
+	set_prefix_key_1(out);
+	report_pass("no exception");
+	report_prefix_pop();
+
+	report_prefix_push("mismatching key, fetch protection");
+	set_storage_key(pagebuf, 0x28, 0);
+	expect_pgm_int();
+	*out = 0xdeadbeef;
+	set_prefix_key_1(out);
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	asm volatile("stpx	%0" : "=Q"(*out));
+	report(*out != 0xdeadbeef, "no fetch occurred");
+	report_prefix_pop();
+
+	register_pgm_cleanup_func(dat_fixup_pgm_int);
+
+	report_prefix_push("mismatching key, remapped page, fetch protection");
+	set_storage_key(pagebuf, 0x28, 0);
+	expect_pgm_int();
+	WRITE_ONCE(*out, 0xdeadbeef);
+	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);
+	asm volatile("stpx	%0" : "=Q"(*out));
+	report(*out != 0xdeadbeef, "no fetch occurred");
+	report_prefix_pop();
+
+	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
+
+	report_prefix_push("mismatching key, fetch protection override applies");
+	set_storage_key(pagebuf, 0x28, 0);
+	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
+	set_prefix_key_1((uint32_t *)0);
+	install_page(root, 0, 0);
+	report_pass("no exception");
+	report_prefix_pop();
+
+	report_prefix_push("mismatching key, fetch protection override does not apply");
+	out = (uint32_t *)(pagebuf + 2048);
+	set_storage_key(pagebuf, 0x28, 0);
+	expect_pgm_int();
+	WRITE_ONCE(*out, 0xdeadbeef);
+	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
+	set_prefix_key_1((uint32_t *)2048);
+	install_page(root, 0, 0);
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	asm volatile("stpx	%0" : "=Q"(*out));
+	report(*out != 0xdeadbeef, "no fetch occurred");
+	report_prefix_pop();
+
+	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
+	set_storage_key(pagebuf, 0x00, 0);
+	register_pgm_cleanup_func(NULL);
+	report_prefix_pop();
+}
+
 int main(void)
 {
 	report_prefix_push("skey");
@@ -130,6 +340,11 @@ int main(void)
 	test_set();
 	test_set_mb();
 	test_chg();
+	test_test_protection();
+	test_store_cpu_address();
+
+	setup_vm();
+	test_set_prefix();
 done:
 	report_prefix_pop();
 	return report_summary();

base-commit: 6a7a83ed106211fc0ee530a3a05f171f6a4c4e66
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions
  2022-04-21  9:04 [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions Janis Schoetterl-Glausch
@ 2022-04-22 11:56 ` Claudio Imbrenda
  2022-04-22 12:19   ` Janis Schoetterl-Glausch
  0 siblings, 1 reply; 3+ messages in thread
From: Claudio Imbrenda @ 2022-04-22 11:56 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch
  Cc: Thomas Huth, Janosch Frank, David Hildenbrand, kvm, linux-s390

On Thu, 21 Apr 2022 11:04:21 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> Some instructions are emulated by KVM. Test that KVM correctly emulates
> storage key checking for two of those instructions (STORE CPU ADDRESS,
> SET PREFIX).
> Test success and error conditions, including coverage of storage and
> fetch protection override.
> Also add test for TEST PROTECTION, even if that instruction will not be
> emulated by KVM under normal conditions.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
> v2 -> v3:
>  * fix asm for SET PREFIX zero key test: make input
>  * implement Thomas' suggestions:
>    https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@redhat.com/
> 
> v1 -> v2:
>  * use install_page instead of manual page table entry manipulation
>  * check that no store occurred if none is expected
>  * try to check that no fetch occured if not expected, although in
>    practice a fetch would probably cause the test to crash
>  * reset storage key to 0 after test
> 
> Range-diff against v2:
> 1:  a2e076d3 ! 1:  dc4ae46f s390x: Test effect of storage keys on some instructions
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	unsigned long addr = (unsigned long)pagebuf;
>      +
>      +	report_prefix_push("TPROT");
>     ++
>      +	set_storage_key(pagebuf, 0x10, 0);
>      +	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
>      +	report(tprot(addr, 1) == 0, "access key matches -> no protection");
>      +	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
>     ++
>      +	set_storage_key(pagebuf, 0x18, 0);
>      +	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
>     ++
>     ++	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>     ++	set_storage_key(pagebuf, 0x90, 0);
>     ++	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
>     ++	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>     ++
>     ++	set_storage_key(pagebuf, 0x00, 0);
>      +	report_prefix_pop();
>      +}
>      +
>     ++/*
>     ++ * Perform STORE CPU ADDRESS (STAP) instruction while temporarily executing
>     ++ * with access key 1.
>     ++ */
>      +static void store_cpu_address_key_1(uint16_t *out)
>      +{
>      +	asm volatile (
>      +		"spka 0x10(0)\n\t"
>      +		"stap %0\n\t"
>      +		"spka 0(0)\n"
>     -+	     : "+Q" (*out) /* exception: old value remains in out -> + constraint*/
>     ++	     : "+Q" (*out) /* exception: old value remains in out -> + constraint */
>      +	);
>      +}
>      +
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	uint16_t *out = (uint16_t *)pagebuf;
>      +	uint16_t cpu_addr;
>      +
>     ++	report_prefix_push("STORE CPU ADDRESS");
>      +	asm ("stap %0" : "=Q" (cpu_addr));
>      +
>     -+	report_prefix_push("STORE CPU ADDRESS, zero key");
>     ++	report_prefix_push("zero key");
>      +	set_storage_key(pagebuf, 0x20, 0);
>     -+	*out = 0xbeef;
>     ++	WRITE_ONCE(*out, 0xbeef);
>      +	asm ("stap %0" : "=Q" (*out));
>      +	report(*out == cpu_addr, "store occurred");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("STORE CPU ADDRESS, matching key");
>     ++	report_prefix_push("matching key");
>      +	set_storage_key(pagebuf, 0x10, 0);
>      +	*out = 0xbeef;
>      +	store_cpu_address_key_1(out);
>      +	report(*out == cpu_addr, "store occurred");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("STORE CPU ADDRESS, mismatching key");
>     ++	report_prefix_push("mismatching key");
>      +	set_storage_key(pagebuf, 0x20, 0);
>      +	expect_pgm_int();
>      +	*out = 0xbeef;
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +
>      +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>      +
>     -+	report_prefix_push("STORE CPU ADDRESS, storage-protection override, invalid key");
>     ++	report_prefix_push("storage-protection override, invalid key");
>      +	set_storage_key(pagebuf, 0x20, 0);
>      +	expect_pgm_int();
>      +	*out = 0xbeef;
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	report(*out == 0xbeef, "no store occurred");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("STORE CPU ADDRESS, storage-protection override, override key");
>     ++	report_prefix_push("storage-protection override, override key");
>      +	set_storage_key(pagebuf, 0x90, 0);
>      +	*out = 0xbeef;
>      +	store_cpu_address_key_1(out);
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	report_prefix_pop();
>      +
>      +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>     ++
>     ++	report_prefix_push("storage-protection override disabled, override key");
>     ++	set_storage_key(pagebuf, 0x90, 0);
>     ++	expect_pgm_int();
>     ++	*out = 0xbeef;
>     ++	store_cpu_address_key_1(out);
>     ++	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>     ++	report(*out == 0xbeef, "no store occurred");
>     ++	report_prefix_pop();
>     ++
>      +	set_storage_key(pagebuf, 0x00, 0);
>     ++	report_prefix_pop();
>      +}
>      +
>     ++/*
>     ++ * Perform SET PREFIX (SPX) instruction while temporarily executing
>     ++ * with access key 1.
>     ++ */
>      +static void set_prefix_key_1(uint32_t *out)
>      +{
>      +	asm volatile (
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +
>      +/*
>      + * We remapped page 0, making the lowcore inaccessible, which breaks the normal
>     -+ * hanlder and breaks skipping the faulting instruction.
>     ++ * handler and breaks skipping the faulting instruction.
>      + * Just disable dynamic address translation to make things work.
>      + */
>      +static void dat_fixup_pgm_int(void)
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	uint32_t *out = (uint32_t *)pagebuf;
>      +	pgd_t *root;
>      +
>     ++	report_prefix_push("SET PREFIX");
>      +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
>      +
>      +	asm volatile("stpx	%0" : "=Q"(*out));
>      +
>     -+	report_prefix_push("SET PREFIX, zero key");
>     ++	report_prefix_push("zero key");
>      +	set_storage_key(pagebuf, 0x20, 0);
>     -+	asm volatile("spx	%0" : "=Q" (*out));
>     ++	asm volatile("spx	%0" :: "Q" (*out));
>      +	report_pass("no exception");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("SET PREFIX, matching key");
>     ++	report_prefix_push("matching key");
>      +	set_storage_key(pagebuf, 0x10, 0);
>      +	set_prefix_key_1(out);
>      +	report_pass("no exception");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("SET PREFIX, mismatching key, no fetch protection");
>     ++	report_prefix_push("mismatching key, no fetch protection");
>      +	set_storage_key(pagebuf, 0x20, 0);
>      +	set_prefix_key_1(out);
>      +	report_pass("no exception");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection");
>     ++	report_prefix_push("mismatching key, fetch protection");
>      +	set_storage_key(pagebuf, 0x28, 0);
>      +	expect_pgm_int();
>      +	*out = 0xdeadbeef;
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	report_prefix_pop();
>      +
>      +	register_pgm_cleanup_func(dat_fixup_pgm_int);
>     ++
>     ++	report_prefix_push("mismatching key, remapped page, fetch protection");
>     ++	set_storage_key(pagebuf, 0x28, 0);
>     ++	expect_pgm_int();
>     ++	WRITE_ONCE(*out, 0xdeadbeef);
>     ++	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);
>     ++	asm volatile("stpx	%0" : "=Q"(*out));
>     ++	report(*out != 0xdeadbeef, "no fetch occurred");
>     ++	report_prefix_pop();
>     ++
>      +	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>      +
>     -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection override applies");
>     ++	report_prefix_push("mismatching key, fetch protection override applies");
>      +	set_storage_key(pagebuf, 0x28, 0);
>      +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>     -+	set_prefix_key_1(0);
>     ++	set_prefix_key_1((uint32_t *)0);
>      +	install_page(root, 0, 0);
>      +	report_pass("no exception");
>      +	report_prefix_pop();
>      +
>     -+	report_prefix_push("SET PREFIX, mismatching key, fetch protection override does not apply");
>     ++	report_prefix_push("mismatching key, fetch protection override does not apply");
>      +	out = (uint32_t *)(pagebuf + 2048);
>      +	set_storage_key(pagebuf, 0x28, 0);
>      +	expect_pgm_int();
>     -+	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>      +	WRITE_ONCE(*out, 0xdeadbeef);
>     ++	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>      +	set_prefix_key_1((uint32_t *)2048);
>      +	install_page(root, 0, 0);
>      +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>      +	set_storage_key(pagebuf, 0x00, 0);
>      +	register_pgm_cleanup_func(NULL);
>     ++	report_prefix_pop();
>      +}
>      +
>       int main(void)
> 
>  lib/s390x/asm/arch_def.h |  20 ++--
>  s390x/skey.c             | 215 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 226 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index bab3c374..676a2753 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -55,15 +55,17 @@ struct psw {
>  #define PSW_MASK_BA			0x0000000080000000UL
>  #define PSW_MASK_64			(PSW_MASK_BA | PSW_MASK_EA)
>  
> -#define CTL0_LOW_ADDR_PROT		(63 - 35)
> -#define CTL0_EDAT			(63 - 40)
> -#define CTL0_IEP			(63 - 43)
> -#define CTL0_AFP			(63 - 45)
> -#define CTL0_VECTOR			(63 - 46)
> -#define CTL0_EMERGENCY_SIGNAL		(63 - 49)
> -#define CTL0_EXTERNAL_CALL		(63 - 50)
> -#define CTL0_CLOCK_COMPARATOR		(63 - 52)
> -#define CTL0_SERVICE_SIGNAL		(63 - 54)
> +#define CTL0_LOW_ADDR_PROT			(63 - 35)
> +#define CTL0_EDAT				(63 - 40)
> +#define CTL0_FETCH_PROTECTION_OVERRIDE		(63 - 38)
> +#define CTL0_STORAGE_PROTECTION_OVERRIDE	(63 - 39)
> +#define CTL0_IEP				(63 - 43)
> +#define CTL0_AFP				(63 - 45)
> +#define CTL0_VECTOR				(63 - 46)
> +#define CTL0_EMERGENCY_SIGNAL			(63 - 49)
> +#define CTL0_EXTERNAL_CALL			(63 - 50)
> +#define CTL0_CLOCK_COMPARATOR			(63 - 52)
> +#define CTL0_SERVICE_SIGNAL			(63 - 54)
>  #define CR0_EXTM_MASK			0x0000000000006200UL /* Combined external masks */
>  
>  #define CTL2_GUARDED_STORAGE		(63 - 59)
> diff --git a/s390x/skey.c b/s390x/skey.c
> index edad53e9..849d105f 100644
> --- a/s390x/skey.c
> +++ b/s390x/skey.c
> @@ -10,6 +10,7 @@
>  #include <libcflat.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/interrupt.h>
> +#include <vmalloc.h>
>  #include <asm/page.h>
>  #include <asm/facility.h>
>  #include <asm/mem.h>
> @@ -118,6 +119,215 @@ static void test_invalid_address(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_test_protection(void)
> +{
> +	unsigned long addr = (unsigned long)pagebuf;
> +
> +	report_prefix_push("TPROT");
> +
> +	set_storage_key(pagebuf, 0x10, 0);
> +	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
> +	report(tprot(addr, 1) == 0, "access key matches -> no protection");
> +	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
> +
> +	set_storage_key(pagebuf, 0x18, 0);
> +	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
> +
> +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +	set_storage_key(pagebuf, 0x90, 0);
> +	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
> +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +
> +	set_storage_key(pagebuf, 0x00, 0);
> +	report_prefix_pop();
> +}
> +
> +/*
> + * Perform STORE CPU ADDRESS (STAP) instruction while temporarily executing
> + * with access key 1.
> + */
> +static void store_cpu_address_key_1(uint16_t *out)
> +{
> +	asm volatile (
> +		"spka 0x10(0)\n\t"
> +		"stap %0\n\t"
> +		"spka 0(0)\n"
> +	     : "+Q" (*out) /* exception: old value remains in out -> + constraint */
> +	);
> +}
> +
> +static void test_store_cpu_address(void)
> +{
> +	uint16_t *out = (uint16_t *)pagebuf;
> +	uint16_t cpu_addr;
> +
> +	report_prefix_push("STORE CPU ADDRESS");
> +	asm ("stap %0" : "=Q" (cpu_addr));
> +
> +	report_prefix_push("zero key");
> +	set_storage_key(pagebuf, 0x20, 0);
> +	WRITE_ONCE(*out, 0xbeef);
> +	asm ("stap %0" : "=Q" (*out));
> +	report(*out == cpu_addr, "store occurred");
> +	report_prefix_pop();
> +
> +	report_prefix_push("matching key");
> +	set_storage_key(pagebuf, 0x10, 0);
> +	*out = 0xbeef;
> +	store_cpu_address_key_1(out);
> +	report(*out == cpu_addr, "store occurred");
> +	report_prefix_pop();
> +
> +	report_prefix_push("mismatching key");
> +	set_storage_key(pagebuf, 0x20, 0);
> +	expect_pgm_int();
> +	*out = 0xbeef;
> +	store_cpu_address_key_1(out);
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	report(*out == 0xbeef, "no store occurred");
> +	report_prefix_pop();
> +
> +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +
> +	report_prefix_push("storage-protection override, invalid key");
> +	set_storage_key(pagebuf, 0x20, 0);
> +	expect_pgm_int();
> +	*out = 0xbeef;
> +	store_cpu_address_key_1(out);
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	report(*out == 0xbeef, "no store occurred");
> +	report_prefix_pop();
> +
> +	report_prefix_push("storage-protection override, override key");
> +	set_storage_key(pagebuf, 0x90, 0);
> +	*out = 0xbeef;
> +	store_cpu_address_key_1(out);
> +	report(*out == cpu_addr, "override occurred");
> +	report_prefix_pop();
> +
> +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +
> +	report_prefix_push("storage-protection override disabled, override key");
> +	set_storage_key(pagebuf, 0x90, 0);
> +	expect_pgm_int();
> +	*out = 0xbeef;
> +	store_cpu_address_key_1(out);
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	report(*out == 0xbeef, "no store occurred");
> +	report_prefix_pop();
> +
> +	set_storage_key(pagebuf, 0x00, 0);
> +	report_prefix_pop();
> +}
> +
> +/*
> + * Perform SET PREFIX (SPX) instruction while temporarily executing
> + * with access key 1.
> + */
> +static void set_prefix_key_1(uint32_t *out)
> +{
> +	asm volatile (
> +		"spka 0x10(0)\n\t"
> +		"spx	%0\n\t"
> +		"spka 0(0)\n"
> +	     :: "Q" (*out)
> +	);
> +}
> +
> +/*
> + * 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);
> +}
> +
> +static void test_set_prefix(void)
> +{
> +	uint32_t *out = (uint32_t *)pagebuf;
> +	pgd_t *root;
> +
> +	report_prefix_push("SET PREFIX");
> +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
> +
> +	asm volatile("stpx	%0" : "=Q"(*out));
> +
> +	report_prefix_push("zero key");
> +	set_storage_key(pagebuf, 0x20, 0);
> +	asm volatile("spx	%0" :: "Q" (*out));

so you are changing the prefix to... the old prefix (so nothing
changes). how do you know that something happened at all?

(see my longer comment below)

> +	report_pass("no exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("matching key");
> +	set_storage_key(pagebuf, 0x10, 0);
> +	set_prefix_key_1(out);
> +	report_pass("no exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("mismatching key, no fetch protection");
> +	set_storage_key(pagebuf, 0x20, 0);
> +	set_prefix_key_1(out);
> +	report_pass("no exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("mismatching key, fetch protection");
> +	set_storage_key(pagebuf, 0x28, 0);
> +	expect_pgm_int();
> +	*out = 0xdeadbeef;

so here you are trying to set 0xdeadbeef as prefix, right?
which would fail for other reasons I guess, since that would be outside
memory (unless otherwise specified, kvm unit tests run with 128M of ram)

> +	set_prefix_key_1(out);
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	asm volatile("stpx	%0" : "=Q"(*out));
> +	report(*out != 0xdeadbeef, "no fetch occurred");

and here you check that the prefix has not changed to that "wrong
value", which would be impossible anyway because it would be outside of
memory. moreover the address you give is not even in the lower 2G, so
in case the address has been changed, it would not match your magic
value anyway.

for this (and the following) tests, I propose the following:

add a new
char tmplowcore[2 * PAGE_SIZE] __attribute((aligned(2*PAGE_SIZE)));

initialize it properly (memcpy the actual lowcore into it), and use
that address for SPX. this way if SPX does not fail, at least you would
have a valid lowcore. and at that point you can check against that
address instead of your magic

> +	report_prefix_pop();
> +
> +	register_pgm_cleanup_func(dat_fixup_pgm_int);
> +
> +	report_prefix_push("mismatching key, remapped page, fetch protection");
> +	set_storage_key(pagebuf, 0x28, 0);
> +	expect_pgm_int();
> +	WRITE_ONCE(*out, 0xdeadbeef);
> +	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);
> +	asm volatile("stpx	%0" : "=Q"(*out));
> +	report(*out != 0xdeadbeef, "no fetch occurred");
> +	report_prefix_pop();
> +
> +	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
> +
> +	report_prefix_push("mismatching key, fetch protection override applies");
> +	set_storage_key(pagebuf, 0x28, 0);
> +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
> +	set_prefix_key_1((uint32_t *)0);
> +	install_page(root, 0, 0);
> +	report_pass("no exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("mismatching key, fetch protection override does not apply");
> +	out = (uint32_t *)(pagebuf + 2048);
> +	set_storage_key(pagebuf, 0x28, 0);
> +	expect_pgm_int();
> +	WRITE_ONCE(*out, 0xdeadbeef);
> +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
> +	set_prefix_key_1((uint32_t *)2048);
> +	install_page(root, 0, 0);
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	asm volatile("stpx	%0" : "=Q"(*out));
> +	report(*out != 0xdeadbeef, "no fetch occurred");
> +	report_prefix_pop();
> +
> +	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
> +	set_storage_key(pagebuf, 0x00, 0);
> +	register_pgm_cleanup_func(NULL);
> +	report_prefix_pop();
> +}
> +
>  int main(void)
>  {
>  	report_prefix_push("skey");
> @@ -130,6 +340,11 @@ int main(void)
>  	test_set();
>  	test_set_mb();
>  	test_chg();
> +	test_test_protection();
> +	test_store_cpu_address();
> +
> +	setup_vm();
> +	test_set_prefix();
>  done:
>  	report_prefix_pop();
>  	return report_summary();
> 
> base-commit: 6a7a83ed106211fc0ee530a3a05f171f6a4c4e66


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions
  2022-04-22 11:56 ` Claudio Imbrenda
@ 2022-04-22 12:19   ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 3+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-04-22 12:19 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Thomas Huth, Janosch Frank, David Hildenbrand, kvm, linux-s390

On 4/22/22 13:56, Claudio Imbrenda wrote:
> On Thu, 21 Apr 2022 11:04:21 +0200
> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
> 
>> Some instructions are emulated by KVM. Test that KVM correctly emulates
>> storage key checking for two of those instructions (STORE CPU ADDRESS,
>> SET PREFIX).
>> Test success and error conditions, including coverage of storage and
>> fetch protection override.
>> Also add test for TEST PROTECTION, even if that instruction will not be
>> emulated by KVM under normal conditions.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>> v2 -> v3:
>>  * fix asm for SET PREFIX zero key test: make input
>>  * implement Thomas' suggestions:
>>    https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@redhat.com/
>>
>> v1 -> v2:
>>  * use install_page instead of manual page table entry manipulation
>>  * check that no store occurred if none is expected
>>  * try to check that no fetch occured if not expected, although in
>>    practice a fetch would probably cause the test to crash
>>  * reset storage key to 0 after test
>>

[...]

>> +static void test_set_prefix(void)
>> +{
>> +	uint32_t *out = (uint32_t *)pagebuf;
>> +	pgd_t *root;
>> +
>> +	report_prefix_push("SET PREFIX");
>> +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
>> +
>> +	asm volatile("stpx	%0" : "=Q"(*out));
>> +
>> +	report_prefix_push("zero key");
>> +	set_storage_key(pagebuf, 0x20, 0);
>> +	asm volatile("spx	%0" :: "Q" (*out));
> 
> so you are changing the prefix to... the old prefix (so nothing
> changes). how do you know that something happened at all?

Basically, I'm only checking that no exception occurs, which would
crash the test. 
> 
> (see my longer comment below)
> 
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("matching key");
>> +	set_storage_key(pagebuf, 0x10, 0);
>> +	set_prefix_key_1(out);
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("mismatching key, no fetch protection");
>> +	set_storage_key(pagebuf, 0x20, 0);
>> +	set_prefix_key_1(out);
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("mismatching key, fetch protection");
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	expect_pgm_int();
>> +	*out = 0xdeadbeef;
> 
> so here you are trying to set 0xdeadbeef as prefix, right?

In the sense that I'm executing SPX with that as operand, yes, but
the hypervisor is not supposed to ever read that value, so the
content should not matter.

> which would fail for other reasons I guess, since that would be outside
> memory (unless otherwise specified, kvm unit tests run with 128M of ram)
> 
>> +	set_prefix_key_1(out);
>> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);

This ^ is the important check, the two lines below indeed questionable,
I considered just dropping them.

>> +	asm volatile("stpx	%0" : "=Q"(*out));
>> +	report(*out != 0xdeadbeef, "no fetch occurred");
> 
> and here you check that the prefix has not changed to that "wrong
> value", which would be impossible anyway because it would be outside of
> memory. moreover the address you give is not even in the lower 2G, so
> in case the address has been changed, it would not match your magic
> value anyway.
> 
> for this (and the following) tests, I propose the following:
> 
> add a new
> char tmplowcore[2 * PAGE_SIZE] __attribute((aligned(2*PAGE_SIZE)));
> 
> initialize it properly (memcpy the actual lowcore into it), and use
> that address for SPX. this way if SPX does not fail, at least you would
> have a valid lowcore. and at that point you can check against that
> address instead of your magic

I'll try it out, maybe it will make tcg not crash and just fail the test.

[...]


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-22 12:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21  9:04 [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions Janis Schoetterl-Glausch
2022-04-22 11:56 ` Claudio Imbrenda
2022-04-22 12:19   ` Janis Schoetterl-Glausch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.