Re: [PATCH v3 1/3] KVM: s390: Improve floating IRQ injection behavior

On 8/19/26 9:13 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incorrect bitmask computation for floating I/O interrupts causes thundering herd performance regressions or lost wakeups.
> --
> 
> --- Patch [1]: [PATCH v3 1/3] KVM: s390: Improve floating IRQ injection behavior ---
> commit 8a0b82ce7a926dc2db73a2448ac2bcc105213707
> Author: Janosch Frank <[email protected]>
> 
> KVM: s390: Improve floating IRQ injection behavior
> 
> This commit improves floating IRQ injection by checking if a VCPU has pending
> IRQs instead of blindly waking up the last VCPU that went to sleep. This aims
> to prevent deadlocks when a VCPU is sleeping without an open mask.
> 
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 8f24bcd1a6d36..3af060ec5be8e 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
> [ ... ]
>> @@ -1915,49 +1915,93 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>>   	return 0;
>>   }
>>   
>> +static u64 inti_to_irq_pend_mask(u64 type, int isc)
>> +{
>> +	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 isc_to_irq_type(isc);
> 
> [Severity: High]
> Does this correctly return a bitmask? The other cases return BIT() masks, but
> this returns a raw integer resulting from isc_to_irq_type().
> >
> [ ... ]
>> @@ -1974,6 +2018,8 @@ static int __inject_vm(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>>   		rc = __inject_pfault_done(kvm, inti);
>>   		break;
>>   	case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
>> +		/* Grab isc here since __inject_io() might free inti */
>> +		isc = isc_to_irq_type(int_word_to_isc(inti->io.io_int_word));
> 
> [Severity: High]
> Are we applying isc_to_irq_type() twice for I/O interrupts?
> 
> This converts the ISC to an IRQ bit index before assigning it to `isc`. Later,
> __floating_irq_kick() passes it to inti_to_irq_pend_mask(), which calls
> isc_to_irq_type() a second time on the same value.
> 
> Since isc_to_irq_type() is implemented as `IRQ_PEND_IO_ISC_0 - isc`, calling it
> twice will reverse the calculation and yield the original ISC value instead
> of the expected IRQ bit index.
Rebase damage, will fix both.