| Message ID | 20230516130456.256205-6-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Add support for running guests without MSO/MSL | expand |
On Tue, 16 May 2023 15:04:55 +0200 Nico Boehr <nrb@linux.ibm.com> wrote: > At the moment, when a PGM int occurs while in SIE, we will just reenter > SIE after the interrupt handler was called. > > This is because sie() has a loop which checks icptcode and re-enters SIE > if it is zero. > > However, this behaviour is quite undesirable for SIE tests, since it > doesn't give the host the chance to assert on the PGM int. Instead, we > will just re-enter SIE, on nullifing conditions even causing the > exception again. > > In sie(), check whether a pgm int code is set in lowcore. If it has, > exit the loop so the test can react to the interrupt. Add a new function > read_pgm_int_code() to obtain the interrupt code. > > Note that this introduces a slight oddity with sie and pgm int in > certain cases: If a PGM int occurs between a expect_pgm_int() and sie(), > we will now never enter SIE until the pgm_int_code is cleared by e.g. > clear_pgm_int(). > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- > lib/s390x/asm/interrupt.h | 1 + > lib/s390x/interrupt.c | 15 +++++++++++++++ > lib/s390x/sie.c | 4 +++- > lib/s390x/sie.h | 1 - > 4 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h > index 55759002dce2..2d7eb1907458 100644 > --- a/lib/s390x/asm/interrupt.h > +++ b/lib/s390x/asm/interrupt.h > @@ -81,6 +81,7 @@ void handle_svc_int(void); > void expect_pgm_int(void); > void expect_ext_int(void); > uint16_t clear_pgm_int(void); > +uint16_t read_pgm_int_code(void); > void check_pgm_int_code(uint16_t code); > > #define IRQ_DAT_ON true > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c > index 2e5309cee40f..82b4259d433c 100644 > --- a/lib/s390x/interrupt.c > +++ b/lib/s390x/interrupt.c > @@ -60,6 +60,21 @@ uint16_t clear_pgm_int(void) > return code; > } > > +/** > + * read_pgm_int_code - Get the program interruption code of the last pgm int > + * on the current CPU. > + * > + * This is similar to clear_pgm_int(), except that it doesn't clear the > + * interruption information from lowcore. > + * > + * Returns 0 when none occured. > + */ > +uint16_t read_pgm_int_code(void) could this whole function go in the header as static inline? > +{ > + mb(); is the mb really needed? > + return lowcore.pgm_int_code; > +} > + > /** > * check_pgm_int_code - Check the program interrupt code on the current CPU. > * @code the expected program interrupt code on the current CPU > diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c > index ffa8ec91a423..632740edd431 100644 > --- a/lib/s390x/sie.c > +++ b/lib/s390x/sie.c > @@ -13,6 +13,7 @@ > #include <libcflat.h> > #include <sie.h> > #include <asm/page.h> > +#include <asm/interrupt.h> > #include <libcflat.h> > #include <alloc_page.h> > > @@ -65,7 +66,8 @@ void sie(struct vm *vm) > /* also handle all interruptions in home space while in SIE */ > irq_set_dat_mode(IRQ_DAT_ON, AS_HOME); > > - while (vm->sblk->icptcode == 0) { > + /* leave SIE when we have an intercept or an interrupt so the test can react to it */ > + while (vm->sblk->icptcode == 0 && !read_pgm_int_code()) { > sie64a(vm->sblk, &vm->save_area); > sie_handle_validity(vm); > } > diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h > index 0b00fb709776..147cb0f2a556 100644 > --- a/lib/s390x/sie.h > +++ b/lib/s390x/sie.h > @@ -284,6 +284,5 @@ void sie_handle_validity(struct vm *vm); > void sie_guest_sca_create(struct vm *vm); > void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len); > void sie_guest_destroy(struct vm *vm); > -bool sie_had_pgm_int(struct vm *vm); ... ? > > #endif /* _S390X_SIE_H_ */
Quoting Claudio Imbrenda (2023-05-16 19:30:18) [...] > > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c > > index 2e5309cee40f..82b4259d433c 100644 > > --- a/lib/s390x/interrupt.c > > +++ b/lib/s390x/interrupt.c [...] > > +/** > > + * read_pgm_int_code - Get the program interruption code of the last pgm int > > + * on the current CPU. > > + * > > + * This is similar to clear_pgm_int(), except that it doesn't clear the > > + * interruption information from lowcore. > > + * > > + * Returns 0 when none occured. > > + */ > > +uint16_t read_pgm_int_code(void) > > could this whole function go in the header as static inline? Yes, sure, changed. > > +{ > > + mb(); > > is the mb really needed? No, I don't think so, since this is a function, I'll remove this.
diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h index 55759002dce2..2d7eb1907458 100644 --- a/lib/s390x/asm/interrupt.h +++ b/lib/s390x/asm/interrupt.h @@ -81,6 +81,7 @@ void handle_svc_int(void); void expect_pgm_int(void); void expect_ext_int(void); uint16_t clear_pgm_int(void); +uint16_t read_pgm_int_code(void); void check_pgm_int_code(uint16_t code); #define IRQ_DAT_ON true diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c index 2e5309cee40f..82b4259d433c 100644 --- a/lib/s390x/interrupt.c +++ b/lib/s390x/interrupt.c @@ -60,6 +60,21 @@ uint16_t clear_pgm_int(void) return code; } +/** + * read_pgm_int_code - Get the program interruption code of the last pgm int + * on the current CPU. + * + * This is similar to clear_pgm_int(), except that it doesn't clear the + * interruption information from lowcore. + * + * Returns 0 when none occured. + */ +uint16_t read_pgm_int_code(void) +{ + mb(); + return lowcore.pgm_int_code; +} + /** * check_pgm_int_code - Check the program interrupt code on the current CPU. * @code the expected program interrupt code on the current CPU diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c index ffa8ec91a423..632740edd431 100644 --- a/lib/s390x/sie.c +++ b/lib/s390x/sie.c @@ -13,6 +13,7 @@ #include <libcflat.h> #include <sie.h> #include <asm/page.h> +#include <asm/interrupt.h> #include <libcflat.h> #include <alloc_page.h> @@ -65,7 +66,8 @@ void sie(struct vm *vm) /* also handle all interruptions in home space while in SIE */ irq_set_dat_mode(IRQ_DAT_ON, AS_HOME); - while (vm->sblk->icptcode == 0) { + /* leave SIE when we have an intercept or an interrupt so the test can react to it */ + while (vm->sblk->icptcode == 0 && !read_pgm_int_code()) { sie64a(vm->sblk, &vm->save_area); sie_handle_validity(vm); } diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h index 0b00fb709776..147cb0f2a556 100644 --- a/lib/s390x/sie.h +++ b/lib/s390x/sie.h @@ -284,6 +284,5 @@ void sie_handle_validity(struct vm *vm); void sie_guest_sca_create(struct vm *vm); void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len); void sie_guest_destroy(struct vm *vm); -bool sie_had_pgm_int(struct vm *vm); #endif /* _S390X_SIE_H_ */
At the moment, when a PGM int occurs while in SIE, we will just reenter SIE after the interrupt handler was called. This is because sie() has a loop which checks icptcode and re-enters SIE if it is zero. However, this behaviour is quite undesirable for SIE tests, since it doesn't give the host the chance to assert on the PGM int. Instead, we will just re-enter SIE, on nullifing conditions even causing the exception again. In sie(), check whether a pgm int code is set in lowcore. If it has, exit the loop so the test can react to the interrupt. Add a new function read_pgm_int_code() to obtain the interrupt code. Note that this introduces a slight oddity with sie and pgm int in certain cases: If a PGM int occurs between a expect_pgm_int() and sie(), we will now never enter SIE until the pgm_int_code is cleared by e.g. clear_pgm_int(). Signed-off-by: Nico Boehr <nrb@linux.ibm.com> --- lib/s390x/asm/interrupt.h | 1 + lib/s390x/interrupt.c | 15 +++++++++++++++ lib/s390x/sie.c | 4 +++- lib/s390x/sie.h | 1 - 4 files changed, 19 insertions(+), 2 deletions(-)