On 27/08/2019 15.49, Janosch Frank wrote:
> The storage key removal facility (stfle bit 169) makes all key related
> instructions result in a special operation exception if they handle a
> key.
>
> Let's make sure that the skey and pfmf tests only run non key code
> (pfmf) or not at all (skey).
>
> Also let's test this new facility. As lots of instructions are
> affected by this, only some of them are tested for now.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
> s390x/Makefile | 1 +
> s390x/pfmf.c | 10 ++++
> s390x/skey.c | 5 ++
> s390x/skrf.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 146 insertions(+)
> create mode 100644 s390x/skrf.c
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 76db0bb..007611e 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -14,6 +14,7 @@ tests += $(TEST_DIR)/iep.elf
> tests += $(TEST_DIR)/cpumodel.elf
> tests += $(TEST_DIR)/diag288.elf
> tests += $(TEST_DIR)/stsi.elf
> +tests += $(TEST_DIR)/skrf.elf
> tests_binary = $(patsubst %.elf,%.bin,$(tests))
>
> all: directories test_cases test_cases_binary
> diff --git a/s390x/pfmf.c b/s390x/pfmf.c
> index 2840cf5..78b4a73 100644
> --- a/s390x/pfmf.c
> +++ b/s390x/pfmf.c
> @@ -34,6 +34,10 @@ static void test_4k_key(void)
> union skey skey;
>
> report_prefix_push("4K");
> + if (test_facility(169)) {
> + report_skip("storage key removal facility is active");
> + goto out;
> + }
> r1.val = 0;
> r1.reg.sk = 1;
> r1.reg.fsc = PFMF_FSC_4K;
> @@ -42,6 +46,7 @@ static void test_4k_key(void)
> skey.val = get_storage_key(pagebuf);
> skey.val &= SKEY_ACC | SKEY_FP;
> report("set storage keys", skey.val == 0x30);
> +out:
> report_prefix_pop();
> }
>
> @@ -54,6 +59,10 @@ static void test_1m_key(void)
> void *addr = pagebuf;
>
> report_prefix_push("1M");
> + if (test_facility(169)) {
> + report_skip("storage key removal facility is active");
> + goto out;
> + }
> r1.val = 0;
> r1.reg.sk = 1;
> r1.reg.fsc = PFMF_FSC_1M;
> @@ -70,6 +79,7 @@ static void test_1m_key(void)
> }
> }
> report("set storage keys", rp);
> +out:
> report_prefix_pop();
> }
>
> diff --git a/s390x/skey.c b/s390x/skey.c
> index efc4eca..5020e99 100644
> --- a/s390x/skey.c
> +++ b/s390x/skey.c
> @@ -126,10 +126,15 @@ static void test_priv(void)
> int main(void)
> {
> report_prefix_push("skey");
> + if (test_facility(169)) {
> + report_skip("storage key removal facility is active");
> + goto done;
> + }
> test_priv();
> test_set();
> test_set_mb();
> test_chg();
> +done:
> report_prefix_pop();
> return report_summary();
> }
> diff --git a/s390x/skrf.c b/s390x/skrf.c
> new file mode 100644
> index 0000000..8e5baea
> --- /dev/null
> +++ b/s390x/skrf.c
> @@ -0,0 +1,130 @@
> +/*
> + * Storage key removal facility tests
> + *
> + * Copyright (c) 2019 IBM Corp
> + *
> + * Authors:
> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2.
> + */
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/interrupt.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +#include <asm/mem.h>
> +
> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
> +
> +static void test_facilities(void)
> +{
> + report_prefix_push("facilities");
> + report("!10", !test_facility(10));
> + report("!14", !test_facility(14));
> + report("!66", !test_facility(66));
> + report("!145", !test_facility(145));
> + report("!149", !test_facility(140));
> + report_prefix_pop();
> +}
> +
> +static void test_skey(void)
> +{
> + report_prefix_push("(i|s)ske");
> + expect_pgm_int();
> + set_storage_key(pagebuf, 0x30, 0);
> + check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> + expect_pgm_int();
> + get_storage_key(pagebuf);
> + check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> + report_prefix_pop();
Wouldn't it be better to have distinct prefixes for the two tests?
> +}
> +
> +static void test_pfmf(void)
> +{
> + union pfmf_r1 r1;
> +
> + report_prefix_push("pfmf");
> + r1.val = 0;
> + r1.reg.sk = 1;
> + r1.reg.fsc = PFMF_FSC_4K;
> + r1.reg.key = 0x30;
> + expect_pgm_int();
> + pfmf(r1.val, pagebuf);
> + check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> + report_prefix_pop();
> +}
> +
> +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);
> + check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> + report_prefix_pop();
> +}
> +
> +static void test_mvcos(void)
> +{
> + uint64_t r3 = 64;
> + uint8_t *src = pagebuf;
> + uint8_t *dst = pagebuf + PAGE_SIZE;
> + /* K bit set, as well as keys */
> + register unsigned long oac asm("0") = 0xf002f002;
> +
> + report_prefix_push("mvcos");
> + expect_pgm_int();
> + asm volatile(".machine \"z10\"\n"
> + ".machine \"push\"\n"
Shouldn't that be the other way round? first push the current one, then
set the new one?
Anyway, I've now also checked this patch in the CI:
diff a/s390x/Makefile b/s390x/Makefile
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -25,7 +25,7 @@ CFLAGS += -std=gnu99
CFLAGS += -ffreestanding
CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/s390x -I lib
CFLAGS += -O2
-CFLAGS += -march=z900
+CFLAGS += -march=z10
CFLAGS += -fno-delete-null-pointer-checks
LDFLAGS += -nostdlib -Wl,--build-id=none
... and it also seems to work fine with the TCG there:
https://gitlab.com/huth/kvm-unit-tests/-/jobs/281450598
So I think you can simply change it in the Makefile instead.
Thomas
> + "mvcos %[dst],%[src],%[len]\n"
> + ".machine \"pop\"\n"
> + : [dst] "+Q" (*(dst))
> + : [src] "Q" (*(src)), [len] "d" (r3), "d" (oac)
> + : "cc", "memory");
> + check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> + report_prefix_pop();
> +}
