| Message ID | 20241031120316.25462-1-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] s390/kvm: mask extra bits from program interrupt code | expand |
On Thu, Oct 31, 2024 at 01:03:16PM +0100, Claudio Imbrenda wrote: > The program interrupt code has some extra bits that are sometimes set > by hardware for various reasons; those bits should be ignored when the > program interrupt number is needed for interrupt handling. > > Fixes: ce2b276ebe51 ("s390/mm/fault: Handle guest-related program interrupts in KVM") > Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 8b3afda99397..f2d1351f6992 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4737,7 +4737,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu) > if (kvm_s390_cur_gmap_fault_is_write()) > flags = FAULT_FLAG_WRITE; > > - switch (current->thread.gmap_int_code) { > + switch (current->thread.gmap_int_code & PGM_INT_CODE_MASK) { Can you give an example? When reviewing your patch I was aware of this, but actually thought we do want to know when this happens, since the kernel did something which causes such bits to be set; e.g. single stepping with PER on the sie instruction. If that happens then such program interruptions should not be passed for kvm handling, since that would indicate a host kernel bug (the sie instruction is not allowed to be single stepped). Or in other words: this should never happen. Of course I might have missed something; so when could this happen where this is not a bug and the bits should be ignored?
On Thu, 31 Oct 2024 13:38:15 +0100 Heiko Carstens <hca@linux.ibm.com> wrote: > On Thu, Oct 31, 2024 at 01:03:16PM +0100, Claudio Imbrenda wrote: > > The program interrupt code has some extra bits that are sometimes set > > by hardware for various reasons; those bits should be ignored when the > > program interrupt number is needed for interrupt handling. > > > > Fixes: ce2b276ebe51 ("s390/mm/fault: Handle guest-related program interrupts in KVM") > > Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > --- > > arch/s390/kvm/kvm-s390.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > > index 8b3afda99397..f2d1351f6992 100644 > > --- a/arch/s390/kvm/kvm-s390.c > > +++ b/arch/s390/kvm/kvm-s390.c > > @@ -4737,7 +4737,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu) > > if (kvm_s390_cur_gmap_fault_is_write()) > > flags = FAULT_FLAG_WRITE; > > > > - switch (current->thread.gmap_int_code) { > > + switch (current->thread.gmap_int_code & PGM_INT_CODE_MASK) { > > Can you give an example? When reviewing your patch I was aware of this, but > actually thought we do want to know when this happens, since the kernel did > something which causes such bits to be set; e.g. single stepping with PER > on the sie instruction. If that happens then such program interruptions > should not be passed for kvm handling, since that would indicate a host > kernel bug (the sie instruction is not allowed to be single stepped). > > Or in other words: this should never happen. Of course I might have missed > something; so when could this happen where this is not a bug and the bits > should be ignored? in some cases some guest indication bits might be set when a host exception happens. I was also unaware of those and found out the hard way.
Am 31.10.24 um 13:03 schrieb Claudio Imbrenda: > The program interrupt code has some extra bits that are sometimes set > by hardware for various reasons; those bits should be ignored when the > program interrupt number is needed for interrupt handling. > > Fixes: ce2b276ebe51 ("s390/mm/fault: Handle guest-related program interrupts in KVM") > Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> seems to fix my issue: Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 8b3afda99397..f2d1351f6992 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4737,7 +4737,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu) > if (kvm_s390_cur_gmap_fault_is_write()) > flags = FAULT_FLAG_WRITE; > > - switch (current->thread.gmap_int_code) { > + switch (current->thread.gmap_int_code & PGM_INT_CODE_MASK) { > case 0: > vcpu->stat.exit_null++; > break;
On Thu, Oct 31, 2024 at 02:01:13PM +0100, Claudio Imbrenda wrote: > On Thu, 31 Oct 2024 13:38:15 +0100 > Heiko Carstens <hca@linux.ibm.com> wrote: > > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > > > index 8b3afda99397..f2d1351f6992 100644 > > > --- a/arch/s390/kvm/kvm-s390.c > > > +++ b/arch/s390/kvm/kvm-s390.c > > > @@ -4737,7 +4737,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu) > > > if (kvm_s390_cur_gmap_fault_is_write()) > > > flags = FAULT_FLAG_WRITE; > > > > > > - switch (current->thread.gmap_int_code) { > > > + switch (current->thread.gmap_int_code & PGM_INT_CODE_MASK) { > > > > Can you give an example? When reviewing your patch I was aware of this, but > > actually thought we do want to know when this happens, since the kernel did > > something which causes such bits to be set; e.g. single stepping with PER > > on the sie instruction. If that happens then such program interruptions > > should not be passed for kvm handling, since that would indicate a host > > kernel bug (the sie instruction is not allowed to be single stepped). > > > > Or in other words: this should never happen. Of course I might have missed > > something; so when could this happen where this is not a bug and the bits > > should be ignored? > > in some cases some guest indication bits might be set when a > host exception happens. > > I was also unaware of those and found out the hard way. Thanks for explaining. Chances are that we need another patch to address this for the vsie code as well (handle_fault()). But that would be another patch. Applied, thanks!
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 8b3afda99397..f2d1351f6992 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4737,7 +4737,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu) if (kvm_s390_cur_gmap_fault_is_write()) flags = FAULT_FLAG_WRITE; - switch (current->thread.gmap_int_code) { + switch (current->thread.gmap_int_code & PGM_INT_CODE_MASK) { case 0: vcpu->stat.exit_null++; break;
The program interrupt code has some extra bits that are sometimes set by hardware for various reasons; those bits should be ignored when the program interrupt number is needed for interrupt handling. Fixes: ce2b276ebe51 ("s390/mm/fault: Handle guest-related program interrupts in KVM") Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)