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

[Janosch Frank <frankja@linux.ibm.com>]

On 9/25/26 11:26 AM, Christian Borntraeger wrote:
> 
> 
> Am 20.08.26 um 14:12 schrieb 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 checking if the VCPU has pending IRQs and if not try
>                                          can deliver that particular IRQ> to find another VCPU which can take the IRQ.
>>

Ok

>> 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.
> 
> 
> One question below.
> 
> 
> probably not the last version of that code ara but certainly a necessary
> fix in the right direction.
> 
> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> We should apply this and add it to kvms390. We will get some
> performance regression testing via the next kernel then and wait
> for the results before sending out a pull request.

Sure
Knowing our own CI timeouts a change in either direction should be 
pretty obvious as well.

> 
> 
>> [...]
>>    /*
>>     * 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, u64 type, int isc)
>>    {
>>    	struct kvm_vcpu *dst_vcpu;
>>    	int sigcpu, online_vcpus, nr_tries = 0;
>> +	u64 irq_pend_mask;
>> +	unsigned long i;
>>    
>>    	online_vcpus = atomic_read(&kvm->online_vcpus);
>>    	if (!online_vcpus)
>>    		return;
>>    
>> +	irq_pend_mask = inti_to_irq_pend_mask(type, isc);
>>    	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;
>> +		if (nr_tries++ >= online_vcpus * 2) {
> 
> why 2 rounds?

I honestly can't recall why
I can remove that change if you want.