* [kvm-unit-tests RFC 0/2] s390x: Add snippet support
@ 2021-05-20 9:47 Janosch Frank
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-20 9:47 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
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.
I heard you liked tests so I put tests inside tests so you can test
while you test.
Janosch Frank (2):
s390x: Add guest snippet support
s390x: mvpg: Add SIE mvpg test
.gitignore | 2 +
s390x/Makefile | 29 ++++++-
s390x/mvpg-sie.c | 139 ++++++++++++++++++++++++++++++++
s390x/snippets/c/cstart.S | 13 +++
s390x/snippets/c/flat.lds | 51 ++++++++++++
s390x/snippets/c/mvpg-snippet.c | 33 ++++++++
s390x/unittests.cfg | 3 +
7 files changed, 267 insertions(+), 3 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-05-20 9:47 [kvm-unit-tests RFC 0/2] s390x: Add snippet support Janosch Frank
@ 2021-05-20 9:47 ` Janosch Frank
2021-05-25 16:44 ` Claudio Imbrenda
2021-06-21 10:10 ` Thomas Huth
2021-05-20 9:47 ` [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test Janosch Frank
2021-05-20 13:36 ` [kvm-unit-tests RFC 0/2] s390x: Add snippet support David Hildenbrand
2 siblings, 2 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-20 9:47 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
Snippets can be used to easily write and run guest (SIE) tests.
The snippet is linked into the test binaries and can therefore be
accessed via a ptr.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
.gitignore | 2 ++
s390x/Makefile | 28 ++++++++++++++++++---
s390x/snippets/c/cstart.S | 13 ++++++++++
s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
4 files changed, 91 insertions(+), 3 deletions(-)
create mode 100644 s390x/snippets/c/cstart.S
create mode 100644 s390x/snippets/c/flat.lds
diff --git a/.gitignore b/.gitignore
index 784cb2dd..29d3635b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -22,3 +22,5 @@ cscope.*
/api/dirty-log
/api/dirty-log-perf
/s390x/*.bin
+/s390x/snippets/*/*.bin
+/s390x/snippets/*/*.gbin
diff --git a/s390x/Makefile b/s390x/Makefile
index 8de926ab..fe267011 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
FLATLIBS = $(libcflat)
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
+
+SNIPPET_DIR = $(TEST_DIR)/snippets
+
+# C snippets that need to be linked
+snippets-c =
+
+# ASM snippets that are directly compiled and converted to a *.gbin
+snippets-a =
+
+snippets = $(snippets-a)$(snippets-c)
+snippets-o += $(patsubst %.gbin,%.o,$(snippets))
+
+$(snippets-a): $(snippets-o) $(FLATLIBS)
+ $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
+ $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
+
+$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
+ $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
+ $(filter %.o, $^) $(FLATLIBS)
+ $(OBJCOPY) -O binary $@ $@
+ $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
+
+%.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) $(@:.elf=.aux.o)
+ $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
$(RM) $(@:.elf=.aux.o)
@chmod a-x $@
@@ -93,7 +115,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} $(SNIPPET_DIR)/c/*.{o,elf,bin,gbin} $(SNIPPET_DIR)/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
generated-files = $(asm-offsets)
$(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
new file mode 100644
index 00000000..02a3338b
--- /dev/null
+++ b/s390x/snippets/c/cstart.S
@@ -0,0 +1,13 @@
+#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
+ brasl %r14, main
+ sigp %r1, %r0, SIGP_STOP
diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
new file mode 100644
index 00000000..5e707325
--- /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 0x10000 (cstart64.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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-05-20 9:47 [kvm-unit-tests RFC 0/2] s390x: Add snippet support Janosch Frank
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
@ 2021-05-20 9:47 ` Janosch Frank
2021-05-25 17:37 ` Claudio Imbrenda
2021-06-21 10:23 ` Thomas Huth
2021-05-20 13:36 ` [kvm-unit-tests RFC 0/2] s390x: Add snippet support David Hildenbrand
2 siblings, 2 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-20 9:47 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
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 | 3 +-
s390x/mvpg-sie.c | 139 ++++++++++++++++++++++++++++++++
s390x/snippets/c/mvpg-snippet.c | 33 ++++++++
s390x/unittests.cfg | 3 +
4 files changed, 177 insertions(+), 1 deletion(-)
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 fe267011..6692cf73 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
tests += $(TEST_DIR)/sie.elf
tests += $(TEST_DIR)/mvpg.elf
tests += $(TEST_DIR)/uv-host.elf
+tests += $(TEST_DIR)/mvpg-sie.elf
tests_binary = $(patsubst %.elf,%.bin,$(tests))
ifneq ($(HOST_KEY_DOCUMENT),)
@@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
SNIPPET_DIR = $(TEST_DIR)/snippets
# C snippets that need to be linked
-snippets-c =
+snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
# ASM snippets that are directly compiled and converted to a *.gbin
snippets-a =
diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
new file mode 100644
index 00000000..a617704b
--- /dev/null
+++ b/s390x/mvpg-sie.c
@@ -0,0 +1,139 @@
+#include <libcflat.h>
+#include <asm/asm-offsets.h>
+#include <asm-generic/barrier.h>
+#include <asm/interrupt.h>
+#include <asm/pgtable.h>
+#include <mmu.h>
+#include <asm/page.h>
+#include <asm/facility.h>
+#include <asm/mem.h>
+#include <asm/sigp.h>
+#include <smp.h>
+#include <alloc_page.h>
+#include <bitops.h>
+#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;
+
+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 handle_validity(struct vm *vm)
+{
+ report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
+}
+
+static void sie(struct vm *vm)
+{
+ /* Reset icptcode so we don't trip below */
+ vm->sblk->icptcode = 0;
+
+ while (vm->sblk->icptcode == 0) {
+ sie64a(vm->sblk, &vm->save_area);
+ if (vm->sblk->icptcode == ICPT_VALIDITY)
+ 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);
+ uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk + 0xc8);
+
+ report_prefix_push("pei");
+ protect_page(guest + 0x6000, PAGE_ENTRY_I);
+ sie(&vm);
+ report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial execution");
+ report((uintptr_t)**pei_src == ((uintptr_t)vm.sblk->mso) + 0x6000 + PAGE_ENTRY_I, "PEI_SRC correct");
+ report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000, "PEI_DST correct");
+ /* Jump over the diag44 */
+ sie(&vm);
+
+ /* Clear PEI data for next check */
+ memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
+ unprotect_page(guest + 0x6000, PAGE_ENTRY_I);
+ protect_page(guest + 0x5000, PAGE_ENTRY_I);
+ sie(&vm);
+ report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial execution");
+ report((uintptr_t)**pei_src == vm.sblk->mso + 0x6000, "PEI_SRC correct");
+ report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000 + PAGE_ENTRY_I, "PEI_DST correct");
+
+ 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;
+}
+
+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 00000000..96b70c9c
--- /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 9f81a608..8634b1b1 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -103,3 +103,6 @@ file = sie.elf
[mvpg]
file = mvpg.elf
timeout = 10
+
+[mvpg-sie]
+file = mvpg-sie.elf
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 0/2] s390x: Add snippet support
2021-05-20 9:47 [kvm-unit-tests RFC 0/2] s390x: Add snippet support Janosch Frank
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
2021-05-20 9:47 ` [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test Janosch Frank
@ 2021-05-20 13:36 ` David Hildenbrand
2 siblings, 0 replies; 18+ messages in thread
From: David Hildenbrand @ 2021-05-20 13:36 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, thuth, cohuck
On 20.05.21 11:47, Janosch Frank wrote:
> 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.
>
> I heard you liked tests so I put tests inside tests so you can test
> while you test.
The idea sounds sane to me.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
@ 2021-05-25 16:44 ` Claudio Imbrenda
2021-05-26 10:12 ` Janosch Frank
2021-06-21 10:10 ` Thomas Huth
1 sibling, 1 reply; 18+ messages in thread
From: Claudio Imbrenda @ 2021-05-25 16:44 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck
On Thu, 20 May 2021 09:47:29 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> Snippets can be used to easily write and run guest (SIE) tests.
> The snippet is linked into the test binaries and can therefore be
> accessed via a ptr.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> .gitignore | 2 ++
> s390x/Makefile | 28 ++++++++++++++++++---
> s390x/snippets/c/cstart.S | 13 ++++++++++
> s390x/snippets/c/flat.lds | 51
> +++++++++++++++++++++++++++++++++++++++ 4 files changed, 91
> insertions(+), 3 deletions(-) create mode 100644
> s390x/snippets/c/cstart.S create mode 100644 s390x/snippets/c/flat.lds
>
> diff --git a/.gitignore b/.gitignore
> index 784cb2dd..29d3635b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -22,3 +22,5 @@ cscope.*
> /api/dirty-log
> /api/dirty-log-perf
> /s390x/*.bin
> +/s390x/snippets/*/*.bin
> +/s390x/snippets/*/*.gbin
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 8de926ab..fe267011 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>
> FLATLIBS = $(libcflat)
> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
> +
> +SNIPPET_DIR = $(TEST_DIR)/snippets
> +
> +# C snippets that need to be linked
> +snippets-c =
> +
> +# ASM snippets that are directly compiled and converted to a *.gbin
> +snippets-a =
> +
> +snippets = $(snippets-a)$(snippets-c)
↑↑
I'm not a Makefile expert, but, don't you need a space between the two
variable expansions?
> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
> +
> +$(snippets-a): $(snippets-o) $(FLATLIBS)
> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
> +
> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
> + $(filter %.o, $^) $(FLATLIBS)
> + $(OBJCOPY) -O binary $@ $@
> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
> +
> +%.elf: $(snippets) %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
I would keep the %.o as the first in the list
> $(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)
> + $(filter %.o, $^) $(FLATLIBS) $(snippets)
so all the snippets are always baked in every test?
> $(@:.elf=.aux.o) $(RM) $(@:.elf=.aux.o)
> @chmod a-x $@
>
> @@ -93,7 +115,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}
> $(SNIPPET_DIR)/c/*.{o,elf,bin,gbin} $(SNIPPET_DIR)/.*.d
> $(TEST_DIR)/.*.d lib/s390x/.*.d generated-files = $(asm-offsets)
> $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> new file mode 100644
> index 00000000..02a3338b
> --- /dev/null
> +++ b/s390x/snippets/c/cstart.S
> @@ -0,0 +1,13 @@
> +#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
> + brasl %r14, main
> + sigp %r1, %r0, SIGP_STOP
> diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
> new file mode 100644
> index 00000000..5e707325
> --- /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 0x10000 (cstart64.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);
> +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-05-20 9:47 ` [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test Janosch Frank
@ 2021-05-25 17:37 ` Claudio Imbrenda
2021-05-26 10:17 ` Janosch Frank
2021-05-27 14:35 ` Janosch Frank
2021-06-21 10:23 ` Thomas Huth
1 sibling, 2 replies; 18+ messages in thread
From: Claudio Imbrenda @ 2021-05-25 17:37 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck
On Thu, 20 May 2021 09:47:30 +0000
Janosch Frank <frankja@linux.ibm.com> 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 | 3 +-
> s390x/mvpg-sie.c | 139
> ++++++++++++++++++++++++++++++++ s390x/snippets/c/mvpg-snippet.c |
> 33 ++++++++ s390x/unittests.cfg | 3 +
> 4 files changed, 177 insertions(+), 1 deletion(-)
> 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 fe267011..6692cf73 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
> tests += $(TEST_DIR)/sie.elf
> tests += $(TEST_DIR)/mvpg.elf
> tests += $(TEST_DIR)/uv-host.elf
> +tests += $(TEST_DIR)/mvpg-sie.elf
>
> tests_binary = $(patsubst %.elf,%.bin,$(tests))
> ifneq ($(HOST_KEY_DOCUMENT),)
> @@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
> SNIPPET_DIR = $(TEST_DIR)/snippets
>
> # C snippets that need to be linked
> -snippets-c =
> +snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>
> # ASM snippets that are directly compiled and converted to a *.gbin
> snippets-a =
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> new file mode 100644
> index 00000000..a617704b
> --- /dev/null
> +++ b/s390x/mvpg-sie.c
> @@ -0,0 +1,139 @@
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm-generic/barrier.h>
> +#include <asm/interrupt.h>
> +#include <asm/pgtable.h>
> +#include <mmu.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +#include <asm/mem.h>
> +#include <asm/sigp.h>
> +#include <smp.h>
> +#include <alloc_page.h>
> +#include <bitops.h>
> +#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;
> +
> +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 handle_validity(struct vm *vm)
> +{
> + report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
I think an assert would be better. This should not happen, and if it
happens something went very wrong and we have no guarantee that we will
be able to continue
> +}
> +
> +static void sie(struct vm *vm)
> +{
> + /* Reset icptcode so we don't trip below */
> + vm->sblk->icptcode = 0;
> +
> + while (vm->sblk->icptcode == 0) {
> + sie64a(vm->sblk, &vm->save_area);
> + if (vm->sblk->icptcode == ICPT_VALIDITY)
> + 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);
> + uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk +
> 0xc8); +
> + report_prefix_push("pei");
maybe clear the destination buffer...
> + protect_page(guest + 0x6000, PAGE_ENTRY_I);
> + sie(&vm);
> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
> execution");
> + report((uintptr_t)**pei_src == ((uintptr_t)vm.sblk->mso) +
> 0x6000 + PAGE_ENTRY_I, "PEI_SRC correct");
> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000,
> "PEI_DST correct");
... and check that the page was not copied
> + /* Jump over the diag44 */
> + sie(&vm);
I would check if you really got a diag44
> + /* Clear PEI data for next check */
> + memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
> + unprotect_page(guest + 0x6000, PAGE_ENTRY_I);
> + protect_page(guest + 0x5000, PAGE_ENTRY_I);
> + sie(&vm);
> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
> execution");
> + report((uintptr_t)**pei_src == vm.sblk->mso + 0x6000,
> "PEI_SRC correct");
> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000 +
> PAGE_ENTRY_I, "PEI_DST correct"); +
> + 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
or maybe you can clear the destination buffer here, if you prefer
> 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;
> +}
> +
> +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 00000000..96b70c9c
> --- /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 9f81a608..8634b1b1 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -103,3 +103,6 @@ file = sie.elf
> [mvpg]
> file = mvpg.elf
> timeout = 10
> +
> +[mvpg-sie]
> +file = mvpg-sie.elf
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-05-25 16:44 ` Claudio Imbrenda
@ 2021-05-26 10:12 ` Janosch Frank
0 siblings, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-26 10:12 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, thuth, cohuck
On 5/25/21 6:44 PM, Claudio Imbrenda wrote:
> On Thu, 20 May 2021 09:47:29 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> Snippets can be used to easily write and run guest (SIE) tests.
>> The snippet is linked into the test binaries and can therefore be
>> accessed via a ptr.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> .gitignore | 2 ++
>> s390x/Makefile | 28 ++++++++++++++++++---
>> s390x/snippets/c/cstart.S | 13 ++++++++++
>> s390x/snippets/c/flat.lds | 51
>> +++++++++++++++++++++++++++++++++++++++ 4 files changed, 91
>> insertions(+), 3 deletions(-) create mode 100644
>> s390x/snippets/c/cstart.S create mode 100644 s390x/snippets/c/flat.lds
>>
>> diff --git a/.gitignore b/.gitignore
>> index 784cb2dd..29d3635b 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -22,3 +22,5 @@ cscope.*
>> /api/dirty-log
>> /api/dirty-log-perf
>> /s390x/*.bin
>> +/s390x/snippets/*/*.bin
>> +/s390x/snippets/*/*.gbin
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 8de926ab..fe267011 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>
>> FLATLIBS = $(libcflat)
>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>> +
>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>> +
>> +# C snippets that need to be linked
>> +snippets-c =
>> +
>> +# ASM snippets that are directly compiled and converted to a *.gbin
>> +snippets-a =
>> +
>> +snippets = $(snippets-a)$(snippets-c)
> ↑↑
> I'm not a Makefile expert, but, don't you need a space between the two
> variable expansions?
>
Yup, I already fixed that one
>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>> +
>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>> +
>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>> + $(filter %.o, $^) $(FLATLIBS)
>> + $(OBJCOPY) -O binary $@ $@
>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>> +
>> +%.elf: $(snippets) %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>
> I would keep the %.o as the first in the list>
>> $(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)
>> + $(filter %.o, $^) $(FLATLIBS) $(snippets)
>
> so all the snippets are always baked in every test?
Currently yes.
Do you have better ideas?
>
>> $(@:.elf=.aux.o) $(RM) $(@:.elf=.aux.o)
>> @chmod a-x $@
>>
>> @@ -93,7 +115,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}
>> $(SNIPPET_DIR)/c/*.{o,elf,bin,gbin} $(SNIPPET_DIR)/.*.d
>> $(TEST_DIR)/.*.d lib/s390x/.*.d generated-files = $(asm-offsets)
>> $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>> new file mode 100644
>> index 00000000..02a3338b
>> --- /dev/null
>> +++ b/s390x/snippets/c/cstart.S
>> @@ -0,0 +1,13 @@
>> +#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
>> + brasl %r14, main
>> + sigp %r1, %r0, SIGP_STOP
>> diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
>> new file mode 100644
>> index 00000000..5e707325
>> --- /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 0x10000 (cstart64.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);
>> +}
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-05-25 17:37 ` Claudio Imbrenda
@ 2021-05-26 10:17 ` Janosch Frank
2021-05-27 14:35 ` Janosch Frank
1 sibling, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-26 10:17 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, thuth, cohuck
On 5/25/21 7:37 PM, Claudio Imbrenda wrote:
> On Thu, 20 May 2021 09:47:30 +0000
> Janosch Frank <frankja@linux.ibm.com> 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 | 3 +-
>> s390x/mvpg-sie.c | 139
>> ++++++++++++++++++++++++++++++++ s390x/snippets/c/mvpg-snippet.c |
>> 33 ++++++++ s390x/unittests.cfg | 3 +
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> 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 fe267011..6692cf73 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
>> tests += $(TEST_DIR)/sie.elf
>> tests += $(TEST_DIR)/mvpg.elf
>> tests += $(TEST_DIR)/uv-host.elf
>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>
>> tests_binary = $(patsubst %.elf,%.bin,$(tests))
>> ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
>> SNIPPET_DIR = $(TEST_DIR)/snippets
>>
>> # C snippets that need to be linked
>> -snippets-c =
>> +snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>>
>> # ASM snippets that are directly compiled and converted to a *.gbin
>> snippets-a =
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> new file mode 100644
>> index 00000000..a617704b
>> --- /dev/null
>> +++ b/s390x/mvpg-sie.c
>> @@ -0,0 +1,139 @@
>> +#include <libcflat.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm-generic/barrier.h>
>> +#include <asm/interrupt.h>
>> +#include <asm/pgtable.h>
>> +#include <mmu.h>
>> +#include <asm/page.h>
>> +#include <asm/facility.h>
>> +#include <asm/mem.h>
>> +#include <asm/sigp.h>
>> +#include <smp.h>
>> +#include <alloc_page.h>
>> +#include <bitops.h>
>> +#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;
>> +
>> +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 handle_validity(struct vm *vm)
>> +{
>> + report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
>
> I think an assert would be better. This should not happen, and if it
> happens something went very wrong and we have no guarantee that we will
> be able to continue
That depends on the test. If you use multiple gbins/snippets in a test,
then a validity for one of those does not necessarily break the others.
Especially when you think about PV tests where the destroy will clean
out all the state anyway.
>
>> +}
>> +
>> +static void sie(struct vm *vm)
>> +{
>> + /* Reset icptcode so we don't trip below */
>> + vm->sblk->icptcode = 0;
>> +
>> + while (vm->sblk->icptcode == 0) {
>> + sie64a(vm->sblk, &vm->save_area);
>> + if (vm->sblk->icptcode == ICPT_VALIDITY)
>> + 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);
>> + uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk +
>> 0xc8); +
>> + report_prefix_push("pei");
>
> maybe clear the destination buffer...
>
>> + protect_page(guest + 0x6000, PAGE_ENTRY_I);
>> + sie(&vm);
>> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
>> execution");
>> + report((uintptr_t)**pei_src == ((uintptr_t)vm.sblk->mso) +
>> 0x6000 + PAGE_ENTRY_I, "PEI_SRC correct");
>> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000,
>> "PEI_DST correct");
>
> ... and check that the page was not copied
Sure
>
>> + /* Jump over the diag44 */
>> + sie(&vm);
>
> I would check if you really got a diag44
Sure, reporting doesn't make sense to me but an assert should be fine
>
>> + /* Clear PEI data for next check */
>> + memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
>> + unprotect_page(guest + 0x6000, PAGE_ENTRY_I);
>> + protect_page(guest + 0x5000, PAGE_ENTRY_I);
>> + sie(&vm);
>> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
>> execution");
>> + report((uintptr_t)**pei_src == vm.sblk->mso + 0x6000,
>> "PEI_SRC correct");
>> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000 +
>> PAGE_ENTRY_I, "PEI_DST correct"); +
>> + 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
>
> or maybe you can clear the destination buffer here, if you prefer
I'll have a look, thanks for the review!
>
>> 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;
>> +}
>> +
>> +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 00000000..96b70c9c
>> --- /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 9f81a608..8634b1b1 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -103,3 +103,6 @@ file = sie.elf
>> [mvpg]
>> file = mvpg.elf
>> timeout = 10
>> +
>> +[mvpg-sie]
>> +file = mvpg-sie.elf
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-05-25 17:37 ` Claudio Imbrenda
2021-05-26 10:17 ` Janosch Frank
@ 2021-05-27 14:35 ` Janosch Frank
1 sibling, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2021-05-27 14:35 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, thuth, cohuck
On 5/25/21 7:37 PM, Claudio Imbrenda wrote:
> On Thu, 20 May 2021 09:47:30 +0000
> Janosch Frank <frankja@linux.ibm.com> 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 | 3 +-
>> s390x/mvpg-sie.c | 139
>> ++++++++++++++++++++++++++++++++ s390x/snippets/c/mvpg-snippet.c |
>> 33 ++++++++ s390x/unittests.cfg | 3 +
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> 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 fe267011..6692cf73 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
>> tests += $(TEST_DIR)/sie.elf
>> tests += $(TEST_DIR)/mvpg.elf
>> tests += $(TEST_DIR)/uv-host.elf
>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>
>> tests_binary = $(patsubst %.elf,%.bin,$(tests))
>> ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
>> SNIPPET_DIR = $(TEST_DIR)/snippets
>>
>> # C snippets that need to be linked
>> -snippets-c =
>> +snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>>
>> # ASM snippets that are directly compiled and converted to a *.gbin
>> snippets-a =
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> new file mode 100644
>> index 00000000..a617704b
>> --- /dev/null
>> +++ b/s390x/mvpg-sie.c
>> @@ -0,0 +1,139 @@
>> +#include <libcflat.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm-generic/barrier.h>
>> +#include <asm/interrupt.h>
>> +#include <asm/pgtable.h>
>> +#include <mmu.h>
>> +#include <asm/page.h>
>> +#include <asm/facility.h>
>> +#include <asm/mem.h>
>> +#include <asm/sigp.h>
>> +#include <smp.h>
>> +#include <alloc_page.h>
>> +#include <bitops.h>
>> +#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;
>> +
>> +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 handle_validity(struct vm *vm)
>> +{
>> + report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
>
> I think an assert would be better. This should not happen, and if it
> happens something went very wrong and we have no guarantee that we will
> be able to continue
>
>> +}
>> +
>> +static void sie(struct vm *vm)
>> +{
>> + /* Reset icptcode so we don't trip below */
>> + vm->sblk->icptcode = 0;
>> +
>> + while (vm->sblk->icptcode == 0) {
>> + sie64a(vm->sblk, &vm->save_area);
>> + if (vm->sblk->icptcode == ICPT_VALIDITY)
>> + 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);
>> + uint64_t **pei_src = (uint64_t **)((uintptr_t) vm.sblk +
>> 0xc8); +
>> + report_prefix_push("pei");
>
> maybe clear the destination buffer...
>
>> + protect_page(guest + 0x6000, PAGE_ENTRY_I);
>> + sie(&vm);
>> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
>> execution");
>> + report((uintptr_t)**pei_src == ((uintptr_t)vm.sblk->mso) +
>> 0x6000 + PAGE_ENTRY_I, "PEI_SRC correct");
>> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000,
>> "PEI_DST correct");
>
> ... and check that the page was not copied
>
>> + /* Jump over the diag44 */
>> + sie(&vm);
>
> I would check if you really got a diag44
>
>> + /* Clear PEI data for next check */
>> + memset((uint64_t *)((uintptr_t) vm.sblk + 0xc0), 0, 16);
>> + unprotect_page(guest + 0x6000, PAGE_ENTRY_I);
I'll also move these over to src/dst so we only use the magic constants
once.
>> + protect_page(guest + 0x5000, PAGE_ENTRY_I);
>> + sie(&vm);
>> + report(vm.sblk->icptcode == ICPT_PARTEXEC, "Partial
>> execution");
>> + report((uintptr_t)**pei_src == vm.sblk->mso + 0x6000,
>> "PEI_SRC correct");
>> + report((uintptr_t)**pei_dst == vm.sblk->mso + 0x5000 +
>> PAGE_ENTRY_I, "PEI_DST correct"); +
>> + 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
>
> or maybe you can clear the destination buffer here, if you prefer
>
>> 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;
>> +}
>> +
>> +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 00000000..96b70c9c
>> --- /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 9f81a608..8634b1b1 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -103,3 +103,6 @@ file = sie.elf
>> [mvpg]
>> file = mvpg.elf
>> timeout = 10
>> +
>> +[mvpg-sie]
>> +file = mvpg-sie.elf
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
2021-05-25 16:44 ` Claudio Imbrenda
@ 2021-06-21 10:10 ` Thomas Huth
2021-06-21 12:19 ` Janosch Frank
1 sibling, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2021-06-21 10:10 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 20/05/2021 11.47, Janosch Frank wrote:
> Snippets can be used to easily write and run guest (SIE) tests.
> The snippet is linked into the test binaries and can therefore be
> accessed via a ptr.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> .gitignore | 2 ++
> s390x/Makefile | 28 ++++++++++++++++++---
> s390x/snippets/c/cstart.S | 13 ++++++++++
> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 91 insertions(+), 3 deletions(-)
> create mode 100644 s390x/snippets/c/cstart.S
> create mode 100644 s390x/snippets/c/flat.lds
>
> diff --git a/.gitignore b/.gitignore
> index 784cb2dd..29d3635b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -22,3 +22,5 @@ cscope.*
> /api/dirty-log
> /api/dirty-log-perf
> /s390x/*.bin
> +/s390x/snippets/*/*.bin
> +/s390x/snippets/*/*.gbin
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 8de926ab..fe267011 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>
> FLATLIBS = $(libcflat)
> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
> +
> +SNIPPET_DIR = $(TEST_DIR)/snippets
> +
> +# C snippets that need to be linked
> +snippets-c =
> +
> +# ASM snippets that are directly compiled and converted to a *.gbin
> +snippets-a =
Could you please call this snippets-s instead of ...-a ? The -a suffix looks
like an archive to me otherwise.
> +snippets = $(snippets-a)$(snippets-c)
Shouldn't there be a space between the two?
> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
> +
> +$(snippets-a): $(snippets-o) $(FLATLIBS)
> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
> +
> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
> + $(filter %.o, $^) $(FLATLIBS)
> + $(OBJCOPY) -O binary $@ $@
> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
> +
> +%.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) $(@:.elf=.aux.o)
> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
Does this link the snippets into all elf files? ... wouldn't it be better to
restrict it somehow to the files that really need them?
> $(RM) $(@:.elf=.aux.o)
> @chmod a-x $@
>
> @@ -93,7 +115,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} $(SNIPPET_DIR)/c/*.{o,elf,bin,gbin} $(SNIPPET_DIR)/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
>
> generated-files = $(asm-offsets)
> $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> new file mode 100644
> index 00000000..02a3338b
> --- /dev/null
> +++ b/s390x/snippets/c/cstart.S
> @@ -0,0 +1,13 @@
> +#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
> + brasl %r14, main
> + sigp %r1, %r0, SIGP_STOP
I think you should clear r0 before using it here?
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-05-20 9:47 ` [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test Janosch Frank
2021-05-25 17:37 ` Claudio Imbrenda
@ 2021-06-21 10:23 ` Thomas Huth
2021-06-21 12:41 ` Janosch Frank
1 sibling, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2021-06-21 10:23 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 20/05/2021 11.47, 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 | 3 +-
> s390x/mvpg-sie.c | 139 ++++++++++++++++++++++++++++++++
> s390x/snippets/c/mvpg-snippet.c | 33 ++++++++
> s390x/unittests.cfg | 3 +
> 4 files changed, 177 insertions(+), 1 deletion(-)
> 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 fe267011..6692cf73 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
> tests += $(TEST_DIR)/sie.elf
> tests += $(TEST_DIR)/mvpg.elf
> tests += $(TEST_DIR)/uv-host.elf
> +tests += $(TEST_DIR)/mvpg-sie.elf
>
> tests_binary = $(patsubst %.elf,%.bin,$(tests))
> ifneq ($(HOST_KEY_DOCUMENT),)
> @@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
> SNIPPET_DIR = $(TEST_DIR)/snippets
> report_abort
> # C snippets that need to be linked
> -snippets-c =
> +snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>
> # ASM snippets that are directly compiled and converted to a *.gbin
> snippets-a =
> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
> new file mode 100644
> index 00000000..a617704b
> --- /dev/nullreport_abort
> +++ b/s390x/mvpg-sie.c
> @@ -0,0 +1,139 @@
> +#include <libcflat.h>
> +#include <asm/asm-offsets.h>
> +#include <asm-generic/barrier.h>
> +#include <asm/interrupt.h>
> +#include <asm/pgtable.h>
> +#include <mmu.h>
> +#include <asm/page.h>
> +#include <asm/facility.h>
> +#include <asm/mem.h>
> +#include <asm/sigp.h>
> +#include <smp.h>
> +#include <alloc_page.h>
> +#include <bitops.h>
> +#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;
> +
> +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 handle_validity(struct vm *vm)
> +{
> + report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
> +}
Maybe rather use report_abort() in that case? Or does it make sense to
continue running the other tests afterwards?
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 10:10 ` Thomas Huth
@ 2021-06-21 12:19 ` Janosch Frank
2021-06-21 12:32 ` Thomas Huth
0 siblings, 1 reply; 18+ messages in thread
From: Janosch Frank @ 2021-06-21 12:19 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 6/21/21 12:10 PM, Thomas Huth wrote:
> On 20/05/2021 11.47, Janosch Frank wrote:
>> Snippets can be used to easily write and run guest (SIE) tests.
>> The snippet is linked into the test binaries and can therefore be
>> accessed via a ptr.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> .gitignore | 2 ++
>> s390x/Makefile | 28 ++++++++++++++++++---
>> s390x/snippets/c/cstart.S | 13 ++++++++++
>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 91 insertions(+), 3 deletions(-)
>> create mode 100644 s390x/snippets/c/cstart.S
>> create mode 100644 s390x/snippets/c/flat.lds
>>
>> diff --git a/.gitignore b/.gitignore
>> index 784cb2dd..29d3635b 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -22,3 +22,5 @@ cscope.*
>> /api/dirty-log
>> /api/dirty-log-perf
>> /s390x/*.bin
>> +/s390x/snippets/*/*.bin
>> +/s390x/snippets/*/*.gbin
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 8de926ab..fe267011 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>
>> FLATLIBS = $(libcflat)
>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>> +
>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>> +
>> +# C snippets that need to be linked
>> +snippets-c =
>> +
>> +# ASM snippets that are directly compiled and converted to a *.gbin
>> +snippets-a =
>
> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
> like an archive to me otherwise.
Sure
>
>> +snippets = $(snippets-a)$(snippets-c)
>
> Shouldn't there be a space between the two?
Yes, already fixed that a long while ago
I thought I had sent out a new version already, maybe that was an
illusion as I can't seem to find it right now.
>
>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>> +
>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>> +
>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>> + $(filter %.o, $^) $(FLATLIBS)
>> + $(OBJCOPY) -O binary $@ $@
>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>> +
>> +%.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) $(@:.elf=.aux.o)
>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>
> Does this link the snippets into all elf files? ... wouldn't it be better to
> restrict it somehow to the files that really need them?
Yes it does.
I'd like to avoid having to specify a makefile rule for every test that
uses snippets as we already have more than the mvpg one in the queue.
So I'm having Steffen looking into a solution for this problem. My first
idea was to bring the used snippets into the unittests.cfg but I
disliked that we then would have compile instructions in another file.
Maybe there's a way to include that into the makefile in a clever way?
>
>> $(RM) $(@:.elf=.aux.o)
>> @chmod a-x $@
>>
>> @@ -93,7 +115,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} $(SNIPPET_DIR)/c/*.{o,elf,bin,gbin} $(SNIPPET_DIR)/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
>>
>> generated-files = $(asm-offsets)
>> $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>> new file mode 100644
>> index 00000000..02a3338b
>> --- /dev/null
>> +++ b/s390x/snippets/c/cstart.S
>> @@ -0,0 +1,13 @@
>> +#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
>> + brasl %r14, main
>> + sigp %r1, %r0, SIGP_STOP
>
> I think you should clear r0 before using it here?
Right. will fix
>
> Thomas
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 12:19 ` Janosch Frank
@ 2021-06-21 12:32 ` Thomas Huth
2021-06-21 12:39 ` Janosch Frank
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2021-06-21 12:32 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 21/06/2021 14.19, Janosch Frank wrote:
> On 6/21/21 12:10 PM, Thomas Huth wrote:
>> On 20/05/2021 11.47, Janosch Frank wrote:
>>> Snippets can be used to easily write and run guest (SIE) tests.
>>> The snippet is linked into the test binaries and can therefore be
>>> accessed via a ptr.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>> .gitignore | 2 ++
>>> s390x/Makefile | 28 ++++++++++++++++++---
>>> s390x/snippets/c/cstart.S | 13 ++++++++++
>>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>> create mode 100644 s390x/snippets/c/cstart.S
>>> create mode 100644 s390x/snippets/c/flat.lds
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 784cb2dd..29d3635b 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -22,3 +22,5 @@ cscope.*
>>> /api/dirty-log
>>> /api/dirty-log-perf
>>> /s390x/*.bin
>>> +/s390x/snippets/*/*.bin
>>> +/s390x/snippets/*/*.gbin
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index 8de926ab..fe267011 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>>
>>> FLATLIBS = $(libcflat)
>>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>> +
>>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>>> +
>>> +# C snippets that need to be linked
>>> +snippets-c =
>>> +
>>> +# ASM snippets that are directly compiled and converted to a *.gbin
>>> +snippets-a =
>>
>> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
>> like an archive to me otherwise.
>
> Sure
>
>>
>>> +snippets = $(snippets-a)$(snippets-c)
>>
>> Shouldn't there be a space between the two?
>
> Yes, already fixed that a long while ago
> I thought I had sent out a new version already, maybe that was an
> illusion as I can't seem to find it right now.
>
>>
>>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>>> +
>>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>> +
>>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>>> + $(filter %.o, $^) $(FLATLIBS)
>>> + $(OBJCOPY) -O binary $@ $@
>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>> +
>>> +%.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) $(@:.elf=.aux.o)
>>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>
>> Does this link the snippets into all elf files? ... wouldn't it be better to
>> restrict it somehow to the files that really need them?
>
> Yes it does.
> I'd like to avoid having to specify a makefile rule for every test that
> uses snippets as we already have more than the mvpg one in the queue.
>
> So I'm having Steffen looking into a solution for this problem. My first
> idea was to bring the used snippets into the unittests.cfg but I
> disliked that we then would have compile instructions in another file.
> Maybe there's a way to include that into the makefile in a clever way?
I haven't tried, but maybe you could replace the $(snippets) in the last
line with
$(wildcard snippets/$@.gbin)
or something similar?
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 12:32 ` Thomas Huth
@ 2021-06-21 12:39 ` Janosch Frank
2021-06-21 13:28 ` Thomas Huth
0 siblings, 1 reply; 18+ messages in thread
From: Janosch Frank @ 2021-06-21 12:39 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 6/21/21 2:32 PM, Thomas Huth wrote:
> On 21/06/2021 14.19, Janosch Frank wrote:
>> On 6/21/21 12:10 PM, Thomas Huth wrote:
>>> On 20/05/2021 11.47, Janosch Frank wrote:
>>>> Snippets can be used to easily write and run guest (SIE) tests.
>>>> The snippet is linked into the test binaries and can therefore be
>>>> accessed via a ptr.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>> .gitignore | 2 ++
>>>> s390x/Makefile | 28 ++++++++++++++++++---
>>>> s390x/snippets/c/cstart.S | 13 ++++++++++
>>>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>>> create mode 100644 s390x/snippets/c/cstart.S
>>>> create mode 100644 s390x/snippets/c/flat.lds
>>>>
>>>> diff --git a/.gitignore b/.gitignore
>>>> index 784cb2dd..29d3635b 100644
>>>> --- a/.gitignore
>>>> +++ b/.gitignore
>>>> @@ -22,3 +22,5 @@ cscope.*
>>>> /api/dirty-log
>>>> /api/dirty-log-perf
>>>> /s390x/*.bin
>>>> +/s390x/snippets/*/*.bin
>>>> +/s390x/snippets/*/*.gbin
>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>> index 8de926ab..fe267011 100644
>>>> --- a/s390x/Makefile
>>>> +++ b/s390x/Makefile
>>>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>>>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>>>
>>>> FLATLIBS = $(libcflat)
>>>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>>> +
>>>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>>>> +
>>>> +# C snippets that need to be linked
>>>> +snippets-c =
>>>> +
>>>> +# ASM snippets that are directly compiled and converted to a *.gbin
>>>> +snippets-a =
>>>
>>> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
>>> like an archive to me otherwise.
>>
>> Sure
>>
>>>
>>>> +snippets = $(snippets-a)$(snippets-c)
>>>
>>> Shouldn't there be a space between the two?
>>
>> Yes, already fixed that a long while ago
>> I thought I had sent out a new version already, maybe that was an
>> illusion as I can't seem to find it right now.
>>
>>>
>>>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>>>> +
>>>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>>>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>> +
>>>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>>>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>>>> + $(filter %.o, $^) $(FLATLIBS)
>>>> + $(OBJCOPY) -O binary $@ $@
>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>> +
>>>> +%.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) $(@:.elf=.aux.o)
>>>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>>
>>> Does this link the snippets into all elf files? ... wouldn't it be better to
>>> restrict it somehow to the files that really need them?
>>
>> Yes it does.
>> I'd like to avoid having to specify a makefile rule for every test that
>> uses snippets as we already have more than the mvpg one in the queue.
>>
>> So I'm having Steffen looking into a solution for this problem. My first
>> idea was to bring the used snippets into the unittests.cfg but I
>> disliked that we then would have compile instructions in another file.
>> Maybe there's a way to include that into the makefile in a clever way?
>
> I haven't tried, but maybe you could replace the $(snippets) in the last
> line with
>
> $(wildcard snippets/$@.gbin)
>
> or something similar?
That starts falling apart when multiple tests use the same snippet, no?
>
> Thomas
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test
2021-06-21 10:23 ` Thomas Huth
@ 2021-06-21 12:41 ` Janosch Frank
0 siblings, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2021-06-21 12:41 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 6/21/21 12:23 PM, Thomas Huth wrote:
> On 20/05/2021 11.47, 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 | 3 +-
>> s390x/mvpg-sie.c | 139 ++++++++++++++++++++++++++++++++
>> s390x/snippets/c/mvpg-snippet.c | 33 ++++++++
>> s390x/unittests.cfg | 3 +
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> 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 fe267011..6692cf73 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -22,6 +22,7 @@ tests += $(TEST_DIR)/uv-guest.elf
>> tests += $(TEST_DIR)/sie.elf
>> tests += $(TEST_DIR)/mvpg.elf
>> tests += $(TEST_DIR)/uv-host.elf
>> +tests += $(TEST_DIR)/mvpg-sie.elf
>>
>> tests_binary = $(patsubst %.elf,%.bin,$(tests))
>> ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -79,7 +80,7 @@ FLATLIBS = $(libcflat)
>> SNIPPET_DIR = $(TEST_DIR)/snippets
>> report_abort
>> # C snippets that need to be linked
>> -snippets-c =
>> +snippets-c = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>>
>> # ASM snippets that are directly compiled and converted to a *.gbin
>> snippets-a =
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> new file mode 100644
>> index 00000000..a617704b
>> --- /dev/nullreport_abort
>> +++ b/s390x/mvpg-sie.c
>> @@ -0,0 +1,139 @@
>> +#include <libcflat.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm-generic/barrier.h>
>> +#include <asm/interrupt.h>
>> +#include <asm/pgtable.h>
>> +#include <mmu.h>
>> +#include <asm/page.h>
>> +#include <asm/facility.h>
>> +#include <asm/mem.h>
>> +#include <asm/sigp.h>
>> +#include <smp.h>
>> +#include <alloc_page.h>
>> +#include <bitops.h>
>> +#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;
>> +
>> +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 handle_validity(struct vm *vm)
>> +{
>> + report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
>> +}
>
> Maybe rather use report_abort() in that case? Or does it make sense to
> continue running the other tests afterwards?
>
> Thomas
>
Hmm, right this is not yet in the SIE lib.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 12:39 ` Janosch Frank
@ 2021-06-21 13:28 ` Thomas Huth
2021-06-21 14:42 ` Janosch Frank
0 siblings, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2021-06-21 13:28 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 21/06/2021 14.39, Janosch Frank wrote:
> On 6/21/21 2:32 PM, Thomas Huth wrote:
>> On 21/06/2021 14.19, Janosch Frank wrote:
>>> On 6/21/21 12:10 PM, Thomas Huth wrote:
>>>> On 20/05/2021 11.47, Janosch Frank wrote:
>>>>> Snippets can be used to easily write and run guest (SIE) tests.
>>>>> The snippet is linked into the test binaries and can therefore be
>>>>> accessed via a ptr.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>> ---
>>>>> .gitignore | 2 ++
>>>>> s390x/Makefile | 28 ++++++++++++++++++---
>>>>> s390x/snippets/c/cstart.S | 13 ++++++++++
>>>>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>>>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>>>> create mode 100644 s390x/snippets/c/cstart.S
>>>>> create mode 100644 s390x/snippets/c/flat.lds
>>>>>
>>>>> diff --git a/.gitignore b/.gitignore
>>>>> index 784cb2dd..29d3635b 100644
>>>>> --- a/.gitignore
>>>>> +++ b/.gitignore
>>>>> @@ -22,3 +22,5 @@ cscope.*
>>>>> /api/dirty-log
>>>>> /api/dirty-log-perf
>>>>> /s390x/*.bin
>>>>> +/s390x/snippets/*/*.bin
>>>>> +/s390x/snippets/*/*.gbin
>>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>>> index 8de926ab..fe267011 100644
>>>>> --- a/s390x/Makefile
>>>>> +++ b/s390x/Makefile
>>>>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>>>>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>>>>
>>>>> FLATLIBS = $(libcflat)
>>>>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>>>> +
>>>>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>>>>> +
>>>>> +# C snippets that need to be linked
>>>>> +snippets-c =
>>>>> +
>>>>> +# ASM snippets that are directly compiled and converted to a *.gbin
>>>>> +snippets-a =
>>>>
>>>> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
>>>> like an archive to me otherwise.
>>>
>>> Sure
>>>
>>>>
>>>>> +snippets = $(snippets-a)$(snippets-c)
>>>>
>>>> Shouldn't there be a space between the two?
>>>
>>> Yes, already fixed that a long while ago
>>> I thought I had sent out a new version already, maybe that was an
>>> illusion as I can't seem to find it right now.
>>>
>>>>
>>>>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>>>>> +
>>>>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>>>>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>> +
>>>>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>>>>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>>>>> + $(filter %.o, $^) $(FLATLIBS)
>>>>> + $(OBJCOPY) -O binary $@ $@
>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>> +
>>>>> +%.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) $(@:.elf=.aux.o)
>>>>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>>>
>>>> Does this link the snippets into all elf files? ... wouldn't it be better to
>>>> restrict it somehow to the files that really need them?
>>>
>>> Yes it does.
>>> I'd like to avoid having to specify a makefile rule for every test that
>>> uses snippets as we already have more than the mvpg one in the queue.
>>>
>>> So I'm having Steffen looking into a solution for this problem. My first
>>> idea was to bring the used snippets into the unittests.cfg but I
>>> disliked that we then would have compile instructions in another file.
>>> Maybe there's a way to include that into the makefile in a clever way?
>>
>> I haven't tried, but maybe you could replace the $(snippets) in the last
>> line with
>>
>> $(wildcard snippets/$@.gbin)
>>
>> or something similar?
>
> That starts falling apart when multiple tests use the same snippet, no?
That's true ... Maybe something like:
$(filter %.gbin,$^)
?
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 13:28 ` Thomas Huth
@ 2021-06-21 14:42 ` Janosch Frank
2021-06-21 14:59 ` Thomas Huth
0 siblings, 1 reply; 18+ messages in thread
From: Janosch Frank @ 2021-06-21 14:42 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 6/21/21 3:28 PM, Thomas Huth wrote:
> On 21/06/2021 14.39, Janosch Frank wrote:
>> On 6/21/21 2:32 PM, Thomas Huth wrote:
>>> On 21/06/2021 14.19, Janosch Frank wrote:
>>>> On 6/21/21 12:10 PM, Thomas Huth wrote:
>>>>> On 20/05/2021 11.47, Janosch Frank wrote:
>>>>>> Snippets can be used to easily write and run guest (SIE) tests.
>>>>>> The snippet is linked into the test binaries and can therefore be
>>>>>> accessed via a ptr.
>>>>>>
>>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>>> ---
>>>>>> .gitignore | 2 ++
>>>>>> s390x/Makefile | 28 ++++++++++++++++++---
>>>>>> s390x/snippets/c/cstart.S | 13 ++++++++++
>>>>>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>>>>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>>>>> create mode 100644 s390x/snippets/c/cstart.S
>>>>>> create mode 100644 s390x/snippets/c/flat.lds
>>>>>>
>>>>>> diff --git a/.gitignore b/.gitignore
>>>>>> index 784cb2dd..29d3635b 100644
>>>>>> --- a/.gitignore
>>>>>> +++ b/.gitignore
>>>>>> @@ -22,3 +22,5 @@ cscope.*
>>>>>> /api/dirty-log
>>>>>> /api/dirty-log-perf
>>>>>> /s390x/*.bin
>>>>>> +/s390x/snippets/*/*.bin
>>>>>> +/s390x/snippets/*/*.gbin
>>>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>>>> index 8de926ab..fe267011 100644
>>>>>> --- a/s390x/Makefile
>>>>>> +++ b/s390x/Makefile
>>>>>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>>>>>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>>>>>
>>>>>> FLATLIBS = $(libcflat)
>>>>>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>>>>> +
>>>>>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>>>>>> +
>>>>>> +# C snippets that need to be linked
>>>>>> +snippets-c =
>>>>>> +
>>>>>> +# ASM snippets that are directly compiled and converted to a *.gbin
>>>>>> +snippets-a =
>>>>>
>>>>> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
>>>>> like an archive to me otherwise.
>>>>
>>>> Sure
>>>>
>>>>>
>>>>>> +snippets = $(snippets-a)$(snippets-c)
>>>>>
>>>>> Shouldn't there be a space between the two?
>>>>
>>>> Yes, already fixed that a long while ago
>>>> I thought I had sent out a new version already, maybe that was an
>>>> illusion as I can't seem to find it right now.
>>>>
>>>>>
>>>>>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>>>>>> +
>>>>>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>>>>>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>>> +
>>>>>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>>>>>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>>>>>> + $(filter %.o, $^) $(FLATLIBS)
>>>>>> + $(OBJCOPY) -O binary $@ $@
>>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>>> +
>>>>>> +%.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) $(@:.elf=.aux.o)
>>>>>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>>>>
>>>>> Does this link the snippets into all elf files? ... wouldn't it be better to
>>>>> restrict it somehow to the files that really need them?
>>>>
>>>> Yes it does.
>>>> I'd like to avoid having to specify a makefile rule for every test that
>>>> uses snippets as we already have more than the mvpg one in the queue.
>>>>
>>>> So I'm having Steffen looking into a solution for this problem. My first
>>>> idea was to bring the used snippets into the unittests.cfg but I
>>>> disliked that we then would have compile instructions in another file.
>>>> Maybe there's a way to include that into the makefile in a clever way?
>>>
>>> I haven't tried, but maybe you could replace the $(snippets) in the last
>>> line with
>>>
>>> $(wildcard snippets/$@.gbin)
>>>
>>> or something similar?
>>
>> That starts falling apart when multiple tests use the same snippet, no?
>
> That's true ... Maybe something like:
>
> $(filter %.gbin,$^)
That filters all files that are not gbins from the prereqs, right?
In what way is that different to linking all the snippets? After all
they are currently a prereq for %.elf or am I missing something here?
Just to expand our use-case a bit more with an example since the code is
still too rough to post right now:
We test diagnose handling for Secure Execution / PV in one host test
where we have a gbin each for most diagnose codes 0x44, 0x9c, 0x288,
0x500 (0x308 needs a whole test and possible multiple gbins on its own
due to its complexity).
Other test bundles are PV IO, misc (stsi, sthyi, ...) and of course
other VSIE instruction handling tests in the future.
>
> ?
>
> Thomas
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] s390x: Add guest snippet support
2021-06-21 14:42 ` Janosch Frank
@ 2021-06-21 14:59 ` Thomas Huth
0 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2021-06-21 14:59 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck
On 21/06/2021 16.42, Janosch Frank wrote:
> On 6/21/21 3:28 PM, Thomas Huth wrote:
>> On 21/06/2021 14.39, Janosch Frank wrote:
>>> On 6/21/21 2:32 PM, Thomas Huth wrote:
>>>> On 21/06/2021 14.19, Janosch Frank wrote:
>>>>> On 6/21/21 12:10 PM, Thomas Huth wrote:
>>>>>> On 20/05/2021 11.47, Janosch Frank wrote:
>>>>>>> Snippets can be used to easily write and run guest (SIE) tests.
>>>>>>> The snippet is linked into the test binaries and can therefore be
>>>>>>> accessed via a ptr.
>>>>>>>
>>>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>>>> ---
>>>>>>> .gitignore | 2 ++
>>>>>>> s390x/Makefile | 28 ++++++++++++++++++---
>>>>>>> s390x/snippets/c/cstart.S | 13 ++++++++++
>>>>>>> s390x/snippets/c/flat.lds | 51 +++++++++++++++++++++++++++++++++++++++
>>>>>>> 4 files changed, 91 insertions(+), 3 deletions(-)
>>>>>>> create mode 100644 s390x/snippets/c/cstart.S
>>>>>>> create mode 100644 s390x/snippets/c/flat.lds
>>>>>>>
>>>>>>> diff --git a/.gitignore b/.gitignore
>>>>>>> index 784cb2dd..29d3635b 100644
>>>>>>> --- a/.gitignore
>>>>>>> +++ b/.gitignore
>>>>>>> @@ -22,3 +22,5 @@ cscope.*
>>>>>>> /api/dirty-log
>>>>>>> /api/dirty-log-perf
>>>>>>> /s390x/*.bin
>>>>>>> +/s390x/snippets/*/*.bin
>>>>>>> +/s390x/snippets/*/*.gbin
>>>>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>>>>> index 8de926ab..fe267011 100644
>>>>>>> --- a/s390x/Makefile
>>>>>>> +++ b/s390x/Makefile
>>>>>>> @@ -75,11 +75,33 @@ OBJDIRS += lib/s390x
>>>>>>> asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>>>>>>>
>>>>>>> FLATLIBS = $(libcflat)
>>>>>>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>>>>>> +
>>>>>>> +SNIPPET_DIR = $(TEST_DIR)/snippets
>>>>>>> +
>>>>>>> +# C snippets that need to be linked
>>>>>>> +snippets-c =
>>>>>>> +
>>>>>>> +# ASM snippets that are directly compiled and converted to a *.gbin
>>>>>>> +snippets-a =
>>>>>>
>>>>>> Could you please call this snippets-s instead of ...-a ? The -a suffix looks
>>>>>> like an archive to me otherwise.
>>>>>
>>>>> Sure
>>>>>
>>>>>>
>>>>>>> +snippets = $(snippets-a)$(snippets-c)
>>>>>>
>>>>>> Shouldn't there be a space between the two?
>>>>>
>>>>> Yes, already fixed that a long while ago
>>>>> I thought I had sent out a new version already, maybe that was an
>>>>> illusion as I can't seem to find it right now.
>>>>>
>>>>>>
>>>>>>> +snippets-o += $(patsubst %.gbin,%.o,$(snippets))
>>>>>>> +
>>>>>>> +$(snippets-a): $(snippets-o) $(FLATLIBS)
>>>>>>> + $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>>>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>>>> +
>>>>>>> +$(snippets-c): $(snippets-o) $(SNIPPET_DIR)/c/cstart.o $(FLATLIBS)
>>>>>>> + $(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds \
>>>>>>> + $(filter %.o, $^) $(FLATLIBS)
>>>>>>> + $(OBJCOPY) -O binary $@ $@
>>>>>>> + $(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>>>> +
>>>>>>> +%.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) $(@:.elf=.aux.o)
>>>>>>> + $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>>>>>
>>>>>> Does this link the snippets into all elf files? ... wouldn't it be better to
>>>>>> restrict it somehow to the files that really need them?
>>>>>
>>>>> Yes it does.
>>>>> I'd like to avoid having to specify a makefile rule for every test that
>>>>> uses snippets as we already have more than the mvpg one in the queue.
>>>>>
>>>>> So I'm having Steffen looking into a solution for this problem. My first
>>>>> idea was to bring the used snippets into the unittests.cfg but I
>>>>> disliked that we then would have compile instructions in another file.
>>>>> Maybe there's a way to include that into the makefile in a clever way?
>>>>
>>>> I haven't tried, but maybe you could replace the $(snippets) in the last
>>>> line with
>>>>
>>>> $(wildcard snippets/$@.gbin)
>>>>
>>>> or something similar?
>>>
>>> That starts falling apart when multiple tests use the same snippet, no?
>>
>> That's true ... Maybe something like:
>>
>> $(filter %.gbin,$^)
>
> That filters all files that are not gbins from the prereqs, right?
> In what way is that different to linking all the snippets? After all
> they are currently a prereq for %.elf or am I missing something here?
Sorry, I meant we could then add the prereqs manually for each tests that
require it, e.g. add a single line like:
mvpg-sie.elf: snippets/mvpg.gbin
(without specifying a rule, just the dependency)
... not sure whether that works, though...
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-06-21 14:59 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20 9:47 [kvm-unit-tests RFC 0/2] s390x: Add snippet support Janosch Frank
2021-05-20 9:47 ` [kvm-unit-tests RFC 1/2] s390x: Add guest " Janosch Frank
2021-05-25 16:44 ` Claudio Imbrenda
2021-05-26 10:12 ` Janosch Frank
2021-06-21 10:10 ` Thomas Huth
2021-06-21 12:19 ` Janosch Frank
2021-06-21 12:32 ` Thomas Huth
2021-06-21 12:39 ` Janosch Frank
2021-06-21 13:28 ` Thomas Huth
2021-06-21 14:42 ` Janosch Frank
2021-06-21 14:59 ` Thomas Huth
2021-05-20 9:47 ` [kvm-unit-tests RFC 2/2] s390x: mvpg: Add SIE mvpg test Janosch Frank
2021-05-25 17:37 ` Claudio Imbrenda
2021-05-26 10:17 ` Janosch Frank
2021-05-27 14:35 ` Janosch Frank
2021-06-21 10:23 ` Thomas Huth
2021-06-21 12:41 ` Janosch Frank
2021-05-20 13:36 ` [kvm-unit-tests RFC 0/2] s390x: Add snippet support David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.