[kvm-unit-tests PATCH v2 0/3] s390x: Add snippet support

[kvm-unit-tests PATCH v2 0/3] s390x: Add snippet support

[Janosch Frank]

The SIE support allows us to run guests and test the hypervisor's
(V)SIE implementation. However it requires that the guest instructions
are binary which limits the complexity of the guest code.

The snippet support provides a way to write guest code as ASM or C and
simply memcpy it into guest memory. Some of the KVM-unit-test library
can be re-used which further speeds up guest code development.

The included mvpg-sie test helped us to deliver the KVM mvpg fixes
which Claudio posted a short while ago. In the future I'll post Secure
Execution snippet support patches which was my initial goal with this
series anyway.

v2:
    * Minor flat.lds fixes
    * mvpg-sie.c include cleanup
    * mvpg-sie.c added some comments
    * mvpg-sie.c replace pei report calculations with variables src/dst

RFC -> V1:
    * Replaced handle_validity() with assert()
    * Added clearing of registers before sigp in cstart.S
    * Added a nice way to compile the snippets

Janosch Frank (2):
  s390x: snippets: Add gitignore as well as linker script and start
    assembly
  s390x: mvpg: Add SIE mvpg test

Steffen Eiden (1):
  s390x: snippets: Add snippet compilation

 .gitignore                      |   1 +
 s390x/Makefile                  |  29 ++++--
 s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
 s390x/snippets/c/cstart.S       |  16 ++++
 s390x/snippets/c/flat.lds       |  51 +++++++++++
 s390x/snippets/c/mvpg-snippet.c |  33 +++++++
 s390x/unittests.cfg             |   3 +
 7 files changed, 278 insertions(+), 6 deletions(-)
 create mode 100644 s390x/mvpg-sie.c
 create mode 100644 s390x/snippets/c/cstart.S
 create mode 100644 s390x/snippets/c/flat.lds
 create mode 100644 s390x/snippets/c/mvpg-snippet.c

-- 
2.30.2

[kvm-unit-tests PATCH v2 1/3] s390x: snippets: Add gitignore as well as linker script and start assembly

[Janosch Frank]

Snippets are small guests That can be run under a unit test as the
hypervisor. They can be written in C or assembly. The C code needs a
linker script and a start assembly file that jumps to main to work
properly. So let's add that as well as a gitignore entry for the new
files.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 .gitignore                |  1 +
 s390x/snippets/c/cstart.S | 16 ++++++++++++
 s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)
 create mode 100644 s390x/snippets/c/cstart.S
 create mode 100644 s390x/snippets/c/flat.lds

diff --git a/.gitignore b/.gitignore
index 8534fb7..b3cf2cb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,3 +23,4 @@ cscope.*
 /api/dirty-log
 /api/dirty-log-perf
 /s390x/*.bin
+/s390x/snippets/*/*.gbin
diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
new file mode 100644
index 0000000..242568d
--- /dev/null
+++ b/s390x/snippets/c/cstart.S
@@ -0,0 +1,16 @@
+#include <asm/sigp.h>
+
+.section .init
+	.globl start
+start:
+	/* XOR all registers with themselves to clear them fully. */
+	.irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+	xgr \i,\i
+	.endr
+	/* 0x3000 is the stack page for now */
+	lghi	%r15, 0x4000 - 160
+	sam64
+	brasl	%r14, main
+	/* For now let's only use cpu 0 in snippets so this will always work. */
+	xgr	%r0, %r0
+	sigp    %r2, %r0, SIGP_STOP
diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
new file mode 100644
index 0000000..ce3bfd6
--- /dev/null
+++ b/s390x/snippets/c/flat.lds
@@ -0,0 +1,51 @@
+SECTIONS
+{
+	.lowcore : {
+		/*
+		 * Initial short psw for disk boot, with 31 bit addressing for
+		 * non z/Arch environment compatibility and the instruction
+		 * address 0x4000 (cstart.S .init).
+		 */
+		. = 0;
+		 LONG(0x00080000)
+		 LONG(0x80004000)
+		 /* Restart new PSW for booting via PSW restart. */
+		 . = 0x1a0;
+		 QUAD(0x0000000180000000)
+		 QUAD(0x0000000000004000)
+	}
+	. = 0x4000;
+	.text : {
+		*(.init)
+		*(.text)
+		*(.text.*)
+	}
+	. = ALIGN(64K);
+	etext = .;
+	.opd : { *(.opd) }
+	. = ALIGN(16);
+	.dynamic : {
+		dynamic_start = .;
+		*(.dynamic)
+	}
+	.dynsym : {
+		dynsym_start = .;
+		*(.dynsym)
+	}
+	.rela.dyn : { *(.rela*) }
+	. = ALIGN(16);
+	.data : {
+		*(.data)
+		*(.data.rel*)
+	}
+	. = ALIGN(16);
+	.rodata : { *(.rodata) *(.rodata.*) }
+	. = ALIGN(16);
+	__bss_start = .;
+	.bss : { *(.bss) }
+	__bss_end = .;
+	. = ALIGN(64K);
+	edata = .;
+	. += 64K;
+	. = ALIGN(64K);
+}
-- 
2.30.2

[kvm-unit-tests PATCH v2 2/3] s390x: snippets: Add snippet compilation

[Janosch Frank]

From: Steffen Eiden <seiden@linux.ibm.com>

This patch provides the necessary make targets to properly compile
snippets and link them into their associated host.

To define the guest-host definition, we use the make-feature
`SECONDEXPANSION` in combination with `Target specific Variable
Values`. The variable `snippets` has different values depending on the
current target. This enables us to define which snippets (=guests)
belong to which hosts. Furthermore, using the second-expansion, we can
use `snippets` in the perquisites of the host's `%.elf` rule, which
otherwise would be not set.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/Makefile | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/s390x/Makefile b/s390x/Makefile
index 8820e99..ba32f4c 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -76,11 +76,26 @@ OBJDIRS += lib/s390x
 asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
 
 FLATLIBS = $(libcflat)
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
-	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) \
-		$(SRCDIR)/lib/auxinfo.c -DPROGNAME=\"$@\"
-	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
-		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
+
+SNIPPET_DIR = $(TEST_DIR)/snippets
+snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
+
+# perquisites (=guests) for the snippet hosts.
+# $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
+
+$(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
+	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
+	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
+
+$(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_asmlib) $(FLATLIBS)
+	$(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds $(patsubst %.gbin,%.o,$@) $(snippet_asmlib) $(FLATLIBS)
+	$(OBJCOPY) -O binary $@ $@
+	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
+
+.SECONDEXPANSION:
+%.elf: $$(snippets) %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
+	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(SRCDIR)/lib/auxinfo.c -DPROGNAME=\"$@\"
+	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
 	$(RM) $(@:.elf=.aux.o)
 	@chmod a-x $@
 
@@ -94,7 +109,7 @@ FLATLIBS = $(libcflat)
 	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
 
 arch_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
+	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d $(SNIPPET_DIR)/c/*.{o,gbin} $(SNIPPET_DIR)/c/.*.d lib/s390x/.*.d
 
 generated-files = $(asm-offsets)
 $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
-- 
2.30.2

[kvm-unit-tests PATCH v2 3/3] s390x: mvpg: Add SIE mvpg test

[Janosch Frank]

Let's also check the PEI values to make sure our VSIE implementation
is correct.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/Makefile                  |   2 +
 s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
 s390x/snippets/c/mvpg-snippet.c |  33 +++++++
 s390x/unittests.cfg             |   3 +
 4 files changed, 189 insertions(+)
 create mode 100644 s390x/mvpg-sie.c
 create mode 100644 s390x/snippets/c/mvpg-snippet.c

diff --git a/s390x/Makefile b/s390x/Makefile
index ba32f4c..07af26d 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -23,6 +23,7 @@ tests += $(TEST_DIR)/sie.elf
 tests += $(TEST_DIR)/mvpg.elf
 tests += $(TEST_DIR)/uv-host.elf
 tests += $(TEST_DIR)/edat.elf
+tests += $(TEST_DIR)/mvpg-sie.elf
 
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 ifneq ($(HOST_KEY_DOCUMENT),)
@@ -82,6 +83,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.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
 
 $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
 	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
new file mode 100644
index 0000000..3536c6a
--- /dev/null
+++ b/s390x/mvpg-sie.c
@@ -0,0 +1,151 @@
+#include <libcflat.h>
+#include <asm/asm-offsets.h>
+#include <asm-generic/barrier.h>
+#include <asm/pgtable.h>
+#include <mmu.h>
+#include <asm/page.h>
+#include <asm/facility.h>
+#include <asm/mem.h>
+#include <alloc_page.h>
+#include <vm.h>
+#include <sclp.h>
+#include <sie.h>
+
+static u8 *guest;
+static u8 *guest_instr;
+static struct vm vm;
+
+static uint8_t *src;
+static uint8_t *dst;
+static uint8_t *cmp;
+
+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);
+		if (vm->sblk->icptcode == ICPT_VALIDITY)
+			assert(0);
+	}
+	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);
+	uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
+
+	report_prefix_push("pei");
+
+	report_prefix_push("src");
+	memset(dst, 0, PAGE_SIZE);
+	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");
+	unprotect_page(src, PAGE_ENTRY_I);
+	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
+	/*
+	 * We need to execute the diag44 which is used as a blocker
+	 * behind the mvpg. It makes sure we fail the tests above if
+	 * the mvpg wouldn't have intercepted.
+	 */
+	sie(&vm);
+	/* Make sure we intercepted for the diag44 and nothing else */
+	assert(vm.sblk->icptcode == ICPT_INST &&
+	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
+	report_prefix_pop();
+
+	/* Clear PEI data for next check */
+	report_prefix_push("dst");
+	memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
+	memset(dst, 0, PAGE_SIZE);
+	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");
+	/* Needed for the memcmp and general cleanup */
+	unprotect_page(dst, PAGE_ENTRY_I);
+	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
+	report_prefix_pop();
+
+	report_prefix_pop();
+}
+
+static void test_mvpg(void)
+{
+	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_end -
+			   (uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_start);
+
+	memcpy(guest, _binary_s390x_snippets_c_mvpg_snippet_gbin_start, binary_size);
+	memset(src, 0x42, PAGE_SIZE);
+	memset(dst, 0x43, PAGE_SIZE);
+	sie(&vm);
+	mb();
+	report(!memcmp(src, dst, PAGE_SIZE) && *dst == 0x42, "Page moved");
+}
+
+static void setup_guest(void)
+{
+	setup_vm();
+
+	/* Allocate 1MB as guest memory */
+	guest = alloc_pages(8);
+	/* 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();
+
+	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
+	vm.sblk->gpsw.mask = 0x0000000180000000ULL;
+	vm.sblk->ictl = ICTL_OPEREXC | ICTL_PINT;
+	/* Enable MVPG interpretation as we want to test KVM and not ourselves */
+	vm.sblk->eca = ECA_MVPGI;
+
+	src = guest + PAGE_SIZE * 6;
+	dst = guest + PAGE_SIZE * 5;
+	cmp = alloc_page();
+	memset(cmp, 0, PAGE_SIZE);
+}
+
+int main(void)
+{
+	report_prefix_push("mvpg-sie");
+	if (!sclp_facilities.has_sief2) {
+		report_skip("SIEF2 facility unavailable");
+		goto done;
+	}
+
+	setup_guest();
+	test_mvpg();
+	test_mvpg_pei();
+
+done:
+	report_prefix_pop();
+	return report_summary();
+
+}
diff --git a/s390x/snippets/c/mvpg-snippet.c b/s390x/snippets/c/mvpg-snippet.c
new file mode 100644
index 0000000..c1eb5d7
--- /dev/null
+++ b/s390x/snippets/c/mvpg-snippet.c
@@ -0,0 +1,33 @@
+#include <libcflat.h>
+
+static inline void force_exit(void)
+{
+	asm volatile("	diag	0,0,0x44\n");
+}
+
+static inline int mvpg(unsigned long r0, void *dest, void *src)
+{
+	register unsigned long reg0 asm ("0") = r0;
+	int cc;
+
+	asm volatile("	mvpg    %1,%2\n"
+		     "	ipm     %0\n"
+		     "	srl     %0,28"
+		     : "=d" (cc) : "a" (dest), "a" (src), "d" (reg0)
+		     : "memory", "cc");
+	return cc;
+}
+
+static void test_mvpg_real(void)
+{
+	mvpg(0, (void *)0x5000, (void *)0x6000);
+	force_exit();
+}
+
+__attribute__((section(".text"))) int main(void)
+{
+	test_mvpg_real();
+	test_mvpg_real();
+	test_mvpg_real();
+	return 0;
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index a0ec886..9e1802f 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -106,3 +106,6 @@ timeout = 10
 
 [edat]
 file = edat.elf
+
+[mvpg-sie]
+file = mvpg-sie.elf
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2 1/3] s390x: snippets: Add gitignore as well as linker script and start assembly

[Thomas Huth]

On 29/06/2021 15.18, Janosch Frank wrote:
> Snippets are small guests That can be run under a unit test as the
> hypervisor.

s/That/that/. Maybe also rather something like:

Snippets are small guests which can be run under a unit test that implements 
a simple hypervisor.

?

> They can be written in C or assembly. The C code needs a
> linker script and a start assembly file that jumps to main to work
> properly. So let's add that as well as a gitignore entry for the new
> files.

Rest of the file looks fine to me.

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

Re: [kvm-unit-tests PATCH v2 3/3] s390x: mvpg: Add SIE mvpg test

[Thomas Huth]

On 29/06/2021 15.18, Janosch Frank wrote:
> Let's also check the PEI values to make sure our VSIE implementation
> is correct.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   s390x/Makefile                  |   2 +
>   s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
>   s390x/snippets/c/mvpg-snippet.c |  33 +++++++
>   s390x/unittests.cfg             |   3 +
>   4 files changed, 189 insertions(+)
>   create mode 100644 s390x/mvpg-sie.c
>   create mode 100644 s390x/snippets/c/mvpg-snippet.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ba32f4c..07af26d 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -23,6 +23,7 @@ tests += $(TEST_DIR)/sie.elf
>   tests += $(TEST_DIR)/mvpg.elf
>   tests += $(TEST_DIR)/uv-host.elf
>   tests += $(TEST_DIR)/edat.elf
> +tests += $(TEST_DIR)/mvpg-sie.elf
>   
>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>   ifneq ($(HOST_KEY_DOCUMENT),)
> @@ -82,6 +83,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.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
>   
>   $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>   	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> new file mode 100644
> index 0000000..3536c6a
> --- /dev/null
> +++ b/s390x/mvpg-sie.c
> @@ -0,0 +1,151 @@
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm-generic/barrier.h>
> +#include <asm/pgtable.h>
> +#include <mmu.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +#include <asm/mem.h>
> +#include <alloc_page.h>
> +#include <vm.h>
> +#include <sclp.h>
> +#include <sie.h>
> +
> +static u8 *guest;
> +static u8 *guest_instr;
> +static struct vm vm;
> +
> +static uint8_t *src;
> +static uint8_t *dst;
> +static uint8_t *cmp;
> +
> +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);
> +		if (vm->sblk->icptcode == ICPT_VALIDITY)
> +			assert(0);

Please replace the above two lines with:

		assert(vm->sblk->icptcode != ICPT_VALIDITY);

> +	}
> +	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);
> +	uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
> +
> +	report_prefix_push("pei");
> +
> +	report_prefix_push("src");
> +	memset(dst, 0, PAGE_SIZE);
> +	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");
> +	unprotect_page(src, PAGE_ENTRY_I);
> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
> +	/*
> +	 * We need to execute the diag44 which is used as a blocker
> +	 * behind the mvpg. It makes sure we fail the tests above if
> +	 * the mvpg wouldn't have intercepted.
> +	 */
> +	sie(&vm);
> +	/* Make sure we intercepted for the diag44 and nothing else */
> +	assert(vm.sblk->icptcode == ICPT_INST &&
> +	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
> +	report_prefix_pop();
> +
> +	/* Clear PEI data for next check */
> +	report_prefix_push("dst");
> +	memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
> +	memset(dst, 0, PAGE_SIZE);
> +	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");
> +	/* Needed for the memcmp and general cleanup */
> +	unprotect_page(dst, PAGE_ENTRY_I);
> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
> +	report_prefix_pop();
> +
> +	report_prefix_pop();
> +}

Still quite a lot of magic values in above code ... any chance to introduce 
some #defines finally?

> +static void test_mvpg(void)
> +{
> +	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_end -
> +			   (uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_start);
> +
> +	memcpy(guest, _binary_s390x_snippets_c_mvpg_snippet_gbin_start, binary_size);
> +	memset(src, 0x42, PAGE_SIZE);
> +	memset(dst, 0x43, PAGE_SIZE);
> +	sie(&vm);
> +	mb();

I think you don't need the mb() here.

> +	report(!memcmp(src, dst, PAGE_SIZE) && *dst == 0x42, "Page moved");
> +}

  Thomas

Re: [kvm-unit-tests PATCH v2 3/3] s390x: mvpg: Add SIE mvpg test

[Janosch Frank]

On 7/5/21 9:24 AM, Thomas Huth wrote:
> On 29/06/2021 15.18, Janosch Frank wrote:
>> Let's also check the PEI values to make sure our VSIE implementation
>> is correct.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   s390x/Makefile                  |   2 +
>>   s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
>>   s390x/snippets/c/mvpg-snippet.c |  33 +++++++
>>   s390x/unittests.cfg             |   3 +
>>   4 files changed, 189 insertions(+)
>>   create mode 100644 s390x/mvpg-sie.c
>>   create mode 100644 s390x/snippets/c/mvpg-snippet.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ba32f4c..07af26d 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -23,6 +23,7 @@ tests += $(TEST_DIR)/sie.elf
>>   tests += $(TEST_DIR)/mvpg.elf
>>   tests += $(TEST_DIR)/uv-host.elf
>>   tests += $(TEST_DIR)/edat.elf
>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>   
>>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>   ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -82,6 +83,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.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
>>   
>>   $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>   	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> new file mode 100644
>> index 0000000..3536c6a
>> --- /dev/null
>> +++ b/s390x/mvpg-sie.c
>> @@ -0,0 +1,151 @@
>> +#include <libcflat.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm-generic/barrier.h>
>> +#include <asm/pgtable.h>
>> +#include <mmu.h>
>> +#include <asm/page.h>
>> +#include <asm/facility.h>
>> +#include <asm/mem.h>
>> +#include <alloc_page.h>
>> +#include <vm.h>
>> +#include <sclp.h>
>> +#include <sie.h>
>> +
>> +static u8 *guest;
>> +static u8 *guest_instr;
>> +static struct vm vm;
>> +
>> +static uint8_t *src;
>> +static uint8_t *dst;
>> +static uint8_t *cmp;
>> +
>> +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);
>> +		if (vm->sblk->icptcode == ICPT_VALIDITY)
>> +			assert(0);
> 
> Please replace the above two lines with:
> 
> 		assert(vm->sblk->icptcode != ICPT_VALIDITY);

Sure

> 
>> +	}
>> +	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);
>> +	uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
>> +
>> +	report_prefix_push("pei");
>> +
>> +	report_prefix_push("src");
>> +	memset(dst, 0, PAGE_SIZE);
>> +	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");
>> +	unprotect_page(src, PAGE_ENTRY_I);
>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>> +	/*
>> +	 * We need to execute the diag44 which is used as a blocker
>> +	 * behind the mvpg. It makes sure we fail the tests above if
>> +	 * the mvpg wouldn't have intercepted.
>> +	 */
>> +	sie(&vm);
>> +	/* Make sure we intercepted for the diag44 and nothing else */
>> +	assert(vm.sblk->icptcode == ICPT_INST &&
>> +	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
>> +	report_prefix_pop();
>> +
>> +	/* Clear PEI data for next check */
>> +	report_prefix_push("dst");
>> +	memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
>> +	memset(dst, 0, PAGE_SIZE);
>> +	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");
>> +	/* Needed for the memcmp and general cleanup */
>> +	unprotect_page(dst, PAGE_ENTRY_I);
>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_pop();
>> +}
> 
> Still quite a lot of magic values in above code ... any chance to introduce 
> some #defines finally?

Currently not really.
I added a comment for the diag 44 which should be enough right now. If
needed I can add a comment to the pei variables as well.

> 
>> +static void test_mvpg(void)
>> +{
>> +	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_end -
>> +			   (uintptr_t)_binary_s390x_snippets_c_mvpg_snippet_gbin_start);
>> +
>> +	memcpy(guest, _binary_s390x_snippets_c_mvpg_snippet_gbin_start, binary_size);
>> +	memset(src, 0x42, PAGE_SIZE);
>> +	memset(dst, 0x43, PAGE_SIZE);
>> +	sie(&vm);
>> +	mb();
> 
> I think you don't need the mb() here.

Right

> 
>> +	report(!memcmp(src, dst, PAGE_SIZE) && *dst == 0x42, "Page moved");
>> +}
> 
>   Thomas
> 

Re: [kvm-unit-tests PATCH v2 3/3] s390x: mvpg: Add SIE mvpg test

[Thomas Huth]

On 05/07/2021 11.37, Janosch Frank wrote:
> On 7/5/21 9:24 AM, Thomas Huth wrote:
>> On 29/06/2021 15.18, Janosch Frank wrote:
>>> Let's also check the PEI values to make sure our VSIE implementation
>>> is correct.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>    s390x/Makefile                  |   2 +
>>>    s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
>>>    s390x/snippets/c/mvpg-snippet.c |  33 +++++++
>>>    s390x/unittests.cfg             |   3 +
>>>    4 files changed, 189 insertions(+)
>>>    create mode 100644 s390x/mvpg-sie.c
>>>    create mode 100644 s390x/snippets/c/mvpg-snippet.c
>>>
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index ba32f4c..07af26d 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -23,6 +23,7 @@ tests += $(TEST_DIR)/sie.elf
>>>    tests += $(TEST_DIR)/mvpg.elf
>>>    tests += $(TEST_DIR)/uv-host.elf
>>>    tests += $(TEST_DIR)/edat.elf
>>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>>    
>>>    tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>>    ifneq ($(HOST_KEY_DOCUMENT),)
>>> @@ -82,6 +83,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.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
>>>    
>>>    $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>>    	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>>> new file mode 100644
>>> index 0000000..3536c6a
>>> --- /dev/null
>>> +++ b/s390x/mvpg-sie.c
>>> @@ -0,0 +1,151 @@
>>> +#include <libcflat.h>
>>> +#include <asm/asm-offsets.h>
>>> +#include <asm-generic/barrier.h>
>>> +#include <asm/pgtable.h>
>>> +#include <mmu.h>
>>> +#include <asm/page.h>
>>> +#include <asm/facility.h>
>>> +#include <asm/mem.h>
>>> +#include <alloc_page.h>
>>> +#include <vm.h>
>>> +#include <sclp.h>
>>> +#include <sie.h>
>>> +
>>> +static u8 *guest;
>>> +static u8 *guest_instr;
>>> +static struct vm vm;
>>> +
>>> +static uint8_t *src;
>>> +static uint8_t *dst;
>>> +static uint8_t *cmp;
>>> +
>>> +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);
>>> +		if (vm->sblk->icptcode == ICPT_VALIDITY)
>>> +			assert(0);
>>
>> Please replace the above two lines with:
>>
>> 		assert(vm->sblk->icptcode != ICPT_VALIDITY);
> 
> Sure
> 
>>
>>> +	}
>>> +	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);
>>> +	uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
>>> +
>>> +	report_prefix_push("pei");
>>> +
>>> +	report_prefix_push("src");
>>> +	memset(dst, 0, PAGE_SIZE);
>>> +	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");
>>> +	unprotect_page(src, PAGE_ENTRY_I);
>>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>>> +	/*
>>> +	 * We need to execute the diag44 which is used as a blocker
>>> +	 * behind the mvpg. It makes sure we fail the tests above if
>>> +	 * the mvpg wouldn't have intercepted.
>>> +	 */
>>> +	sie(&vm);
>>> +	/* Make sure we intercepted for the diag44 and nothing else */
>>> +	assert(vm.sblk->icptcode == ICPT_INST &&
>>> +	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
>>> +	report_prefix_pop();
>>> +
>>> +	/* Clear PEI data for next check */
>>> +	report_prefix_push("dst");
>>> +	memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
>>> +	memset(dst, 0, PAGE_SIZE);
>>> +	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");
>>> +	/* Needed for the memcmp and general cleanup */
>>> +	unprotect_page(dst, PAGE_ENTRY_I);
>>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>>> +	report_prefix_pop();
>>> +
>>> +	report_prefix_pop();
>>> +}
>>
>> Still quite a lot of magic values in above code ... any chance to introduce
>> some #defines finally?
> 
> Currently not really.
> I added a comment for the diag 44 which should be enough right now. If
> needed I can add a comment to the pei variables as well.

Ok, fine for me, we can still clean up later if necessary. Thus with the 
assert() fixed and the mb() removed:

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

Re: [kvm-unit-tests PATCH v2 3/3] s390x: mvpg: Add SIE mvpg test

[Janosch Frank]

On 7/5/21 11:52 AM, Thomas Huth wrote:
> On 05/07/2021 11.37, Janosch Frank wrote:
>> On 7/5/21 9:24 AM, Thomas Huth wrote:
>>> On 29/06/2021 15.18, Janosch Frank wrote:
>>>> Let's also check the PEI values to make sure our VSIE implementation
>>>> is correct.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>    s390x/Makefile                  |   2 +
>>>>    s390x/mvpg-sie.c                | 151 ++++++++++++++++++++++++++++++++
>>>>    s390x/snippets/c/mvpg-snippet.c |  33 +++++++
>>>>    s390x/unittests.cfg             |   3 +
>>>>    4 files changed, 189 insertions(+)
>>>>    create mode 100644 s390x/mvpg-sie.c
>>>>    create mode 100644 s390x/snippets/c/mvpg-snippet.c
>>>>
>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>> index ba32f4c..07af26d 100644
>>>> --- a/s390x/Makefile
>>>> +++ b/s390x/Makefile
>>>> @@ -23,6 +23,7 @@ tests += $(TEST_DIR)/sie.elf
>>>>    tests += $(TEST_DIR)/mvpg.elf
>>>>    tests += $(TEST_DIR)/uv-host.elf
>>>>    tests += $(TEST_DIR)/edat.elf
>>>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>>>    
>>>>    tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>>>    ifneq ($(HOST_KEY_DOCUMENT),)
>>>> @@ -82,6 +83,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.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
>>>>    
>>>>    $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>>>    	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>>>> new file mode 100644
>>>> index 0000000..3536c6a
>>>> --- /dev/null
>>>> +++ b/s390x/mvpg-sie.c
>>>> @@ -0,0 +1,151 @@
>>>> +#include <libcflat.h>
>>>> +#include <asm/asm-offsets.h>
>>>> +#include <asm-generic/barrier.h>
>>>> +#include <asm/pgtable.h>
>>>> +#include <mmu.h>
>>>> +#include <asm/page.h>
>>>> +#include <asm/facility.h>
>>>> +#include <asm/mem.h>
>>>> +#include <alloc_page.h>
>>>> +#include <vm.h>
>>>> +#include <sclp.h>
>>>> +#include <sie.h>
>>>> +
>>>> +static u8 *guest;
>>>> +static u8 *guest_instr;
>>>> +static struct vm vm;
>>>> +
>>>> +static uint8_t *src;
>>>> +static uint8_t *dst;
>>>> +static uint8_t *cmp;
>>>> +
>>>> +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);
>>>> +		if (vm->sblk->icptcode == ICPT_VALIDITY)
>>>> +			assert(0);
>>>
>>> Please replace the above two lines with:
>>>
>>> 		assert(vm->sblk->icptcode != ICPT_VALIDITY);
>>
>> Sure
>>
>>>
>>>> +	}
>>>> +	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);
>>>> +	uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
>>>> +
>>>> +	report_prefix_push("pei");
>>>> +
>>>> +	report_prefix_push("src");
>>>> +	memset(dst, 0, PAGE_SIZE);
>>>> +	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");
>>>> +	unprotect_page(src, PAGE_ENTRY_I);
>>>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>>>> +	/*
>>>> +	 * We need to execute the diag44 which is used as a blocker
>>>> +	 * behind the mvpg. It makes sure we fail the tests above if
>>>> +	 * the mvpg wouldn't have intercepted.
>>>> +	 */
>>>> +	sie(&vm);
>>>> +	/* Make sure we intercepted for the diag44 and nothing else */
>>>> +	assert(vm.sblk->icptcode == ICPT_INST &&
>>>> +	       vm.sblk->ipa == 0x8300 && vm.sblk->ipb == 0x440000);
>>>> +	report_prefix_pop();
>>>> +
>>>> +	/* Clear PEI data for next check */
>>>> +	report_prefix_push("dst");
>>>> +	memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
>>>> +	memset(dst, 0, PAGE_SIZE);
>>>> +	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");
>>>> +	/* Needed for the memcmp and general cleanup */
>>>> +	unprotect_page(dst, PAGE_ENTRY_I);
>>>> +	report(!memcmp(cmp, dst, PAGE_SIZE), "Destination intact");
>>>> +	report_prefix_pop();
>>>> +
>>>> +	report_prefix_pop();
>>>> +}
>>>
>>> Still quite a lot of magic values in above code ... any chance to introduce
>>> some #defines finally?
>>
>> Currently not really.
>> I added a comment for the diag 44 which should be enough right now. If
>> needed I can add a comment to the pei variables as well.
> 
> Ok, fine for me, we can still clean up later if necessary. Thus with the 
> assert() fixed and the mb() removed:
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 

Thanks, I'll fix this up and queue the series