Message ID | 20190820105550.4991-4-frankja@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | s390x: More emulation tests | expand |
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
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 >
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
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