On Tue, 16 May 2023 15:04:55 +0200
Nico Boehr <nrb@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
> 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_ */
