| Message ID | 20230327082118.2177-4-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Add support for running guests without MSO/MSL | expand |
On 3/27/23 10:21, Nico Boehr 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(). We could simply check "!lowcore.pgm_int_code" by introducing: uint16_t read_pgm_int(void) { mb(); return lowcore.pgm_int_code; } into interrupt.c. Or to be a bit more verbose: I don't see a reason why we'd want to store a per sblk PGM in SIE bit when all we want to know is either: was there a PGM at all (to stop the SIE loop) or was there a PGM between the expect and the check_pgm_int_code().
Quoting Janosch Frank (2023-03-28 15:42:26) > On 3/27/23 10:21, Nico Boehr 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(). > > We could simply check "!lowcore.pgm_int_code" by introducing: > uint16_t read_pgm_int(void) > { > mb(); > return lowcore.pgm_int_code; > } > > into interrupt.c. > > > Or to be a bit more verbose: > I don't see a reason why we'd want to store a per sblk PGM in SIE bit > when all we want to know is either: was there a PGM at all (to stop the > SIE loop) or was there a PGM between the expect and the > check_pgm_int_code(). Yes, makes perfect sense, I just didn't see this possiblity. Thanks, will change.
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 */
Quoting Claudio Imbrenda (2023-03-28 19:01:06) [...] > > 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 Thanks, would do, but this is gonna go away anyways in favor of Janoschs suggestion.
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) uint32_t prog0c; /* 0x000c */ union { uint8_t reserved10[16]; /* 0x0010 */
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(-)