[PATCH] KVM: s390: only deliver the set service event bits

[PATCH] KVM: s390: only deliver the set service event bits

[Eric Farman]

The SCLP driver code masks off the last two bits of the parameter [1]
to determine if a read is required, but doesn't care about the
contents of those bits. Meanwhile, the KVM code that delivers
event interrupts masks off those two bits but sends both to the
guest, even if only one was specified by userspace [2].

This works for the driver code, but it means any nuances of those
bits gets lost. Use the event pending mask as an actual mask, and
only send the bit(s) that were specified in the pending interrupt.

[1] Linux: sclp_interrupt_handler() (drivers/s390/char/sclp.c:658)
[2] QEMU: service_interrupt() (hw/s390x/sclp.c:360..363)

Fixes: 0890ddea1a90 ("KVM: s390: protvirt: Add SCLP interrupt handling")
Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index fc4007cc067a..20e080e9150b 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1031,7 +1031,7 @@ static int __must_check __deliver_service_ev(struct kvm_vcpu *vcpu)
 		return 0;
 	}
 	ext = fi->srv_signal;
-	/* only clear the event bit */
+	/* only clear the event bits */
 	fi->srv_signal.ext_params &= ~SCCB_EVENT_PENDING;
 	clear_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs);
 	spin_unlock(&fi->lock);
@@ -1041,7 +1041,7 @@ static int __must_check __deliver_service_ev(struct kvm_vcpu *vcpu)
 	trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, KVM_S390_INT_SERVICE,
 					 ext.ext_params, 0);
 
-	return write_sclp(vcpu, SCCB_EVENT_PENDING);
+	return write_sclp(vcpu, ext.ext_params & SCCB_EVENT_PENDING);
 }
 
 static int __must_check __deliver_pfault_done(struct kvm_vcpu *vcpu)
-- 
2.40.1

Re: [PATCH] KVM: s390: only deliver the set service event bits

[Christian Borntraeger]

Am 05.02.24 um 22:43 schrieb Eric Farman:
> The SCLP driver code masks off the last two bits of the parameter [1]
> to determine if a read is required, but doesn't care about the
> contents of those bits. Meanwhile, the KVM code that delivers
> event interrupts masks off those two bits but sends both to the
> guest, even if only one was specified by userspace [2].
> 
> This works for the driver code, but it means any nuances of those
> bits gets lost. Use the event pending mask as an actual mask, and
> only send the bit(s) that were specified in the pending interrupt.
> 
> [1] Linux: sclp_interrupt_handler() (drivers/s390/char/sclp.c:658)
> [2] QEMU: service_interrupt() (hw/s390x/sclp.c:360..363)
> 
> Fixes: 0890ddea1a90 ("KVM: s390: protvirt: Add SCLP interrupt handling")
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

makes sense

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>


> ---
>   arch/s390/kvm/interrupt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index fc4007cc067a..20e080e9150b 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -1031,7 +1031,7 @@ static int __must_check __deliver_service_ev(struct kvm_vcpu *vcpu)
>   		return 0;
>   	}
>   	ext = fi->srv_signal;
> -	/* only clear the event bit */
> +	/* only clear the event bits */
>   	fi->srv_signal.ext_params &= ~SCCB_EVENT_PENDING;
>   	clear_bit(IRQ_PEND_EXT_SERVICE_EV, &fi->pending_irqs);
>   	spin_unlock(&fi->lock);
> @@ -1041,7 +1041,7 @@ static int __must_check __deliver_service_ev(struct kvm_vcpu *vcpu)
>   	trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, KVM_S390_INT_SERVICE,
>   					 ext.ext_params, 0);
>   
> -	return write_sclp(vcpu, SCCB_EVENT_PENDING);
> +	return write_sclp(vcpu, ext.ext_params & SCCB_EVENT_PENDING);
>   }
>   
>   static int __must_check __deliver_pfault_done(struct kvm_vcpu *vcpu)

Re: [PATCH] KVM: s390: only deliver the set service event bits

[Janosch Frank]

On 2/5/24 22:43, Eric Farman wrote:
> The SCLP driver code masks off the last two bits of the parameter [1]
> to determine if a read is required, but doesn't care about the
> contents of those bits. Meanwhile, the KVM code that delivers
> event interrupts masks off those two bits but sends both to the
> guest, even if only one was specified by userspace [2].
> 
> This works for the driver code, but it means any nuances of those
> bits gets lost. Use the event pending mask as an actual mask, and
> only send the bit(s) that were specified in the pending interrupt.
> 
> [1] Linux: sclp_interrupt_handler() (drivers/s390/char/sclp.c:658)
> [2] QEMU: service_interrupt() (hw/s390x/sclp.c:360..363)

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [PATCH] KVM: s390: only deliver the set service event bits

[Janosch Frank]

On 2/5/24 22:43, Eric Farman wrote:
> The SCLP driver code masks off the last two bits of the parameter [1]
> to determine if a read is required, but doesn't care about the
> contents of those bits. Meanwhile, the KVM code that delivers
> event interrupts masks off those two bits but sends both to the
> guest, even if only one was specified by userspace [2].
> 
> This works for the driver code, but it means any nuances of those
> bits gets lost. Use the event pending mask as an actual mask, and
> only send the bit(s) that were specified in the pending interrupt.

Thanks, picked