| Message ID | 20220630113059.229221-4-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add panic test support | expand |
On 6/30/22 13:30, Nico Boehr wrote: > An invalid PSW causes a program interrupt. When an invalid PSW is > introduced in the pgm_new_psw, an interrupt loop occurs as soon as a > program interrupt is caused. > > QEMU should detect that and panick the guest, hence add a test for it. Why is that, after all in LPAR it would just spin, right? Also, panicK. How do you assert that the guest doesn't spin forever, is there a timeout?
On 30/06/2022 16.38, Janis Schoetterl-Glausch wrote: > On 6/30/22 13:30, Nico Boehr wrote: >> An invalid PSW causes a program interrupt. When an invalid PSW is >> introduced in the pgm_new_psw, an interrupt loop occurs as soon as a >> program interrupt is caused. >> >> QEMU should detect that and panick the guest, hence add a test for it. > > Why is that, after all in LPAR it would just spin, right? Not sure what the LPAR is doing, but the guest is certainly completely unusable, so a panic event is the right thing to do here for QEMU. > Also, panicK. > How do you assert that the guest doesn't spin forever, is there a timeout? I agree, it would be good to have a "timeout" set in the unittests.cfg for this test here (some few seconds should be enough). Thomas
On 30/06/2022 13.30, Nico Boehr wrote: > An invalid PSW causes a program interrupt. When an invalid PSW is > introduced in the pgm_new_psw, an interrupt loop occurs as soon as a > program interrupt is caused. > > QEMU should detect that and panick the guest, hence add a test for it. > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- .... > +int main(void) > +{ > + report_prefix_push("pgmint-loop"); > + > + lowcore.pgm_new_psw.addr = (uint64_t) pgm_int_handler; > + /* bit 12 set is invalid */ > + lowcore.pgm_new_psw.mask = extract_psw_mask() | BIT(63 - 12); Basically patch looks fine to me ... just an idea for an extension (but that could also be done later): Looking at the is_valid_psw() function in the Linux kernel sources, there are a couple of additional condition that could cause a PGM interrupt loop ... you could maybe check them here, too, e.g. by adding a "extra_params = -append '...'" in the unittests.cfg file to select the indiviual tests via argv[] ? Thomas
Quoting Janis Schoetterl-Glausch (2022-06-30 16:38:47) > On 6/30/22 13:30, Nico Boehr wrote: > > An invalid PSW causes a program interrupt. When an invalid PSW is > > introduced in the pgm_new_psw, an interrupt loop occurs as soon as a > > program interrupt is caused. > > > > QEMU should detect that and panick the guest, hence add a test for it. > > Why is that, after all in LPAR it would just spin, right? The test doesn't spin for me under LPAR so it seems like LPAR can detect this as well. KVM has code to detect this situation, see handle_prog() in intercept.c, which then exits to userspace. > Also, panicK. Right, fixed. > How do you assert that the guest doesn't spin forever, is there a timeout? There is the default kvm-unit-tests timeout of 90 seconds, but that is probably too much for this test. I think 5 seconds should be plenty, will add.
Quoting Thomas Huth (2022-06-30 19:25:57) > On 30/06/2022 13.30, Nico Boehr wrote: > > An invalid PSW causes a program interrupt. When an invalid PSW is > > introduced in the pgm_new_psw, an interrupt loop occurs as soon as a > > program interrupt is caused. > > > > QEMU should detect that and panick the guest, hence add a test for it. > > > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > > --- > .... > > +int main(void) > > +{ > > + report_prefix_push("pgmint-loop"); > > + > > + lowcore.pgm_new_psw.addr = (uint64_t) pgm_int_handler; > > + /* bit 12 set is invalid */ > > + lowcore.pgm_new_psw.mask = extract_psw_mask() | BIT(63 - 12); > > Basically patch looks fine to me ... just an idea for an extension (but that > could also be done later): > > Looking at the is_valid_psw() function in the Linux kernel sources, there > are a couple of additional condition that could cause a PGM interrupt loop > ... you could maybe check them here, too, e.g. by adding a "extra_params = > -append '...'" in the unittests.cfg file to select the indiviual tests via > argv[] ? It is a good idea, I have it on my TODO and will address it in a upcoming patchset.
On 6/30/22 19:11, Thomas Huth wrote: > On 30/06/2022 16.38, Janis Schoetterl-Glausch wrote: >> On 6/30/22 13:30, Nico Boehr wrote: >>> An invalid PSW causes a program interrupt. When an invalid PSW is >>> introduced in the pgm_new_psw, an interrupt loop occurs as soon as a >>> program interrupt is caused. >>> >>> QEMU should detect that and panick the guest, hence add a test for it. >> >> Why is that, after all in LPAR it would just spin, right? > > Not sure what the LPAR is doing, but the guest is certainly completely unusable, so a panic event is the right thing to do here for QEMU. I suppose some other kind of interrupt could fix things up somehow, but I guess in practice panicking does indeed make more sense.
On 6/30/22 13:30, Nico Boehr wrote: > An invalid PSW causes a program interrupt. When an invalid PSW is > introduced in the pgm_new_psw, an interrupt loop occurs as soon as a > program interrupt is caused. > > QEMU should detect that and panick the guest, hence add a test for it. > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> Test is fine but the same general comments as in patch #2 apply. > --- > s390x/Makefile | 1 + > s390x/pgmint-loop.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 4 ++++ > 3 files changed, 51 insertions(+) > create mode 100644 s390x/pgmint-loop.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index 92a020234c9f..a600dbfb3f4c 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -35,6 +35,7 @@ tests += $(TEST_DIR)/pv-attest.elf > tests += $(TEST_DIR)/migration-cmm.elf > tests += $(TEST_DIR)/migration-skey.elf > tests += $(TEST_DIR)/extint-loop.elf > +tests += $(TEST_DIR)/pgmint-loop.elf > > pv-tests += $(TEST_DIR)/pv-diags.elf > > diff --git a/s390x/pgmint-loop.c b/s390x/pgmint-loop.c > new file mode 100644 > index 000000000000..5b74f26dbc3d > --- /dev/null > +++ b/s390x/pgmint-loop.c > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Program interrupt loop test > + * > + * Copyright IBM Corp. 2022 > + * > + * Authors: > + * Nico Boehr <nrb@linux.ibm.com> > + */ > +#include <libcflat.h> > +#include <bitops.h> > +#include <asm/interrupt.h> > +#include <asm/barrier.h> > + > +static void pgm_int_handler(void) > +{ > + /* > + * return to pgm_old_psw. This gives us the chance to print the return_fail > + * in case something goes wrong. > + */ > + asm volatile ( > + "lpswe %[pgm_old_psw]\n" > + : > + : [pgm_old_psw] "Q"(lowcore.pgm_old_psw) > + : "memory" > + ); > +} > + > +int main(void) > +{ > + report_prefix_push("pgmint-loop"); > + > + lowcore.pgm_new_psw.addr = (uint64_t) pgm_int_handler; > + /* bit 12 set is invalid */ > + lowcore.pgm_new_psw.mask = extract_psw_mask() | BIT(63 - 12); > + mb(); > + > + /* cause a pgm int */ > + *((int *)-4) = 0x42; > + mb(); > + > + report_fail("survived pgmint loop"); > + > + report_prefix_pop(); > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 7d408f2d5310..c3073bfc4363 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -188,3 +188,7 @@ groups = migration > [extint-loop] > file = extint-loop.elf > groups = panic > + > +[pgmint-loop] > +file = pgmint-loop.elf > +groups = panic
diff --git a/s390x/Makefile b/s390x/Makefile index 92a020234c9f..a600dbfb3f4c 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -35,6 +35,7 @@ tests += $(TEST_DIR)/pv-attest.elf tests += $(TEST_DIR)/migration-cmm.elf tests += $(TEST_DIR)/migration-skey.elf tests += $(TEST_DIR)/extint-loop.elf +tests += $(TEST_DIR)/pgmint-loop.elf pv-tests += $(TEST_DIR)/pv-diags.elf diff --git a/s390x/pgmint-loop.c b/s390x/pgmint-loop.c new file mode 100644 index 000000000000..5b74f26dbc3d --- /dev/null +++ b/s390x/pgmint-loop.c @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Program interrupt loop test + * + * Copyright IBM Corp. 2022 + * + * Authors: + * Nico Boehr <nrb@linux.ibm.com> + */ +#include <libcflat.h> +#include <bitops.h> +#include <asm/interrupt.h> +#include <asm/barrier.h> + +static void pgm_int_handler(void) +{ + /* + * return to pgm_old_psw. This gives us the chance to print the return_fail + * in case something goes wrong. + */ + asm volatile ( + "lpswe %[pgm_old_psw]\n" + : + : [pgm_old_psw] "Q"(lowcore.pgm_old_psw) + : "memory" + ); +} + +int main(void) +{ + report_prefix_push("pgmint-loop"); + + lowcore.pgm_new_psw.addr = (uint64_t) pgm_int_handler; + /* bit 12 set is invalid */ + lowcore.pgm_new_psw.mask = extract_psw_mask() | BIT(63 - 12); + mb(); + + /* cause a pgm int */ + *((int *)-4) = 0x42; + mb(); + + report_fail("survived pgmint loop"); + + report_prefix_pop(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 7d408f2d5310..c3073bfc4363 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -188,3 +188,7 @@ groups = migration [extint-loop] file = extint-loop.elf groups = panic + +[pgmint-loop] +file = pgmint-loop.elf +groups = panic
An invalid PSW causes a program interrupt. When an invalid PSW is introduced in the pgm_new_psw, an interrupt loop occurs as soon as a program interrupt is caused. QEMU should detect that and panick the guest, hence add a test for it. Signed-off-by: Nico Boehr <nrb@linux.ibm.com> --- s390x/Makefile | 1 + s390x/pgmint-loop.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 4 ++++ 3 files changed, 51 insertions(+) create mode 100644 s390x/pgmint-loop.c