[PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[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>
---
 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;
-- 
2.47.0

Re: [PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[Heiko Carstens]

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?

Re: [PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[Claudio Imbrenda]

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.

Re: [PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[Heiko Carstens]

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!

Re: [PATCH v1 1/1] s390/kvm: mask extra bits from program interrupt code

[Christian Borntraeger]

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;