| Message ID | 20211005091153.1863139-2-scgl@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v2,1/2] s390x: Remove assert from arch_def.h | expand |
Oops, forgot to Cc the lists on the cover letter, see below.
When specification exception interpretation is enabled, specification
exceptions need not result in interceptions.
However, if the exception is due to an invalid program new PSW,
interception must occur.
Test this.
Also test that interpretation does occur if interpretation is disabled.
v1 -> v2
Add license and test description
Use lowcore pointer instead of magic value for program new PSW
-> need to get rid of assert in arch_def.h
Do not use odd program new PSW, even if irrelevant
Use SIE lib
Reword messages
Fix nits
Janis Schoetterl-Glausch (2):
s390x: Remove assert from arch_def.h
s390x: Add specification exception interception test
s390x/Makefile | 2 +
lib/s390x/asm/arch_def.h | 5 ++-
lib/s390x/sie.h | 1 +
s390x/snippets/c/spec_ex.c | 20 +++++++++
s390x/spec_ex-sie.c | 83 ++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 3 ++
6 files changed, 112 insertions(+), 2 deletions(-)
create mode 100644 s390x/snippets/c/spec_ex.c
create mode 100644 s390x/spec_ex-sie.c
Range-diff against v1:
-: ------- > 1: 8ad817c s390x: Remove assert from arch_def.h
1: 9e5e161 ! 2: f8abbae s390x: Add specification exception interception test
@@ lib/s390x/sie.h: struct kvm_s390_sie_block {
## s390x/snippets/c/spec_ex.c (new) ##
@@
++// SPDX-License-Identifier: GPL-2.0-only
++/*
++ * © Copyright IBM Corp. 2021
++ *
++ * Snippet used by specification exception interception test.
++ */
+#include <stdint.h>
+#include <asm/arch_def.h>
+
+__attribute__((section(".text"))) int main(void)
+{
++ struct lowcore *lowcore = (struct lowcore *) 0;
+ uint64_t bad_psw = 0;
-+ struct psw *pgm_new = (struct psw *)464;
+
-+ pgm_new->mask = 1UL << (63 - 12); //invalid program new PSW
-+ pgm_new->addr = 0xdeadbeef;
++ /* PSW bit 12 has no name or meaning and must be 0 */
++ lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
++ lowcore->pgm_new_psw.addr = 0xdeadbeee;
+ asm volatile ("lpsw %0" :: "Q"(bad_psw));
+ return 0;
+}
## s390x/spec_ex-sie.c (new) ##
@@
++// SPDX-License-Identifier: GPL-2.0-only
++/*
++ * © Copyright IBM Corp. 2021
++ *
++ * Specification exception interception test.
++ * Checks that specification exception interceptions occur as expected when
++ * specification exception interpretation is off/on.
++ */
+#include <libcflat.h>
+#include <sclp.h>
+#include <asm/page.h>
@@ s390x/spec_ex-sie.c (new)
+ (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
+
+ setup_vm();
-+
-+ /* Allocate 1MB as guest memory */
+ guest = alloc_pages(8);
-+ /* The first two pages are the lowcore */
-+
-+ vm.sblk = alloc_page();
-+
-+ vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
-+ vm.sblk->prefix = 0;
-+ /*
-+ * Pageable guest with the same ASCE as the test program, 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();
-+
+ memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
++ sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
+}
+
+static void reset_guest(void)
+{
+ vm.sblk->gpsw.addr = PAGE_SIZE * 4;
-+ vm.sblk->gpsw.mask = 0x0000000180000000ULL;
++ vm.sblk->gpsw.mask = PSW_MASK_64;
++ vm.sblk->icptcode = 0;
+}
+
+static void test_spec_ex_sie(void)
+{
+ setup_guest();
+
-+ report_prefix_push("spec ex interpretation off");
++ report_prefix_push("SIE spec ex interpretation");
++ report_prefix_push("off");
+ reset_guest();
-+ sie64a(vm.sblk, &vm.save_area);
-+ //interpretation off -> initial exception must cause interception
++ sie(&vm);
++ /* interpretation off -> initial exception must cause interception */
+ report(vm.sblk->icptcode == ICPT_PROGI
+ && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
-+ && vm.sblk->gpsw.addr != 0xdeadbeef,
-+ "Received specification exception intercept for non program new PSW");
++ && vm.sblk->gpsw.addr != 0xdeadbeee,
++ "Received specification exception intercept for initial exception");
+ report_prefix_pop();
+
-+ report_prefix_push("spec ex interpretation on");
++ report_prefix_push("on");
+ vm.sblk->ecb |= ECB_SPECI;
+ reset_guest();
-+ sie64a(vm.sblk, &vm.save_area);
-+ // interpretation on -> configuration dependent if initial exception causes
-+ // interception, but invalid new program PSW must
++ sie(&vm);
++ /* interpretation on -> configuration dependent if initial exception causes
++ * interception, but invalid new program PSW must
++ */
+ report(vm.sblk->icptcode == ICPT_PROGI
+ && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
+ "Received specification exception intercept");
-+ if (vm.sblk->gpsw.addr == 0xdeadbeef)
-+ report_info("Interpreted non program new PSW specification exception");
++ if (vm.sblk->gpsw.addr == 0xdeadbeee)
++ report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
+ else
-+ report_info("Did not interpret non program new PSW specification exception");
++ report_info("Did not interpret initial exception");
++ report_prefix_pop();
+ report_prefix_pop();
+}
+
base-commit: fe26131eec769cef7ad7e2e1e4e192aa0bdb2bba
On Tue, 5 Oct 2021 11:11:52 +0200 Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote: > Do not use asserts in arch_def.h so it can be included by snippets. > The caller in stsi.c does not need to be adjusted, returning -1 causes > the test to fail. > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/s390x/asm/arch_def.h | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 302ef1f..4167e2b 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2) > return cc; > } > > -static inline unsigned long stsi_get_fc(void) > +static inline int stsi_get_fc(void) > { > register unsigned long r0 asm("0") = 0; > register unsigned long r1 asm("1") = 0; > @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void) > : "+d" (r0), [cc] "=d" (cc) > : "d" (r1) > : "cc", "memory"); > - assert(!cc); > + if (cc != 0) > + return -1; > return r0 >> 28; > } >
On 10/5/21 11:11, Janis Schoetterl-Glausch wrote: > Do not use asserts in arch_def.h so it can be included by snippets. > The caller in stsi.c does not need to be adjusted, returning -1 causes > the test to fail. > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> A few days ago I had a minute to investigate what needed to be added to be able to link the libcflat. Fortunately it wasn't a lot and I'll try to post it this week so this patch can hopefully be dropped. > --- > lib/s390x/asm/arch_def.h | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 302ef1f..4167e2b 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2) > return cc; > } > > -static inline unsigned long stsi_get_fc(void) > +static inline int stsi_get_fc(void) > { > register unsigned long r0 asm("0") = 0; > register unsigned long r1 asm("1") = 0; > @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void) > : "+d" (r0), [cc] "=d" (cc) > : "d" (r1) > : "cc", "memory"); > - assert(!cc); > + if (cc != 0) > + return -1; > return r0 >> 28; > } > >
On 10/5/21 2:51 PM, Janosch Frank wrote: > On 10/5/21 11:11, Janis Schoetterl-Glausch wrote: >> Do not use asserts in arch_def.h so it can be included by snippets. >> The caller in stsi.c does not need to be adjusted, returning -1 causes >> the test to fail. >> >> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > > A few days ago I had a minute to investigate what needed to be added to be able to link the libcflat. Fortunately it wasn't a lot and I'll try to post it this week so this patch can hopefully be dropped. One could argue that cc being != 0 is part of the test and so should go through report() and not assert(). Which happens naturally, since the caller will likely compare it to some positive expected value. > >> --- >> lib/s390x/asm/arch_def.h | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >> index 302ef1f..4167e2b 100644 >> --- a/lib/s390x/asm/arch_def.h >> +++ b/lib/s390x/asm/arch_def.h >> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2) >> return cc; >> } >> -static inline unsigned long stsi_get_fc(void) >> +static inline int stsi_get_fc(void) >> { >> register unsigned long r0 asm("0") = 0; >> register unsigned long r1 asm("1") = 0; >> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void) >> : "+d" (r0), [cc] "=d" (cc) >> : "d" (r1) >> : "cc", "memory"); >> - assert(!cc); >> + if (cc != 0) >> + return -1; >> return r0 >> 28; >> } >> >
On 10/5/21 15:51, Janis Schoetterl-Glausch wrote: > On 10/5/21 2:51 PM, Janosch Frank wrote: >> On 10/5/21 11:11, Janis Schoetterl-Glausch wrote: >>> Do not use asserts in arch_def.h so it can be included by snippets. >>> The caller in stsi.c does not need to be adjusted, returning -1 causes >>> the test to fail. >>> >>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> >> >> A few days ago I had a minute to investigate what needed to be added to be able to link the libcflat. Fortunately it wasn't a lot and I'll try to post it this week so this patch can hopefully be dropped. > > One could argue that cc being != 0 is part of the test and so should go through report() and not assert(). > Which happens naturally, since the caller will likely compare it to some positive expected value. Yes, I can also live with that if you change the commit message then :) >> >>> --- >>> lib/s390x/asm/arch_def.h | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h >>> index 302ef1f..4167e2b 100644 >>> --- a/lib/s390x/asm/arch_def.h >>> +++ b/lib/s390x/asm/arch_def.h >>> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2) >>> return cc; >>> } >>> -static inline unsigned long stsi_get_fc(void) >>> +static inline int stsi_get_fc(void) >>> { >>> register unsigned long r0 asm("0") = 0; >>> register unsigned long r1 asm("1") = 0; >>> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void) >>> : "+d" (r0), [cc] "=d" (cc) >>> : "d" (r1) >>> : "cc", "memory"); >>> - assert(!cc); >>> + if (cc != 0) >>> + return -1; >>> return r0 >> 28; >>> } >>> >> >
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index 302ef1f..4167e2b 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2) return cc; } -static inline unsigned long stsi_get_fc(void) +static inline int stsi_get_fc(void) { register unsigned long r0 asm("0") = 0; register unsigned long r1 asm("1") = 0; @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void) : "+d" (r0), [cc] "=d" (cc) : "d" (r1) : "cc", "memory"); - assert(!cc); + if (cc != 0) + return -1; return r0 >> 28; }
Do not use asserts in arch_def.h so it can be included by snippets. The caller in stsi.c does not need to be adjusted, returning -1 causes the test to fail. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> --- lib/s390x/asm/arch_def.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)