Re: [kvm-unit-tests PATCH v1 3/4] s390x: lib: sie: don't reenter SIE on pgm int

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Mon, 27 Mar 2023 10:21:17 +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.
> 
> Add a flag PROG_PGM_IN_SIE set by the pgm int fixup which indicates a
> program interrupt has occured in SIE. Check for the flag in sie() and if
> it's set return from sie() to give the host the ability to react on the
> exception. The host may check if a PGM int has occured in the guest
> using the new function sie_had_pgm_int().
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>  lib/s390x/interrupt.c |  6 ++++++
>  lib/s390x/sie.c       | 10 +++++++++-
>  lib/s390x/sie.h       |  1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index eb3d6a9b701d..9baf7a003f52 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -106,10 +106,16 @@ void register_ext_cleanup_func(void (*f)(struct stack_frame_int *))
>  
>  static void fixup_pgm_int(struct stack_frame_int *stack)
>  {
> +	struct kvm_s390_sie_block *sblk;
> +
>  	/* If we have an error on SIE we directly move to sie_exit */
>  	if (lowcore.pgm_old_psw.addr >= (uint64_t)&sie_entry &&
>  	    lowcore.pgm_old_psw.addr <= (uint64_t)&sie_exit) {
>  		lowcore.pgm_old_psw.addr = (uint64_t)&sie_exit;
> +
> +		/* set a marker in sie_block that a PGM int occured */
> +		sblk = *((struct kvm_s390_sie_block **)(stack->grs0[13] + __SF_SIE_CONTROL));
> +		sblk->prog0c |= PROG_PGM_IN_SIE;
>  		return;
>  	}
>  
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index 22141ded1a90..5e9ae7115c47 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -44,6 +44,11 @@ void sie_handle_validity(struct vm *vm)
>  	vm->validity_expected = false;
>  }
>  
> +bool sie_had_pgm_int(struct vm *vm)
> +{
> +	return vm->sblk->prog0c & PROG_PGM_IN_SIE;
> +}
> +
>  void sie(struct vm *vm)
>  {
>  	uint64_t old_cr13;
> @@ -68,7 +73,10 @@ void sie(struct vm *vm)
>  	lowcore.io_new_psw.mask |= PSW_MASK_DAT_HOME;
>  	mb();
>  
> -	while (vm->sblk->icptcode == 0) {
> +	/* clear PGM int marker, which might still be set */
> +	vm->sblk->prog0c &= ~PROG_PGM_IN_SIE;
> +
> +	while (vm->sblk->icptcode == 0 && !sie_had_pgm_int(vm)) {
>  		sie64a(vm->sblk, &vm->save_area);
>  		sie_handle_validity(vm);
>  	}
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index 0b00fb709776..8ab755dc9456 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -37,6 +37,7 @@ struct kvm_s390_sie_block {
>  	uint32_t 	ibc : 12;
>  	uint8_t		reserved08[4];		/* 0x0008 */
>  #define PROG_IN_SIE (1<<0)
> +#define PROG_PGM_IN_SIE (1<<1)

please align the body of the macros with tabs, so they are more readable

>  	uint32_t	prog0c;			/* 0x000c */
>  union {
>  		uint8_t	reserved10[16];		/* 0x0010 */