KVM: x86: Fix misleading variable names and add more comments for PIR=>IRR flow

Rename kvm_apic_update_irr()'s "irr_updated" and vmx_sync_pir_to_irr()'s
"got_posted_interrupt" to a more accurate "max_irr_is_from_pir", as neither
"irr_updated" nor "got_posted_interrupt" is accurate.
__kvm_apic_update_irr() and thus kvm_apic_update_irr() specifically return
true if and only if the highest priority IRQ, i.e. max_irr, is a "new"
pending IRQ from the PIR.  I.e. it's possible for the IRR to be updated,
i.e. for a posted IRQ to be "got", *without* the APIs returning true.

Expand vmx_sync_pir_to_irr()'s comment to explain why it's necessary to
set KVM_REQ_EVENT only if a "new" IRQ was found, and to explain why it's
safe to do so only if a new IRQ is also the highest priority pending IRQ.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://patch.msgid.link/20260503201703.108231-3-pbonzini@redhat.com/
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 5ee14d6bc2883fe5d9a54f67aebee6cb063ea6b4..4078e624ca6675d1af4b3182f8279f76ed38bdf6 100644 (file)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -667,14 +667,14 @@ bool __kvm_apic_update_irr(unsigned long *pir, void *regs, int *max_irr)
        u32 *__pir = (void *)pir_vals;
        u32 i, vec;
        u32 irr_val, prev_irr_val;
-       int max_updated_irr;
+       int max_new_irr;
 
        if (!pi_harvest_pir(pir, pir_vals)) {
                *max_irr = apic_find_highest_vector(regs + APIC_IRR);
                return false;
        }
 
-       max_updated_irr = -1;
+       max_new_irr = -1;
        *max_irr = -1;
 
        for (i = vec = 0; i <= 7; i++, vec += 32) {
@@ -690,25 +690,25 @@ bool __kvm_apic_update_irr(unsigned long *pir, void *regs, int *max_irr)
                                 !try_cmpxchg(p_irr, &prev_irr_val, irr_val));
 
                        if (prev_irr_val != irr_val)
-                               max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec;
+                               max_new_irr = __fls(irr_val ^ prev_irr_val) + vec;
                }
                if (irr_val)
                        *max_irr = __fls(irr_val) + vec;
        }
 
-       return ((max_updated_irr != -1) &&
-               (max_updated_irr == *max_irr));
+       return max_new_irr != -1 && max_new_irr == *max_irr;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(__kvm_apic_update_irr);
 
 bool kvm_apic_update_irr(struct kvm_vcpu *vcpu, unsigned long *pir, int *max_irr)
 {
        struct kvm_lapic *apic = vcpu->arch.apic;
-       bool irr_updated = __kvm_apic_update_irr(pir, apic->regs, max_irr);
+       bool max_irr_is_from_pir;
 
-       if (unlikely(!apic->apicv_active && irr_updated))
+       max_irr_is_from_pir = __kvm_apic_update_irr(pir, apic->regs, max_irr);
+       if (unlikely(!apic->apicv_active && max_irr_is_from_pir))
                apic->irr_pending = true;
-       return irr_updated;
+       return max_irr_is_from_pir;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_apic_update_irr);
 

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index a29896a9ef145601e52934b812b00cd2b983f3c2..5c2c33a5f7dc9a0bc7ca8349c8ece340ed51594b 100644 (file)
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7029,8 +7029,8 @@ static void vmx_set_rvi(int vector)
 int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
 {
        struct vcpu_vt *vt = to_vt(vcpu);
+       bool max_irr_is_from_pir;
        int max_irr;
-       bool got_posted_interrupt;
 
        if (KVM_BUG_ON(!enable_apicv, vcpu->kvm))
                return -EIO;
@@ -7042,17 +7042,22 @@ int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
                 * But on x86 this is just a compiler barrier anyway.
                 */
                smp_mb__after_atomic();
-               got_posted_interrupt =
-                       kvm_apic_update_irr(vcpu, vt->pi_desc.pir, &max_irr);
+               max_irr_is_from_pir = kvm_apic_update_irr(vcpu, vt->pi_desc.pir,
+                                                         &max_irr);
        } else {
                max_irr = kvm_lapic_find_highest_irr(vcpu);
-               got_posted_interrupt = false;
+               max_irr_is_from_pir = false;
        }
 
        /*
-        * Newly recognized interrupts are injected via either virtual interrupt
-        * delivery (RVI) or KVM_REQ_EVENT.  Virtual interrupt delivery is
-        * disabled in two cases:
+        * If APICv is enabled and L2 is not active, then update the Requesting
+        * Virtual Interrupt (RVI) portion of vmcs01.GUEST_INTR_STATUS with the
+        * highest priority IRR to deliver the IRQ via Virtual Interrupt
+        * Delivery.  Note, this is required even if the highest priority IRQ
+        * was already pending in the IRR, as RVI isn't updated in lockstep with
+        * the IRR (unlike apic->irr_pending).
+        *
+        * For the cases where Virtual Interrupt Delivery can't be used:
         *
         * 1) If L2 is running and the vCPU has a new pending interrupt.  If L1
         * wants to exit on interrupts, KVM_REQ_EVENT is needed to synthesize a
@@ -7063,10 +7068,29 @@ int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu)
         * 2) If APICv is disabled for this vCPU, assigned devices may still
         * attempt to post interrupts.  The posted interrupt vector will cause
         * a VM-Exit and the subsequent entry will call sync_pir_to_irr.
+        *
+        * In both cases, set KVM_REQ_EVENT if and only if the highest priority
+        * pending IRQ came from the PIR, as setting KVM_REQ_EVENT if any IRQ
+        * is pending may put the vCPU into an infinite loop, e.g. if the IRQ
+        * is blocked, then it will stay pending until an IRQ window is opened.
+        *
+        * Note!  It's possible that one or more IRQs were moved from the PIR
+        * to the IRR _without_ max_irr_is_from_pir being true!  I.e. if there
+        * was a higher priority IRQ already pending in the IRR.  Not setting
+        * KVM_REQ_EVENT in this case is intentional and safe.  If APICv is
+        * inactive, or L2 is running with exit-on-interrupt off (in vmcs12),
+        * i.e. without nested virtual interrupt delivery, then there's no need
+        * to request an IRQ window as the lower priority IRQ only needs to be
+        * delivered when the higher priority IRQ is dismissed from the ISR,
+        * i.e. on the next EOI, and EOIs are always intercepted if APICv is
+        * disabled or if L2 is running without nested VID.  If L2 is running
+        * exit-on-interrupt on (in vmcs12), then the higher priority IRQ will
+        * trigger a nested VM-Exit, at which point KVM will re-evaluate L1's
+        * pending IRQs.
         */
        if (!is_guest_mode(vcpu) && kvm_vcpu_apicv_active(vcpu))
                vmx_set_rvi(max_irr);
-       else if (got_posted_interrupt)
+       else if (max_irr_is_from_pir)
                kvm_make_request(KVM_REQ_EVENT, vcpu);
 
        return max_irr;