* [kvm-unit-tests PATCH 0/3] s390x: More emulation tests
@ 2019-08-20 10:55 Janosch Frank
2019-08-20 10:55 ` [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot Janosch Frank
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Janosch Frank @ 2019-08-20 10:55 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, david, thuth
The first patch allows for CECSIM booting via PSW restart.
The other ones add diag288 and STSI tests.
I chose to start with these since they are low controversy. My queue
still contains the sclp patches and a simple smp library with
tests. They will follow later.
Janosch Frank (3):
s390x: Support PSW restart boot
s390x: Diag288 test
s390x: STSI tests
s390x/Makefile | 2 +
s390x/diag288.c | 111 +++++++++++++++++++++++++++++++++++++++
s390x/flat.lds | 14 +++--
s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 7 +++
5 files changed, 252 insertions(+), 5 deletions(-)
create mode 100644 s390x/diag288.c
create mode 100644 s390x/stsi.c
--
2.17.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank @ 2019-08-20 10:55 ` Janosch Frank 2019-08-20 11:40 ` Thomas Huth 2019-08-20 10:55 ` [kvm-unit-tests PATCH 2/3] s390x: Diag288 test Janosch Frank ` (3 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-20 10:55 UTC (permalink / raw) To: kvm; +Cc: linux-s390, david, thuth Add a boot PSW to PSW restart new, so we can also boot via a PSW restart. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/flat.lds | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/s390x/flat.lds b/s390x/flat.lds index 403d967..86dffac 100644 --- a/s390x/flat.lds +++ b/s390x/flat.lds @@ -1,14 +1,18 @@ SECTIONS { - /* - * Initial short psw for disk boot, with 31 bit addressing for - * non z/Arch environment compatibility and the instruction - * address 0x10000 (cstart64.S .init). - */ .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(0x80010000) + /* Restart new PSW for booting via PSW restart. */ + . = 0x1a0; + QUAD(0x0000000180000000) + QUAD(0x0000000000010000) } . = 0x10000; .text : { -- 2.17.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot 2019-08-20 10:55 ` [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot Janosch Frank @ 2019-08-20 11:40 ` Thomas Huth 0 siblings, 0 replies; 18+ messages in thread From: Thomas Huth @ 2019-08-20 11:40 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, david On 8/20/19 12:55 PM, Janosch Frank wrote: > Add a boot PSW to PSW restart new, so we can also boot via a PSW > restart. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/flat.lds | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/s390x/flat.lds b/s390x/flat.lds > index 403d967..86dffac 100644 > --- a/s390x/flat.lds > +++ b/s390x/flat.lds > @@ -1,14 +1,18 @@ > SECTIONS > { > - /* > - * Initial short psw for disk boot, with 31 bit addressing for > - * non z/Arch environment compatibility and the instruction > - * address 0x10000 (cstart64.S .init). > - */ > .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(0x80010000) > + /* Restart new PSW for booting via PSW restart. */ > + . = 0x1a0; > + QUAD(0x0000000180000000) > + QUAD(0x0000000000010000) > } > . = 0x10000; > .text : { > Reviewed-by: Thomas Huth <thuth@redhat.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank 2019-08-20 10:55 ` [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot Janosch Frank @ 2019-08-20 10:55 ` Janosch Frank 2019-08-20 11:59 ` Thomas Huth 2019-08-20 10:55 ` [kvm-unit-tests PATCH 3/3] s390x: STSI tests Janosch Frank ` (2 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-20 10:55 UTC (permalink / raw) To: kvm; +Cc: linux-s390, david, thuth A small test for the watchdog via diag288. Minimum timer value is 15 (seconds) and the only supported action with QEMU is restart. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/Makefile | 1 + s390x/diag288.c | 111 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 4 ++ 3 files changed, 116 insertions(+) create mode 100644 s390x/diag288.c diff --git a/s390x/Makefile b/s390x/Makefile index 1f21ddb..b654c56 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf tests += $(TEST_DIR)/vector.elf tests += $(TEST_DIR)/gs.elf tests += $(TEST_DIR)/iep.elf +tests += $(TEST_DIR)/diag288.elf tests_binary = $(patsubst %.elf,%.bin,$(tests)) all: directories test_cases test_cases_binary diff --git a/s390x/diag288.c b/s390x/diag288.c new file mode 100644 index 0000000..5abcec4 --- /dev/null +++ b/s390x/diag288.c @@ -0,0 +1,111 @@ +/* + * Timer Event DIAG288 test + * + * Copyright (c) 2019 IBM Corp + * + * Authors: + * Janosch Frank <frankja@linux.ibm.com> + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU Library General Public License version 2. + */ + +#include <libcflat.h> +#include <asm/asm-offsets.h> +#include <asm/interrupt.h> + +struct lowcore *lc = (void *)0x0; + +#define CODE_INIT 0 +#define CODE_CHANGE 1 +#define CODE_CANCEL 2 + +#define ACTION_RESTART 0 + +static inline void diag288(unsigned long code, unsigned long time, + unsigned long action) +{ + register unsigned long fc asm("0") = code; + register unsigned long tm asm("1") = time; + register unsigned long ac asm("2") = action; + + asm volatile("diag %0,%2,0x288" + : : "d" (fc), "d" (tm), "d" (ac)); +} + +static inline void diag288_uneven(void) +{ + register unsigned long fc asm("1") = 0; + register unsigned long time asm("1") = 15; + register unsigned long action asm("2") = 0; + + asm volatile("diag %0,%2,0x288" + : : "d" (fc), "d" (time), "d" (action)); +} + +static void test_specs(void) +{ + report_prefix_push("spec ex"); + + report_prefix_push("uneven"); + expect_pgm_int(); + diag288_uneven(); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("unsup act"); + expect_pgm_int(); + diag288(CODE_INIT, 15, 42); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("unsup fctn"); + expect_pgm_int(); + diag288(42, 15, ACTION_RESTART); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("no init"); + expect_pgm_int(); + diag288(CODE_CANCEL, 15, ACTION_RESTART); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("min timer"); + expect_pgm_int(); + diag288(CODE_INIT, 14, ACTION_RESTART); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_pop(); +} + +static void test_priv(void) +{ + report_prefix_push("privileged"); + expect_pgm_int(); + enter_pstate(); + diag288(0, 15, 0); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + report_prefix_pop(); +} + +static void test_bite(void) +{ + if (lc->restart_old_psw.addr) { + report("restart", true); + return; + } + lc->restart_new_psw.addr = (uint64_t)test_bite; + diag288(CODE_INIT, 15, ACTION_RESTART); + while(1) {}; +} + +int main(void) +{ + report_prefix_push("diag288"); + test_priv(); + test_specs(); + test_bite(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 546b1f2..ca10f38 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -61,3 +61,7 @@ file = gs.elf [iep] file = iep.elf + +[diag288] +file = diag288.elf +extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi \ No newline at end of file -- 2.17.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 10:55 ` [kvm-unit-tests PATCH 2/3] s390x: Diag288 test Janosch Frank @ 2019-08-20 11:59 ` Thomas Huth 2019-08-20 12:25 ` Janosch Frank 0 siblings, 1 reply; 18+ messages in thread From: Thomas Huth @ 2019-08-20 11:59 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, david On 8/20/19 12:55 PM, Janosch Frank wrote: > A small test for the watchdog via diag288. > > Minimum timer value is 15 (seconds) and the only supported action with > QEMU is restart. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/Makefile | 1 + > s390x/diag288.c | 111 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 4 ++ > 3 files changed, 116 insertions(+) > create mode 100644 s390x/diag288.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index 1f21ddb..b654c56 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf > tests += $(TEST_DIR)/vector.elf > tests += $(TEST_DIR)/gs.elf > tests += $(TEST_DIR)/iep.elf > +tests += $(TEST_DIR)/diag288.elf > tests_binary = $(patsubst %.elf,%.bin,$(tests)) > > all: directories test_cases test_cases_binary > diff --git a/s390x/diag288.c b/s390x/diag288.c > new file mode 100644 > index 0000000..5abcec4 > --- /dev/null > +++ b/s390x/diag288.c > @@ -0,0 +1,111 @@ > +/* > + * Timer Event DIAG288 test > + * > + * Copyright (c) 2019 IBM Corp > + * > + * Authors: > + * Janosch Frank <frankja@linux.ibm.com> > + * > + * This code is free software; you can redistribute it and/or modify it > + * under the terms of the GNU Library General Public License version 2. > + */ > + > +#include <libcflat.h> > +#include <asm/asm-offsets.h> > +#include <asm/interrupt.h> > + > +struct lowcore *lc = (void *)0x0; Maybe use "NULL" instead of "(void *)0x0" ? ... maybe we could also introduce such a variable as a global variable in lib/s390x/ since this is already the third or fourth time that we use it in the kvm-unit-tests... > +#define CODE_INIT 0 > +#define CODE_CHANGE 1 > +#define CODE_CANCEL 2 > + > +#define ACTION_RESTART 0 > + > +static inline void diag288(unsigned long code, unsigned long time, > + unsigned long action) > +{ > + register unsigned long fc asm("0") = code; > + register unsigned long tm asm("1") = time; > + register unsigned long ac asm("2") = action; > + > + asm volatile("diag %0,%2,0x288" > + : : "d" (fc), "d" (tm), "d" (ac)); > +} > + > +static inline void diag288_uneven(void) > +{ > + register unsigned long fc asm("1") = 0; > + register unsigned long time asm("1") = 15; So you're setting register 1 twice? And "time" is not really used in the inline assembly below? How's that supposed to work? Looks like a bug to me... if not, please explain with a comment in the code here. > + register unsigned long action asm("2") = 0; > + > + asm volatile("diag %0,%2,0x288" > + : : "d" (fc), "d" (time), "d" (action)); > +} > + > +static void test_specs(void) > +{ > + report_prefix_push("spec ex"); After all those Spectre bugs in the last year, "spec ex" makes me think of speculative execution first... maybe better use "specification" as prefix? > + report_prefix_push("uneven"); > + expect_pgm_int(); > + diag288_uneven(); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("unsup act"); "unsupported action" ? ... it's not that long, is it? > + expect_pgm_int(); > + diag288(CODE_INIT, 15, 42); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("unsup fctn"); "unsupported function" ? > + expect_pgm_int(); > + diag288(42, 15, ACTION_RESTART); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("no init"); > + expect_pgm_int(); > + diag288(CODE_CANCEL, 15, ACTION_RESTART); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("min timer"); > + expect_pgm_int(); > + diag288(CODE_INIT, 14, ACTION_RESTART); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_pop(); > +} > + > +static void test_priv(void) > +{ > + report_prefix_push("privileged"); > + expect_pgm_int(); > + enter_pstate(); > + diag288(0, 15, 0); diag288(CODE_INIT, 0, ACTION_RESTART) ? > + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); > + report_prefix_pop(); > +} > + > +static void test_bite(void) > +{ > + if (lc->restart_old_psw.addr) { > + report("restart", true); > + return; > + } > + lc->restart_new_psw.addr = (uint64_t)test_bite; > + diag288(CODE_INIT, 15, ACTION_RESTART); > + while(1) {}; Should this maybe timeout after a minute or so? > +} > + > +int main(void) > +{ > + report_prefix_push("diag288"); > + test_priv(); > + test_specs(); > + test_bite(); > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 546b1f2..ca10f38 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -61,3 +61,7 @@ file = gs.elf > > [iep] > file = iep.elf > + > +[diag288] > +file = diag288.elf > +extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi > \ No newline at end of file Nit: Add newline (well, it gets added by the next patch, but if you touch this patch again anyway...) Thomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 11:59 ` Thomas Huth @ 2019-08-20 12:25 ` Janosch Frank 2019-08-20 12:55 ` Thomas Huth 0 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-20 12:25 UTC (permalink / raw) To: Thomas Huth, kvm; +Cc: linux-s390, david On 8/20/19 1:59 PM, Thomas Huth wrote: > On 8/20/19 12:55 PM, Janosch Frank wrote: >> A small test for the watchdog via diag288. >> >> Minimum timer value is 15 (seconds) and the only supported action with >> QEMU is restart. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/Makefile | 1 + >> s390x/diag288.c | 111 ++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 4 ++ >> 3 files changed, 116 insertions(+) >> create mode 100644 s390x/diag288.c >> >> diff --git a/s390x/Makefile b/s390x/Makefile >> index 1f21ddb..b654c56 100644 >> --- a/s390x/Makefile >> +++ b/s390x/Makefile >> @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf >> tests += $(TEST_DIR)/vector.elf >> tests += $(TEST_DIR)/gs.elf >> tests += $(TEST_DIR)/iep.elf >> +tests += $(TEST_DIR)/diag288.elf >> tests_binary = $(patsubst %.elf,%.bin,$(tests)) >> >> all: directories test_cases test_cases_binary >> diff --git a/s390x/diag288.c b/s390x/diag288.c >> new file mode 100644 >> index 0000000..5abcec4 >> --- /dev/null >> +++ b/s390x/diag288.c >> @@ -0,0 +1,111 @@ >> +/* >> + * Timer Event DIAG288 test >> + * >> + * Copyright (c) 2019 IBM Corp >> + * >> + * Authors: >> + * Janosch Frank <frankja@linux.ibm.com> >> + * >> + * This code is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU Library General Public License version 2. >> + */ >> + >> +#include <libcflat.h> >> +#include <asm/asm-offsets.h> >> +#include <asm/interrupt.h> >> + >> +struct lowcore *lc = (void *)0x0; > > Maybe use "NULL" instead of "(void *)0x0" ? Well I'd rather have: struct lowcore *lc = (struct lowcore *)0x0; Than using NULL. > > ... maybe we could also introduce such a variable as a global variable > in lib/s390x/ since this is already the third or fourth time that we use > it in the kvm-unit-tests... Sure I also thought about that, any particular place? > >> +#define CODE_INIT 0 >> +#define CODE_CHANGE 1 >> +#define CODE_CANCEL 2 >> + >> +#define ACTION_RESTART 0 >> + >> +static inline void diag288(unsigned long code, unsigned long time, >> + unsigned long action) >> +{ >> + register unsigned long fc asm("0") = code; >> + register unsigned long tm asm("1") = time; >> + register unsigned long ac asm("2") = action; >> + >> + asm volatile("diag %0,%2,0x288" >> + : : "d" (fc), "d" (tm), "d" (ac)); >> +} >> + >> +static inline void diag288_uneven(void) >> +{ >> + register unsigned long fc asm("1") = 0; >> + register unsigned long time asm("1") = 15; > > So you're setting register 1 twice? And "time" is not really used in the > inline assembly below? How's that supposed to work? Looks like a bug to > me... if not, please explain with a comment in the code here. Well I'm waiting for a spec exception here, so it doesn't have to work. I'll probably just remove the register variables and do a: "diag %r1,%r2,0x288" > >> + register unsigned long action asm("2") = 0; >> + >> + asm volatile("diag %0,%2,0x288" >> + : : "d" (fc), "d" (time), "d" (action)); >> +} >> + >> +static void test_specs(void) >> +{ >> + report_prefix_push("spec ex"); > > After all those Spectre bugs in the last year, "spec ex" makes me think > of speculative execution first... maybe better use "specification" as > prefix? Sure, I'll take the review for the prefixes. I thought a short prefix makes that more readable, but if it only confuses, let's use a longer one. > >> + report_prefix_push("uneven"); >> + expect_pgm_int(); >> + diag288_uneven(); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + >> + report_prefix_push("unsup act"); > > "unsupported action" ? ... it's not that long, is it? > >> + expect_pgm_int(); >> + diag288(CODE_INIT, 15, 42); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + >> + report_prefix_push("unsup fctn"); > > "unsupported function" ? > >> + expect_pgm_int(); >> + diag288(42, 15, ACTION_RESTART); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + >> + report_prefix_push("no init"); >> + expect_pgm_int(); >> + diag288(CODE_CANCEL, 15, ACTION_RESTART); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + >> + report_prefix_push("min timer"); >> + expect_pgm_int(); >> + diag288(CODE_INIT, 14, ACTION_RESTART); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + >> + report_prefix_pop(); >> +} >> + >> +static void test_priv(void) >> +{ >> + report_prefix_push("privileged"); >> + expect_pgm_int(); >> + enter_pstate(); >> + diag288(0, 15, 0); > diag288(CODE_INIT, 0, ACTION_RESTART) ? > >> + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); >> + report_prefix_pop(); >> +} >> + >> +static void test_bite(void) >> +{ >> + if (lc->restart_old_psw.addr) { >> + report("restart", true); >> + return; >> + } >> + lc->restart_new_psw.addr = (uint64_t)test_bite; >> + diag288(CODE_INIT, 15, ACTION_RESTART); >> + while(1) {}; > > Should this maybe timeout after a minute or so? Well run_tests.sh does timeout externally. Do you need it backed into the test? > >> +} >> + >> +int main(void) >> +{ >> + report_prefix_push("diag288"); >> + test_priv(); >> + test_specs(); >> + test_bite(); >> + return report_summary(); >> +} >> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg >> index 546b1f2..ca10f38 100644 >> --- a/s390x/unittests.cfg >> +++ b/s390x/unittests.cfg >> @@ -61,3 +61,7 @@ file = gs.elf >> >> [iep] >> file = iep.elf >> + >> +[diag288] >> +file = diag288.elf >> +extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi >> \ No newline at end of file > > Nit: Add newline (well, it gets added by the next patch, but if you > touch this patch again anyway...) Ok > > Thomas > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 12:25 ` Janosch Frank @ 2019-08-20 12:55 ` Thomas Huth 2019-08-20 15:21 ` Janosch Frank 0 siblings, 1 reply; 18+ messages in thread From: Thomas Huth @ 2019-08-20 12:55 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, david On 8/20/19 2:25 PM, Janosch Frank wrote: > On 8/20/19 1:59 PM, Thomas Huth wrote: >> On 8/20/19 12:55 PM, Janosch Frank wrote: >>> A small test for the watchdog via diag288. >>> >>> Minimum timer value is 15 (seconds) and the only supported action with >>> QEMU is restart. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/Makefile | 1 + >>> s390x/diag288.c | 111 ++++++++++++++++++++++++++++++++++++++++++++ >>> s390x/unittests.cfg | 4 ++ >>> 3 files changed, 116 insertions(+) >>> create mode 100644 s390x/diag288.c >>> >>> diff --git a/s390x/Makefile b/s390x/Makefile >>> index 1f21ddb..b654c56 100644 >>> --- a/s390x/Makefile >>> +++ b/s390x/Makefile >>> @@ -11,6 +11,7 @@ tests += $(TEST_DIR)/cmm.elf >>> tests += $(TEST_DIR)/vector.elf >>> tests += $(TEST_DIR)/gs.elf >>> tests += $(TEST_DIR)/iep.elf >>> +tests += $(TEST_DIR)/diag288.elf >>> tests_binary = $(patsubst %.elf,%.bin,$(tests)) >>> >>> all: directories test_cases test_cases_binary >>> diff --git a/s390x/diag288.c b/s390x/diag288.c >>> new file mode 100644 >>> index 0000000..5abcec4 >>> --- /dev/null >>> +++ b/s390x/diag288.c >>> @@ -0,0 +1,111 @@ >>> +/* >>> + * Timer Event DIAG288 test >>> + * >>> + * Copyright (c) 2019 IBM Corp >>> + * >>> + * Authors: >>> + * Janosch Frank <frankja@linux.ibm.com> >>> + * >>> + * This code is free software; you can redistribute it and/or modify it >>> + * under the terms of the GNU Library General Public License version 2. >>> + */ >>> + >>> +#include <libcflat.h> >>> +#include <asm/asm-offsets.h> >>> +#include <asm/interrupt.h> >>> + >>> +struct lowcore *lc = (void *)0x0; >> >> Maybe use "NULL" instead of "(void *)0x0" ? > > Well I'd rather have: > struct lowcore *lc = (struct lowcore *)0x0; Fine for me, too. >> ... maybe we could also introduce such a variable as a global variable >> in lib/s390x/ since this is already the third or fourth time that we use >> it in the kvm-unit-tests... > > Sure I also thought about that, any particular place? No clue. Maybe lib/s390x/mmu.c ? Or a new file called lowcore.c ? >>> +static inline void diag288_uneven(void) >>> +{ >>> + register unsigned long fc asm("1") = 0; >>> + register unsigned long time asm("1") = 15; >> >> So you're setting register 1 twice? And "time" is not really used in the >> inline assembly below? How's that supposed to work? Looks like a bug to >> me... if not, please explain with a comment in the code here. > > Well I'm waiting for a spec exception here, so it doesn't have to work.> I'll probably just remove the register variables and do a: > > "diag %r1,%r2,0x288" Yes, I think that's easier to understand. BTW, is there another documentation of diag 288 beside the "CP programming services" manual? At least my version of that specification does not say that the fc register has to be even... >>> +static void test_bite(void) >>> +{ >>> + if (lc->restart_old_psw.addr) { >>> + report("restart", true); >>> + return; >>> + } >>> + lc->restart_new_psw.addr = (uint64_t)test_bite; >>> + diag288(CODE_INIT, 15, ACTION_RESTART); >>> + while(1) {}; >> >> Should this maybe timeout after a minute or so? > > Well run_tests.sh does timeout externally. > Do you need it backed into the test? I sometimes also run the tests without the wrapper script, so in that case it would be convenient ... but I can also quit QEMU manually in that case, so it's not a big issue. Thomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 12:55 ` Thomas Huth @ 2019-08-20 15:21 ` Janosch Frank 2019-08-20 15:29 ` Thomas Huth 0 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-20 15:21 UTC (permalink / raw) To: Thomas Huth, kvm; +Cc: linux-s390, david [-- Attachment #1.1: Type: text/plain, Size: 2091 bytes --] On 8/20/19 2:55 PM, Thomas Huth wrote: > On 8/20/19 2:25 PM, Janosch Frank wrote: >> On 8/20/19 1:59 PM, Thomas Huth wrote: >>> On 8/20/19 12:55 PM, Janosch Frank wrote: [...] >>> ... maybe we could also introduce such a variable as a global variable >>> in lib/s390x/ since this is already the third or fourth time that we use >>> it in the kvm-unit-tests... >> >> Sure I also thought about that, any particular place? > > No clue. Maybe lib/s390x/mmu.c ? Or a new file called lowcore.c ? > >>>> +static inline void diag288_uneven(void) >>>> +{ >>>> + register unsigned long fc asm("1") = 0; >>>> + register unsigned long time asm("1") = 15; >>> >>> So you're setting register 1 twice? And "time" is not really used in the >>> inline assembly below? How's that supposed to work? Looks like a bug to >>> me... if not, please explain with a comment in the code here. >> >> Well I'm waiting for a spec exception here, so it doesn't have to work.> I'll probably just remove the register variables and do a: >> >> "diag %r1,%r2,0x288" > > Yes, I think that's easier to understand. > > BTW, is there another documentation of diag 288 beside the "CP > programming services" manual? At least my version of that specification > does not say that the fc register has to be even... I used the non-public lpar documentation... > >>>> +static void test_bite(void) >>>> +{ >>>> + if (lc->restart_old_psw.addr) { >>>> + report("restart", true); >>>> + return; >>>> + } >>>> + lc->restart_new_psw.addr = (uint64_t)test_bite; >>>> + diag288(CODE_INIT, 15, ACTION_RESTART); >>>> + while(1) {}; >>> >>> Should this maybe timeout after a minute or so? >> >> Well run_tests.sh does timeout externally. >> Do you need it backed into the test? > > I sometimes also run the tests without the wrapper script, so in that > case it would be convenient ... but I can also quit QEMU manually in > that case, so it's not a big issue. How about setting the clock comparator, that should trigger an unexpected external interrupt? > > Thomas > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] s390x: Diag288 test 2019-08-20 15:21 ` Janosch Frank @ 2019-08-20 15:29 ` Thomas Huth 0 siblings, 0 replies; 18+ messages in thread From: Thomas Huth @ 2019-08-20 15:29 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, david [-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --] On 8/20/19 5:21 PM, Janosch Frank wrote: > On 8/20/19 2:55 PM, Thomas Huth wrote: >> On 8/20/19 2:25 PM, Janosch Frank wrote: >>> On 8/20/19 1:59 PM, Thomas Huth wrote: >>>> On 8/20/19 12:55 PM, Janosch Frank wrote: > [...] >>>> ... maybe we could also introduce such a variable as a global variable >>>> in lib/s390x/ since this is already the third or fourth time that we use >>>> it in the kvm-unit-tests... >>> >>> Sure I also thought about that, any particular place? >> >> No clue. Maybe lib/s390x/mmu.c ? Or a new file called lowcore.c ? >> >>>>> +static inline void diag288_uneven(void) >>>>> +{ >>>>> + register unsigned long fc asm("1") = 0; >>>>> + register unsigned long time asm("1") = 15; >>>> >>>> So you're setting register 1 twice? And "time" is not really used in the >>>> inline assembly below? How's that supposed to work? Looks like a bug to >>>> me... if not, please explain with a comment in the code here. >>> >>> Well I'm waiting for a spec exception here, so it doesn't have to work.> I'll probably just remove the register variables and do a: >>> >>> "diag %r1,%r2,0x288" >> >> Yes, I think that's easier to understand. >> >> BTW, is there another documentation of diag 288 beside the "CP >> programming services" manual? At least my version of that specification >> does not say that the fc register has to be even... > > I used the non-public lpar documentation... Ok, if it's specified there, then the check is fine with me. >>>>> +static void test_bite(void) >>>>> +{ >>>>> + if (lc->restart_old_psw.addr) { >>>>> + report("restart", true); >>>>> + return; >>>>> + } >>>>> + lc->restart_new_psw.addr = (uint64_t)test_bite; >>>>> + diag288(CODE_INIT, 15, ACTION_RESTART); >>>>> + while(1) {}; >>>> >>>> Should this maybe timeout after a minute or so? >>> >>> Well run_tests.sh does timeout externally. >>> Do you need it backed into the test? >> >> I sometimes also run the tests without the wrapper script, so in that >> case it would be convenient ... but I can also quit QEMU manually in >> that case, so it's not a big issue. > > How about setting the clock comparator, that should trigger an > unexpected external interrupt? Sounds like an idea (if this is not getting too complicated... otherwise just leave it as it is). Thomas [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 3/3] s390x: STSI tests 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank 2019-08-20 10:55 ` [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot Janosch Frank 2019-08-20 10:55 ` [kvm-unit-tests PATCH 2/3] s390x: Diag288 test Janosch Frank @ 2019-08-20 10:55 ` Janosch Frank 2019-08-20 13:21 ` Thomas Huth 2019-08-20 11:11 ` [kvm-unit-tests PATCH 0/3] s390x: More emulation tests David Hildenbrand 2019-08-20 19:04 ` David Hildenbrand 4 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-20 10:55 UTC (permalink / raw) To: kvm; +Cc: linux-s390, david, thuth For now let's concentrate on the error conditions. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/Makefile | 1 + s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 5 +- 3 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 s390x/stsi.c diff --git a/s390x/Makefile b/s390x/Makefile index b654c56..311ab77 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -12,6 +12,7 @@ tests += $(TEST_DIR)/vector.elf tests += $(TEST_DIR)/gs.elf tests += $(TEST_DIR)/iep.elf tests += $(TEST_DIR)/diag288.elf +tests += $(TEST_DIR)/stsi.elf tests_binary = $(patsubst %.elf,%.bin,$(tests)) all: directories test_cases test_cases_binary diff --git a/s390x/stsi.c b/s390x/stsi.c new file mode 100644 index 0000000..005f337 --- /dev/null +++ b/s390x/stsi.c @@ -0,0 +1,123 @@ +/* + * Store System Information tests + * + * Copyright (c) 2019 IBM Corp + * + * Authors: + * Janosch Frank <frankja@linux.ibm.com> + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU Library General Public License version 2. + */ + +#include <libcflat.h> +#include <asm/page.h> +#include <asm/asm-offsets.h> +#include <asm/interrupt.h> + +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); + +static inline unsigned long stsi(unsigned long *addr, + unsigned long fc, uint8_t sel1, uint8_t sel2) +{ + register unsigned long r0 asm("0") = (fc << 28) | sel1; + register unsigned long r1 asm("1") = sel2; + int cc; + + asm volatile("stsi 0(%3)\n" + "ipm %[cc]\n" + "srl %[cc],28\n" + : "+d" (r0), [cc] "=d" (cc) + : "d" (r1), "a" (addr) + : "cc", "memory"); + return cc; +} + +static inline void stsi_zero_r0(unsigned long *addr, + unsigned long fc, uint8_t sel1, uint8_t sel2) +{ + register unsigned long r0 asm("0") = (fc << 28) | (1 << 8) | sel1; + register unsigned long r1 asm("1") = sel2; + + + asm volatile("stsi 0(%2)" + : "+d" (r0) + : "d" (r1), "a" (addr) + : "cc", "memory"); +} + +static inline void stsi_zero_r1(unsigned long *addr, + unsigned long fc, uint8_t sel1, uint8_t sel2) +{ + register unsigned long r0 asm("0") = (fc << 28) | sel1; + register unsigned long r1 asm("1") = (1 << 16) | sel2; + + + asm volatile("stsi 0(%2)" + : "+d" (r0) + : "d" (r1), "a" (addr) + : "cc", "memory"); +} + +static inline unsigned long stsi_get_fc(unsigned long *addr) +{ + register unsigned long r0 asm("0") = 0; + register unsigned long r1 asm("1") = 0; + + + asm volatile("stsi 0(%2)" + : "+d" (r0) + : "d" (r1), "a" (addr) + : "cc", "memory"); + return r0 >> 28; +} + +static void test_specs(void) +{ + report_prefix_push("spec ex"); + + report_prefix_push("inv r0"); + expect_pgm_int(); + stsi_zero_r0((void *)pagebuf, 1, 0, 0); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("inv r1"); + expect_pgm_int(); + stsi_zero_r1((void *)pagebuf, 1, 0, 0); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_push("unaligned"); + expect_pgm_int(); + stsi((void *)pagebuf + 42, 1, 0, 0); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + + report_prefix_pop(); +} + +static void test_priv(void) +{ + report_prefix_push("privileged"); + expect_pgm_int(); + enter_pstate(); + stsi((void *)pagebuf, 0, 0, 0); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + report_prefix_pop(); +} + +static void test_fc(void) +{ + report("cc == 3", stsi((void *)pagebuf, 7, 0, 0)); + report("r0 == 3", stsi_get_fc((void *)pagebuf)); +} + +int main(void) +{ + report_prefix_push("stsi"); + test_priv(); + test_specs(); + test_fc(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index ca10f38..c56258a 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -64,4 +64,7 @@ file = iep.elf [diag288] file = diag288.elf -extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi \ No newline at end of file +extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi + +[stsi] +file = stsi.elf -- 2.17.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 3/3] s390x: STSI tests 2019-08-20 10:55 ` [kvm-unit-tests PATCH 3/3] s390x: STSI tests Janosch Frank @ 2019-08-20 13:21 ` Thomas Huth 2019-08-21 8:46 ` Janosch Frank 0 siblings, 1 reply; 18+ messages in thread From: Thomas Huth @ 2019-08-20 13:21 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, david On 8/20/19 12:55 PM, Janosch Frank wrote: > For now let's concentrate on the error conditions. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/Makefile | 1 + > s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 5 +- > 3 files changed, 128 insertions(+), 1 deletion(-) > create mode 100644 s390x/stsi.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index b654c56..311ab77 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -12,6 +12,7 @@ tests += $(TEST_DIR)/vector.elf > tests += $(TEST_DIR)/gs.elf > tests += $(TEST_DIR)/iep.elf > tests += $(TEST_DIR)/diag288.elf > +tests += $(TEST_DIR)/stsi.elf > tests_binary = $(patsubst %.elf,%.bin,$(tests)) > > all: directories test_cases test_cases_binary > diff --git a/s390x/stsi.c b/s390x/stsi.c > new file mode 100644 > index 0000000..005f337 > --- /dev/null > +++ b/s390x/stsi.c > @@ -0,0 +1,123 @@ > +/* > + * Store System Information tests > + * > + * Copyright (c) 2019 IBM Corp > + * > + * Authors: > + * Janosch Frank <frankja@linux.ibm.com> > + * > + * This code is free software; you can redistribute it and/or modify it > + * under the terms of the GNU Library General Public License version 2. > + */ > + > +#include <libcflat.h> > +#include <asm/page.h> > +#include <asm/asm-offsets.h> > +#include <asm/interrupt.h> > + > +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); > + > +static inline unsigned long stsi(unsigned long *addr, > + unsigned long fc, uint8_t sel1, uint8_t sel2) Return code should be "int", not "long". I'd also suggest to use "void *addr" instead of "unsigned long *addr", then you don't have to cast the pagebuf when you're calling this function. > +{ > + register unsigned long r0 asm("0") = (fc << 28) | sel1; > + register unsigned long r1 asm("1") = sel2; > + int cc; > + > + asm volatile("stsi 0(%3)\n" > + "ipm %[cc]\n" > + "srl %[cc],28\n" > + : "+d" (r0), [cc] "=d" (cc) > + : "d" (r1), "a" (addr) > + : "cc", "memory"); > + return cc; > +} Bonus points for putting that function into a header and re-use it in skey.c (maybe in a separate patch, though). > +static inline void stsi_zero_r0(unsigned long *addr, > + unsigned long fc, uint8_t sel1, uint8_t sel2) > +{ > + register unsigned long r0 asm("0") = (fc << 28) | (1 << 8) | sel1; > + register unsigned long r1 asm("1") = sel2; > + > + Please remove one empty line. > + asm volatile("stsi 0(%2)" > + : "+d" (r0) > + : "d" (r1), "a" (addr) > + : "cc", "memory"); > +} > + > +static inline void stsi_zero_r1(unsigned long *addr, > + unsigned long fc, uint8_t sel1, uint8_t sel2) > +{ > + register unsigned long r0 asm("0") = (fc << 28) | sel1; > + register unsigned long r1 asm("1") = (1 << 16) | sel2; > + > + dito > + asm volatile("stsi 0(%2)" > + : "+d" (r0) > + : "d" (r1), "a" (addr) > + : "cc", "memory"); > +} Also not sure whether you need separate functions for these tests ... you could also change the type of sel1 and sel2 from "uint8_t" to "int" in the stsi() function and then pass the invalid types to that function instead? > +static inline unsigned long stsi_get_fc(unsigned long *addr) > +{ > + register unsigned long r0 asm("0") = 0; > + register unsigned long r1 asm("1") = 0; > + > + Superfluous empty line again. > + asm volatile("stsi 0(%2)" > + : "+d" (r0) > + : "d" (r1), "a" (addr) > + : "cc", "memory"); Maybe assert that the CC is 0 after the call? > + return r0 >> 28; > +} > + > +static void test_specs(void) > +{ > + report_prefix_push("spec ex"); s/spec ex/specification/ please > + report_prefix_push("inv r0"); > + expect_pgm_int(); > + stsi_zero_r0((void *)pagebuf, 1, 0, 0); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("inv r1"); > + expect_pgm_int(); > + stsi_zero_r1((void *)pagebuf, 1, 0, 0); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_push("unaligned"); > + expect_pgm_int(); > + stsi((void *)pagebuf + 42, 1, 0, 0); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + > + report_prefix_pop(); > +} > + > +static void test_priv(void) > +{ > + report_prefix_push("privileged"); > + expect_pgm_int(); > + enter_pstate(); > + stsi((void *)pagebuf, 0, 0, 0); > + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); > + report_prefix_pop(); > +} > + > +static void test_fc(void) > +{ > + report("cc == 3", stsi((void *)pagebuf, 7, 0, 0)); Shouldn't that line look like this instead: report("cc == 3", stsi((void *)pagebuf, 7, 0, 0) == 3); ? > + report("r0 == 3", stsi_get_fc((void *)pagebuf)); report("r0 >= 3", stsi_get_fc((void *)pagebuf) >= 3); ? > +} > + > +int main(void) > +{ > + report_prefix_push("stsi"); > + test_priv(); > + test_specs(); > + test_fc(); > + return report_summary(); > +} How about adding another test for access exceptions? Activate low address protection, then store to address 4096 ... and/or check "stsi((void *)-0xdeadadd, 1, 0, 0);" ? Thomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 3/3] s390x: STSI tests 2019-08-20 13:21 ` Thomas Huth @ 2019-08-21 8:46 ` Janosch Frank 0 siblings, 0 replies; 18+ messages in thread From: Janosch Frank @ 2019-08-21 8:46 UTC (permalink / raw) To: Thomas Huth, kvm; +Cc: linux-s390, david [-- Attachment #1.1: Type: text/plain, Size: 3395 bytes --] On 8/20/19 3:21 PM, Thomas Huth wrote: > On 8/20/19 12:55 PM, Janosch Frank wrote: >> For now let's concentrate on the error conditions. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/Makefile | 1 + >> s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 5 +- >> 3 files changed, 128 insertions(+), 1 deletion(-) >> create mode 100644 s390x/stsi.c >> >> diff --git a/s390x/Makefile b/s390x/Makefile >> index b654c56..311ab77 100644 >> --- a/s390x/Makefile >> +++ b/s390x/Makefile >> @@ -12,6 +12,7 @@ tests += $(TEST_DIR)/vector.elf >> tests += $(TEST_DIR)/gs.elf >> tests += $(TEST_DIR)/iep.elf >> tests += $(TEST_DIR)/diag288.elf >> +tests += $(TEST_DIR)/stsi.elf >> tests_binary = $(patsubst %.elf,%.bin,$(tests)) >> >> all: directories test_cases test_cases_binary >> diff --git a/s390x/stsi.c b/s390x/stsi.c >> new file mode 100644 >> index 0000000..005f337 >> --- /dev/null >> +++ b/s390x/stsi.c >> @@ -0,0 +1,123 @@ >> +/* >> + * Store System Information tests >> + * >> + * Copyright (c) 2019 IBM Corp >> + * >> + * Authors: >> + * Janosch Frank <frankja@linux.ibm.com> >> + * >> + * This code is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU Library General Public License version 2. >> + */ >> + >> +#include <libcflat.h> >> +#include <asm/page.h> >> +#include <asm/asm-offsets.h> >> +#include <asm/interrupt.h> >> + >> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); >> + >> +static inline unsigned long stsi(unsigned long *addr, >> + unsigned long fc, uint8_t sel1, uint8_t sel2) > > Return code should be "int", not "long". > > I'd also suggest to use "void *addr" instead of "unsigned long *addr", > then you don't have to cast the pagebuf when you're calling this function. Ok > >> +{ >> + register unsigned long r0 asm("0") = (fc << 28) | sel1; >> + register unsigned long r1 asm("1") = sel2; >> + int cc; >> + >> + asm volatile("stsi 0(%3)\n" >> + "ipm %[cc]\n" >> + "srl %[cc],28\n" >> + : "+d" (r0), [cc] "=d" (cc) >> + : "d" (r1), "a" (addr) >> + : "cc", "memory"); >> + return cc; >> +} > > Bonus points for putting that function into a header and re-use it in > skey.c (maybe in a separate patch, though). I forgot that you added that... How about moving it to lib/s390/asm/arch_def.h ? [...] >> +static void test_fc(void) >> +{ >> + report("cc == 3", stsi((void *)pagebuf, 7, 0, 0)); > > Shouldn't that line look like this instead: > > report("cc == 3", stsi((void *)pagebuf, 7, 0, 0) == 3); > > ? Yes > >> + report("r0 == 3", stsi_get_fc((void *)pagebuf)); > > report("r0 >= 3", stsi_get_fc((void *)pagebuf) >= 3); > > ? Well rather >= 2 because we can also run on lpar with some additional patches applied. Time to test this under lpar... > >> +} >> + >> +int main(void) >> +{ >> + report_prefix_push("stsi"); >> + test_priv(); >> + test_specs(); >> + test_fc(); >> + return report_summary(); >> +} > > How about adding another test for access exceptions? Activate low > address protection, then store to address 4096 ... and/or check > "stsi((void *)-0xdeadadd, 1, 0, 0);" ? Sounds good > > Thomas > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank ` (2 preceding siblings ...) 2019-08-20 10:55 ` [kvm-unit-tests PATCH 3/3] s390x: STSI tests Janosch Frank @ 2019-08-20 11:11 ` David Hildenbrand 2019-08-20 11:49 ` Janosch Frank 2019-08-20 19:04 ` David Hildenbrand 4 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-08-20 11:11 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, thuth On 20.08.19 12:55, Janosch Frank wrote: > The first patch allows for CECSIM booting via PSW restart. > The other ones add diag288 and STSI tests. > > I chose to start with these since they are low controversy. My queue > still contains the sclp patches and a simple smp library with > tests. They will follow later. > > Janosch Frank (3): > s390x: Support PSW restart boot > s390x: Diag288 test > s390x: STSI tests > > s390x/Makefile | 2 + > s390x/diag288.c | 111 +++++++++++++++++++++++++++++++++++++++ > s390x/flat.lds | 14 +++-- > s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 7 +++ > 5 files changed, 252 insertions(+), 5 deletions(-) > create mode 100644 s390x/diag288.c > create mode 100644 s390x/stsi.c > Just wondering, did you try them with TCG as well? (or do I have to test) -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-20 11:11 ` [kvm-unit-tests PATCH 0/3] s390x: More emulation tests David Hildenbrand @ 2019-08-20 11:49 ` Janosch Frank 0 siblings, 0 replies; 18+ messages in thread From: Janosch Frank @ 2019-08-20 11:49 UTC (permalink / raw) To: David Hildenbrand, kvm; +Cc: linux-s390, thuth [-- Attachment #1.1: Type: text/plain, Size: 1182 bytes --] On 8/20/19 1:11 PM, David Hildenbrand wrote: > On 20.08.19 12:55, Janosch Frank wrote: >> The first patch allows for CECSIM booting via PSW restart. >> The other ones add diag288 and STSI tests. >> >> I chose to start with these since they are low controversy. My queue >> still contains the sclp patches and a simple smp library with >> tests. They will follow later. >> >> Janosch Frank (3): >> s390x: Support PSW restart boot >> s390x: Diag288 test >> s390x: STSI tests >> >> s390x/Makefile | 2 + >> s390x/diag288.c | 111 +++++++++++++++++++++++++++++++++++++++ >> s390x/flat.lds | 14 +++-- >> s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 7 +++ >> 5 files changed, 252 insertions(+), 5 deletions(-) >> create mode 100644 s390x/diag288.c >> create mode 100644 s390x/stsi.c >> > > Just wondering, did you try them with TCG as well? (or do I have to test) > No, they are also pending for z/VM and LPAR testing (well at least for STSI). They have been running with a fmt2 and a fmt4 SIE description though. I'll speak with the CI people here to add a TCG test. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank ` (3 preceding siblings ...) 2019-08-20 11:11 ` [kvm-unit-tests PATCH 0/3] s390x: More emulation tests David Hildenbrand @ 2019-08-20 19:04 ` David Hildenbrand 2019-08-21 8:48 ` Janosch Frank 4 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-08-20 19:04 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, thuth On 20.08.19 12:55, Janosch Frank wrote: > The first patch allows for CECSIM booting via PSW restart. > The other ones add diag288 and STSI tests. > > I chose to start with these since they are low controversy. My queue > still contains the sclp patches and a simple smp library with > tests. They will follow later. On which branch do these patches apply? I fail to am 2+3 on master (well I didn't try too hard to resolve ;) ). Do you have a branch somewhere? -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-20 19:04 ` David Hildenbrand @ 2019-08-21 8:48 ` Janosch Frank 2019-08-21 8:53 ` David Hildenbrand 0 siblings, 1 reply; 18+ messages in thread From: Janosch Frank @ 2019-08-21 8:48 UTC (permalink / raw) To: David Hildenbrand, kvm; +Cc: linux-s390, thuth [-- Attachment #1.1: Type: text/plain, Size: 745 bytes --] On 8/20/19 9:04 PM, David Hildenbrand wrote: > On 20.08.19 12:55, Janosch Frank wrote: >> The first patch allows for CECSIM booting via PSW restart. >> The other ones add diag288 and STSI tests. >> >> I chose to start with these since they are low controversy. My queue >> still contains the sclp patches and a simple smp library with >> tests. They will follow later. > > On which branch do these patches apply? I fail to am 2+3 on master (well > I didn't try too hard to resolve ;) ). Do you have a branch somewhere? > That is currently on top of master (24efc22), the only merge conflicts might be s390x/Makefile or unittests.conf if your branch is not clean. I'm trying to get a public github account for that and qemu. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-21 8:48 ` Janosch Frank @ 2019-08-21 8:53 ` David Hildenbrand 2019-08-21 9:28 ` Janosch Frank 0 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-08-21 8:53 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, thuth On 21.08.19 10:48, Janosch Frank wrote: > On 8/20/19 9:04 PM, David Hildenbrand wrote: >> On 20.08.19 12:55, Janosch Frank wrote: >>> The first patch allows for CECSIM booting via PSW restart. >>> The other ones add diag288 and STSI tests. >>> >>> I chose to start with these since they are low controversy. My queue >>> still contains the sclp patches and a simple smp library with >>> tests. They will follow later. >> >> On which branch do these patches apply? I fail to am 2+3 on master (well >> I didn't try too hard to resolve ;) ). Do you have a branch somewhere? >> > > That is currently on top of master (24efc22), the only merge conflicts > might be s390x/Makefile or unittests.conf if your branch is not clean. > I'm trying to get a public github account for that and qemu. > t460s: ~/git/kvm-unit-tests master $ git fetch origin t460s: ~/git/kvm-unit-tests master $ git reset --hard origin/master HEAD is now at 03b1e45 x86: Support environments without test-devices t460s: ~/git/kvm-unit-tests master $ git am \[kvm-unit-tests\ PATCH\ * Applying: s390x: Support PSW restart boot Applying: s390x: Diag288 test error: patch failed: s390x/Makefile:11 error: s390x/Makefile: patch does not apply error: patch failed: s390x/unittests.cfg:61 error: s390x/unittests.cfg: patch does not apply Patch failed at 0002 s390x: Diag288 test hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Are you sure? -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 0/3] s390x: More emulation tests 2019-08-21 8:53 ` David Hildenbrand @ 2019-08-21 9:28 ` Janosch Frank 0 siblings, 0 replies; 18+ messages in thread From: Janosch Frank @ 2019-08-21 9:28 UTC (permalink / raw) To: David Hildenbrand, kvm; +Cc: linux-s390, thuth [-- Attachment #1.1: Type: text/plain, Size: 1837 bytes --] On 8/21/19 10:53 AM, David Hildenbrand wrote: > On 21.08.19 10:48, Janosch Frank wrote: >> On 8/20/19 9:04 PM, David Hildenbrand wrote: >>> On 20.08.19 12:55, Janosch Frank wrote: >>>> The first patch allows for CECSIM booting via PSW restart. >>>> The other ones add diag288 and STSI tests. >>>> >>>> I chose to start with these since they are low controversy. My queue >>>> still contains the sclp patches and a simple smp library with >>>> tests. They will follow later. >>> >>> On which branch do these patches apply? I fail to am 2+3 on master (well >>> I didn't try too hard to resolve ;) ). Do you have a branch somewhere? >>> >> >> That is currently on top of master (24efc22), the only merge conflicts >> might be s390x/Makefile or unittests.conf if your branch is not clean. >> I'm trying to get a public github account for that and qemu. >> > > t460s: ~/git/kvm-unit-tests master $ git fetch origin > t460s: ~/git/kvm-unit-tests master $ git reset --hard origin/master > HEAD is now at 03b1e45 x86: Support environments without test-devices > t460s: ~/git/kvm-unit-tests master $ git am \[kvm-unit-tests\ PATCH\ * > Applying: s390x: Support PSW restart boot > Applying: s390x: Diag288 test > error: patch failed: s390x/Makefile:11 > error: s390x/Makefile: patch does not apply > error: patch failed: s390x/unittests.cfg:61 > error: s390x/unittests.cfg: patch does not apply > Patch failed at 0002 s390x: Diag288 test > hint: Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > Are you sure? > The internal mirror did not pick up the changes of the last few days... [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-08-21 9:28 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-20 10:55 [kvm-unit-tests PATCH 0/3] s390x: More emulation tests Janosch Frank 2019-08-20 10:55 ` [kvm-unit-tests PATCH 1/3] s390x: Support PSW restart boot Janosch Frank 2019-08-20 11:40 ` Thomas Huth 2019-08-20 10:55 ` [kvm-unit-tests PATCH 2/3] s390x: Diag288 test Janosch Frank 2019-08-20 11:59 ` Thomas Huth 2019-08-20 12:25 ` Janosch Frank 2019-08-20 12:55 ` Thomas Huth 2019-08-20 15:21 ` Janosch Frank 2019-08-20 15:29 ` Thomas Huth 2019-08-20 10:55 ` [kvm-unit-tests PATCH 3/3] s390x: STSI tests Janosch Frank 2019-08-20 13:21 ` Thomas Huth 2019-08-21 8:46 ` Janosch Frank 2019-08-20 11:11 ` [kvm-unit-tests PATCH 0/3] s390x: More emulation tests David Hildenbrand 2019-08-20 11:49 ` Janosch Frank 2019-08-20 19:04 ` David Hildenbrand 2019-08-21 8:48 ` Janosch Frank 2019-08-21 8:53 ` David Hildenbrand 2019-08-21 9:28 ` Janosch Frank
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.