[kvm-unit-tests PATCH v7 0/8] s390x: Add support for running guests without MSO/MSL

[kvm-unit-tests PATCH v7 0/8] s390x: Add support for running guests without MSO/MSL

[Nico Boehr]

v7:
---
* pick up r-bs
* fixup unneeded linebreak (thanks Janosch)
* remove outdated comments (thanks Janosch)
* remove unneeded includes (thanks Janosch)

v6:
---
* add commit "s390x: add test source dir to include paths" and share define with number of pages between snippet and test (Thomas)
* add commit "lib: s390x: interrupt: remove TEID_ASCE defines"
* rename dat -> use_dat (thanks Thomas)
* remove IRQ_DAT_ON/_OFF defines (thanks Thomas)
* add a comment to explain why we switch to home space when entering SIE (thanks Thomas)
* clarify register_ext_cleanup_func() doesn't touch address space mode when DAT is off (thanks Thomas)
* convert address space defines to enum (thanks Thomas)
* switch bitfield member to uint64_t (thanks Claudio)
* upercase hex number
* in selftest, set the bitfield value to a define and then assert on the bitfield (thanks Thomas)

v5:
---
* fix a big oopsie in irq_set_dat_mode() which caused parts of lowcore being
  overwritten (thanks Claudio)

v4:
---
- add static assert for PSW bitfield (Janosch, Claudio)
- remove unneeded includes (Janosch)
- move variable decls to function start (Janosch)
- remove unneeded imports (Janosch)
- lowerocase hex (Janosch)
- remove unneeded attr (Janosch)
- tyop :-) fixes (Janosch)

v3:
---
* introduce bitfield for the PSW to make handling less clumsy
* some variable renames (Claudio)
* remove unneeded barriers (Claudio)
* remove rebase leftover sie_had_pgm_int (Claudio)
* move read_pgm_int_code to header (Claudio)
* squash include fix commit into the one causing the issue (Claudio)

v2:
---
* add function to change DAT/AS mode for all irq handlers (Janosch, Claudio)
* instead of a new flag in PROG0C, check the pgm int code in lowcore (Janosch)
* fix indents, comments (Nina)

Right now, all SIE tests in kvm-unit-tests (i.e. where kvm-unit-test is the
hypervisor) run using MSO/MSL.

This is convenient, because it's simple. But it also comes with
disadvantages, for example some features are unavailabe with MSO/MSL.

This series adds support for running guests without MSO/MSL with dedicated
guest page tables for the GPA->HPA translation.

Since SIE implicitly uses the primary space mode for the guest, the host
can't run in the primary space mode, too. To avoid moving all tests to the
home space mode, only switch to home space mode when it is actually needed.

This series also comes with various bugfixes that were caught while
develoing this.

Nico Boehr (8):
  lib: s390x: introduce bitfield for PSW mask
  s390x: add function to set DAT mode for all interrupts
  s390x: sie: switch to home space mode before entering SIE
  s390x: lib: don't forward PSW when handling exception in SIE
  s390x: lib: sie: don't reenter SIE on pgm int
  s390x: add test source dir to include paths
  s390x: add a test for SIE without MSO/MSL
  lib: s390x: interrupt: remove TEID_ASCE defines

 lib/s390x/asm/arch_def.h   |  36 ++++++++++--
 lib/s390x/asm/interrupt.h  |  21 +++++--
 lib/s390x/asm/mem.h        |   1 +
 lib/s390x/interrupt.c      |  36 ++++++++++++
 lib/s390x/mmu.c            |   5 +-
 lib/s390x/sie.c            |  30 +++++++++-
 s390x/Makefile             |   4 +-
 s390x/selftest.c           |  34 ++++++++++++
 s390x/sie-dat.c            | 110 +++++++++++++++++++++++++++++++++++++
 s390x/snippets/c/sie-dat.c |  52 ++++++++++++++++++
 s390x/snippets/c/sie-dat.h |   2 +
 s390x/unittests.cfg        |   3 +
 12 files changed, 320 insertions(+), 14 deletions(-)
 create mode 100644 s390x/sie-dat.c
 create mode 100644 s390x/snippets/c/sie-dat.c
 create mode 100644 s390x/snippets/c/sie-dat.h

-- 
2.41.0

[kvm-unit-tests PATCH v7 1/8] lib: s390x: introduce bitfield for PSW mask

[Nico Boehr]

Changing the PSW mask is currently little clumsy, since there is only the
PSW_MASK_* defines. This makes it hard to change e.g. only the address
space in the current PSW without a lot of bit fiddling.

Introduce a bitfield for the PSW mask. This makes this kind of
modifications much simpler and easier to read.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/asm/arch_def.h | 25 ++++++++++++++++++++++++-
 s390x/selftest.c         | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index bb26e008cc68..f629b6d0a17f 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -37,9 +37,32 @@ struct stack_frame_int {
 };
 
 struct psw {
-	uint64_t	mask;
+	union {
+		uint64_t	mask;
+		struct {
+			uint64_t reserved00:1;
+			uint64_t per:1;
+			uint64_t reserved02:3;
+			uint64_t dat:1;
+			uint64_t io:1;
+			uint64_t ext:1;
+			uint64_t key:4;
+			uint64_t reserved12:1;
+			uint64_t mchk:1;
+			uint64_t wait:1;
+			uint64_t pstate:1;
+			uint64_t as:2;
+			uint64_t cc:2;
+			uint64_t prg_mask:4;
+			uint64_t reserved24:7;
+			uint64_t ea:1;
+			uint64_t ba:1;
+			uint64_t reserved33:31;
+		};
+	};
 	uint64_t	addr;
 };
+_Static_assert(sizeof(struct psw) == 16, "PSW size");
 
 #define PSW(m, a) ((struct psw){ .mask = (m), .addr = (uint64_t)(a) })
 
diff --git a/s390x/selftest.c b/s390x/selftest.c
index 13fd36bc06f8..92ed4e5d35eb 100644
--- a/s390x/selftest.c
+++ b/s390x/selftest.c
@@ -74,6 +74,39 @@ static void test_malloc(void)
 	report_prefix_pop();
 }
 
+static void test_psw_mask(void)
+{
+	uint64_t expected_key = 0xf;
+	struct psw test_psw = PSW(0, 0);
+
+	report_prefix_push("PSW mask");
+	test_psw.mask = PSW_MASK_DAT;
+	report(test_psw.dat, "DAT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_DAT, test_psw.mask);
+
+	test_psw.mask = PSW_MASK_IO;
+	report(test_psw.io, "IO matches expected=0x%016lx actual=0x%016lx", PSW_MASK_IO, test_psw.mask);
+
+	test_psw.mask = PSW_MASK_EXT;
+	report(test_psw.ext, "EXT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_EXT, test_psw.mask);
+
+	test_psw.mask = expected_key << (63 - 11);
+	report(test_psw.key == expected_key, "PSW Key matches expected=0x%lx actual=0x%x", expected_key, test_psw.key);
+
+	test_psw.mask = 1UL << (63 - 13);
+	report(test_psw.mchk, "MCHK matches");
+
+	test_psw.mask = PSW_MASK_WAIT;
+	report(test_psw.wait, "Wait matches expected=0x%016lx actual=0x%016lx", PSW_MASK_WAIT, test_psw.mask);
+
+	test_psw.mask = PSW_MASK_PSTATE;
+	report(test_psw.pstate, "Pstate matches expected=0x%016lx actual=0x%016lx", PSW_MASK_PSTATE, test_psw.mask);
+
+	test_psw.mask = PSW_MASK_64;
+	report(test_psw.ea && test_psw.ba, "BA/EA matches expected=0x%016lx actual=0x%016lx", PSW_MASK_64, test_psw.mask);
+
+	report_prefix_pop();
+}
+
 int main(int argc, char**argv)
 {
 	report_prefix_push("selftest");
@@ -89,6 +122,7 @@ int main(int argc, char**argv)
 	test_fp();
 	test_pgm_int();
 	test_malloc();
+	test_psw_mask();
 
 	return report_summary();
 }
-- 
2.41.0

[kvm-unit-tests PATCH v7 2/8] s390x: add function to set DAT mode for all interrupts

[Nico Boehr]

When toggling DAT or switch address space modes, it is likely that
interrupts should be handled in the same DAT or address space mode.

Add a function which toggles DAT and address space mode for all
interruptions, except restart interrupts.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h  | 10 ++++++----
 lib/s390x/asm/interrupt.h |  2 ++
 lib/s390x/interrupt.c     | 35 +++++++++++++++++++++++++++++++++++
 lib/s390x/mmu.c           |  5 +++--
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index f629b6d0a17f..5beaf15b57e7 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -84,10 +84,12 @@ struct cpu {
 	bool in_interrupt_handler;
 };
 
-#define AS_PRIM				0
-#define AS_ACCR				1
-#define AS_SECN				2
-#define AS_HOME				3
+enum address_space {
+	AS_PRIM = 0,
+	AS_ACCR = 1,
+	AS_SECN = 2,
+	AS_HOME = 3
+};
 
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_IO			0x0200000000000000UL
diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index 35c1145f0349..d01f8a89641a 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -83,6 +83,8 @@ void expect_ext_int(void);
 uint16_t clear_pgm_int(void);
 void check_pgm_int_code(uint16_t code);
 
+void irq_set_dat_mode(bool use_dat, enum address_space as);
+
 /* Activate low-address protection */
 static inline void low_prot_enable(void)
 {
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 3f993a363ae2..e0a1713349f6 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -9,6 +9,7 @@
  */
 #include <libcflat.h>
 #include <asm/barrier.h>
+#include <asm/mem.h>
 #include <asm/asm-offsets.h>
 #include <sclp.h>
 #include <interrupt.h>
@@ -104,6 +105,40 @@ void register_ext_cleanup_func(void (*f)(struct stack_frame_int *))
 	THIS_CPU->ext_cleanup_func = f;
 }
 
+/**
+ * irq_set_dat_mode - Set the DAT mode of all interrupt handlers, except for
+ * restart.
+ * This will update the DAT mode and address space mode of all interrupt new
+ * PSWs.
+ *
+ * Since enabling DAT needs initialized CRs and the restart new PSW is often used
+ * to initialize CRs, the restart new PSW is never touched to avoid the chicken
+ * and egg situation.
+ *
+ * @use_dat specifies whether to use DAT or not
+ * @as specifies the address space mode to use. Not set if use_dat is false.
+ */
+void irq_set_dat_mode(bool use_dat, enum address_space as)
+{
+	struct psw* irq_psws[] = {
+		OPAQUE_PTR(GEN_LC_EXT_NEW_PSW),
+		OPAQUE_PTR(GEN_LC_SVC_NEW_PSW),
+		OPAQUE_PTR(GEN_LC_PGM_NEW_PSW),
+		OPAQUE_PTR(GEN_LC_MCCK_NEW_PSW),
+		OPAQUE_PTR(GEN_LC_IO_NEW_PSW),
+	};
+	struct psw *psw;
+
+	assert(as == AS_PRIM || as == AS_ACCR || as == AS_SECN || as == AS_HOME);
+
+	for (size_t i = 0; i < ARRAY_SIZE(irq_psws); i++) {
+		psw = irq_psws[i];
+		psw->dat = use_dat;
+		if (use_dat)
+			psw->as = as;
+	}
+}
+
 static void fixup_pgm_int(struct stack_frame_int *stack)
 {
 	/* If we have an error on SIE we directly move to sie_exit */
diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
index b474d7021d3f..9a179d6b8ec5 100644
--- a/lib/s390x/mmu.c
+++ b/lib/s390x/mmu.c
@@ -12,6 +12,7 @@
 #include <asm/pgtable.h>
 #include <asm/arch_def.h>
 #include <asm/barrier.h>
+#include <asm/interrupt.h>
 #include <vmalloc.h>
 #include "mmu.h"
 
@@ -41,8 +42,8 @@ static void mmu_enable(pgd_t *pgtable)
 	/* enable dat (primary == 0 set as default) */
 	enable_dat();
 
-	/* we can now also use DAT unconditionally in our PGM handler */
-	lowcore.pgm_new_psw.mask |= PSW_MASK_DAT;
+	/* we can now also use DAT in all interrupt handlers */
+	irq_set_dat_mode(true, AS_PRIM);
 }
 
 /*
-- 
2.41.0

[kvm-unit-tests PATCH v7 3/8] s390x: sie: switch to home space mode before entering SIE

[Nico Boehr]

This is to prepare for running guests without MSO/MSL, which is
currently not possible.

We already have code in sie64a to setup a guest primary ASCE before
entering SIE, so we can in theory switch to the page tables which
translate gpa to hpa.

But the host is running in primary space mode already, so changing the
primary ASCE before entering SIE will also affect the host's code and
data.

To make this switch useful, the host should run in a different address
space mode. Hence, set up and change to home address space mode before
installing the guest ASCE.

The home space ASCE is just copied over from the primary space ASCE, so
no functional change is intended, also for tests that want to use
MSO/MSL. If a test intends to use a different primary space ASCE, it can
now just set the guest.asce in the save_area.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/asm/arch_def.h |  1 +
 lib/s390x/sie.c          | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 5beaf15b57e7..745a33878de5 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -92,6 +92,7 @@ enum address_space {
 };
 
 #define PSW_MASK_DAT			0x0400000000000000UL
+#define PSW_MASK_HOME			0x0000C00000000000UL
 #define PSW_MASK_IO			0x0200000000000000UL
 #define PSW_MASK_EXT			0x0100000000000000UL
 #define PSW_MASK_KEY			0x00F0000000000000UL
diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index b44febdeaaac..7f4474555ff7 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -52,6 +52,8 @@ void sie_handle_validity(struct vm *vm)
 
 void sie(struct vm *vm)
 {
+	uint64_t old_cr13;
+
 	if (vm->sblk->sdf == 2)
 		memcpy(vm->sblk->pv_grregs, vm->save_area.guest.grs,
 		       sizeof(vm->save_area.guest.grs));
@@ -59,6 +61,24 @@ void sie(struct vm *vm)
 	/* Reset icptcode so we don't trip over it below */
 	vm->sblk->icptcode = 0;
 
+	/*
+	 * Set up home address space to match primary space. Instead of running
+	 * in home space all the time, we switch every time in sie() because:
+	 * - tests that depend on running in primary space mode don't need to be
+	 *   touched
+	 * - it avoids regressions in tests
+	 * - switching every time makes it easier to extend this in the future,
+	 *   for example to allow tests to run in whatever space they want
+	 */
+	old_cr13 = stctg(13);
+	lctlg(13, stctg(1));
+
+	/* switch to home space so guest tables can be different from host */
+	psw_mask_set_bits(PSW_MASK_HOME);
+
+	/* also handle all interruptions in home space while in SIE */
+	irq_set_dat_mode(true, AS_HOME);
+
 	while (vm->sblk->icptcode == 0) {
 		sie64a(vm->sblk, &vm->save_area);
 		sie_handle_validity(vm);
@@ -66,6 +86,12 @@ void sie(struct vm *vm)
 	vm->save_area.guest.grs[14] = vm->sblk->gg14;
 	vm->save_area.guest.grs[15] = vm->sblk->gg15;
 
+	irq_set_dat_mode(true, AS_PRIM);
+	psw_mask_clear_bits(PSW_MASK_HOME);
+
+	/* restore the old CR 13 */
+	lctlg(13, old_cr13);
+
 	if (vm->sblk->sdf == 2)
 		memcpy(vm->save_area.guest.grs, vm->sblk->pv_grregs,
 		       sizeof(vm->save_area.guest.grs));
-- 
2.41.0

[kvm-unit-tests PATCH v7 4/8] s390x: lib: don't forward PSW when handling exception in SIE

[Nico Boehr]

When we're handling a pgm int in SIE, we want to return to the SIE
cleanup after handling the exception. That's why we set pgm_old_psw to
the sie_exit label in fixup_pgm_int.

On nullifing pgm ints, fixup_pgm_int will also forward the old PSW such
that we don't cause an pgm int again.

However, when we want to return to the sie_exit label, this is not
needed (since we've manually set pgm_old_psw). Instead, forwarding the
PSW might cause us to skip an instruction or end up in the middle of an
instruction.

So, let's just skip the rest of the fixup in case we're inside SIE.

Note that we're intentionally not fixing up the PSW in the guest; that's
best left to the test at hand by registering their own psw fixup.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/interrupt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index e0a1713349f6..f33467957dbf 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -145,6 +145,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
 	if (lowcore.pgm_old_psw.addr >= (uint64_t)&sie_entry &&
 	    lowcore.pgm_old_psw.addr <= (uint64_t)&sie_exit) {
 		lowcore.pgm_old_psw.addr = (uint64_t)&sie_exit;
+		return;
 	}
 
 	switch (lowcore.pgm_int_code) {
-- 
2.41.0

[kvm-unit-tests PATCH v7 5/8] s390x: lib: sie: don't reenter SIE on pgm int

[Nico Boehr]

At the moment, when a PGM int occurs while in SIE, we will just reenter
SIE after the interrupt handler was called.

This is because sie() has a loop which checks icptcode and re-enters SIE
if it is zero.

However, this behaviour is quite undesirable for SIE tests, since it
doesn't give the host the chance to assert on the PGM int. Instead, we
will just re-enter SIE, on nullifing conditions even causing the
exception again.

In sie(), check whether a pgm int code is set in lowcore. If it has,
exit the loop so the test can react to the interrupt. Add a new function
read_pgm_int_code() to obtain the interrupt code.

Note that this introduces a slight oddity with sie and pgm int in
certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
we will now never enter SIE until the pgm_int_code is cleared by e.g.
clear_pgm_int().

Also add missing include of facility.h to mem.h.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/asm/interrupt.h | 14 ++++++++++++++
 lib/s390x/asm/mem.h       |  1 +
 lib/s390x/sie.c           |  4 +++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index d01f8a89641a..7f73d473b346 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -97,4 +97,18 @@ static inline void low_prot_disable(void)
 	ctl_clear_bit(0, CTL0_LOW_ADDR_PROT);
 }
 
+/**
+ * read_pgm_int_code - Get the program interruption code of the last pgm int
+ * on the current CPU.
+ *
+ * This is similar to clear_pgm_int(), except that it doesn't clear the
+ * interruption information from lowcore.
+ *
+ * Returns 0 when none occurred.
+ */
+static inline uint16_t read_pgm_int_code(void)
+{
+	return lowcore.pgm_int_code;
+}
+
 #endif
diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h
index 64ef59b546a4..94d58c34f53f 100644
--- a/lib/s390x/asm/mem.h
+++ b/lib/s390x/asm/mem.h
@@ -8,6 +8,7 @@
 #ifndef _ASMS390X_MEM_H_
 #define _ASMS390X_MEM_H_
 #include <asm/arch_def.h>
+#include <asm/facility.h>
 
 /* create pointer while avoiding compiler warnings */
 #define OPAQUE_PTR(x) ((void *)(((uint64_t)&lowcore) + (x)))
diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index 7f4474555ff7..a97096948460 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -13,6 +13,7 @@
 #include <libcflat.h>
 #include <sie.h>
 #include <asm/page.h>
+#include <asm/interrupt.h>
 #include <libcflat.h>
 #include <alloc_page.h>
 
@@ -79,7 +80,8 @@ void sie(struct vm *vm)
 	/* also handle all interruptions in home space while in SIE */
 	irq_set_dat_mode(true, AS_HOME);
 
-	while (vm->sblk->icptcode == 0) {
+	/* leave SIE when we have an intercept or an interrupt so the test can react to it */
+	while (vm->sblk->icptcode == 0 && !read_pgm_int_code()) {
 		sie64a(vm->sblk, &vm->save_area);
 		sie_handle_validity(vm);
 	}
-- 
2.41.0

[kvm-unit-tests PATCH v7 6/8] s390x: add test source dir to include paths

[Nico Boehr]

Sometimes, it is useful to share some defines between a snippet and a
test. By adding the source directory to include paths, header files can
be placed in the snippet directory and included from the test (or vice
versa).

This is a prerequisite for "s390x: add a test for SIE without MSO/MSL".

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 s390x/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/s390x/Makefile b/s390x/Makefile
index 6e967194ae0d..947a4344738f 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -67,7 +67,7 @@ test_cases: $(tests)
 test_cases_binary: $(tests_binary)
 test_cases_pv: $(tests_pv_binary)
 
-INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x
+INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x $(SRCDIR)/s390x
 # Include generated header files (e.g. in case of out-of-source builds)
 INCLUDE_PATHS += lib
 CPPFLAGS = $(addprefix -I,$(INCLUDE_PATHS))
-- 
2.41.0

[kvm-unit-tests PATCH v7 7/8] s390x: add a test for SIE without MSO/MSL

[Nico Boehr]

Since we now have the ability to run guests without MSO/MSL, add a test
to make sure this doesn't break.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 s390x/Makefile             |   2 +
 s390x/sie-dat.c            | 110 +++++++++++++++++++++++++++++++++++++
 s390x/snippets/c/sie-dat.c |  52 ++++++++++++++++++
 s390x/snippets/c/sie-dat.h |   2 +
 s390x/unittests.cfg        |   3 +
 5 files changed, 169 insertions(+)
 create mode 100644 s390x/sie-dat.c
 create mode 100644 s390x/snippets/c/sie-dat.c
 create mode 100644 s390x/snippets/c/sie-dat.h

diff --git a/s390x/Makefile b/s390x/Makefile
index 947a4344738f..f79fd0098312 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -41,6 +41,7 @@ tests += $(TEST_DIR)/migration-sck.elf
 tests += $(TEST_DIR)/exittime.elf
 tests += $(TEST_DIR)/ex.elf
 tests += $(TEST_DIR)/topology.elf
+tests += $(TEST_DIR)/sie-dat.elf
 
 pv-tests += $(TEST_DIR)/pv-diags.elf
 pv-tests += $(TEST_DIR)/pv-icptcode.elf
@@ -123,6 +124,7 @@ snippet_lib = $(snippet_asmlib) lib/auxinfo.o
 # perquisites (=guests) for the snippet hosts.
 # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
 $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
+$(TEST_DIR)/sie-dat.elf: snippets = $(SNIPPET_DIR)/c/sie-dat.gbin
 $(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
 
 $(TEST_DIR)/pv-diags.elf: pv-snippets += $(SNIPPET_DIR)/asm/pv-diag-yield.gbin
diff --git a/s390x/sie-dat.c b/s390x/sie-dat.c
new file mode 100644
index 000000000000..89e2c4c13db2
--- /dev/null
+++ b/s390x/sie-dat.c
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tests SIE with paging.
+ *
+ * Copyright 2023 IBM Corp.
+ *
+ * Authors:
+ *    Nico Boehr <nrb@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include <vmalloc.h>
+#include <asm/pgtable.h>
+#include <mmu.h>
+#include <asm/page.h>
+#include <asm/interrupt.h>
+#include <alloc_page.h>
+#include <sclp.h>
+#include <sie.h>
+#include <snippet.h>
+#include "snippets/c/sie-dat.h"
+
+static struct vm vm;
+static pgd_t *guest_root;
+
+static void test_sie_dat(void)
+{
+	uint64_t test_page_gpa, test_page_hpa;
+	uint8_t *test_page_hva, expected_val;
+	bool contents_match;
+	uint8_t r1;
+
+	/* guest will tell us the guest physical address of the test buffer */
+	sie(&vm);
+
+	r1 = (vm.sblk->ipa & 0xf0) >> 4;
+	test_page_gpa = vm.save_area.guest.grs[r1];
+	test_page_hpa = virt_to_pte_phys(guest_root, (void*)test_page_gpa);
+	test_page_hva = __va(test_page_hpa);
+	assert(vm.sblk->icptcode == ICPT_INST &&
+	       (vm.sblk->ipa & 0xff00) == 0x8300 && vm.sblk->ipb == 0x9c0000);
+	report_info("test buffer gpa=0x%lx hva=%p", test_page_gpa, test_page_hva);
+
+	/* guest will now write to the test buffer and we verify the contents */
+	sie(&vm);
+	assert(vm.sblk->icptcode == ICPT_INST &&
+	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
+
+	contents_match = true;
+	for (unsigned int i = 0; i < GUEST_TEST_PAGE_COUNT; i++) {
+		expected_val = 42 + i;
+		if (test_page_hva[i * PAGE_SIZE] != expected_val) {
+			report_fail("page %u mismatch actual_val=%x expected_val=%x",
+				    i, test_page_hva[i], expected_val);
+			contents_match = false;
+		}
+	}
+	report(contents_match, "test buffer contents match");
+
+	/* the guest will now write to an unmapped address and we check that this causes a segment translation exception */
+	report_prefix_push("guest write to unmapped");
+	expect_pgm_int();
+	sie(&vm);
+	check_pgm_int_code(PGM_INT_CODE_SEGMENT_TRANSLATION);
+	report_prefix_pop();
+}
+
+static void setup_guest(void)
+{
+	extern const char SNIPPET_NAME_START(c, sie_dat)[];
+	extern const char SNIPPET_NAME_END(c, sie_dat)[];
+	uint64_t guest_max_addr;
+
+	setup_vm();
+	snippet_setup_guest(&vm, false);
+
+	/* allocate a region-1 table */
+	guest_root = pgd_alloc_one();
+
+	/* map guest memory 1:1 */
+	guest_max_addr = GUEST_TOTAL_PAGE_COUNT * PAGE_SIZE;
+	for (uint64_t i = 0; i < guest_max_addr; i += PAGE_SIZE)
+		install_page(guest_root, __pa(vm.guest_mem + i), (void *)i);
+
+	/* set up storage limit supression - leave mso and msl intact they are ignored anyways */
+	vm.sblk->cpuflags |= CPUSTAT_SM;
+
+	/* set up the guest asce */
+	vm.save_area.guest.asce = __pa(guest_root) | ASCE_DT_REGION1 | REGION_TABLE_LENGTH;
+
+	snippet_init(&vm, SNIPPET_NAME_START(c, sie_dat),
+		     SNIPPET_LEN(c, sie_dat), SNIPPET_UNPACK_OFF);
+}
+
+int main(void)
+{
+	report_prefix_push("sie-dat");
+	if (!sclp_facilities.has_sief2) {
+		report_skip("SIEF2 facility unavailable");
+		goto done;
+	}
+
+	setup_guest();
+	test_sie_dat();
+	sie_guest_destroy(&vm);
+
+done:
+	report_prefix_pop();
+	return report_summary();
+
+}
diff --git a/s390x/snippets/c/sie-dat.c b/s390x/snippets/c/sie-dat.c
new file mode 100644
index 000000000000..e07136bf7430
--- /dev/null
+++ b/s390x/snippets/c/sie-dat.c
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Snippet used by the sie-dat.c test to verify paging without MSO/MSL
+ *
+ * Copyright (c) 2023 IBM Corp
+ *
+ * Authors:
+ *  Nico Boehr <nrb@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include <asm-generic/page.h>
+#include "sie-dat.h"
+
+static uint8_t test_page[GUEST_TEST_PAGE_COUNT * PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
+
+static inline void force_exit(void)
+{
+	asm volatile("diag	0,0,0x44\n");
+}
+
+static inline void force_exit_value(uint64_t val)
+{
+	asm volatile(
+		"diag	%[val],0,0x9c\n"
+		: : [val] "d"(val)
+	);
+}
+
+int main(void)
+{
+	uint8_t *invalid_ptr;
+
+	memset(test_page, 0, sizeof(test_page));
+	/* tell the host the page's physical address (we're running DAT off) */
+	force_exit_value((uint64_t)test_page);
+
+	/* write some value to the page so the host can verify it */
+	for (size_t i = 0; i < GUEST_TEST_PAGE_COUNT; i++)
+		test_page[i * PAGE_SIZE] = 42 + i;
+
+	/* indicate we've written all pages */
+	force_exit();
+
+	/* the first unmapped address */
+	invalid_ptr = (uint8_t *)(GUEST_TOTAL_PAGE_COUNT * PAGE_SIZE);
+	*invalid_ptr = 42;
+
+	/* indicate we've written the non-allowed page (should never get here) */
+	force_exit();
+
+	return 0;
+}
diff --git a/s390x/snippets/c/sie-dat.h b/s390x/snippets/c/sie-dat.h
new file mode 100644
index 000000000000..ed3f99f75f9c
--- /dev/null
+++ b/s390x/snippets/c/sie-dat.h
@@ -0,0 +1,2 @@
+#define GUEST_TEST_PAGE_COUNT 10
+#define GUEST_TOTAL_PAGE_COUNT 256
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 68e119e4fcaa..184658ff7d8d 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -247,3 +247,6 @@ file = topology.elf
 [topology-2]
 file = topology.elf
 extra_params = -cpu max,ctop=on -smp sockets=31,cores=8,maxcpus=248  -append '-sockets 31 -cores 8'
+
+[sie-dat]
+file = sie-dat.elf
-- 
2.41.0

[kvm-unit-tests PATCH v7 8/8] lib: s390x: interrupt: remove TEID_ASCE defines

[Nico Boehr]

These defines were - I can only guess - meant for the asce_id field.
Since print_decode_teid() used AS_PRIM and friends instead, I see little
benefit in keeping these around.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 lib/s390x/asm/interrupt.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
index 7f73d473b346..48bd78fa1515 100644
--- a/lib/s390x/asm/interrupt.h
+++ b/lib/s390x/asm/interrupt.h
@@ -13,11 +13,6 @@
 #define EXT_IRQ_EXTERNAL_CALL	0x1202
 #define EXT_IRQ_SERVICE_SIG	0x2401
 
-#define TEID_ASCE_PRIMARY	0
-#define TEID_ASCE_AR		1
-#define TEID_ASCE_SECONDARY	2
-#define TEID_ASCE_HOME		3
-
 union teid {
 	unsigned long val;
 	union {
-- 
2.41.0

Re: [kvm-unit-tests PATCH v7 1/8] lib: s390x: introduce bitfield for PSW mask

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:30 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Changing the PSW mask is currently little clumsy, since there is only the
> PSW_MASK_* defines. This makes it hard to change e.g. only the address
> space in the current PSW without a lot of bit fiddling.
> 
> Introduce a bitfield for the PSW mask. This makes this kind of
> modifications much simpler and easier to read.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/asm/arch_def.h | 25 ++++++++++++++++++++++++-
>  s390x/selftest.c         | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index bb26e008cc68..f629b6d0a17f 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -37,9 +37,32 @@ struct stack_frame_int {
>  };
>  
>  struct psw {
> -	uint64_t	mask;
> +	union {
> +		uint64_t	mask;
> +		struct {
> +			uint64_t reserved00:1;
> +			uint64_t per:1;
> +			uint64_t reserved02:3;
> +			uint64_t dat:1;
> +			uint64_t io:1;
> +			uint64_t ext:1;
> +			uint64_t key:4;
> +			uint64_t reserved12:1;
> +			uint64_t mchk:1;
> +			uint64_t wait:1;
> +			uint64_t pstate:1;
> +			uint64_t as:2;
> +			uint64_t cc:2;
> +			uint64_t prg_mask:4;
> +			uint64_t reserved24:7;
> +			uint64_t ea:1;
> +			uint64_t ba:1;
> +			uint64_t reserved33:31;
> +		};
> +	};
>  	uint64_t	addr;
>  };
> +_Static_assert(sizeof(struct psw) == 16, "PSW size");
>  
>  #define PSW(m, a) ((struct psw){ .mask = (m), .addr = (uint64_t)(a) })
>  
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index 13fd36bc06f8..92ed4e5d35eb 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -74,6 +74,39 @@ static void test_malloc(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_psw_mask(void)
> +{
> +	uint64_t expected_key = 0xf;
> +	struct psw test_psw = PSW(0, 0);
> +
> +	report_prefix_push("PSW mask");
> +	test_psw.mask = PSW_MASK_DAT;
> +	report(test_psw.dat, "DAT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_DAT, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_IO;
> +	report(test_psw.io, "IO matches expected=0x%016lx actual=0x%016lx", PSW_MASK_IO, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_EXT;
> +	report(test_psw.ext, "EXT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_EXT, test_psw.mask);
> +
> +	test_psw.mask = expected_key << (63 - 11);
> +	report(test_psw.key == expected_key, "PSW Key matches expected=0x%lx actual=0x%x", expected_key, test_psw.key);
> +
> +	test_psw.mask = 1UL << (63 - 13);
> +	report(test_psw.mchk, "MCHK matches");
> +
> +	test_psw.mask = PSW_MASK_WAIT;
> +	report(test_psw.wait, "Wait matches expected=0x%016lx actual=0x%016lx", PSW_MASK_WAIT, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_PSTATE;
> +	report(test_psw.pstate, "Pstate matches expected=0x%016lx actual=0x%016lx", PSW_MASK_PSTATE, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_64;
> +	report(test_psw.ea && test_psw.ba, "BA/EA matches expected=0x%016lx actual=0x%016lx", PSW_MASK_64, test_psw.mask);
> +
> +	report_prefix_pop();
> +}
> +
>  int main(int argc, char**argv)
>  {
>  	report_prefix_push("selftest");
> @@ -89,6 +122,7 @@ int main(int argc, char**argv)
>  	test_fp();
>  	test_pgm_int();
>  	test_malloc();
> +	test_psw_mask();
>  
>  	return report_summary();
>  }

Re: [kvm-unit-tests PATCH v7 3/8] s390x: sie: switch to home space mode before entering SIE

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> This is to prepare for running guests without MSO/MSL, which is
> currently not possible.
> 
> We already have code in sie64a to setup a guest primary ASCE before
> entering SIE, so we can in theory switch to the page tables which
> translate gpa to hpa.
> 
> But the host is running in primary space mode already, so changing the
> primary ASCE before entering SIE will also affect the host's code and
> data.
> 
> To make this switch useful, the host should run in a different address
> space mode. Hence, set up and change to home address space mode before
> installing the guest ASCE.
> 
> The home space ASCE is just copied over from the primary space ASCE, so
> no functional change is intended, also for tests that want to use
> MSO/MSL. If a test intends to use a different primary space ASCE, it can
> now just set the guest.asce in the save_area.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [kvm-unit-tests PATCH v7 2/8] s390x: add function to set DAT mode for all interrupts

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:31 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> When toggling DAT or switch address space modes, it is likely that
> interrupts should be handled in the same DAT or address space mode.
> 
> Add a function which toggles DAT and address space mode for all
> interruptions, except restart interrupts.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/asm/arch_def.h  | 10 ++++++----
>  lib/s390x/asm/interrupt.h |  2 ++
>  lib/s390x/interrupt.c     | 35 +++++++++++++++++++++++++++++++++++
>  lib/s390x/mmu.c           |  5 +++--
>  4 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index f629b6d0a17f..5beaf15b57e7 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -84,10 +84,12 @@ struct cpu {
>  	bool in_interrupt_handler;
>  };
>  
> -#define AS_PRIM				0
> -#define AS_ACCR				1
> -#define AS_SECN				2
> -#define AS_HOME				3
> +enum address_space {
> +	AS_PRIM = 0,
> +	AS_ACCR = 1,
> +	AS_SECN = 2,
> +	AS_HOME = 3
> +};
>  
>  #define PSW_MASK_DAT			0x0400000000000000UL
>  #define PSW_MASK_IO			0x0200000000000000UL
> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index 35c1145f0349..d01f8a89641a 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -83,6 +83,8 @@ void expect_ext_int(void);
>  uint16_t clear_pgm_int(void);
>  void check_pgm_int_code(uint16_t code);
>  
> +void irq_set_dat_mode(bool use_dat, enum address_space as);
> +
>  /* Activate low-address protection */
>  static inline void low_prot_enable(void)
>  {
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 3f993a363ae2..e0a1713349f6 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -9,6 +9,7 @@
>   */
>  #include <libcflat.h>
>  #include <asm/barrier.h>
> +#include <asm/mem.h>
>  #include <asm/asm-offsets.h>
>  #include <sclp.h>
>  #include <interrupt.h>
> @@ -104,6 +105,40 @@ void register_ext_cleanup_func(void (*f)(struct stack_frame_int *))
>  	THIS_CPU->ext_cleanup_func = f;
>  }
>  
> +/**
> + * irq_set_dat_mode - Set the DAT mode of all interrupt handlers, except for
> + * restart.

(here)

> + * This will update the DAT mode and address space mode of all interrupt new
> + * PSWs.
> + *
> + * Since enabling DAT needs initialized CRs and the restart new PSW is often used
> + * to initialize CRs, the restart new PSW is never touched to avoid the chicken
> + * and egg situation.
> + *
> + * @use_dat specifies whether to use DAT or not
> + * @as specifies the address space mode to use. Not set if use_dat is false.

add the colon

 * @use_dat: 
 * @as: 

and move them up there ^

with that fixed:

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> + */
> +void irq_set_dat_mode(bool use_dat, enum address_space as)
> +{
> +	struct psw* irq_psws[] = {
> +		OPAQUE_PTR(GEN_LC_EXT_NEW_PSW),
> +		OPAQUE_PTR(GEN_LC_SVC_NEW_PSW),
> +		OPAQUE_PTR(GEN_LC_PGM_NEW_PSW),
> +		OPAQUE_PTR(GEN_LC_MCCK_NEW_PSW),
> +		OPAQUE_PTR(GEN_LC_IO_NEW_PSW),
> +	};
> +	struct psw *psw;
> +
> +	assert(as == AS_PRIM || as == AS_ACCR || as == AS_SECN || as == AS_HOME);
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(irq_psws); i++) {
> +		psw = irq_psws[i];
> +		psw->dat = use_dat;
> +		if (use_dat)
> +			psw->as = as;
> +	}
> +}
> +
>  static void fixup_pgm_int(struct stack_frame_int *stack)
>  {
>  	/* If we have an error on SIE we directly move to sie_exit */
> diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
> index b474d7021d3f..9a179d6b8ec5 100644
> --- a/lib/s390x/mmu.c
> +++ b/lib/s390x/mmu.c
> @@ -12,6 +12,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/arch_def.h>
>  #include <asm/barrier.h>
> +#include <asm/interrupt.h>
>  #include <vmalloc.h>
>  #include "mmu.h"
>  
> @@ -41,8 +42,8 @@ static void mmu_enable(pgd_t *pgtable)
>  	/* enable dat (primary == 0 set as default) */
>  	enable_dat();
>  
> -	/* we can now also use DAT unconditionally in our PGM handler */
> -	lowcore.pgm_new_psw.mask |= PSW_MASK_DAT;
> +	/* we can now also use DAT in all interrupt handlers */
> +	irq_set_dat_mode(true, AS_PRIM);
>  }
>  
>  /*

Re: [kvm-unit-tests PATCH v7 1/8] lib: s390x: introduce bitfield for PSW mask

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> Changing the PSW mask is currently little clumsy, since there is only the
> PSW_MASK_* defines. This makes it hard to change e.g. only the address
> space in the current PSW without a lot of bit fiddling.
> 
> Introduce a bitfield for the PSW mask. This makes this kind of
> modifications much simpler and easier to read.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>   lib/s390x/asm/arch_def.h | 25 ++++++++++++++++++++++++-
>   s390x/selftest.c         | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index bb26e008cc68..f629b6d0a17f 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -37,9 +37,32 @@ struct stack_frame_int {
>   };
>   
>   struct psw {
> -	uint64_t	mask;
> +	union {
> +		uint64_t	mask;
> +		struct {
> +			uint64_t reserved00:1;
> +			uint64_t per:1;
> +			uint64_t reserved02:3;
> +			uint64_t dat:1;
> +			uint64_t io:1;
> +			uint64_t ext:1;
> +			uint64_t key:4;
> +			uint64_t reserved12:1;
> +			uint64_t mchk:1;
> +			uint64_t wait:1;
> +			uint64_t pstate:1;
> +			uint64_t as:2;
> +			uint64_t cc:2;
> +			uint64_t prg_mask:4;
> +			uint64_t reserved24:7;
> +			uint64_t ea:1;
> +			uint64_t ba:1;
> +			uint64_t reserved33:31;
> +		};
> +	};
>   	uint64_t	addr;
>   };
> +_Static_assert(sizeof(struct psw) == 16, "PSW size");
>   
>   #define PSW(m, a) ((struct psw){ .mask = (m), .addr = (uint64_t)(a) })
>   
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index 13fd36bc06f8..92ed4e5d35eb 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -74,6 +74,39 @@ static void test_malloc(void)
>   	report_prefix_pop();
>   }
>   
> +static void test_psw_mask(void)
> +{
> +	uint64_t expected_key = 0xf;
> +	struct psw test_psw = PSW(0, 0);
> +
> +	report_prefix_push("PSW mask");
> +	test_psw.mask = PSW_MASK_DAT;
> +	report(test_psw.dat, "DAT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_DAT, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_IO;
> +	report(test_psw.io, "IO matches expected=0x%016lx actual=0x%016lx", PSW_MASK_IO, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_EXT;
> +	report(test_psw.ext, "EXT matches expected=0x%016lx actual=0x%016lx", PSW_MASK_EXT, test_psw.mask);
> +
> +	test_psw.mask = expected_key << (63 - 11);
> +	report(test_psw.key == expected_key, "PSW Key matches expected=0x%lx actual=0x%x", expected_key, test_psw.key);
> +
> +	test_psw.mask = 1UL << (63 - 13);
> +	report(test_psw.mchk, "MCHK matches");
> +
> +	test_psw.mask = PSW_MASK_WAIT;
> +	report(test_psw.wait, "Wait matches expected=0x%016lx actual=0x%016lx", PSW_MASK_WAIT, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_PSTATE;
> +	report(test_psw.pstate, "Pstate matches expected=0x%016lx actual=0x%016lx", PSW_MASK_PSTATE, test_psw.mask);
> +
> +	test_psw.mask = PSW_MASK_64;
> +	report(test_psw.ea && test_psw.ba, "BA/EA matches expected=0x%016lx actual=0x%016lx", PSW_MASK_64, test_psw.mask);
> +
> +	report_prefix_pop();
> +}
> +
>   int main(int argc, char**argv)
>   {
>   	report_prefix_push("selftest");
> @@ -89,6 +122,7 @@ int main(int argc, char**argv)
>   	test_fp();
>   	test_pgm_int();
>   	test_malloc();
> +	test_psw_mask();
>   
>   	return report_summary();
>   }

Re: [kvm-unit-tests PATCH v7 3/8] s390x: sie: switch to home space mode before entering SIE

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:32 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> This is to prepare for running guests without MSO/MSL, which is
> currently not possible.
> 
> We already have code in sie64a to setup a guest primary ASCE before
> entering SIE, so we can in theory switch to the page tables which
> translate gpa to hpa.
> 
> But the host is running in primary space mode already, so changing the
> primary ASCE before entering SIE will also affect the host's code and
> data.
> 
> To make this switch useful, the host should run in a different address
> space mode. Hence, set up and change to home address space mode before
> installing the guest ASCE.
> 
> The home space ASCE is just copied over from the primary space ASCE, so
> no functional change is intended, also for tests that want to use
> MSO/MSL. If a test intends to use a different primary space ASCE, it can
> now just set the guest.asce in the save_area.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/asm/arch_def.h |  1 +
>  lib/s390x/sie.c          | 26 ++++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 5beaf15b57e7..745a33878de5 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -92,6 +92,7 @@ enum address_space {
>  };
>  
>  #define PSW_MASK_DAT			0x0400000000000000UL
> +#define PSW_MASK_HOME			0x0000C00000000000UL
>  #define PSW_MASK_IO			0x0200000000000000UL
>  #define PSW_MASK_EXT			0x0100000000000000UL
>  #define PSW_MASK_KEY			0x00F0000000000000UL
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index b44febdeaaac..7f4474555ff7 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -52,6 +52,8 @@ void sie_handle_validity(struct vm *vm)
>  
>  void sie(struct vm *vm)
>  {
> +	uint64_t old_cr13;
> +
>  	if (vm->sblk->sdf == 2)
>  		memcpy(vm->sblk->pv_grregs, vm->save_area.guest.grs,
>  		       sizeof(vm->save_area.guest.grs));
> @@ -59,6 +61,24 @@ void sie(struct vm *vm)
>  	/* Reset icptcode so we don't trip over it below */
>  	vm->sblk->icptcode = 0;
>  
> +	/*
> +	 * Set up home address space to match primary space. Instead of running
> +	 * in home space all the time, we switch every time in sie() because:
> +	 * - tests that depend on running in primary space mode don't need to be
> +	 *   touched
> +	 * - it avoids regressions in tests
> +	 * - switching every time makes it easier to extend this in the future,
> +	 *   for example to allow tests to run in whatever space they want
> +	 */
> +	old_cr13 = stctg(13);
> +	lctlg(13, stctg(1));
> +
> +	/* switch to home space so guest tables can be different from host */
> +	psw_mask_set_bits(PSW_MASK_HOME);
> +
> +	/* also handle all interruptions in home space while in SIE */
> +	irq_set_dat_mode(true, AS_HOME);
> +
>  	while (vm->sblk->icptcode == 0) {
>  		sie64a(vm->sblk, &vm->save_area);
>  		sie_handle_validity(vm);
> @@ -66,6 +86,12 @@ void sie(struct vm *vm)
>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;
>  
> +	irq_set_dat_mode(true, AS_PRIM);
> +	psw_mask_clear_bits(PSW_MASK_HOME);
> +
> +	/* restore the old CR 13 */
> +	lctlg(13, old_cr13);
> +
>  	if (vm->sblk->sdf == 2)
>  		memcpy(vm->save_area.guest.grs, vm->sblk->pv_grregs,
>  		       sizeof(vm->save_area.guest.grs));

Re: [kvm-unit-tests PATCH v7 5/8] s390x: lib: sie: don't reenter SIE on pgm int

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:34 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

[...]

> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index d01f8a89641a..7f73d473b346 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -97,4 +97,18 @@ static inline void low_prot_disable(void)
>  	ctl_clear_bit(0, CTL0_LOW_ADDR_PROT);
>  }
>  
> +/**
> + * read_pgm_int_code - Get the program interruption code of the last pgm int
> + * on the current CPU.
> + *
> + * This is similar to clear_pgm_int(), except that it doesn't clear the
> + * interruption information from lowcore.
> + *
> + * Returns 0 when none occurred.

 * Return:

with that fixed:

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

[...]

Re: [kvm-unit-tests PATCH v7 5/8] s390x: lib: sie: don't reenter SIE on pgm int

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> At the moment, when a PGM int occurs while in SIE, we will just reenter
> SIE after the interrupt handler was called.
> 
> This is because sie() has a loop which checks icptcode and re-enters SIE
> if it is zero.
> 
> However, this behaviour is quite undesirable for SIE tests, since it
> doesn't give the host the chance to assert on the PGM int. Instead, we
> will just re-enter SIE, on nullifing conditions even causing the
> exception again.
> 
> In sie(), check whether a pgm int code is set in lowcore. If it has,
> exit the loop so the test can react to the interrupt. Add a new function
> read_pgm_int_code() to obtain the interrupt code.
> 
> Note that this introduces a slight oddity with sie and pgm int in
> certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
> we will now never enter SIE until the pgm_int_code is cleared by e.g.
> clear_pgm_int().

Is there any use in NOT having an assert(!read_pgm_int_code()) before 
entering the loop?

Re: [kvm-unit-tests PATCH v7 6/8] s390x: add test source dir to include paths

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> Sometimes, it is useful to share some defines between a snippet and a
> test. By adding the source directory to include paths, header files can
> be placed in the snippet directory and included from the test (or vice
> versa).
> 
> This is a prerequisite for "s390x: add a test for SIE without MSO/MSL".

"This is a prerequisite for future tests."

I'd never directly reference a future commit.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   s390x/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 6e967194ae0d..947a4344738f 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -67,7 +67,7 @@ test_cases: $(tests)
>   test_cases_binary: $(tests_binary)
>   test_cases_pv: $(tests_pv_binary)
>   
> -INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x
> +INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x $(SRCDIR)/s390x
>   # Include generated header files (e.g. in case of out-of-source builds)
>   INCLUDE_PATHS += lib
>   CPPFLAGS = $(addprefix -I,$(INCLUDE_PATHS))

Re: [kvm-unit-tests PATCH v7 6/8] s390x: add test source dir to include paths

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:35 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Sometimes, it is useful to share some defines between a snippet and a
> test. By adding the source directory to include paths, header files can
> be placed in the snippet directory and included from the test (or vice
> versa).
> 
> This is a prerequisite for "s390x: add a test for SIE without MSO/MSL".
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  s390x/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 6e967194ae0d..947a4344738f 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -67,7 +67,7 @@ test_cases: $(tests)
>  test_cases_binary: $(tests_binary)
>  test_cases_pv: $(tests_pv_binary)
>  
> -INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x
> +INCLUDE_PATHS = $(SRCDIR)/lib $(SRCDIR)/lib/s390x $(SRCDIR)/s390x
>  # Include generated header files (e.g. in case of out-of-source builds)
>  INCLUDE_PATHS += lib
>  CPPFLAGS = $(addprefix -I,$(INCLUDE_PATHS))

Re: [kvm-unit-tests PATCH v7 7/8] s390x: add a test for SIE without MSO/MSL

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:36 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Since we now have the ability to run guests without MSO/MSL, add a test
> to make sure this doesn't break.

[...]

> +	/* the guest will now write to an unmapped address and we check that this causes a segment translation exception */
> +	report_prefix_push("guest write to unmapped");
> +	expect_pgm_int();
> +	sie(&vm);
> +	check_pgm_int_code(PGM_INT_CODE_SEGMENT_TRANSLATION);

can you also check that the faulting address is the one we expect?

> +	report_prefix_pop();
> +}
> +

[...]

> diff --git a/s390x/snippets/c/sie-dat.c b/s390x/snippets/c/sie-dat.c
> new file mode 100644
> index 000000000000..e07136bf7430
> --- /dev/null
> +++ b/s390x/snippets/c/sie-dat.c
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Snippet used by the sie-dat.c test to verify paging without MSO/MSL
> + *
> + * Copyright (c) 2023 IBM Corp
> + *
> + * Authors:
> + *  Nico Boehr <nrb@linux.ibm.com>
> + */
> +#include <libcflat.h>
> +#include <asm-generic/page.h>
> +#include "sie-dat.h"
> +
> +static uint8_t test_page[GUEST_TEST_PAGE_COUNT * PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));

I would call it test_pages

[...]

Re: [kvm-unit-tests PATCH v7 8/8] lib: s390x: interrupt: remove TEID_ASCE defines

[Claudio Imbrenda]

On Fri,  3 Nov 2023 10:29:37 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> These defines were - I can only guess - meant for the asce_id field.
> Since print_decode_teid() used AS_PRIM and friends instead, I see little
> benefit in keeping these around.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/asm/interrupt.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
> index 7f73d473b346..48bd78fa1515 100644
> --- a/lib/s390x/asm/interrupt.h
> +++ b/lib/s390x/asm/interrupt.h
> @@ -13,11 +13,6 @@
>  #define EXT_IRQ_EXTERNAL_CALL	0x1202
>  #define EXT_IRQ_SERVICE_SIG	0x2401
>  
> -#define TEID_ASCE_PRIMARY	0
> -#define TEID_ASCE_AR		1
> -#define TEID_ASCE_SECONDARY	2
> -#define TEID_ASCE_HOME		3
> -
>  union teid {
>  	unsigned long val;
>  	union {

Re: [kvm-unit-tests PATCH v7 7/8] s390x: add a test for SIE without MSO/MSL

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> Since we now have the ability to run guests without MSO/MSL, add a test
> to make sure this doesn't break.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Two small nits below

> +
> +static void test_sie_dat(void)
> +{
> +	uint64_t test_page_gpa, test_page_hpa;
> +	uint8_t *test_page_hva, expected_val;
> +	bool contents_match;
> +	uint8_t r1;
> +
> +	/* guest will tell us the guest physical address of the test buffer */
> +	sie(&vm);
> +
> +	r1 = (vm.sblk->ipa & 0xf0) >> 4;
> +	test_page_gpa = vm.save_area.guest.grs[r1];
> +	test_page_hpa = virt_to_pte_phys(guest_root, (void*)test_page_gpa);
> +	test_page_hva = __va(test_page_hpa);
> +	assert(vm.sblk->icptcode == ICPT_INST &&
> +	       (vm.sblk->ipa & 0xff00) == 0x8300 && vm.sblk->ipb == 0x9c0000);

Move the assert right under the sie() call.

> +	report_info("test buffer gpa=0x%lx hva=%p", test_page_gpa, test_page_hva);
> +
> +	/* guest will now write to the test buffer and we verify the contents */
> +	sie(&vm);
> +	assert(vm.sblk->icptcode == ICPT_INST &&
> +	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
> +

[...]

> +static void setup_guest(void)
> +{
> +	extern const char SNIPPET_NAME_START(c, sie_dat)[];
> +	extern const char SNIPPET_NAME_END(c, sie_dat)[];
> +	uint64_t guest_max_addr;
> +
> +	setup_vm();
> +	snippet_setup_guest(&vm, false);
> +
> +	/* allocate a region-1 table */
> +	guest_root = pgd_alloc_one();
> +
> +	/* map guest memory 1:1 */

0:guest_max_addr

> +	guest_max_addr = GUEST_TOTAL_PAGE_COUNT * PAGE_SIZE;
> +	for (uint64_t i = 0; i < guest_max_addr; i += PAGE_SIZE)
> +		install_page(guest_root, __pa(vm.guest_mem + i), (void *)i);
> +
> +	/* set up storage limit supression - leave mso and msl intact they are ignored anyways */
> +	vm.sblk->cpuflags |= CPUSTAT_SM;
> +
> +	/* set up the guest asce */
> +	vm.save_area.guest.asce = __pa(guest_root) | ASCE_DT_REGION1 | REGION_TABLE_LENGTH;
> +
> +	snippet_init(&vm, SNIPPET_NAME_START(c, sie_dat),
> +		     SNIPPET_LEN(c, sie_dat), SNIPPET_UNPACK_OFF);
> +}
> +

Re: [kvm-unit-tests PATCH v7 8/8] lib: s390x: interrupt: remove TEID_ASCE defines

[Janosch Frank]

On 11/3/23 10:29, Nico Boehr wrote:
> These defines were - I can only guess - meant for the asce_id field.
> Since print_decode_teid() used AS_PRIM and friends instead, I see little
> benefit in keeping these around.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

For once I'm not responsible so:
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [kvm-unit-tests PATCH v7 7/8] s390x: add a test for SIE without MSO/MSL

[Heiko Carstens]

On Fri, Nov 03, 2023 at 10:29:36AM +0100, Nico Boehr wrote:
> Since we now have the ability to run guests without MSO/MSL, add a test
> to make sure this doesn't break.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  s390x/Makefile             |   2 +
>  s390x/sie-dat.c            | 110 +++++++++++++++++++++++++++++++++++++
>  s390x/snippets/c/sie-dat.c |  52 ++++++++++++++++++
>  s390x/snippets/c/sie-dat.h |   2 +
>  s390x/unittests.cfg        |   3 +
>  5 files changed, 169 insertions(+)
>  create mode 100644 s390x/sie-dat.c
>  create mode 100644 s390x/snippets/c/sie-dat.c
>  create mode 100644 s390x/snippets/c/sie-dat.h
...
> +static uint8_t test_page[GUEST_TEST_PAGE_COUNT * PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
> +
> +static inline void force_exit(void)
> +{
> +	asm volatile("diag	0,0,0x44\n");
> +}
> +
> +static inline void force_exit_value(uint64_t val)
> +{
> +	asm volatile(
> +		"diag	%[val],0,0x9c\n"
> +		: : [val] "d"(val)
> +	);
> +}
> +
> +int main(void)
> +{
> +	uint8_t *invalid_ptr;
> +
> +	memset(test_page, 0, sizeof(test_page));
> +	/* tell the host the page's physical address (we're running DAT off) */
> +	force_exit_value((uint64_t)test_page);
> +
> +	/* write some value to the page so the host can verify it */
> +	for (size_t i = 0; i < GUEST_TEST_PAGE_COUNT; i++)
> +		test_page[i * PAGE_SIZE] = 42 + i;
> +
> +	/* indicate we've written all pages */
> +	force_exit();
> +
> +	/* the first unmapped address */
> +	invalid_ptr = (uint8_t *)(GUEST_TOTAL_PAGE_COUNT * PAGE_SIZE);
> +	*invalid_ptr = 42;
> +
> +	/* indicate we've written the non-allowed page (should never get here) */
> +	force_exit();
> +
> +	return 0;
> +}

The compiler will not necessarily generate the expected code here, since
there is no data dependency between the used inline assemblies and the
memory locations that are changed. That is: the compiler may move the
inline assemblies and/or memory assignments around.

In order to prevent that you could simply add a compiler barrier to both
inline assemblies (add "memory" to the clobber list).

Re: [kvm-unit-tests PATCH v7 7/8] s390x: add a test for SIE without MSO/MSL

[Nico Boehr]

Quoting Janosch Frank (2023-11-03 15:16:36)
[...]
> > +static void setup_guest(void)
> > +{
> > +     extern const char SNIPPET_NAME_START(c, sie_dat)[];
> > +     extern const char SNIPPET_NAME_END(c, sie_dat)[];
> > +     uint64_t guest_max_addr;
> > +
> > +     setup_vm();
> > +     snippet_setup_guest(&vm, false);
> > +
> > +     /* allocate a region-1 table */
> > +     guest_root = pgd_alloc_one();
> > +
> > +     /* map guest memory 1:1 */
> 
> 0:guest_max_addr

Sorry, I don't get what you mean.

Re: [kvm-unit-tests PATCH v7 5/8] s390x: lib: sie: don't reenter SIE on pgm int

[Nico Boehr]

Quoting Janosch Frank (2023-11-03 14:53:17)
> On 11/3/23 10:29, Nico Boehr wrote:
> > At the moment, when a PGM int occurs while in SIE, we will just reenter
> > SIE after the interrupt handler was called.
> > 
> > This is because sie() has a loop which checks icptcode and re-enters SIE
> > if it is zero.
> > 
> > However, this behaviour is quite undesirable for SIE tests, since it
> > doesn't give the host the chance to assert on the PGM int. Instead, we
> > will just re-enter SIE, on nullifing conditions even causing the
> > exception again.
> > 
> > In sie(), check whether a pgm int code is set in lowcore. If it has,
> > exit the loop so the test can react to the interrupt. Add a new function
> > read_pgm_int_code() to obtain the interrupt code.
> > 
> > Note that this introduces a slight oddity with sie and pgm int in
> > certain cases: If a PGM int occurs between a expect_pgm_int() and sie(),
> > we will now never enter SIE until the pgm_int_code is cleared by e.g.
> > clear_pgm_int().
> 
> Is there any use in NOT having an assert(!read_pgm_int_code()) before 
> entering the loop?

I added it, nothing breaks, so probably none. Thanks.