[kvm-unit-tests PATCH v2 0/3] s390x: Align SIE tests to 2GB

[kvm-unit-tests PATCH v2 0/3] s390x: Align SIE tests to 2GB

[Nico Boehr]

v2:
---
* reverse xmas (thanks Claudio)
* kerneldoc (thanks Claudio)
* fix a virtual-physical confusion in mvpg-sie
* pick up Janoschs r-bs

Some environments on s390x require guests to be aligned to 2GB. This is a
problem when kvm-unit-tests act as a hypervisor, since guests run with MSO/MSL
there and guest memory doesn't satisfy this requirement.

This series ensures kvm-unit-tests fulfills the 2G alignment requirement.

Note that these changes make mvpg-sie test case fail under KVM due to a bug in
prefix handling, a fix is was submitted here:
https://lore.kernel.org/kvm/20231102153549.53984-1-imbrenda@linux.ibm.com/

Nico Boehr (3):
  s390x: spec_ex-sie: refactor to use snippet API
  s390x: sie: ensure guests are aligned to 2GB
  s390x: mvpg-sie: fix virtual-physical address confusion

 lib/s390x/mmu.h     |  2 ++
 lib/s390x/sie.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sie.h     |  2 ++
 lib/s390x/snippet.h |  9 +++------
 s390x/mvpg-sie.c    | 15 +++++++++++----
 s390x/sie.c         |  4 ++--
 s390x/spec_ex-sie.c | 13 ++++++-------
 7 files changed, 68 insertions(+), 19 deletions(-)

-- 
2.41.0

[kvm-unit-tests PATCH v2 1/3] s390x: spec_ex-sie: refactor to use snippet API

[Nico Boehr]

spec_ex-sie uses a snippet, but allocated the memory on its own without
obvious reason to do so.

Refactor the test to use snippet_init().

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/spec_ex-sie.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
index 5fa135b81c86..fe2f23ee3d84 100644
--- a/s390x/spec_ex-sie.c
+++ b/s390x/spec_ex-sie.c
@@ -17,19 +17,18 @@
 #include <hardware.h>
 
 static struct vm vm;
-extern const char SNIPPET_NAME_START(c, spec_ex)[];
-extern const char SNIPPET_NAME_END(c, spec_ex)[];
 static bool strict;
 
 static void setup_guest(void)
 {
-	char *guest;
-	int binary_size = SNIPPET_LEN(c, spec_ex);
+	extern const char SNIPPET_NAME_START(c, spec_ex)[];
+	extern const char SNIPPET_NAME_END(c, spec_ex)[];
 
 	setup_vm();
-	guest = alloc_pages(8);
-	memcpy(guest, SNIPPET_NAME_START(c, spec_ex), binary_size);
-	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
+
+	snippet_setup_guest(&vm, false);
+	snippet_init(&vm, SNIPPET_NAME_START(c, spec_ex),
+		     SNIPPET_LEN(c, spec_ex), SNIPPET_UNPACK_OFF);
 }
 
 static void reset_guest(void)
-- 
2.41.0

[kvm-unit-tests PATCH v2 2/3] s390x: sie: ensure guests are aligned to 2GB

[Nico Boehr]

Until now, kvm-unit-tests has aligned guests to 1 MB in the host virtual
address space. Unfortunately, some s390x environments require guests to
be 2GB aligned in the host virtual address space, preventing
kvm-unit-tests which act as a hypervisor from running there.

We can't easily put guests at address 0, since we want to be able to run
with MSO/MSL without having to maintain separate page tables for the
guest physical memory. 2GB is also not a good choice, since the
alloc_pages allocator will place its metadata there when the host has
more than 2GB of memory. In addition, we also want a bit of space after
the end of the host physical memory to be able to catch accesses beyond
the end of physical memory.

The vmalloc allocator unfortunately allocates memory starting at the
highest virtual address which is not suitable for guest memory either
due to additional constraints of some environments.

The physical page allocator in memalign_pages() is also not a optimal
choice, since every test running SIE would then require at least 4GB+1MB
of physical memory.

This results in a few quite complex allocation requirements, hence add a
new function sie_guest_alloc() which allocates memory for a guest and
then establishes a properly aligned virtual space mapping.

Rework snippet test and sie tests to use the new sie_guest_alloc()
function.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 lib/s390x/sie.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sie.h     |  2 ++
 lib/s390x/snippet.h |  9 +++------
 s390x/sie.c         |  4 ++--
 4 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
index b44febdeaaac..69466dbf030a 100644
--- a/lib/s390x/sie.c
+++ b/lib/s390x/sie.c
@@ -15,6 +15,8 @@
 #include <asm/page.h>
 #include <libcflat.h>
 #include <alloc_page.h>
+#include <vmalloc.h>
+#include <sclp.h>
 
 void sie_expect_validity(struct vm *vm)
 {
@@ -111,6 +113,46 @@ void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
 	vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
 }
 
+/**
+ * sie_guest_alloc - Allocate memory for a guest and map it in virtual address
+ * space such that it is properly aligned.
+ * @guest_size the desired size of the guest in bytes.
+ */
+uint8_t *sie_guest_alloc(uint64_t guest_size)
+{
+	static unsigned long guest_counter = 1;
+	u8 *guest_phys, *guest_virt;
+	unsigned long i;
+	pgd_t *root;
+
+	setup_vm();
+	root = (pgd_t *)(stctg(1) & PAGE_MASK);
+
+	/*
+	 * Start of guest memory in host virtual space needs to be aligned to
+	 * 2GB for some environments. It also can't be at 2GB since the memory
+	 * allocator stores its page_states metadata there.
+	 * Thus we use the next multiple of 4GB after the end of physical
+	 * mapping. This also leaves space after end of physical memory so the
+	 * page immediately after physical memory is guaranteed not to be
+	 * present.
+	 */
+	guest_virt = (uint8_t *)ALIGN(get_ram_size() + guest_counter * 4UL * SZ_1G, SZ_2G);
+	guest_counter++;
+
+	guest_phys = alloc_pages(get_order(guest_size) - 12);
+	/*
+	 * Establish a new mapping of the guest memory so it can be 2GB aligned
+	 * without actually requiring 2GB physical memory.
+	 */
+	for (i = 0; i < guest_size; i += PAGE_SIZE) {
+		install_page(root, __pa(guest_phys + i), guest_virt + i);
+	}
+	memset(guest_virt, 0, guest_size);
+
+	return guest_virt;
+}
+
 /* Frees the memory that was gathered on initialization */
 void sie_guest_destroy(struct vm *vm)
 {
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index 147cb0f2a556..c1724cf2f67e 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -285,4 +285,6 @@ void sie_guest_sca_create(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);
 
+uint8_t *sie_guest_alloc(uint64_t guest_size);
+
 #endif /* _S390X_SIE_H_ */
diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
index 11ec54c379e9..910849aa186c 100644
--- a/lib/s390x/snippet.h
+++ b/lib/s390x/snippet.h
@@ -125,14 +125,11 @@ static inline void snippet_pv_init(struct vm *vm, const char *gbin,
 /* Allocates and sets up a snippet based guest */
 static inline void snippet_setup_guest(struct vm *vm, bool is_pv)
 {
-	u8 *guest;
-
-	/* Allocate 1MB as guest memory */
-	guest = alloc_pages(8);
-	memset(guest, 0, HPAGE_SIZE);
+	const unsigned long guest_size = SZ_1M;
+	uint8_t *guest_start = sie_guest_alloc(guest_size);
 
 	/* Initialize the vm struct and allocate control blocks */
-	sie_guest_create(vm, (uint64_t)guest, HPAGE_SIZE);
+	sie_guest_create(vm, (uint64_t)guest_start, guest_size);
 
 	if (is_pv) {
 		/* FMT4 needs a ESCA */
diff --git a/s390x/sie.c b/s390x/sie.c
index cd3cea10d100..ce5b6069bf0b 100644
--- a/s390x/sie.c
+++ b/s390x/sie.c
@@ -89,8 +89,8 @@ static void setup_guest(void)
 {
 	setup_vm();
 
-	/* Allocate 1MB as guest memory */
-	guest = alloc_pages(8);
+	guest = sie_guest_alloc(SZ_1M);
+
 	/* The first two pages are the lowcore */
 	guest_instr = guest + PAGE_SIZE * 2;
 
-- 
2.41.0

[kvm-unit-tests PATCH v2 3/3] s390x: mvpg-sie: fix virtual-physical address confusion

[Nico Boehr]

The addresses reported for the partial execution of mvpg instruction are
physical addresses. Now that MSO is a virtual address, we can't simply
compare the PEI fields in the sie block with MSO, but need to do an
additional translation step.

Add the necessary virtual-physical translations and expose the
virt_to_pte_phys() function in mmu.h which is useful for this kind of
translation.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 lib/s390x/mmu.h  |  2 ++
 s390x/mvpg-sie.c | 15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/s390x/mmu.h b/lib/s390x/mmu.h
index dadc2e600f9a..e9e837236603 100644
--- a/lib/s390x/mmu.h
+++ b/lib/s390x/mmu.h
@@ -95,4 +95,6 @@ static inline void unprotect_page(void *vaddr, unsigned long prot)
 
 void *get_dat_entry(pgd_t *pgtable, void *vaddr, enum pgt_level level);
 
+phys_addr_t virt_to_pte_phys(pgd_t *pgtable, void *vaddr);
+
 #endif /* _ASMS390X_MMU_H_ */
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
index 99f4859bc2df..effb5eab2c0e 100644
--- a/s390x/mvpg-sie.c
+++ b/s390x/mvpg-sie.c
@@ -23,7 +23,9 @@
 static struct vm vm;
 
 static uint8_t *src;
+static phys_addr_t src_phys;
 static uint8_t *dst;
+static phys_addr_t dst_phys;
 static uint8_t *cmp;
 
 static void test_mvpg_pei(void)
@@ -38,8 +40,8 @@ static void test_mvpg_pei(void)
 	protect_page(src, PAGE_ENTRY_I);
 	sie(&vm);
 	report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial execution");
-	report((uintptr_t)**pei_src == (uintptr_t)src + PAGE_ENTRY_I, "PEI_SRC correct");
-	report((uintptr_t)**pei_dst == (uintptr_t)dst, "PEI_DST correct");
+	report((uintptr_t)**pei_src == (uintptr_t)src_phys + PAGE_ENTRY_I, "PEI_SRC correct");
+	report((uintptr_t)**pei_dst == (uintptr_t)dst_phys, "PEI_DST correct");
 	unprotect_page(src, PAGE_ENTRY_I);
 	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
 	/*
@@ -60,8 +62,8 @@ static void test_mvpg_pei(void)
 	protect_page(dst, PAGE_ENTRY_I);
 	sie(&vm);
 	report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial execution");
-	report((uintptr_t)**pei_src == (uintptr_t)src, "PEI_SRC correct");
-	report((uintptr_t)**pei_dst == (uintptr_t)dst + PAGE_ENTRY_I, "PEI_DST correct");
+	report((uintptr_t)**pei_src == (uintptr_t)src_phys, "PEI_SRC correct");
+	report((uintptr_t)**pei_dst == (uintptr_t)dst_phys + PAGE_ENTRY_I, "PEI_DST correct");
 	/* Needed for the memcmp and general cleanup */
 	unprotect_page(dst, PAGE_ENTRY_I);
 	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
@@ -82,8 +84,10 @@ static void setup_guest(void)
 {
 	extern const char SNIPPET_NAME_START(c, mvpg_snippet)[];
 	extern const char SNIPPET_NAME_END(c, mvpg_snippet)[];
+	pgd_t *root;
 
 	setup_vm();
+	root = (pgd_t *)(stctg(1) & PAGE_MASK);
 
 	snippet_setup_guest(&vm, false);
 	snippet_init(&vm, SNIPPET_NAME_START(c, mvpg_snippet),
@@ -94,6 +98,9 @@ static void setup_guest(void)
 
 	src = (uint8_t *) vm.sblk->mso + PAGE_SIZE * 6;
 	dst = (uint8_t *) vm.sblk->mso + PAGE_SIZE * 5;
+	src_phys = virt_to_pte_phys(root, src);
+	dst_phys = virt_to_pte_phys(root, dst);
+
 	cmp = alloc_page();
 	memset(cmp, 0, PAGE_SIZE);
 }
-- 
2.41.0

Re: [kvm-unit-tests PATCH v2 2/3] s390x: sie: ensure guests are aligned to 2GB

[Claudio Imbrenda]

On Mon,  6 Nov 2023 18:08:01 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Until now, kvm-unit-tests has aligned guests to 1 MB in the host virtual
> address space. Unfortunately, some s390x environments require guests to
> be 2GB aligned in the host virtual address space, preventing
> kvm-unit-tests which act as a hypervisor from running there.
> 
> We can't easily put guests at address 0, since we want to be able to run
> with MSO/MSL without having to maintain separate page tables for the
> guest physical memory. 2GB is also not a good choice, since the
> alloc_pages allocator will place its metadata there when the host has
> more than 2GB of memory. In addition, we also want a bit of space after
> the end of the host physical memory to be able to catch accesses beyond
> the end of physical memory.
> 
> The vmalloc allocator unfortunately allocates memory starting at the
> highest virtual address which is not suitable for guest memory either
> due to additional constraints of some environments.
> 
> The physical page allocator in memalign_pages() is also not a optimal
> choice, since every test running SIE would then require at least 4GB+1MB
> of physical memory.
> 
> This results in a few quite complex allocation requirements, hence add a
> new function sie_guest_alloc() which allocates memory for a guest and
> then establishes a properly aligned virtual space mapping.
> 
> Rework snippet test and sie tests to use the new sie_guest_alloc()
> function.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>  lib/s390x/sie.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/sie.h     |  2 ++
>  lib/s390x/snippet.h |  9 +++------
>  s390x/sie.c         |  4 ++--
>  4 files changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index b44febdeaaac..69466dbf030a 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -15,6 +15,8 @@
>  #include <asm/page.h>
>  #include <libcflat.h>
>  #include <alloc_page.h>
> +#include <vmalloc.h>
> +#include <sclp.h>
>  
>  void sie_expect_validity(struct vm *vm)
>  {
> @@ -111,6 +113,46 @@ void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
>  	vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
>  }
>  
> +/**
> + * sie_guest_alloc - Allocate memory for a guest and map it in virtual address

sie_guest_alloc()

> + * space such that it is properly aligned.
> + * @guest_size the desired size of the guest in bytes.

@guest_size:


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

(no need to resend, just fold the fixes when picking)

> + */
> +uint8_t *sie_guest_alloc(uint64_t guest_size)
> +{
> +	static unsigned long guest_counter = 1;
> +	u8 *guest_phys, *guest_virt;
> +	unsigned long i;
> +	pgd_t *root;
> +
> +	setup_vm();
> +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
> +
> +	/*
> +	 * Start of guest memory in host virtual space needs to be aligned to
> +	 * 2GB for some environments. It also can't be at 2GB since the memory
> +	 * allocator stores its page_states metadata there.
> +	 * Thus we use the next multiple of 4GB after the end of physical
> +	 * mapping. This also leaves space after end of physical memory so the
> +	 * page immediately after physical memory is guaranteed not to be
> +	 * present.
> +	 */
> +	guest_virt = (uint8_t *)ALIGN(get_ram_size() + guest_counter * 4UL * SZ_1G, SZ_2G);
> +	guest_counter++;
> +
> +	guest_phys = alloc_pages(get_order(guest_size) - 12);
> +	/*
> +	 * Establish a new mapping of the guest memory so it can be 2GB aligned
> +	 * without actually requiring 2GB physical memory.
> +	 */
> +	for (i = 0; i < guest_size; i += PAGE_SIZE) {
> +		install_page(root, __pa(guest_phys + i), guest_virt + i);
> +	}
> +	memset(guest_virt, 0, guest_size);
> +
> +	return guest_virt;
> +}
> +
>  /* Frees the memory that was gathered on initialization */
>  void sie_guest_destroy(struct vm *vm)
>  {
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 147cb0f2a556..c1724cf2f67e 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -285,4 +285,6 @@ void sie_guest_sca_create(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);
>  
> +uint8_t *sie_guest_alloc(uint64_t guest_size);
> +
>  #endif /* _S390X_SIE_H_ */
> diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
> index 11ec54c379e9..910849aa186c 100644
> --- a/lib/s390x/snippet.h
> +++ b/lib/s390x/snippet.h
> @@ -125,14 +125,11 @@ static inline void snippet_pv_init(struct vm *vm, const char *gbin,
>  /* Allocates and sets up a snippet based guest */
>  static inline void snippet_setup_guest(struct vm *vm, bool is_pv)
>  {
> -	u8 *guest;
> -
> -	/* Allocate 1MB as guest memory */
> -	guest = alloc_pages(8);
> -	memset(guest, 0, HPAGE_SIZE);
> +	const unsigned long guest_size = SZ_1M;
> +	uint8_t *guest_start = sie_guest_alloc(guest_size);
>  
>  	/* Initialize the vm struct and allocate control blocks */
> -	sie_guest_create(vm, (uint64_t)guest, HPAGE_SIZE);
> +	sie_guest_create(vm, (uint64_t)guest_start, guest_size);
>  
>  	if (is_pv) {
>  		/* FMT4 needs a ESCA */
> diff --git a/s390x/sie.c b/s390x/sie.c
> index cd3cea10d100..ce5b6069bf0b 100644
> --- a/s390x/sie.c
> +++ b/s390x/sie.c
> @@ -89,8 +89,8 @@ static void setup_guest(void)
>  {
>  	setup_vm();
>  
> -	/* Allocate 1MB as guest memory */
> -	guest = alloc_pages(8);
> +	guest = sie_guest_alloc(SZ_1M);
> +
>  	/* The first two pages are the lowcore */
>  	guest_instr = guest + PAGE_SIZE * 2;
>  

Re: [kvm-unit-tests PATCH v2 3/3] s390x: mvpg-sie: fix virtual-physical address confusion

[Claudio Imbrenda]

On Mon,  6 Nov 2023 18:08:02 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> The addresses reported for the partial execution of mvpg instruction are
> physical addresses. Now that MSO is a virtual address, we can't simply
> compare the PEI fields in the sie block with MSO, but need to do an
> additional translation step.
> 
> Add the necessary virtual-physical translations and expose the
> virt_to_pte_phys() function in mmu.h which is useful for this kind of
> translation.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>  lib/s390x/mmu.h  |  2 ++
>  s390x/mvpg-sie.c | 15 +++++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/s390x/mmu.h b/lib/s390x/mmu.h
> index dadc2e600f9a..e9e837236603 100644
> --- a/lib/s390x/mmu.h
> +++ b/lib/s390x/mmu.h
> @@ -95,4 +95,6 @@ static inline void unprotect_page(void *vaddr, unsigned long prot)
>  
>  void *get_dat_entry(pgd_t *pgtable, void *vaddr, enum pgt_level level);
>  
> +phys_addr_t virt_to_pte_phys(pgd_t *pgtable, void *vaddr);

or just #include <vmalloc.h> ?

[...]