* [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
* 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 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 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 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 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
* [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 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 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 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 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 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
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.