[PATCH v2] KVM: s390: Improve floating IRQ injection behavior

[PATCH v2] KVM: s390: Improve floating IRQ injection behavior

[Janosch Frank]

Floating IRQs can be handled by any VCPU that opened its masks. The
current design does not check if the mask is open when a floating IRQ
is injected via the FLIC. It will wakeup the last VCPU that went
sleeping hoping it's the correct one.

Improve this by at least checking if the VCPU has pending IRQs and if
not try to find another VCPU which can take the IRQ.

Also add a function to distribute floating IRQs which can be called on
VCPU enter or exit. It will check for pending floating IRQs and wakeup
sleeping VCPUs which have pending IRQs.

This is not the final fix around this topic but we'll eventually
inject a pending IRQ. The current code can easily deadlock a VM and
with this fix we work around that.

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

v2:
 - Rebase onto master
 - Added IRQ type check before delivering

---
 arch/s390/include/asm/kvm_host.h |  1 +
 arch/s390/kvm/interrupt.c        | 63 ++++++++++++++++++++++++++++++--
 arch/s390/kvm/kvm-s390.c         |  3 ++
 arch/s390/kvm/kvm-s390.h         |  1 +
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index b4182ca4435f..1d62bdd7aca5 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -467,6 +467,7 @@ struct kvm_vm_stat {
 	u64 gmap_shadow_r3_entry;
 	u64 gmap_shadow_sg_entry;
 	u64 gmap_shadow_pg_entry;
+	u64 inject_redist;
 };
 
 struct kvm_arch_memory_slot {
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 8f24bcd1a6d3..e67cdedb5aec 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -367,6 +367,37 @@ static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
 	return active_mask;
 }
 
+void distribute_float_irqs(struct kvm *kvm)
+{
+	struct kvm_vcpu *dst_vcpu;
+	int sigcpu, online_vcpus;
+
+	if (!READ_ONCE(kvm->arch.float_int.pending_irqs))
+		return;
+
+	online_vcpus = atomic_read(&kvm->online_vcpus);
+
+	/*
+	 * Not too worried about synchronization for idle_mask. We
+	 * might burn too many cycles but apart from that waking a
+	 * vcpu is not harmful.
+	 */
+	sigcpu = find_first_bit(kvm->arch.idle_mask, online_vcpus);
+	/* Well nobody's sleeping so someone will likely take the IRQ soon */
+	if (sigcpu == online_vcpus)
+		return;
+
+	do {
+		dst_vcpu = kvm_get_vcpu(kvm, sigcpu);
+		if (deliverable_irqs(dst_vcpu)) {
+			kvm->stat.inject_redist++;
+			kvm_s390_vcpu_wakeup(dst_vcpu);
+			break;
+		}
+		sigcpu = find_next_bit(kvm->arch.idle_mask, online_vcpus, ++sigcpu);
+	} while (sigcpu < online_vcpus);
+}
+
 static void __set_cpu_idle(struct kvm_vcpu *vcpu)
 {
 	kvm_s390_set_cpuflags(vcpu, CPUSTAT_WAIT);
@@ -1915,22 +1946,48 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 	return 0;
 }
 
+static u64 inti_to_irq_pend_mask(struct kvm_s390_interrupt_info *inti)
+{
+	u64 type = READ_ONCE(inti->type);
+
+	switch (type) {
+	case KVM_S390_MCHK:
+		/* Only repressible machine checks are floating */
+		return BIT(IRQ_PEND_MCHK_REP);
+	case KVM_S390_INT_VIRTIO:
+		return BIT(IRQ_PEND_VIRTIO);
+	case KVM_S390_INT_SERVICE:
+		return BIT(IRQ_PEND_EXT_SERVICE) |
+		       BIT(IRQ_PEND_EXT_SERVICE_EV);
+	case KVM_S390_INT_PFAULT_DONE:
+		return BIT(IRQ_PEND_PFAULT_DONE);
+	case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
+		return BIT(isc_to_irq_type(int_word_to_isc(inti->io.io_int_word)));
+	default:
+		return 0;
+	}
+}
+
 /*
  * Find a destination VCPU for a floating irq and kick it.
  */
-static void __floating_irq_kick(struct kvm *kvm, u64 type)
+static void __floating_irq_kick(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 {
 	struct kvm_vcpu *dst_vcpu;
 	int sigcpu, online_vcpus, nr_tries = 0;
+	u64 type = READ_ONCE(inti->type);
+	u64 irq_pend_mask;
 
 	online_vcpus = atomic_read(&kvm->online_vcpus);
 	if (!online_vcpus)
 		return;
 
+	irq_pend_mask = inti_to_irq_pend_mask(inti);
 	for (sigcpu = kvm->arch.float_int.last_sleep_cpu; ; sigcpu++) {
 		sigcpu %= online_vcpus;
 		dst_vcpu = kvm_get_vcpu(kvm, sigcpu);
-		if (!is_vcpu_stopped(dst_vcpu))
+		if (!is_vcpu_stopped(dst_vcpu) &&
+		    deliverable_irqs(dst_vcpu) & irq_pend_mask)
 			break;
 		/* avoid endless loops if all vcpus are stopped */
 		if (nr_tries++ >= online_vcpus)
@@ -1982,7 +2039,7 @@ static int __inject_vm(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 	if (rc)
 		return rc;
 
-	__floating_irq_kick(kvm, type);
+	__floating_irq_kick(kvm, inti);
 	return 0;
 }
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 518a69c55e85..d6d46688d980 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -86,6 +86,7 @@ const struct kvm_stats_desc kvm_vm_stats_desc[] = {
 	STATS_DESC_COUNTER(VM, gmap_shadow_r3_entry),
 	STATS_DESC_COUNTER(VM, gmap_shadow_sg_entry),
 	STATS_DESC_COUNTER(VM, gmap_shadow_pg_entry),
+	STATS_DESC_COUNTER(VM, inject_redist),
 };
 
 const struct kvm_stats_header kvm_vm_stats_header = {
@@ -4572,6 +4573,8 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu)
 		rc = kvm_s390_deliver_pending_interrupts(vcpu);
 		if (rc || guestdbg_exit_pending(vcpu))
 			return rc;
+
+		distribute_float_irqs(vcpu->kvm);
 	}
 
 	rc = kvm_s390_handle_requests(vcpu);
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 6d2842fb71a3..a2bcf4acac16 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -375,6 +375,7 @@ enum hrtimer_restart kvm_s390_idle_wakeup(struct hrtimer *timer);
 int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu);
 void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu);
 void kvm_s390_clear_float_irqs(struct kvm *kvm);
+void distribute_float_irqs(struct kvm *kvm);
 int __must_check kvm_s390_inject_vm(struct kvm *kvm,
 				    struct kvm_s390_interrupt *s390int,
 				    struct kvm_s390_interrupt_info *inti);
-- 
2.53.0

Re: [PATCH v2] KVM: s390: Improve floating IRQ injection behavior

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Use-After-Free of `inti` when injecting floating interrupts.
- [High] Missed vCPU kick causes lost or delayed floating interrupts when all vCPUs have masks closed.
--

--- Patch [1]: [PATCH v2] KVM: s390: Improve floating IRQ injection behavior ---
commit e7cb031c2f46e8290e932cd848faad5eec943d11
Author: Janosch Frank <frankja@linux.ibm.com>
Subject: KVM: s390: Improve floating IRQ injection behavior

Floating IRQs can be handled by any VCPU that opened its masks. The
current design does not check if the mask is open when a floating IRQ
is injected via the FLIC. It will wakeup the last VCPU that went
sleeping hoping it's the correct one.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 8f24bcd1a6d36..e67cdedb5aece 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c

[ ... ]

> @@ -1915,22 +1946,48 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)

[ ... ]

>  /*
>   * Find a destination VCPU for a floating irq and kick it.
>   */
> -static void __floating_irq_kick(struct kvm *kvm, u64 type)
> +static void __floating_irq_kick(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>  {
>  	struct kvm_vcpu *dst_vcpu;
>  	int sigcpu, online_vcpus, nr_tries = 0;
> +	u64 type = READ_ONCE(inti->type);
> +	u64 irq_pend_mask;
>  
>  	online_vcpus = atomic_read(&kvm->online_vcpus);
>  	if (!online_vcpus)
>  		return;
>  
> +	irq_pend_mask = inti_to_irq_pend_mask(inti);
>  	for (sigcpu = kvm->arch.float_int.last_sleep_cpu; ; sigcpu++) {
>  		sigcpu %= online_vcpus;
>  		dst_vcpu = kvm_get_vcpu(kvm, sigcpu);
> -		if (!is_vcpu_stopped(dst_vcpu))
> +		if (!is_vcpu_stopped(dst_vcpu) &&
> +		    deliverable_irqs(dst_vcpu) & irq_pend_mask)
>  			break;
>  		/* avoid endless loops if all vcpus are stopped */
>  		if (nr_tries++ >= online_vcpus)
>  			return;

[Severity: High]
Does this code drop the interrupt kick entirely if all vCPUs currently
have their interrupt masks closed?

If deliverable_irqs(dst_vcpu) & irq_pend_mask evaluates to 0 for all running
vCPUs, this loop will exit via the nr_tries check and return early.

Because it returns early, kvm_s390_set_cpuflags() and kvm_s390_vcpu_wakeup()
are never executed, meaning no vCPU drops out of SIE and
set_intercept_indicators() is not called. 

If the guest later unmasks the interrupt, could it remain undelivered since
the hardware intercept was never set up?

[ ... ]

> @@ -1982,7 +2039,7 @@ static int __inject_vm(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>  	if (rc)
>  		return rc;
>  
> -	__floating_irq_kick(kvm, type);
> +	__floating_irq_kick(kvm, inti);

[Severity: Critical]
Does this cause a use-after-free when injecting GISA I/O adapter interrupts?

When __inject_io() handles a GISA I/O interrupt, it frees the interrupt info:

arch/s390/kvm/interrupt.c:__inject_io() {
    ...
    if (gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
        ...
        kfree(inti);
        return 0;
    }
    ...
}

Since it returns 0, __inject_vm() will proceed to call __floating_irq_kick()
with the newly freed inti pointer, which then dereferences it when calling
inti_to_irq_pend_mask(inti).

Also, for non-GISA interrupts, inti is placed on a shared list where another
vCPU might deliver and free it before __floating_irq_kick() runs. 

Should we avoid passing inti to the kick function?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817121631.159451-1-frankja@linux.ibm.com?part=1

Re: [PATCH v2] KVM: s390: Improve floating IRQ injection behavior

[Christian Borntraeger]

Am 17.08.26 um 14:28 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Use-After-Free of `inti` when injecting floating interrupts.
> - [High] Missed vCPU kick causes lost or delayed floating interrupts when all vCPUs have masks closed.
> --
> 
> --- Patch [1]: [PATCH v2] KVM: s390: Improve floating IRQ injection behavior ---
> commit e7cb031c2f46e8290e932cd848faad5eec943d11
> Author: Janosch Frank <frankja@linux.ibm.com>
> Subject: KVM: s390: Improve floating IRQ injection behavior
> 
> Floating IRQs can be handled by any VCPU that opened its masks. The
> current design does not check if the mask is open when a floating IRQ
> is injected via the FLIC. It will wakeup the last VCPU that went
> sleeping hoping it's the correct one.
> 
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 8f24bcd1a6d36..e67cdedb5aece 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
> 
> [ ... ]
> 
>> @@ -1915,22 +1946,48 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
> 
> [ ... ]
> 
>>   /*
>>    * Find a destination VCPU for a floating irq and kick it.
>>    */
>> -static void __floating_irq_kick(struct kvm *kvm, u64 type)
>> +static void __floating_irq_kick(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>>   {
>>   	struct kvm_vcpu *dst_vcpu;
>>   	int sigcpu, online_vcpus, nr_tries = 0;
>> +	u64 type = READ_ONCE(inti->type);
>> +	u64 irq_pend_mask;
>>   
>>   	online_vcpus = atomic_read(&kvm->online_vcpus);
>>   	if (!online_vcpus)
>>   		return;
>>   
>> +	irq_pend_mask = inti_to_irq_pend_mask(inti);
>>   	for (sigcpu = kvm->arch.float_int.last_sleep_cpu; ; sigcpu++) {
>>   		sigcpu %= online_vcpus;
>>   		dst_vcpu = kvm_get_vcpu(kvm, sigcpu);
>> -		if (!is_vcpu_stopped(dst_vcpu))
>> +		if (!is_vcpu_stopped(dst_vcpu) &&
>> +		    deliverable_irqs(dst_vcpu) & irq_pend_mask)
>>   			break;
>>   		/* avoid endless loops if all vcpus are stopped */
>>   		if (nr_tries++ >= online_vcpus)
>>   			return;
> 
> [Severity: High]
> Does this code drop the interrupt kick entirely if all vCPUs currently
> have their interrupt masks closed?

I think this is a corner case but still a valid finding. We can probably consider this
slowpath and wakeup/set cpuflags for ALL cpus? maybe after doing 2 rounds instead of one?