Re: [PATCH] KVM: VMX: Fall back to IRR scan when PIR is empty despite PID.ON being set

[Chenyi Qiang <chenyi.qiang@intel.com>]

On 4/28/2026 3:45 PM, Paolo Bonzini wrote:
> On 4/28/26 09:03, Chenyi Qiang wrote:
>> Fall back to kvm_lapic_find_highest_irr() in vmx_sync_pir_to_irr() when
>> PID.ON is set but PIR turns out to be empty, to correctly report the
>> highest pending interrupt from the existing IRR.
>>
>> In a nested VM stress test, the following WARNING fires in
>> vmx_check_nested_events() when kvm_cpu_has_interrupt() reports a pending
>> interrupt but the subsequent kvm_apic_has_interrupt() (which invokes
>> vmx_sync_pir_to_irr() again) returns -1:
>>
>>    WARNING: CPU: 99 PID: 57767 at arch/x86/kvm/vmx/nested.c:4449 vmx_check_nested_events+0x6bf/0x6e0 [kvm_intel]
>>    Call Trace:
>>     kvm_check_and_inject_events
>>     vcpu_enter_guest.constprop.0
>>     vcpu_run
>>     kvm_arch_vcpu_ioctl_run
>>     kvm_vcpu_ioctl
>>     __x64_sys_ioctl
>>     do_syscall_64
>>     entry_SYSCALL_64_after_hwframe
>>
>> The root cause is a race between vmx_sync_pir_to_irr() on the target vCPU
>> and __vmx_deliver_posted_interrupt() on a sender vCPU.  The sender
>> performs two individually-atomic operations that are not a single
>> transaction:
>>
>>    1. pi_test_and_set_pir(vector)  -- sets the PIR bit
>>    2. pi_test_and_set_on()         -- sets PID.ON
>>
>> The following interleaving triggers the bug:
>>
>>    Sender vCPU (IPI):              Target vCPU (1st sync_pir_to_irr):
>>    B1: set PIR[vector]
>>                                    A1: pi_clear_on()
>>                                    A2: pi_harvest_pir() -> sees B1 bit
>>                                    A3: xchg() -> consumes bit, PIR=0
>>                                        (1st sync returns correct max_irr)
>>    B2: set PID.ON = 1
>>
>>                                    Target vCPU (2nd sync_pir_to_irr):
>>                                    C1: pi_test_on() -> TRUE (from B2)
>>                                    C2: pi_clear_on() -> ON=0
>>                                    C3: pi_harvest_pir() -> PIR empty
>>                                    C4: *max_irr = -1, early return
>>                                        IRR NOT SCANNED
>>
>> The interrupt is not lost (it resides in the IRR from the first sync and
>> is recovered on the next vcpu_enter_guest() iteration), but the incorrect
>> max_irr causes a spurious WARNING and a wasted L2 VM-Enter/VM-Exit cycle.
>>
>> Fixes: b41f8638b9d3 ("KVM: VMX: Isolate pure loads from atomic XCHG when processing PIR")
>> Reported-by: Farrah Chen <farrah.chen@intel.com>
>> Assisted-by: GitHub Copilot:Claude Opus 4.6
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>>
>> ---
>> There is a WARNING call trace during a nested VM stress test. AI
>> provided an analysis of a race condition and the related fix, which
>> looks reasonable to me. With the patch applied, the WARNING can not
>> be reproduced in overnight stress testing.
> 
> The analysis of the race is correct and changing the logic is the
> right thing to do; but I would change directly __kvm_apic_update_irr,
> either like this:
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e3ec4d8607c1..5ee14d6bc288 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -669,12 +669,14 @@ bool __kvm_apic_update_irr(unsigned long *pir, void *regs, int *max_irr)
>      u32 irr_val, prev_irr_val;
>      int max_updated_irr;
>  
> +    if (!pi_harvest_pir(pir, pir_vals)) {
> +        *max_irr = apic_find_highest_vector(regs + APIC_IRR);
> +        return false;
> +    }
> +
>      max_updated_irr = -1;
>      *max_irr = -1;
>  
> -    if (!pi_harvest_pir(pir, pir_vals))
> -        return false;
> -
>      for (i = vec = 0; i <= 7; i++, vec += 32) {
>          u32 *p_irr = (u32 *)(regs + APIC_IRR + i * 0x10);

Make sense. This resolves the problem at the source.
I will verify the change in this way and respin it.

>  
> 
> Or even ignoring altogether the return value of pi_harvest_pir(), always
> going in the loop below for simplicity.
> 
> Paolo
>