* [kvm-unit-tests PATCH v1 0/3] s390x: mvpg test
@ 2021-02-09 18:51 Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Claudio Imbrenda @ 2021-02-09 18:51 UTC (permalink / raw)
To: kvm; +Cc: david, thuth, frankja, cohuck, pmorel, borntraeger
A simple unit test for the MVPG instruction.
The timeout is set to 10 seconds because the test should complete in a
fraction of a second even on busy machines. If the test is run in VSIE
and the host of the host is not handling MVPG properly, the test will
probably hang.
Testing MVPG behaviour in VSIE is the main motivation for this test.
Anything related to storage keys is not tested.
Claudio Imbrenda (3):
s390x: introduce leave_pstate to leave userspace
s390x: check for specific program interrupt
s390x: mvpg: simple test
s390x/Makefile | 1 +
lib/s390x/asm/arch_def.h | 5 +
lib/s390x/asm/interrupt.h | 1 +
lib/s390x/interrupt.c | 18 ++-
s390x/mvpg.c | 266 ++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 4 +
6 files changed, 293 insertions(+), 2 deletions(-)
create mode 100644 s390x/mvpg.c
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace
2021-02-09 18:51 [kvm-unit-tests PATCH v1 0/3] s390x: mvpg test Claudio Imbrenda
@ 2021-02-09 18:51 ` Claudio Imbrenda
2021-02-10 11:16 ` Cornelia Huck
` (2 more replies)
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test Claudio Imbrenda
2 siblings, 3 replies; 10+ messages in thread
From: Claudio Imbrenda @ 2021-02-09 18:51 UTC (permalink / raw)
To: kvm; +Cc: david, thuth, frankja, cohuck, pmorel, borntraeger
In most testcases, we enter problem state (userspace) just to test if a
privileged instruction causes a fault. In some cases, though, we need
to test if an instruction works properly in userspace. This means that
we do not expect a fault, and we need an orderly way to leave problem
state afterwards.
This patch introduces a simple system based on the SVC instruction.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 5 +++++
lib/s390x/interrupt.c | 12 ++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 9c4e330a..9902e9fe 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -276,6 +276,11 @@ static inline void enter_pstate(void)
load_psw_mask(mask);
}
+static inline void leave_pstate(void)
+{
+ asm volatile(" svc 1\n");
+}
+
static inline int stsi(void *addr, int fc, int sel1, int sel2)
{
register int r0 asm("0") = (fc << 28) | sel1;
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 1ce36073..59e01b1a 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -188,6 +188,14 @@ int unregister_io_int_func(void (*f)(void))
void handle_svc_int(void)
{
- report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
- stap(), lc->svc_old_psw.addr);
+ uint16_t code = lc->svc_int_code;
+
+ switch (code) {
+ case 1:
+ lc->svc_old_psw.mask &= ~PSW_MASK_PSTATE;
+ break;
+ default:
+ report_abort("Unexpected supervisor call interrupt: code %#x on cpu %d at %#lx",
+ code, stap(), lc->svc_old_psw.addr);
+ }
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt
2021-02-09 18:51 [kvm-unit-tests PATCH v1 0/3] s390x: mvpg test Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
@ 2021-02-09 18:51 ` Claudio Imbrenda
2021-02-10 11:23 ` Cornelia Huck
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test Claudio Imbrenda
2 siblings, 1 reply; 10+ messages in thread
From: Claudio Imbrenda @ 2021-02-09 18:51 UTC (permalink / raw)
To: kvm; +Cc: david, thuth, frankja, cohuck, pmorel, borntraeger
We already have check_pgm_int_code to check and report if a specific
program interrupt has occourred, but this approach has some issues.
In order to specify which test is being run, it was needed to push and
pop a prefix for each test, which is not nice to read both in the code
and in the output.
Another issue is that sometimes the condition to test for might require
other checks in addition to the interrupt.
The simple function added in this patch tests if the program intteruupt
received is the one expected.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/interrupt.h | 1 +
lib/s390x/interrupt.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index 1a2e2cd8..a33437b1 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -23,6 +23,7 @@ void expect_pgm_int(void);
void expect_ext_int(void);
uint16_t clear_pgm_int(void);
void check_pgm_int_code(uint16_t code);
+int is_pgm(int expected);
/* Activate low-address protection */
static inline void low_prot_enable(void)
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 59e01b1a..6f660285 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -51,6 +51,12 @@ void check_pgm_int_code(uint16_t code)
lc->pgm_int_code);
}
+int is_pgm(int expected)
+{
+ mb();
+ return expected == lc->pgm_int_code;
+}
+
void register_pgm_cleanup_func(void (*f)(void))
{
pgm_cleanup_func = f;
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test
2021-02-09 18:51 [kvm-unit-tests PATCH v1 0/3] s390x: mvpg test Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt Claudio Imbrenda
@ 2021-02-09 18:51 ` Claudio Imbrenda
2021-02-11 9:25 ` Janosch Frank
2 siblings, 1 reply; 10+ messages in thread
From: Claudio Imbrenda @ 2021-02-09 18:51 UTC (permalink / raw)
To: kvm; +Cc: david, thuth, frankja, cohuck, pmorel, borntraeger
A simple unit test for the MVPG instruction.
The timeout is set to 10 seconds because the test should complete in a
fraction of a second even on busy machines. If the test is run in VSIE
and the host of the host is not handling MVPG properly, the test will
probably hang.
Testing MVPG behaviour in VSIE is the main motivation for this test.
Anything related to storage keys is not tested.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/Makefile | 1 +
s390x/mvpg.c | 266 ++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 4 +
3 files changed, 271 insertions(+)
create mode 100644 s390x/mvpg.c
diff --git a/s390x/Makefile b/s390x/Makefile
index 08d85c9f..770eaa0b 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -20,6 +20,7 @@ tests += $(TEST_DIR)/sclp.elf
tests += $(TEST_DIR)/css.elf
tests += $(TEST_DIR)/uv-guest.elf
tests += $(TEST_DIR)/sie.elf
+tests += $(TEST_DIR)/mvpg.elf
tests_binary = $(patsubst %.elf,%.bin,$(tests))
ifneq ($(HOST_KEY_DOCUMENT),)
diff --git a/s390x/mvpg.c b/s390x/mvpg.c
new file mode 100644
index 00000000..fe6fe80a
--- /dev/null
+++ b/s390x/mvpg.c
@@ -0,0 +1,266 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Move Page instruction tests
+ *
+ * Copyright (c) 2020 IBM Corp
+ *
+ * Authors:
+ * Claudio Imbrenda <imbrenda@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include <asm/asm-offsets.h>
+#include <asm-generic/barrier.h>
+#include <asm/interrupt.h>
+#include <asm/pgtable.h>
+#include <mmu.h>
+#include <asm/page.h>
+#include <asm/facility.h>
+#include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
+#include <alloc_page.h>
+#include <bitops.h>
+
+/* Used to build the appropriate test values for register 0 */
+#define KFC(x) ((x) << 10)
+#define CCO 0x100
+
+/* How much memory to allocate for the test */
+#define MEM_ORDER 12
+/* How many iterations to perform in the loops */
+#define ITER 8
+
+/* Used to generate the simple pattern */
+#define MAGIC 42
+
+static uint8_t source[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
+static uint8_t buffer[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
+
+/* Keep track of fresh memory */
+static uint8_t *fresh;
+
+static inline int mvpg(unsigned long r0, void *dest, void *src)
+{
+ register unsigned long reg0 asm ("0") = r0;
+ int cc;
+
+ asm volatile(" mvpg %1,%2\n"
+ " ipm %0\n"
+ " srl %0,28"
+ : "=&d" (cc) : "a" (dest), "a" (src), "d" (reg0)
+ : "memory", "cc");
+ return cc;
+}
+
+/*
+ * Initialize a page with a simple pattern
+ */
+static void init_page(uint8_t *p)
+{
+ int i;
+
+ for (i = 0; i < PAGE_SIZE; i++)
+ p[i] = i + MAGIC;
+}
+
+/*
+ * Check if the given page contains the simple pattern
+ */
+static int page_ok(const uint8_t *p)
+{
+ int i;
+
+ for (i = 0; i < PAGE_SIZE; i++)
+ if (p[i] != (uint8_t)(i + MAGIC))
+ return 0;
+ return 1;
+}
+
+static void test_exceptions(void)
+{
+ int i, expected;
+
+ report_prefix_push("exceptions");
+
+ /*
+ * Key Function Control values 4 and 5 are allowed only in supervisor
+ * state, and even then, only if the move-page-and-set-key facility
+ * is present (STFLE bit 149)
+ */
+ report_prefix_push("privileged");
+ if (test_facility(149)) {
+ expected = PGM_INT_CODE_PRIVILEGED_OPERATION;
+ for (i = 4; i < 6; i++) {
+ expect_pgm_int();
+ enter_pstate();
+ mvpg(KFC(i), buffer, source);
+ report(is_pgm(expected), "Key Function Control value %d", i);
+ }
+ } else {
+ report_skip("Key Function Control value %d", 4);
+ report_skip("Key Function Control value %d", 5);
+ i = 4;
+ }
+ report_prefix_pop();
+
+ /*
+ * Invalid values of the Key Function Control, or setting the
+ * reserved bits, should result in a specification exception
+ */
+ report_prefix_push("specification");
+ expected = PGM_INT_CODE_SPECIFICATION;
+ expect_pgm_int();
+ mvpg(KFC(3), buffer, source);
+ report(is_pgm(expected), "Key Function Control value 3");
+ for (; i < 32; i++) {
+ expect_pgm_int();
+ mvpg(KFC(i), buffer, source);
+ report(is_pgm(expected), "Key Function Control value %d", i);
+ }
+ report_prefix_pop();
+
+ /* Operands outside memory result in addressing exceptions, as usual */
+ report_prefix_push("addressing");
+ expected = PGM_INT_CODE_ADDRESSING;
+ expect_pgm_int();
+ mvpg(0, buffer, (void *)PAGE_MASK);
+ report(is_pgm(expected), "Second operand outside memory");
+
+ expect_pgm_int();
+ mvpg(0, (void *)PAGE_MASK, source);
+ report(is_pgm(expected), "First operand outside memory");
+ report_prefix_pop();
+
+ report_prefix_pop();
+}
+
+static void test_success(void)
+{
+ int cc;
+
+ report_prefix_push("success");
+ /* Test successful scenarios, both in supervisor and problem state */
+ cc = mvpg(0, buffer, source);
+ report(page_ok(buffer) && !cc, "Supervisor state MVPG successful");
+
+ enter_pstate();
+ cc = mvpg(0, buffer, source);
+ leave_pstate();
+ report(page_ok(buffer) && !cc, "Problem state MVPG successful");
+
+ report_prefix_pop();
+}
+
+static void test_small_loop(const void *string)
+{
+ uint8_t *dest;
+ int i, cc;
+
+ /* Looping over cold and warm pages helps catch VSIE bugs */
+ report_prefix_push(string);
+ dest = fresh;
+ for (i = 0; i < ITER; i++) {
+ cc = mvpg(0, fresh, source);
+ report(page_ok(fresh) && !cc, "cold: %p, %p", source, fresh);
+ fresh += PAGE_SIZE;
+ }
+
+ for (i = 0; i < ITER; i++) {
+ memset(dest, 0, PAGE_SIZE);
+ cc = mvpg(0, dest, source);
+ report(page_ok(dest) && !cc, "warm: %p, %p", source, dest);
+ dest += PAGE_SIZE;
+ }
+ report_prefix_pop();
+}
+
+static void test_mmu_prot(void)
+{
+ int cc;
+
+ report_prefix_push("protection");
+ report_prefix_push("cco=0");
+
+ /* MVPG should still succeed when the source is read-only */
+ protect_page(source, PAGE_ENTRY_P);
+ cc = mvpg(0, fresh, source);
+ report(page_ok(fresh) && !cc, "source read only");
+ unprotect_page(source, PAGE_ENTRY_P);
+ fresh += PAGE_SIZE;
+
+ /*
+ * When the source or destination are invalid, a page translation
+ * exception should be raised; when the destination is read-only,
+ * a protection exception should be raised.
+ */
+ protect_page(fresh, PAGE_ENTRY_P);
+ expect_pgm_int();
+ mvpg(0, fresh, source);
+ report(is_pgm(PGM_INT_CODE_PROTECTION), "destination read only");
+ fresh += PAGE_SIZE;
+
+ protect_page(source, PAGE_ENTRY_I);
+ expect_pgm_int();
+ mvpg(0, fresh, source);
+ report(is_pgm(PGM_INT_CODE_PAGE_TRANSLATION), "source invalid");
+ unprotect_page(source, PAGE_ENTRY_I);
+ fresh += PAGE_SIZE;
+
+ protect_page(fresh, PAGE_ENTRY_I);
+ expect_pgm_int();
+ mvpg(0, fresh, source);
+ report(is_pgm(PGM_INT_CODE_PAGE_TRANSLATION), "destination invalid");
+ fresh += PAGE_SIZE;
+
+ report_prefix_pop();
+ report_prefix_push("cco=1");
+ /*
+ * Setting the CCO bit should suppress page translation exceptions,
+ * but not protection exceptions.
+ */
+ protect_page(fresh, PAGE_ENTRY_P);
+ expect_pgm_int();
+ mvpg(CCO, fresh, source);
+ report(is_pgm(PGM_INT_CODE_PROTECTION), "destination read only");
+ fresh += PAGE_SIZE;
+
+ protect_page(fresh, PAGE_ENTRY_I);
+ cc = mvpg(CCO, fresh, source);
+ report(cc == 1, "destination invalid");
+ fresh += PAGE_SIZE;
+
+ protect_page(source, PAGE_ENTRY_I);
+ cc = mvpg(CCO, fresh, source);
+ report(cc == 2, "source invalid");
+ fresh += PAGE_SIZE;
+
+ protect_page(fresh, PAGE_ENTRY_I);
+ cc = mvpg(CCO, fresh, source);
+ report(cc == 2, "source and destination invalid");
+ fresh += PAGE_SIZE;
+
+ unprotect_page(source, PAGE_ENTRY_I);
+ report_prefix_pop();
+ report_prefix_pop();
+}
+
+int main(void)
+{
+ report_prefix_push("mvpg");
+
+ init_page(source);
+ fresh = alloc_pages_flags(MEM_ORDER, FLAG_DONTZERO | FLAG_FRESH);
+ assert(fresh);
+
+ test_exceptions();
+ test_success();
+ test_small_loop("nommu");
+
+ setup_vm();
+
+ test_small_loop("mmu");
+ test_mmu_prot();
+
+ report_prefix_pop();
+ return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 2298be6c..9f81a608 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -99,3 +99,7 @@ file = uv-guest.elf
[sie]
file = sie.elf
+
+[mvpg]
+file = mvpg.elf
+timeout = 10
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
@ 2021-02-10 11:16 ` Cornelia Huck
2021-02-10 11:32 ` Janosch Frank
2021-02-11 10:52 ` David Hildenbrand
2 siblings, 0 replies; 10+ messages in thread
From: Cornelia Huck @ 2021-02-10 11:16 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, david, thuth, frankja, pmorel, borntraeger
On Tue, 9 Feb 2021 19:51:52 +0100
Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> In most testcases, we enter problem state (userspace) just to test if a
> privileged instruction causes a fault. In some cases, though, we need
> to test if an instruction works properly in userspace. This means that
> we do not expect a fault, and we need an orderly way to leave problem
> state afterwards.
>
> This patch introduces a simple system based on the SVC instruction.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 5 +++++
> lib/s390x/interrupt.c | 12 ++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt Claudio Imbrenda
@ 2021-02-10 11:23 ` Cornelia Huck
2021-02-10 11:43 ` Janosch Frank
0 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2021-02-10 11:23 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, david, thuth, frankja, pmorel, borntraeger
On Tue, 9 Feb 2021 19:51:53 +0100
Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> We already have check_pgm_int_code to check and report if a specific
> program interrupt has occourred, but this approach has some issues.
s/occourred/occurred/
>
> In order to specify which test is being run, it was needed to push and
> pop a prefix for each test, which is not nice to read both in the code
> and in the output.
>
> Another issue is that sometimes the condition to test for might require
> other checks in addition to the interrupt.
>
> The simple function added in this patch tests if the program intteruupt
s/intteruupt/interrupt/
> received is the one expected.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/interrupt.h | 1 +
> lib/s390x/interrupt.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index 1a2e2cd8..a33437b1 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -23,6 +23,7 @@ void expect_pgm_int(void);
> void expect_ext_int(void);
> uint16_t clear_pgm_int(void);
> void check_pgm_int_code(uint16_t code);
> +int is_pgm(int expected);
>
> /* Activate low-address protection */
> static inline void low_prot_enable(void)
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 59e01b1a..6f660285 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -51,6 +51,12 @@ void check_pgm_int_code(uint16_t code)
> lc->pgm_int_code);
> }
>
> +int is_pgm(int expected)
is_pgm() is a bit non-descriptive. Maybe check_pgm_int_code_noreport()?
Also, maybe let it take a uint16_t parameter?
> +{
> + mb();
> + return expected == lc->pgm_int_code;
> +}
> +
> void register_pgm_cleanup_func(void (*f)(void))
> {
> pgm_cleanup_func = f;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
2021-02-10 11:16 ` Cornelia Huck
@ 2021-02-10 11:32 ` Janosch Frank
2021-02-11 10:52 ` David Hildenbrand
2 siblings, 0 replies; 10+ messages in thread
From: Janosch Frank @ 2021-02-10 11:32 UTC (permalink / raw)
To: Claudio Imbrenda, kvm; +Cc: david, thuth, cohuck, pmorel, borntraeger
On 2/9/21 7:51 PM, Claudio Imbrenda wrote:
> In most testcases, we enter problem state (userspace) just to test if a
> privileged instruction causes a fault. In some cases, though, we need
> to test if an instruction works properly in userspace. This means that
> we do not expect a fault, and we need an orderly way to leave problem
> state afterwards.
>
> This patch introduces a simple system based on the SVC instruction.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 5 +++++
> lib/s390x/interrupt.c | 12 ++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 9c4e330a..9902e9fe 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -276,6 +276,11 @@ static inline void enter_pstate(void)
> load_psw_mask(mask);
> }
>
> +static inline void leave_pstate(void)
> +{
> + asm volatile(" svc 1\n");
Magic constants being magic :-)
> +}
> +
> static inline int stsi(void *addr, int fc, int sel1, int sel2)
> {
> register int r0 asm("0") = (fc << 28) | sel1;
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 1ce36073..59e01b1a 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -188,6 +188,14 @@ int unregister_io_int_func(void (*f)(void))
>
> void handle_svc_int(void)
> {
> - report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
> - stap(), lc->svc_old_psw.addr);
> + uint16_t code = lc->svc_int_code;
> +
> + switch (code) {
> + case 1:
> + lc->svc_old_psw.mask &= ~PSW_MASK_PSTATE;
> + break;
> + default:
> + report_abort("Unexpected supervisor call interrupt: code %#x on cpu %d at %#lx",
> + code, stap(), lc->svc_old_psw.addr);
> + }
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt
2021-02-10 11:23 ` Cornelia Huck
@ 2021-02-10 11:43 ` Janosch Frank
0 siblings, 0 replies; 10+ messages in thread
From: Janosch Frank @ 2021-02-10 11:43 UTC (permalink / raw)
To: Cornelia Huck, Claudio Imbrenda; +Cc: kvm, david, thuth, pmorel, borntraeger
On 2/10/21 12:23 PM, Cornelia Huck wrote:
> On Tue, 9 Feb 2021 19:51:53 +0100
> Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
>
>> We already have check_pgm_int_code to check and report if a specific
>> program interrupt has occourred, but this approach has some issues.
>
> s/occourred/occurred/
>
>>
>> In order to specify which test is being run, it was needed to push and
>> pop a prefix for each test, which is not nice to read both in the code
>> and in the output.
>>
>> Another issue is that sometimes the condition to test for might require
>> other checks in addition to the interrupt.
>>
>> The simple function added in this patch tests if the program intteruupt
>
> s/intteruupt/interrupt/
>
>> received is the one expected.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>> lib/s390x/asm/interrupt.h | 1 +
>> lib/s390x/interrupt.c | 6 ++++++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
>> index 1a2e2cd8..a33437b1 100644
>> --- a/lib/s390x/asm/interrupt.h
>> +++ b/lib/s390x/asm/interrupt.h
>> @@ -23,6 +23,7 @@ void expect_pgm_int(void);
>> void expect_ext_int(void);
>> uint16_t clear_pgm_int(void);
>> void check_pgm_int_code(uint16_t code);
>> +int is_pgm(int expected);
>>
>> /* Activate low-address protection */
>> static inline void low_prot_enable(void)
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 59e01b1a..6f660285 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -51,6 +51,12 @@ void check_pgm_int_code(uint16_t code)
>> lc->pgm_int_code);
>> }
>>
>> +int is_pgm(int expected)
>
> is_pgm() is a bit non-descriptive. Maybe check_pgm_int_code_noreport()?
>
> Also, maybe let it take a uint16_t parameter?
Could we use clear_pgm_int()?
It returns the last code so you can check yourself.
We could rename it to fetch_and_clear_pgm_int()
>
>> +{
>> + mb();
>> + return expected == lc->pgm_int_code;
>> +}
>> +
>> void register_pgm_cleanup_func(void (*f)(void))
>> {
>> pgm_cleanup_func = f;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test Claudio Imbrenda
@ 2021-02-11 9:25 ` Janosch Frank
0 siblings, 0 replies; 10+ messages in thread
From: Janosch Frank @ 2021-02-11 9:25 UTC (permalink / raw)
To: Claudio Imbrenda, kvm; +Cc: david, thuth, cohuck, pmorel, borntraeger
On 2/9/21 7:51 PM, Claudio Imbrenda wrote:
> A simple unit test for the MVPG instruction.
>
> The timeout is set to 10 seconds because the test should complete in a
> fraction of a second even on busy machines. If the test is run in VSIE
> and the host of the host is not handling MVPG properly, the test will
> probably hang.
>
> Testing MVPG behaviour in VSIE is the main motivation for this test.
>
> Anything related to storage keys is not tested.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/Makefile | 1 +
> s390x/mvpg.c | 266 ++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 4 +
> 3 files changed, 271 insertions(+)
> create mode 100644 s390x/mvpg.c
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 08d85c9f..770eaa0b 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -20,6 +20,7 @@ tests += $(TEST_DIR)/sclp.elf
> tests += $(TEST_DIR)/css.elf
> tests += $(TEST_DIR)/uv-guest.elf
> tests += $(TEST_DIR)/sie.elf
> +tests += $(TEST_DIR)/mvpg.elf
>
> tests_binary = $(patsubst %.elf,%.bin,$(tests))
> ifneq ($(HOST_KEY_DOCUMENT),)
> diff --git a/s390x/mvpg.c b/s390x/mvpg.c
> new file mode 100644
> index 00000000..fe6fe80a
> --- /dev/null
> +++ b/s390x/mvpg.c
> @@ -0,0 +1,266 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Move Page instruction tests
> + *
> + * Copyright (c) 2020 IBM Corp
> + *
> + * Authors:
> + * Claudio Imbrenda <imbrenda@linux.ibm.com>
> + */
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm-generic/barrier.h>
> +#include <asm/interrupt.h>
> +#include <asm/pgtable.h>
> +#include <mmu.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +#include <asm/mem.h>
> +#include <asm/sigp.h>
> +#include <smp.h>
> +#include <alloc_page.h>
> +#include <bitops.h>
> +
> +/* Used to build the appropriate test values for register 0 */
> +#define KFC(x) ((x) << 10)
> +#define CCO 0x100
> +
> +/* How much memory to allocate for the test */
> +#define MEM_ORDER 12
> +/* How many iterations to perform in the loops */
> +#define ITER 8
> +
> +/* Used to generate the simple pattern */
> +#define MAGIC 42
> +
> +static uint8_t source[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
> +static uint8_t buffer[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
> +
> +/* Keep track of fresh memory */
> +static uint8_t *fresh;
> +
> +static inline int mvpg(unsigned long r0, void *dest, void *src)
> +{
> + register unsigned long reg0 asm ("0") = r0;
> + int cc;
> +
> + asm volatile(" mvpg %1,%2\n"
> + " ipm %0\n"
> + " srl %0,28"
> + : "=&d" (cc) : "a" (dest), "a" (src), "d" (reg0)
> + : "memory", "cc");
> + return cc;
> +}
> +
> +/*
> + * Initialize a page with a simple pattern
> + */
> +static void init_page(uint8_t *p)
> +{
> + int i;
> +
> + for (i = 0; i < PAGE_SIZE; i++)
> + p[i] = i + MAGIC;
> +}
> +
> +/*
> + * Check if the given page contains the simple pattern
> + */
> +static int page_ok(const uint8_t *p)
> +{
> + int i;
> +
> + for (i = 0; i < PAGE_SIZE; i++)
> + if (p[i] != (uint8_t)(i + MAGIC))
> + return 0;
> + return 1;
> +}
> +
> +static void test_exceptions(void)
> +{
> + int i, expected;
> +
> + report_prefix_push("exceptions");
> +
> + /*
> + * Key Function Control values 4 and 5 are allowed only in supervisor
> + * state, and even then, only if the move-page-and-set-key facility
> + * is present (STFLE bit 149)
> + */
> + report_prefix_push("privileged");
> + if (test_facility(149)) {
> + expected = PGM_INT_CODE_PRIVILEGED_OPERATION;
> + for (i = 4; i < 6; i++) {
> + expect_pgm_int();
> + enter_pstate();
> + mvpg(KFC(i), buffer, source);
> + report(is_pgm(expected), "Key Function Control value %d", i);
> + }
> + } else {
> + report_skip("Key Function Control value %d", 4);
> + report_skip("Key Function Control value %d", 5);
> + i = 4;
> + }
> + report_prefix_pop();
> +
> + /*
> + * Invalid values of the Key Function Control, or setting the
> + * reserved bits, should result in a specification exception
> + */
> + report_prefix_push("specification");
> + expected = PGM_INT_CODE_SPECIFICATION;
> + expect_pgm_int();
> + mvpg(KFC(3), buffer, source);
> + report(is_pgm(expected), "Key Function Control value 3");
> + for (; i < 32; i++) {
> + expect_pgm_int();
> + mvpg(KFC(i), buffer, source);
> + report(is_pgm(expected), "Key Function Control value %d", i);
> + }
> + report_prefix_pop();
> +
> + /* Operands outside memory result in addressing exceptions, as usual */
> + report_prefix_push("addressing");
> + expected = PGM_INT_CODE_ADDRESSING;
> + expect_pgm_int();
> + mvpg(0, buffer, (void *)PAGE_MASK);
> + report(is_pgm(expected), "Second operand outside memory");
> +
> + expect_pgm_int();
> + mvpg(0, (void *)PAGE_MASK, source);
> + report(is_pgm(expected), "First operand outside memory");
> + report_prefix_pop();
> +
> + report_prefix_pop();
> +}
> +
> +static void test_success(void)
> +{
> + int cc;
> +
> + report_prefix_push("success");
> + /* Test successful scenarios, both in supervisor and problem state */
> + cc = mvpg(0, buffer, source);
> + report(page_ok(buffer) && !cc, "Supervisor state MVPG successful");
> +
> + enter_pstate();
> + cc = mvpg(0, buffer, source);
> + leave_pstate();
> + report(page_ok(buffer) && !cc, "Problem state MVPG successful");
> +
> + report_prefix_pop();
> +}
> +
> +static void test_small_loop(const void *string)
> +{
> + uint8_t *dest;
> + int i, cc;
> +
> + /* Looping over cold and warm pages helps catch VSIE bugs */
> + report_prefix_push(string);
> + dest = fresh;
> + for (i = 0; i < ITER; i++) {
> + cc = mvpg(0, fresh, source);
> + report(page_ok(fresh) && !cc, "cold: %p, %p", source, fresh);
> + fresh += PAGE_SIZE;
> + }
> +
> + for (i = 0; i < ITER; i++) {
> + memset(dest, 0, PAGE_SIZE);
> + cc = mvpg(0, dest, source);
> + report(page_ok(dest) && !cc, "warm: %p, %p", source, dest);
> + dest += PAGE_SIZE;
> + }
> + report_prefix_pop();
> +}
> +
> +static void test_mmu_prot(void)
> +{
> + int cc;
> +
> + report_prefix_push("protection");
> + report_prefix_push("cco=0");
> +
> + /* MVPG should still succeed when the source is read-only */
> + protect_page(source, PAGE_ENTRY_P);
> + cc = mvpg(0, fresh, source);
> + report(page_ok(fresh) && !cc, "source read only");
> + unprotect_page(source, PAGE_ENTRY_P);
> + fresh += PAGE_SIZE;
> +
> + /*
> + * When the source or destination are invalid, a page translation
> + * exception should be raised; when the destination is read-only,
> + * a protection exception should be raised.
> + */
> + protect_page(fresh, PAGE_ENTRY_P);
> + expect_pgm_int();
> + mvpg(0, fresh, source);
> + report(is_pgm(PGM_INT_CODE_PROTECTION), "destination read only");
> + fresh += PAGE_SIZE;
> +
> + protect_page(source, PAGE_ENTRY_I);
> + expect_pgm_int();
> + mvpg(0, fresh, source);
> + report(is_pgm(PGM_INT_CODE_PAGE_TRANSLATION), "source invalid");
> + unprotect_page(source, PAGE_ENTRY_I);
> + fresh += PAGE_SIZE;
> +
> + protect_page(fresh, PAGE_ENTRY_I);
> + expect_pgm_int();
> + mvpg(0, fresh, source);
> + report(is_pgm(PGM_INT_CODE_PAGE_TRANSLATION), "destination invalid");
> + fresh += PAGE_SIZE;
> +
> + report_prefix_pop();
> + report_prefix_push("cco=1");
> + /*
> + * Setting the CCO bit should suppress page translation exceptions,
> + * but not protection exceptions.
> + */
> + protect_page(fresh, PAGE_ENTRY_P);
> + expect_pgm_int();
> + mvpg(CCO, fresh, source);
> + report(is_pgm(PGM_INT_CODE_PROTECTION), "destination read only");
> + fresh += PAGE_SIZE;
> +
> + protect_page(fresh, PAGE_ENTRY_I);
> + cc = mvpg(CCO, fresh, source);
> + report(cc == 1, "destination invalid");
> + fresh += PAGE_SIZE;
> +
> + protect_page(source, PAGE_ENTRY_I);
> + cc = mvpg(CCO, fresh, source);
> + report(cc == 2, "source invalid");
> + fresh += PAGE_SIZE;
> +
> + protect_page(fresh, PAGE_ENTRY_I);
> + cc = mvpg(CCO, fresh, source);
> + report(cc == 2, "source and destination invalid");
> + fresh += PAGE_SIZE;
> +
> + unprotect_page(source, PAGE_ENTRY_I);
> + report_prefix_pop();
> + report_prefix_pop();
> +}
> +
> +int main(void)
> +{
> + report_prefix_push("mvpg");
> +
> + init_page(source);
> + fresh = alloc_pages_flags(MEM_ORDER, FLAG_DONTZERO | FLAG_FRESH);
> + assert(fresh);
> +
> + test_exceptions();
> + test_success();
> + test_small_loop("nommu");
> +
> + setup_vm();
> +
> + test_small_loop("mmu");
> + test_mmu_prot();
> +
> + report_prefix_pop();
> + return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 2298be6c..9f81a608 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -99,3 +99,7 @@ file = uv-guest.elf
>
> [sie]
> file = sie.elf
> +
> +[mvpg]
> +file = mvpg.elf
> +timeout = 10
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
2021-02-10 11:16 ` Cornelia Huck
2021-02-10 11:32 ` Janosch Frank
@ 2021-02-11 10:52 ` David Hildenbrand
2 siblings, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2021-02-11 10:52 UTC (permalink / raw)
To: Claudio Imbrenda, kvm; +Cc: thuth, frankja, cohuck, pmorel, borntraeger
On 09.02.21 19:51, Claudio Imbrenda wrote:
> In most testcases, we enter problem state (userspace) just to test if a
> privileged instruction causes a fault. In some cases, though, we need
> to test if an instruction works properly in userspace. This means that
> we do not expect a fault, and we need an orderly way to leave problem
> state afterwards.
>
> This patch introduces a simple system based on the SVC instruction.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 5 +++++
> lib/s390x/interrupt.c | 12 ++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 9c4e330a..9902e9fe 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -276,6 +276,11 @@ static inline void enter_pstate(void)
> load_psw_mask(mask);
> }
>
> +static inline void leave_pstate(void)
> +{
> + asm volatile(" svc 1\n");
> +}
> +
> static inline int stsi(void *addr, int fc, int sel1, int sel2)
> {
> register int r0 asm("0") = (fc << 28) | sel1;
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 1ce36073..59e01b1a 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -188,6 +188,14 @@ int unregister_io_int_func(void (*f)(void))
>
> void handle_svc_int(void)
> {
> - report_abort("Unexpected supervisor call interrupt: on cpu %d at %#lx",
> - stap(), lc->svc_old_psw.addr);
> + uint16_t code = lc->svc_int_code;
> +
> + switch (code) {
> + case 1:
> + lc->svc_old_psw.mask &= ~PSW_MASK_PSTATE;
> + break;
> + default:
> + report_abort("Unexpected supervisor call interrupt: code %#x on cpu %d at %#lx",
> + code, stap(), lc->svc_old_psw.addr);
> + }
> }
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-11 10:57 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 18:51 [kvm-unit-tests PATCH v1 0/3] s390x: mvpg test Claudio Imbrenda
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 1/3] s390x: introduce leave_pstate to leave userspace Claudio Imbrenda
2021-02-10 11:16 ` Cornelia Huck
2021-02-10 11:32 ` Janosch Frank
2021-02-11 10:52 ` David Hildenbrand
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 2/3] s390x: check for specific program interrupt Claudio Imbrenda
2021-02-10 11:23 ` Cornelia Huck
2021-02-10 11:43 ` Janosch Frank
2021-02-09 18:51 ` [kvm-unit-tests PATCH v1 3/3] s390x: mvpg: simple test Claudio Imbrenda
2021-02-11 9:25 ` Janosch Frank
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).