[kvm-unit-tests PATCH 0/4] s390x: SIE cleanup 2

[kvm-unit-tests PATCH 0/4] s390x: SIE cleanup 2

[Janosch Frank]

Time to deduplicate code before we copy it around even more.

Most problems originated from my desire to rapidly integrate SIE PV
support and as more and more people are using the SIE support it's
time to have a SIE library.

The SIE lib is by no means perfect. For that we might need a few more
iterations but it's good enough that we only need a bit of code in the
tests to get a guest going. This means we have a low entry to test
development which is my main goal.

Janosch Frank (4):
  s390x: sie: Add sie lib validity handling
  s390x: lib: Introduce HPAGE_* constants
  s390x: lib: sie: Add struct vm (de)initialization functions
  lib: s390x: sie: Move sie function into library

 lib/s390x/asm/page.h |  4 +++
 lib/s390x/sie.c      | 84 ++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sie.h      |  7 ++++
 s390x/Makefile       |  1 +
 s390x/mvpg-sie.c     | 31 ++--------------
 s390x/sie.c          | 41 ++-------------------
 6 files changed, 101 insertions(+), 67 deletions(-)
 create mode 100644 lib/s390x/sie.c

-- 
2.30.2

[kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Janosch Frank]

Let's start off the SIE lib with validity handling code since that has
the least amount of dependencies to other files.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sie.h  |  3 +++
 s390x/Makefile   |  1 +
 s390x/mvpg-sie.c |  2 +-
 s390x/sie.c      |  7 +------
 5 files changed, 47 insertions(+), 7 deletions(-)
 create mode 100644 lib/s390x/sie.c

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
new file mode 100644
index 00000000..9107519f
--- /dev/null
+++ b/lib/s390x/sie.c
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Virtualization library that speeds up managing guests.
+ *
+ * Copyright (c) 2021 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <frankja@linux.ibm.com>
+ */
+
+#include <asm/barrier.h>
+#include <libcflat.h>
+#include <sie.h>
+
+static bool validity_expected;
+static uint16_t vir;
+
+void sie_expect_validity(void)
+{
+	validity_expected = true;
+	vir = 0;
+}
+
+void sie_check_validity(uint16_t vir_exp)
+{
+	report(vir_exp == vir, "VALIDITY: %x", vir);
+	mb();
+	vir = 0;
+}
+
+void sie_handle_validity(struct vm *vm)
+{
+	if (vm->sblk->icptcode != ICPT_VALIDITY)
+		return;
+
+	vir = vm->sblk->ipb >> 16;
+
+	if (!validity_expected)
+		report_abort("VALIDITY: %x", vir);
+	validity_expected = false;
+}
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index 6ba858a2..7ff98d2d 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -197,5 +197,8 @@ struct vm {
 extern void sie_entry(void);
 extern void sie_exit(void);
 extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
+void sie_expect_validity(void);
+void sie_check_validity(uint16_t vir_exp);
+void sie_handle_validity(struct vm *vm);
 
 #endif /* _S390X_SIE_H_ */
diff --git a/s390x/Makefile b/s390x/Makefile
index 6565561b..ef8041a6 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -71,6 +71,7 @@ cflatobjs += lib/s390x/css_dump.o
 cflatobjs += lib/s390x/css_lib.o
 cflatobjs += lib/s390x/malloc_io.o
 cflatobjs += lib/s390x/uv.o
+cflatobjs += lib/s390x/sie.o
 
 OBJDIRS += lib/s390x
 
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 5e70f591..2ac91eec 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -39,7 +39,7 @@ static void sie(struct vm *vm)
 
 	while (vm->sblk->icptcode == 0) {
 		sie64a(vm->sblk, &vm->save_area);
-		assert(vm->sblk->icptcode != ICPT_VALIDITY);
+		sie_handle_validity(vm);
 	}
 	vm->save_area.guest.grs[14] = vm->sblk->gg14;
 	vm->save_area.guest.grs[15] = vm->sblk->gg15;
diff --git a/s390x/sie.c b/s390x/sie.c
index 134d3c4f..5c798a9e 100644
--- a/s390x/sie.c
+++ b/s390x/sie.c
@@ -24,17 +24,12 @@ static u8 *guest;
 static u8 *guest_instr;
 static struct vm vm;
 
-static void handle_validity(struct vm *vm)
-{
-	report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
-}
 
 static void sie(struct vm *vm)
 {
 	while (vm->sblk->icptcode == 0) {
 		sie64a(vm->sblk, &vm->save_area);
-		if (vm->sblk->icptcode == ICPT_VALIDITY)
-			handle_validity(vm);
+		sie_handle_validity(vm);
 	}
 	vm->save_area.guest.grs[14] = vm->sblk->gg14;
 	vm->save_area.guest.grs[15] = vm->sblk->gg15;
-- 
2.30.2

[kvm-unit-tests PATCH 2/4] s390x: lib: Introduce HPAGE_* constants

[Janosch Frank]

They come in handy when working with 1MB blocks/addresses.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/page.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/s390x/asm/page.h b/lib/s390x/asm/page.h
index f130f936..2f4afd06 100644
--- a/lib/s390x/asm/page.h
+++ b/lib/s390x/asm/page.h
@@ -35,4 +35,8 @@ typedef struct { pteval_t pte; } pte_t;
 #define __pmd(x)	((pmd_t) { (x) } )
 #define __pte(x)	((pte_t) { (x) } )
 
+#define HPAGE_SHIFT		20
+#define HPAGE_SIZE		(_AC(1,UL) << HPAGE_SHIFT)
+#define HPAGE_MASK		(~(HPAGE_SIZE-1))
+
 #endif
-- 
2.30.2

[kvm-unit-tests PATCH 3/4] s390x: lib: sie: Add struct vm (de)initialization functions

[Janosch Frank]

Before I start copying the same code over and over lets move this into
the library.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sie.c  | 30 ++++++++++++++++++++++++++++++
 lib/s390x/sie.h  |  3 +++
 s390x/mvpg-sie.c | 18 ++----------------
 s390x/sie.c      | 19 +++----------------
 4 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index 9107519f..ec0c4867 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -11,6 +11,9 @@
 #include <asm/barrier.h>
 #include <libcflat.h>
 #include <sie.h>
+#include <asm/page.h>
+#include <libcflat.h>
+#include <alloc_page.h>
 
 static bool validity_expected;
 static uint16_t vir;
@@ -39,3 +42,30 @@ void sie_handle_validity(struct vm *vm)
 		report_abort("VALIDITY: %x", vir);
 	validity_expected = false;
 }
+
+/* Initializes the struct vm members like the SIE control block. */
+void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
+{
+	vm->sblk = alloc_page();
+	memset(vm->sblk, 0, PAGE_SIZE);
+	vm->sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
+	vm->sblk->ihcpu = 0xffff;
+	vm->sblk->prefix = 0;
+
+	/* Guest memory chunks are always 1MB */
+	assert(!(guest_mem_len & ~HPAGE_MASK));
+	/* Currently MSO/MSL is the easiest option */
+	vm->sblk->mso = (uint64_t)guest_mem;
+	vm->sblk->msl = (uint64_t)guest_mem + ((guest_mem_len - 1) & HPAGE_MASK);
+
+	/* CRYCB needs to be in the first 2GB */
+	vm->crycb = alloc_pages_flags(0, AREA_DMA31);
+	vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
+}
+
+/* Frees the memory that was gathered on initialization */
+void sie_guest_destroy(struct vm *vm)
+{
+	free_page(vm->crycb);
+	free_page(vm->sblk);
+}
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index 7ff98d2d..946bd164 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -190,6 +190,7 @@ struct vm_save_area {
 struct vm {
 	struct kvm_s390_sie_block *sblk;
 	struct vm_save_area save_area;
+	uint8_t *crycb;				/* Crypto Control Block */
 	/* Ptr to first guest page */
 	uint8_t *guest_mem;
 };
@@ -200,5 +201,7 @@ extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_ar
 void sie_expect_validity(void);
 void sie_check_validity(uint16_t vir_exp);
 void sie_handle_validity(struct vm *vm);
+void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len);
+void sie_guest_destroy(struct vm *vm);
 
 #endif /* _S390X_SIE_H_ */
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 2ac91eec..71ae4f88 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -110,22 +110,7 @@ static void setup_guest(void)
 	/* The first two pages are the lowcore */
 	guest_instr = guest + PAGE_SIZE * 2;
 
-	vm.sblk = alloc_page();
-
-	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
-	vm.sblk->prefix = 0;
-	/*
-	 * Pageable guest with the same ASCE as the test programm, but
-	 * the guest memory 0x0 is offset to start at the allocated
-	 * guest pages and end after 1MB.
-	 *
-	 * It's not pretty but faster and easier than managing guest ASCEs.
-	 */
-	vm.sblk->mso = (u64)guest;
-	vm.sblk->msl = (u64)guest;
-	vm.sblk->ihcpu = 0xffff;
-
-	vm.sblk->crycbd = (uint64_t)alloc_page();
+	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
 
 	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
 	vm.sblk->gpsw.mask = 0x0000000180000000ULL;
@@ -150,6 +135,7 @@ int main(void)
 	setup_guest();
 	test_mvpg();
 	test_mvpg_pei();
+	sie_guest_destroy(&vm);
 
 done:
 	report_prefix_pop();
diff --git a/s390x/sie.c b/s390x/sie.c
index 5c798a9e..9cb9b055 100644
--- a/s390x/sie.c
+++ b/s390x/sie.c
@@ -84,22 +84,7 @@ static void setup_guest(void)
 	/* The first two pages are the lowcore */
 	guest_instr = guest + PAGE_SIZE * 2;
 
-	vm.sblk = alloc_page();
-
-	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
-	vm.sblk->prefix = 0;
-	/*
-	 * Pageable guest with the same ASCE as the test programm, but
-	 * the guest memory 0x0 is offset to start at the allocated
-	 * guest pages and end after 1MB.
-	 *
-	 * It's not pretty but faster and easier than managing guest ASCEs.
-	 */
-	vm.sblk->mso = (u64)guest;
-	vm.sblk->msl = (u64)guest;
-	vm.sblk->ihcpu = 0xffff;
-
-	vm.sblk->crycbd = (uint64_t)alloc_page();
+	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
 }
 
 int main(void)
@@ -112,6 +97,8 @@ int main(void)
 
 	setup_guest();
 	test_diags();
+	sie_guest_destroy(&vm);
+
 done:
 	report_prefix_pop();
 	return report_summary();
-- 
2.30.2

[kvm-unit-tests PATCH 4/4] lib: s390x: sie: Move sie function into library

[Janosch Frank]

Time to deduplicate more code.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sie.c  | 13 +++++++++++++
 lib/s390x/sie.h  |  1 +
 s390x/mvpg-sie.c | 13 -------------
 s390x/sie.c      | 17 -----------------
 4 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index ec0c4867..d971e825 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -43,6 +43,19 @@ void sie_handle_validity(struct vm *vm)
 	validity_expected = false;
 }
 
+void sie(struct vm *vm)
+{
+	/* Reset icptcode so we don't trip over it below */
+	vm->sblk->icptcode = 0;
+
+	while (vm->sblk->icptcode == 0) {
+		sie64a(vm->sblk, &vm->save_area);
+		sie_handle_validity(vm);
+	}
+	vm->save_area.guest.grs[14] = vm->sblk->gg14;
+	vm->save_area.guest.grs[15] = vm->sblk->gg15;
+}
+
 /* Initializes the struct vm members like the SIE control block. */
 void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
 {
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index 946bd164..ca514ef3 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -198,6 +198,7 @@ struct vm {
 extern void sie_entry(void);
 extern void sie_exit(void);
 extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
+void sie(struct vm *vm);
 void sie_expect_validity(void);
 void sie_check_validity(uint16_t vir_exp);
 void sie_handle_validity(struct vm *vm);
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 71ae4f88..70d2fcfa 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -32,19 +32,6 @@ extern const char _binary_s390x_snippets_c_mvpg_snippet_gbin_start[];
 extern const char _binary_s390x_snippets_c_mvpg_snippet_gbin_end[];
 int binary_size;
 
-static void sie(struct vm *vm)
-{
-	/* Reset icptcode so we don't trip over it below */
-	vm->sblk->icptcode = 0;
-
-	while (vm->sblk->icptcode == 0) {
-		sie64a(vm->sblk, &vm->save_area);
-		sie_handle_validity(vm);
-	}
-	vm->save_area.guest.grs[14] = vm->sblk->gg14;
-	vm->save_area.guest.grs[15] = vm->sblk->gg15;
-}
-
 static void test_mvpg_pei(void)
 {
 	uint64_t **pei_dst = (uint64_t **)((uintptr_t) vm.sblk + 0xc0);
diff --git a/s390x/sie.c b/s390x/sie.c
index 9cb9b055..ed2c3263 100644
--- a/s390x/sie.c
+++ b/s390x/sie.c
@@ -24,22 +24,6 @@ static u8 *guest;
 static u8 *guest_instr;
 static struct vm vm;
 
-
-static void sie(struct vm *vm)
-{
-	while (vm->sblk->icptcode == 0) {
-		sie64a(vm->sblk, &vm->save_area);
-		sie_handle_validity(vm);
-	}
-	vm->save_area.guest.grs[14] = vm->sblk->gg14;
-	vm->save_area.guest.grs[15] = vm->sblk->gg15;
-}
-
-static void sblk_cleanup(struct vm *vm)
-{
-	vm->sblk->icptcode = 0;
-}
-
 static void test_diag(u32 instr)
 {
 	vm.sblk->gpsw.addr = PAGE_SIZE * 2;
@@ -51,7 +35,6 @@ static void test_diag(u32 instr)
 	report(vm.sblk->icptcode == ICPT_INST &&
 	       vm.sblk->ipa == instr >> 16 && vm.sblk->ipb == instr << 16,
 	       "Intercept data");
-	sblk_cleanup(&vm);
 }
 
 static struct {
-- 
2.30.2

Re: [kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Claudio Imbrenda]

On Thu, 29 Jul 2021 13:48:00 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Let's start off the SIE lib with validity handling code since that has
> the least amount of dependencies to other files.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/sie.h  |  3 +++
>  s390x/Makefile   |  1 +
>  s390x/mvpg-sie.c |  2 +-
>  s390x/sie.c      |  7 +------
>  5 files changed, 47 insertions(+), 7 deletions(-)
>  create mode 100644 lib/s390x/sie.c
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> new file mode 100644
> index 00000000..9107519f
> --- /dev/null
> +++ b/lib/s390x/sie.c
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtualization library that speeds up managing guests.
> + *
> + * Copyright (c) 2021 IBM Corp
> + *
> + * Authors:
> + *  Janosch Frank <frankja@linux.ibm.com>
> + */
> +
> +#include <asm/barrier.h>
> +#include <libcflat.h>
> +#include <sie.h>
> +
> +static bool validity_expected;
> +static uint16_t vir;
> +
> +void sie_expect_validity(void)
> +{
> +	validity_expected = true;
> +	vir = 0;
> +}
> +
> +void sie_check_validity(uint16_t vir_exp)
> +{
> +	report(vir_exp == vir, "VALIDITY: %x", vir);
> +	mb();

why the barrier?

> +	vir = 0;
> +}
> +
> +void sie_handle_validity(struct vm *vm)
> +{
> +	if (vm->sblk->icptcode != ICPT_VALIDITY)
> +		return;
> +
> +	vir = vm->sblk->ipb >> 16;
> +
> +	if (!validity_expected)
> +		report_abort("VALIDITY: %x", vir);
> +	validity_expected = false;
> +}
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 6ba858a2..7ff98d2d 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -197,5 +197,8 @@ struct vm {
>  extern void sie_entry(void);
>  extern void sie_exit(void);
>  extern void sie64a(struct kvm_s390_sie_block *sblk, struct
> vm_save_area *save_area); +void sie_expect_validity(void);
> +void sie_check_validity(uint16_t vir_exp);
> +void sie_handle_validity(struct vm *vm);
>  
>  #endif /* _S390X_SIE_H_ */
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 6565561b..ef8041a6 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -71,6 +71,7 @@ cflatobjs += lib/s390x/css_dump.o
>  cflatobjs += lib/s390x/css_lib.o
>  cflatobjs += lib/s390x/malloc_io.o
>  cflatobjs += lib/s390x/uv.o
> +cflatobjs += lib/s390x/sie.o
>  
>  OBJDIRS += lib/s390x
>  
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> index 5e70f591..2ac91eec 100644
> --- a/s390x/mvpg-sie.c
> +++ b/s390x/mvpg-sie.c
> @@ -39,7 +39,7 @@ static void sie(struct vm *vm)
>  
>  	while (vm->sblk->icptcode == 0) {
>  		sie64a(vm->sblk, &vm->save_area);
> -		assert(vm->sblk->icptcode != ICPT_VALIDITY);
> +		sie_handle_validity(vm);
>  	}
>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> diff --git a/s390x/sie.c b/s390x/sie.c
> index 134d3c4f..5c798a9e 100644
> --- a/s390x/sie.c
> +++ b/s390x/sie.c
> @@ -24,17 +24,12 @@ static u8 *guest;
>  static u8 *guest_instr;
>  static struct vm vm;
>  
> -static void handle_validity(struct vm *vm)
> -{
> -	report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
> -}
>  
>  static void sie(struct vm *vm)
>  {
>  	while (vm->sblk->icptcode == 0) {
>  		sie64a(vm->sblk, &vm->save_area);
> -		if (vm->sblk->icptcode == ICPT_VALIDITY)
> -			handle_validity(vm);
> +		sie_handle_validity(vm);
>  	}
>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;

Re: [kvm-unit-tests PATCH 2/4] s390x: lib: Introduce HPAGE_* constants

[Claudio Imbrenda]

On Thu, 29 Jul 2021 13:48:01 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> They come in handy when working with 1MB blocks/addresses.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

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

> ---
>  lib/s390x/asm/page.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/s390x/asm/page.h b/lib/s390x/asm/page.h
> index f130f936..2f4afd06 100644
> --- a/lib/s390x/asm/page.h
> +++ b/lib/s390x/asm/page.h
> @@ -35,4 +35,8 @@ typedef struct { pteval_t pte; } pte_t;
>  #define __pmd(x)	((pmd_t) { (x) } )
>  #define __pte(x)	((pte_t) { (x) } )
>  
> +#define HPAGE_SHIFT		20
> +#define HPAGE_SIZE		(_AC(1,UL) << HPAGE_SHIFT)
> +#define HPAGE_MASK		(~(HPAGE_SIZE-1))
> +
>  #endif

Re: [kvm-unit-tests PATCH 3/4] s390x: lib: sie: Add struct vm (de)initialization functions

[Claudio Imbrenda]

On Thu, 29 Jul 2021 13:48:02 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Before I start copying the same code over and over lets move this into
> the library.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/sie.c  | 30 ++++++++++++++++++++++++++++++
>  lib/s390x/sie.h  |  3 +++
>  s390x/mvpg-sie.c | 18 ++----------------
>  s390x/sie.c      | 19 +++----------------
>  4 files changed, 38 insertions(+), 32 deletions(-)
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index 9107519f..ec0c4867 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -11,6 +11,9 @@
>  #include <asm/barrier.h>
>  #include <libcflat.h>
>  #include <sie.h>
> +#include <asm/page.h>
> +#include <libcflat.h>
> +#include <alloc_page.h>
>  
>  static bool validity_expected;
>  static uint16_t vir;
> @@ -39,3 +42,30 @@ void sie_handle_validity(struct vm *vm)
>  		report_abort("VALIDITY: %x", vir);
>  	validity_expected = false;
>  }
> +
> +/* Initializes the struct vm members like the SIE control block. */
> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
> guest_mem_len) +{
> +	vm->sblk = alloc_page();
> +	memset(vm->sblk, 0, PAGE_SIZE);

you can skip the memset, the page allocator always zeroes the page,
unless you explicitly pass FLAG_DONTZERO

regardless of that:

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

> +	vm->sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
> +	vm->sblk->ihcpu = 0xffff;
> +	vm->sblk->prefix = 0;
> +
> +	/* Guest memory chunks are always 1MB */
> +	assert(!(guest_mem_len & ~HPAGE_MASK));
> +	/* Currently MSO/MSL is the easiest option */
> +	vm->sblk->mso = (uint64_t)guest_mem;
> +	vm->sblk->msl = (uint64_t)guest_mem + ((guest_mem_len - 1) &
> HPAGE_MASK); +
> +	/* CRYCB needs to be in the first 2GB */
> +	vm->crycb = alloc_pages_flags(0, AREA_DMA31);
> +	vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
> +}
> +
> +/* Frees the memory that was gathered on initialization */
> +void sie_guest_destroy(struct vm *vm)
> +{
> +	free_page(vm->crycb);
> +	free_page(vm->sblk);
> +}
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 7ff98d2d..946bd164 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -190,6 +190,7 @@ struct vm_save_area {
>  struct vm {
>  	struct kvm_s390_sie_block *sblk;
>  	struct vm_save_area save_area;
> +	uint8_t *crycb;				/* Crypto
> Control Block */ /* Ptr to first guest page */
>  	uint8_t *guest_mem;
>  };
> @@ -200,5 +201,7 @@ extern void sie64a(struct kvm_s390_sie_block
> *sblk, struct vm_save_area *save_ar void sie_expect_validity(void);
>  void sie_check_validity(uint16_t vir_exp);
>  void sie_handle_validity(struct vm *vm);
> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
> guest_mem_len); +void sie_guest_destroy(struct vm *vm);
>  
>  #endif /* _S390X_SIE_H_ */
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> index 2ac91eec..71ae4f88 100644
> --- a/s390x/mvpg-sie.c
> +++ b/s390x/mvpg-sie.c
> @@ -110,22 +110,7 @@ static void setup_guest(void)
>  	/* The first two pages are the lowcore */
>  	guest_instr = guest + PAGE_SIZE * 2;
>  
> -	vm.sblk = alloc_page();
> -
> -	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
> -	vm.sblk->prefix = 0;
> -	/*
> -	 * Pageable guest with the same ASCE as the test programm,
> but
> -	 * the guest memory 0x0 is offset to start at the allocated
> -	 * guest pages and end after 1MB.
> -	 *
> -	 * It's not pretty but faster and easier than managing guest
> ASCEs.
> -	 */
> -	vm.sblk->mso = (u64)guest;
> -	vm.sblk->msl = (u64)guest;
> -	vm.sblk->ihcpu = 0xffff;
> -
> -	vm.sblk->crycbd = (uint64_t)alloc_page();
> +	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>  
>  	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
>  	vm.sblk->gpsw.mask = 0x0000000180000000ULL;
> @@ -150,6 +135,7 @@ int main(void)
>  	setup_guest();
>  	test_mvpg();
>  	test_mvpg_pei();
> +	sie_guest_destroy(&vm);
>  
>  done:
>  	report_prefix_pop();
> diff --git a/s390x/sie.c b/s390x/sie.c
> index 5c798a9e..9cb9b055 100644
> --- a/s390x/sie.c
> +++ b/s390x/sie.c
> @@ -84,22 +84,7 @@ static void setup_guest(void)
>  	/* The first two pages are the lowcore */
>  	guest_instr = guest + PAGE_SIZE * 2;
>  
> -	vm.sblk = alloc_page();
> -
> -	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
> -	vm.sblk->prefix = 0;
> -	/*
> -	 * Pageable guest with the same ASCE as the test programm,
> but
> -	 * the guest memory 0x0 is offset to start at the allocated
> -	 * guest pages and end after 1MB.
> -	 *
> -	 * It's not pretty but faster and easier than managing guest
> ASCEs.
> -	 */
> -	vm.sblk->mso = (u64)guest;
> -	vm.sblk->msl = (u64)guest;
> -	vm.sblk->ihcpu = 0xffff;
> -
> -	vm.sblk->crycbd = (uint64_t)alloc_page();
> +	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>  }
>  
>  int main(void)
> @@ -112,6 +97,8 @@ int main(void)
>  
>  	setup_guest();
>  	test_diags();
> +	sie_guest_destroy(&vm);
> +
>  done:
>  	report_prefix_pop();
>  	return report_summary();

Re: [kvm-unit-tests PATCH 4/4] lib: s390x: sie: Move sie function into library

[Claudio Imbrenda]

On Thu, 29 Jul 2021 13:48:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Time to deduplicate more code.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

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

> ---
>  lib/s390x/sie.c  | 13 +++++++++++++
>  lib/s390x/sie.h  |  1 +
>  s390x/mvpg-sie.c | 13 -------------
>  s390x/sie.c      | 17 -----------------
>  4 files changed, 14 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index ec0c4867..d971e825 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -43,6 +43,19 @@ void sie_handle_validity(struct vm *vm)
>  	validity_expected = false;
>  }
>  
> +void sie(struct vm *vm)
> +{
> +	/* Reset icptcode so we don't trip over it below */
> +	vm->sblk->icptcode = 0;
> +
> +	while (vm->sblk->icptcode == 0) {
> +		sie64a(vm->sblk, &vm->save_area);
> +		sie_handle_validity(vm);
> +	}
> +	vm->save_area.guest.grs[14] = vm->sblk->gg14;
> +	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> +}
> +
>  /* Initializes the struct vm members like the SIE control block. */
>  void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
> guest_mem_len) {
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 946bd164..ca514ef3 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -198,6 +198,7 @@ struct vm {
>  extern void sie_entry(void);
>  extern void sie_exit(void);
>  extern void sie64a(struct kvm_s390_sie_block *sblk, struct
> vm_save_area *save_area); +void sie(struct vm *vm);
>  void sie_expect_validity(void);
>  void sie_check_validity(uint16_t vir_exp);
>  void sie_handle_validity(struct vm *vm);
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> index 71ae4f88..70d2fcfa 100644
> --- a/s390x/mvpg-sie.c
> +++ b/s390x/mvpg-sie.c
> @@ -32,19 +32,6 @@ extern const char
> _binary_s390x_snippets_c_mvpg_snippet_gbin_start[]; extern const char
> _binary_s390x_snippets_c_mvpg_snippet_gbin_end[]; int binary_size;
>  
> -static void sie(struct vm *vm)
> -{
> -	/* Reset icptcode so we don't trip over it below */
> -	vm->sblk->icptcode = 0;
> -
> -	while (vm->sblk->icptcode == 0) {
> -		sie64a(vm->sblk, &vm->save_area);
> -		sie_handle_validity(vm);
> -	}
> -	vm->save_area.guest.grs[14] = vm->sblk->gg14;
> -	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> -}
> -
>  static void test_mvpg_pei(void)
>  {
>  	uint64_t **pei_dst = (uint64_t **)((uintptr_t) vm.sblk +
> 0xc0); diff --git a/s390x/sie.c b/s390x/sie.c
> index 9cb9b055..ed2c3263 100644
> --- a/s390x/sie.c
> +++ b/s390x/sie.c
> @@ -24,22 +24,6 @@ static u8 *guest;
>  static u8 *guest_instr;
>  static struct vm vm;
>  
> -
> -static void sie(struct vm *vm)
> -{
> -	while (vm->sblk->icptcode == 0) {
> -		sie64a(vm->sblk, &vm->save_area);
> -		sie_handle_validity(vm);
> -	}
> -	vm->save_area.guest.grs[14] = vm->sblk->gg14;
> -	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> -}
> -
> -static void sblk_cleanup(struct vm *vm)
> -{
> -	vm->sblk->icptcode = 0;
> -}
> -
>  static void test_diag(u32 instr)
>  {
>  	vm.sblk->gpsw.addr = PAGE_SIZE * 2;
> @@ -51,7 +35,6 @@ static void test_diag(u32 instr)
>  	report(vm.sblk->icptcode == ICPT_INST &&
>  	       vm.sblk->ipa == instr >> 16 && vm.sblk->ipb == instr
> << 16, "Intercept data");
> -	sblk_cleanup(&vm);
>  }
>  
>  static struct {

Re: [kvm-unit-tests PATCH 3/4] s390x: lib: sie: Add struct vm (de)initialization functions

[Janosch Frank]

On 7/29/21 4:21 PM, Claudio Imbrenda wrote:
> On Thu, 29 Jul 2021 13:48:02 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Before I start copying the same code over and over lets move this into
>> the library.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  lib/s390x/sie.c  | 30 ++++++++++++++++++++++++++++++
>>  lib/s390x/sie.h  |  3 +++
>>  s390x/mvpg-sie.c | 18 ++----------------
>>  s390x/sie.c      | 19 +++----------------
>>  4 files changed, 38 insertions(+), 32 deletions(-)
>>
>> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
>> index 9107519f..ec0c4867 100644
>> --- a/lib/s390x/sie.c
>> +++ b/lib/s390x/sie.c
>> @@ -11,6 +11,9 @@
>>  #include <asm/barrier.h>
>>  #include <libcflat.h>
>>  #include <sie.h>
>> +#include <asm/page.h>
>> +#include <libcflat.h>
>> +#include <alloc_page.h>
>>  
>>  static bool validity_expected;
>>  static uint16_t vir;
>> @@ -39,3 +42,30 @@ void sie_handle_validity(struct vm *vm)
>>  		report_abort("VALIDITY: %x", vir);
>>  	validity_expected = false;
>>  }
>> +
>> +/* Initializes the struct vm members like the SIE control block. */
>> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
>> guest_mem_len) +{
>> +	vm->sblk = alloc_page();
>> +	memset(vm->sblk, 0, PAGE_SIZE);
> 
> you can skip the memset, the page allocator always zeroes the page,
> unless you explicitly pass FLAG_DONTZERO

Yeah I thought as much but I still like to have that explicitly for ease
of mind.

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

Thanks

> 
>> +	vm->sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> +	vm->sblk->ihcpu = 0xffff;
>> +	vm->sblk->prefix = 0;
>> +
>> +	/* Guest memory chunks are always 1MB */
>> +	assert(!(guest_mem_len & ~HPAGE_MASK));
>> +	/* Currently MSO/MSL is the easiest option */
>> +	vm->sblk->mso = (uint64_t)guest_mem;
>> +	vm->sblk->msl = (uint64_t)guest_mem + ((guest_mem_len - 1) &
>> HPAGE_MASK); +
>> +	/* CRYCB needs to be in the first 2GB */
>> +	vm->crycb = alloc_pages_flags(0, AREA_DMA31);
>> +	vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
>> +}
>> +
>> +/* Frees the memory that was gathered on initialization */
>> +void sie_guest_destroy(struct vm *vm)
>> +{
>> +	free_page(vm->crycb);
>> +	free_page(vm->sblk);
>> +}
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index 7ff98d2d..946bd164 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -190,6 +190,7 @@ struct vm_save_area {
>>  struct vm {
>>  	struct kvm_s390_sie_block *sblk;
>>  	struct vm_save_area save_area;
>> +	uint8_t *crycb;				/* Crypto
>> Control Block */ /* Ptr to first guest page */
>>  	uint8_t *guest_mem;
>>  };
>> @@ -200,5 +201,7 @@ extern void sie64a(struct kvm_s390_sie_block
>> *sblk, struct vm_save_area *save_ar void sie_expect_validity(void);
>>  void sie_check_validity(uint16_t vir_exp);
>>  void sie_handle_validity(struct vm *vm);
>> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
>> guest_mem_len); +void sie_guest_destroy(struct vm *vm);
>>  
>>  #endif /* _S390X_SIE_H_ */
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> index 2ac91eec..71ae4f88 100644
>> --- a/s390x/mvpg-sie.c
>> +++ b/s390x/mvpg-sie.c
>> @@ -110,22 +110,7 @@ static void setup_guest(void)
>>  	/* The first two pages are the lowcore */
>>  	guest_instr = guest + PAGE_SIZE * 2;
>>  
>> -	vm.sblk = alloc_page();
>> -
>> -	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> -	vm.sblk->prefix = 0;
>> -	/*
>> -	 * Pageable guest with the same ASCE as the test programm,
>> but
>> -	 * the guest memory 0x0 is offset to start at the allocated
>> -	 * guest pages and end after 1MB.
>> -	 *
>> -	 * It's not pretty but faster and easier than managing guest
>> ASCEs.
>> -	 */
>> -	vm.sblk->mso = (u64)guest;
>> -	vm.sblk->msl = (u64)guest;
>> -	vm.sblk->ihcpu = 0xffff;
>> -
>> -	vm.sblk->crycbd = (uint64_t)alloc_page();
>> +	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>>  
>>  	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
>>  	vm.sblk->gpsw.mask = 0x0000000180000000ULL;
>> @@ -150,6 +135,7 @@ int main(void)
>>  	setup_guest();
>>  	test_mvpg();
>>  	test_mvpg_pei();
>> +	sie_guest_destroy(&vm);
>>  
>>  done:
>>  	report_prefix_pop();
>> diff --git a/s390x/sie.c b/s390x/sie.c
>> index 5c798a9e..9cb9b055 100644
>> --- a/s390x/sie.c
>> +++ b/s390x/sie.c
>> @@ -84,22 +84,7 @@ static void setup_guest(void)
>>  	/* The first two pages are the lowcore */
>>  	guest_instr = guest + PAGE_SIZE * 2;
>>  
>> -	vm.sblk = alloc_page();
>> -
>> -	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> -	vm.sblk->prefix = 0;
>> -	/*
>> -	 * Pageable guest with the same ASCE as the test programm,
>> but
>> -	 * the guest memory 0x0 is offset to start at the allocated
>> -	 * guest pages and end after 1MB.
>> -	 *
>> -	 * It's not pretty but faster and easier than managing guest
>> ASCEs.
>> -	 */
>> -	vm.sblk->mso = (u64)guest;
>> -	vm.sblk->msl = (u64)guest;
>> -	vm.sblk->ihcpu = 0xffff;
>> -
>> -	vm.sblk->crycbd = (uint64_t)alloc_page();
>> +	sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>>  }
>>  
>>  int main(void)
>> @@ -112,6 +97,8 @@ int main(void)
>>  
>>  	setup_guest();
>>  	test_diags();
>> +	sie_guest_destroy(&vm);
>> +
>>  done:
>>  	report_prefix_pop();
>>  	return report_summary();
> 

Re: [kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Janosch Frank]

On 7/29/21 4:11 PM, Claudio Imbrenda wrote:
> On Thu, 29 Jul 2021 13:48:00 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Let's start off the SIE lib with validity handling code since that has
>> the least amount of dependencies to other files.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>  lib/s390x/sie.h  |  3 +++
>>  s390x/Makefile   |  1 +
>>  s390x/mvpg-sie.c |  2 +-
>>  s390x/sie.c      |  7 +------
>>  5 files changed, 47 insertions(+), 7 deletions(-)
>>  create mode 100644 lib/s390x/sie.c
>>
>> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
>> new file mode 100644
>> index 00000000..9107519f
>> --- /dev/null
>> +++ b/lib/s390x/sie.c
>> @@ -0,0 +1,41 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Virtualization library that speeds up managing guests.
>> + *
>> + * Copyright (c) 2021 IBM Corp
>> + *
>> + * Authors:
>> + *  Janosch Frank <frankja@linux.ibm.com>
>> + */
>> +
>> +#include <asm/barrier.h>
>> +#include <libcflat.h>
>> +#include <sie.h>
>> +
>> +static bool validity_expected;
>> +static uint16_t vir;
>> +
>> +void sie_expect_validity(void)
>> +{
>> +	validity_expected = true;
>> +	vir = 0;
>> +}
>> +
>> +void sie_check_validity(uint16_t vir_exp)
>> +{
>> +	report(vir_exp == vir, "VALIDITY: %x", vir);
>> +	mb();
> 
> why the barrier?

That's left over, I'll remove it.

> 
>> +	vir = 0;
>> +}
>> +
>> +void sie_handle_validity(struct vm *vm)
>> +{
>> +	if (vm->sblk->icptcode != ICPT_VALIDITY)
>> +		return;
>> +
>> +	vir = vm->sblk->ipb >> 16;
>> +
>> +	if (!validity_expected)
>> +		report_abort("VALIDITY: %x", vir);
>> +	validity_expected = false;
>> +}
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index 6ba858a2..7ff98d2d 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -197,5 +197,8 @@ struct vm {
>>  extern void sie_entry(void);
>>  extern void sie_exit(void);
>>  extern void sie64a(struct kvm_s390_sie_block *sblk, struct
>> vm_save_area *save_area); +void sie_expect_validity(void);
>> +void sie_check_validity(uint16_t vir_exp);
>> +void sie_handle_validity(struct vm *vm);
>>  
>>  #endif /* _S390X_SIE_H_ */
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 6565561b..ef8041a6 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -71,6 +71,7 @@ cflatobjs += lib/s390x/css_dump.o
>>  cflatobjs += lib/s390x/css_lib.o
>>  cflatobjs += lib/s390x/malloc_io.o
>>  cflatobjs += lib/s390x/uv.o
>> +cflatobjs += lib/s390x/sie.o
>>  
>>  OBJDIRS += lib/s390x
>>  
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> index 5e70f591..2ac91eec 100644
>> --- a/s390x/mvpg-sie.c
>> +++ b/s390x/mvpg-sie.c
>> @@ -39,7 +39,7 @@ static void sie(struct vm *vm)
>>  
>>  	while (vm->sblk->icptcode == 0) {
>>  		sie64a(vm->sblk, &vm->save_area);
>> -		assert(vm->sblk->icptcode != ICPT_VALIDITY);
>> +		sie_handle_validity(vm);
>>  	}
>>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
>>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;
>> diff --git a/s390x/sie.c b/s390x/sie.c
>> index 134d3c4f..5c798a9e 100644
>> --- a/s390x/sie.c
>> +++ b/s390x/sie.c
>> @@ -24,17 +24,12 @@ static u8 *guest;
>>  static u8 *guest_instr;
>>  static struct vm vm;
>>  
>> -static void handle_validity(struct vm *vm)
>> -{
>> -	report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
>> -}
>>  
>>  static void sie(struct vm *vm)
>>  {
>>  	while (vm->sblk->icptcode == 0) {
>>  		sie64a(vm->sblk, &vm->save_area);
>> -		if (vm->sblk->icptcode == ICPT_VALIDITY)
>> -			handle_validity(vm);
>> +		sie_handle_validity(vm);
>>  	}
>>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
>>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> 

Re: [kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Claudio Imbrenda]

On Thu, 29 Jul 2021 16:33:31 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 7/29/21 4:11 PM, Claudio Imbrenda wrote:
> > On Thu, 29 Jul 2021 13:48:00 +0000
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Let's start off the SIE lib with validity handling code since that
> >> has the least amount of dependencies to other files.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>  lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
> >>  lib/s390x/sie.h  |  3 +++
> >>  s390x/Makefile   |  1 +
> >>  s390x/mvpg-sie.c |  2 +-
> >>  s390x/sie.c      |  7 +------
> >>  5 files changed, 47 insertions(+), 7 deletions(-)
> >>  create mode 100644 lib/s390x/sie.c
> >>
> >> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> >> new file mode 100644
> >> index 00000000..9107519f
> >> --- /dev/null
> >> +++ b/lib/s390x/sie.c
> >> @@ -0,0 +1,41 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only */
> >> +/*
> >> + * Virtualization library that speeds up managing guests.
> >> + *
> >> + * Copyright (c) 2021 IBM Corp
> >> + *
> >> + * Authors:
> >> + *  Janosch Frank <frankja@linux.ibm.com>
> >> + */
> >> +
> >> +#include <asm/barrier.h>
> >> +#include <libcflat.h>
> >> +#include <sie.h>
> >> +
> >> +static bool validity_expected;
> >> +static uint16_t vir;
> >> +
> >> +void sie_expect_validity(void)
> >> +{
> >> +	validity_expected = true;
> >> +	vir = 0;
> >> +}
> >> +
> >> +void sie_check_validity(uint16_t vir_exp)
> >> +{
> >> +	report(vir_exp == vir, "VALIDITY: %x", vir);
> >> +	mb();  
> > 
> > why the barrier?  
> 
> That's left over, I'll remove it.

with the barrier removed:

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

> >   
> >> +	vir = 0;
> >> +}
> >> +
> >> +void sie_handle_validity(struct vm *vm)
> >> +{
> >> +	if (vm->sblk->icptcode != ICPT_VALIDITY)
> >> +		return;
> >> +
> >> +	vir = vm->sblk->ipb >> 16;
> >> +
> >> +	if (!validity_expected)
> >> +		report_abort("VALIDITY: %x", vir);
> >> +	validity_expected = false;
> >> +}
> >> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> >> index 6ba858a2..7ff98d2d 100644
> >> --- a/lib/s390x/sie.h
> >> +++ b/lib/s390x/sie.h
> >> @@ -197,5 +197,8 @@ struct vm {
> >>  extern void sie_entry(void);
> >>  extern void sie_exit(void);
> >>  extern void sie64a(struct kvm_s390_sie_block *sblk, struct
> >> vm_save_area *save_area); +void sie_expect_validity(void);
> >> +void sie_check_validity(uint16_t vir_exp);
> >> +void sie_handle_validity(struct vm *vm);
> >>  
> >>  #endif /* _S390X_SIE_H_ */
> >> diff --git a/s390x/Makefile b/s390x/Makefile
> >> index 6565561b..ef8041a6 100644
> >> --- a/s390x/Makefile
> >> +++ b/s390x/Makefile
> >> @@ -71,6 +71,7 @@ cflatobjs += lib/s390x/css_dump.o
> >>  cflatobjs += lib/s390x/css_lib.o
> >>  cflatobjs += lib/s390x/malloc_io.o
> >>  cflatobjs += lib/s390x/uv.o
> >> +cflatobjs += lib/s390x/sie.o
> >>  
> >>  OBJDIRS += lib/s390x
> >>  
> >> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> >> index 5e70f591..2ac91eec 100644
> >> --- a/s390x/mvpg-sie.c
> >> +++ b/s390x/mvpg-sie.c
> >> @@ -39,7 +39,7 @@ static void sie(struct vm *vm)
> >>  
> >>  	while (vm->sblk->icptcode == 0) {
> >>  		sie64a(vm->sblk, &vm->save_area);
> >> -		assert(vm->sblk->icptcode != ICPT_VALIDITY);
> >> +		sie_handle_validity(vm);
> >>  	}
> >>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
> >>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;
> >> diff --git a/s390x/sie.c b/s390x/sie.c
> >> index 134d3c4f..5c798a9e 100644
> >> --- a/s390x/sie.c
> >> +++ b/s390x/sie.c
> >> @@ -24,17 +24,12 @@ static u8 *guest;
> >>  static u8 *guest_instr;
> >>  static struct vm vm;
> >>  
> >> -static void handle_validity(struct vm *vm)
> >> -{
> >> -	report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
> >> -}
> >>  
> >>  static void sie(struct vm *vm)
> >>  {
> >>  	while (vm->sblk->icptcode == 0) {
> >>  		sie64a(vm->sblk, &vm->save_area);
> >> -		if (vm->sblk->icptcode == ICPT_VALIDITY)
> >> -			handle_validity(vm);
> >> +		sie_handle_validity(vm);
> >>  	}
> >>  	vm->save_area.guest.grs[14] = vm->sblk->gg14;
> >>  	vm->save_area.guest.grs[15] = vm->sblk->gg15;  
> >   
> 

Re: [kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Thomas Huth]

On 29/07/2021 15.48, Janosch Frank wrote:
> Let's start off the SIE lib with validity handling code since that has
> the least amount of dependencies to other files.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>   lib/s390x/sie.h  |  3 +++
>   s390x/Makefile   |  1 +
>   s390x/mvpg-sie.c |  2 +-
>   s390x/sie.c      |  7 +------
>   5 files changed, 47 insertions(+), 7 deletions(-)
>   create mode 100644 lib/s390x/sie.c
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> new file mode 100644
> index 00000000..9107519f
> --- /dev/null
> +++ b/lib/s390x/sie.c
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtualization library that speeds up managing guests.

"speeds up managing guests" ... so this means that guests can be scheduled 
faster? ;-)

> + * Copyright (c) 2021 IBM Corp
> + *
> + * Authors:
> + *  Janosch Frank <frankja@linux.ibm.com>
> + */
> +
> +#include <asm/barrier.h>
> +#include <libcflat.h>
> +#include <sie.h>
> +
> +static bool validity_expected;
> +static uint16_t vir;

What does "vir" stand for? A short comment would be nice.

  Thomas

Re: [kvm-unit-tests PATCH 2/4] s390x: lib: Introduce HPAGE_* constants

[Thomas Huth]

On 29/07/2021 15.48, Janosch Frank wrote:
> They come in handy when working with 1MB blocks/addresses.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/asm/page.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/lib/s390x/asm/page.h b/lib/s390x/asm/page.h
> index f130f936..2f4afd06 100644
> --- a/lib/s390x/asm/page.h
> +++ b/lib/s390x/asm/page.h
> @@ -35,4 +35,8 @@ typedef struct { pteval_t pte; } pte_t;
>   #define __pmd(x)	((pmd_t) { (x) } )
>   #define __pte(x)	((pte_t) { (x) } )
>   
> +#define HPAGE_SHIFT		20
> +#define HPAGE_SIZE		(_AC(1,UL) << HPAGE_SHIFT)
> +#define HPAGE_MASK		(~(HPAGE_SIZE-1))
> +
>   #endif
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 3/4] s390x: lib: sie: Add struct vm (de)initialization functions

[Thomas Huth]

On 29/07/2021 15.48, Janosch Frank wrote:
> Before I start copying the same code over and over lets move this into
> the library.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/sie.c  | 30 ++++++++++++++++++++++++++++++
>   lib/s390x/sie.h  |  3 +++
>   s390x/mvpg-sie.c | 18 ++----------------
>   s390x/sie.c      | 19 +++----------------
>   4 files changed, 38 insertions(+), 32 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 4/4] lib: s390x: sie: Move sie function into library

[Thomas Huth]

On 29/07/2021 15.48, Janosch Frank wrote:
> Time to deduplicate more code.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/sie.c  | 13 +++++++++++++
>   lib/s390x/sie.h  |  1 +
>   s390x/mvpg-sie.c | 13 -------------
>   s390x/sie.c      | 17 -----------------
>   4 files changed, 14 insertions(+), 30 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 1/4] s390x: sie: Add sie lib validity handling

[Janosch Frank]

On 7/30/21 4:45 PM, Thomas Huth wrote:
> On 29/07/2021 15.48, Janosch Frank wrote:
>> Let's start off the SIE lib with validity handling code since that has
>> the least amount of dependencies to other files.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/sie.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/sie.h  |  3 +++
>>   s390x/Makefile   |  1 +
>>   s390x/mvpg-sie.c |  2 +-
>>   s390x/sie.c      |  7 +------
>>   5 files changed, 47 insertions(+), 7 deletions(-)
>>   create mode 100644 lib/s390x/sie.c
>>
>> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
>> new file mode 100644
>> index 00000000..9107519f
>> --- /dev/null
>> +++ b/lib/s390x/sie.c
>> @@ -0,0 +1,41 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Virtualization library that speeds up managing guests.
> 
> "speeds up managing guests" ... so this means that guests can be scheduled 
> faster? ;-)

Maybe once I implement scheduling which is not on my current plan.

> 
>> + * Copyright (c) 2021 IBM Corp
>> + *
>> + * Authors:
>> + *  Janosch Frank <frankja@linux.ibm.com>
>> + */
>> +
>> +#include <asm/barrier.h>
>> +#include <libcflat.h>
>> +#include <sie.h>
>> +
>> +static bool validity_expected;
>> +static uint16_t vir;
> 
> What does "vir" stand for? A short comment would be nice.

So apparently it's called "Validity-Interception Reason" in the
specification which makes sense in the context it's used :)

I'll add a few words.

> 
>   Thomas
> 

[kvm-unit-tests PATCH v2] s390x: sie: Add sie lib validity handling

[Janosch Frank]

Let's start off the SIE lib with validity handling code since that has
the least amount of dependencies to other files.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 lib/s390x/sie.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sie.h  |  3 +++
 s390x/Makefile   |  1 +
 s390x/mvpg-sie.c |  2 +-
 s390x/sie.c      |  7 +------
 5 files changed, 46 insertions(+), 7 deletions(-)
 create mode 100644 lib/s390x/sie.c

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
new file mode 100644
index 00000000..15ba407c
--- /dev/null
+++ b/lib/s390x/sie.c
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Virtualization library that speeds up managing guests.
+ *
+ * Copyright (c) 2021 IBM Corp
+ *
+ * Authors:
+ *  Janosch Frank <frankja@linux.ibm.com>
+ */
+
+#include <asm/barrier.h>
+#include <libcflat.h>
+#include <sie.h>
+
+static bool validity_expected;
+static uint16_t vir;		/* Validity interception reason */
+
+void sie_expect_validity(void)
+{
+	validity_expected = true;
+	vir = 0;
+}
+
+void sie_check_validity(uint16_t vir_exp)
+{
+	report(vir_exp == vir, "VALIDITY: %x", vir);
+	vir = 0;
+}
+
+void sie_handle_validity(struct vm *vm)
+{
+	if (vm->sblk->icptcode != ICPT_VALIDITY)
+		return;
+
+	vir = vm->sblk->ipb >> 16;
+
+	if (!validity_expected)
+		report_abort("VALIDITY: %x", vir);
+	validity_expected = false;
+}
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index 6ba858a2..7ff98d2d 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -197,5 +197,8 @@ struct vm {
 extern void sie_entry(void);
 extern void sie_exit(void);
 extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
+void sie_expect_validity(void);
+void sie_check_validity(uint16_t vir_exp);
+void sie_handle_validity(struct vm *vm);
 
 #endif /* _S390X_SIE_H_ */
diff --git a/s390x/Makefile b/s390x/Makefile
index 6565561b..ef8041a6 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -71,6 +71,7 @@ cflatobjs += lib/s390x/css_dump.o
 cflatobjs += lib/s390x/css_lib.o
 cflatobjs += lib/s390x/malloc_io.o
 cflatobjs += lib/s390x/uv.o
+cflatobjs += lib/s390x/sie.o
 
 OBJDIRS += lib/s390x
 
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 5e70f591..2ac91eec 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -39,7 +39,7 @@ static void sie(struct vm *vm)
 
 	while (vm->sblk->icptcode == 0) {
 		sie64a(vm->sblk, &vm->save_area);
-		assert(vm->sblk->icptcode != ICPT_VALIDITY);
+		sie_handle_validity(vm);
 	}
 	vm->save_area.guest.grs[14] = vm->sblk->gg14;
 	vm->save_area.guest.grs[15] = vm->sblk->gg15;
diff --git a/s390x/sie.c b/s390x/sie.c
index 134d3c4f..5c798a9e 100644
--- a/s390x/sie.c
+++ b/s390x/sie.c
@@ -24,17 +24,12 @@ static u8 *guest;
 static u8 *guest_instr;
 static struct vm vm;
 
-static void handle_validity(struct vm *vm)
-{
-	report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
-}
 
 static void sie(struct vm *vm)
 {
 	while (vm->sblk->icptcode == 0) {
 		sie64a(vm->sblk, &vm->save_area);
-		if (vm->sblk->icptcode == ICPT_VALIDITY)
-			handle_validity(vm);
+		sie_handle_validity(vm);
 	}
 	vm->save_area.guest.grs[14] = vm->sblk->gg14;
 	vm->save_area.guest.grs[15] = vm->sblk->gg15;
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2] s390x: sie: Add sie lib validity handling

[Thomas Huth]

On 03/08/2021 15.57, Janosch Frank wrote:
> Let's start off the SIE lib with validity handling code since that has
> the least amount of dependencies to other files.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   lib/s390x/sie.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>   lib/s390x/sie.h  |  3 +++
>   s390x/Makefile   |  1 +
>   s390x/mvpg-sie.c |  2 +-
>   s390x/sie.c      |  7 +------
>   5 files changed, 46 insertions(+), 7 deletions(-)
>   create mode 100644 lib/s390x/sie.c
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> new file mode 100644
> index 00000000..15ba407c
> --- /dev/null
> +++ b/lib/s390x/sie.c
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtualization library that speeds up managing guests.

Maybe something like:

Library for managing various aspects of (nested) guests

?

Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>