Re: [PATCH] KVM: x86: Try to enable irr_pending state with disabled APICv

[Sean Christopherson <seanjc@google.com>]

On Wed, Oct 23, 2024, Yong He wrote:
> From: Yong He <alexyonghe@tencent.com>
> 
> Try to enable irr_pending when set APIC state, if there is
> pending interrupt in IRR with disabled APICv.
> 
> In save/restore VM scenery with disabled APICv. Qemu/CloudHypervisor
> always send signals to stop running vcpu threads, then save
> entire VM state, including APIC state. There may be a pending
> timer interrupt in the saved APIC IRR that is injected before
> vcpu_run return. But when restoring the VM, since APICv is
> disabled, irr_pending is disabled by default, so this may cause
> the timer interrupt in the IRR to be suspended for a long time,
> until the next interrupt comes.
> 
> Signed-off-by: Yong He <alexyonghe@tencent.com>
> ---
>  arch/x86/kvm/lapic.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 2098dc689088..7373f649958b 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -3099,6 +3099,10 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s)
>  						apic_find_highest_irr(apic));
>  		kvm_x86_call(hwapic_isr_update)(apic_find_highest_isr(apic));
>  	}
> +
> +	/* Search the IRR and enable irr_pending state with disabled APICv*/
> +	if (!enable_apicv && apic_search_irr(apic) != -1)

This can/should be an "else" from the above "if (apic->apicv_active)".  I also
think KVM can safely clear irr_pending in this case, which is also why irr_pending
isn't handling in kvm_apic_update_apicv().  When APICv is disabled (inhibited) at
runtime, an IRQ may be in-flight, i.e. apic_search_irr() can get a false negative.

But when stuffing APIC state, I don't see how that can happen.  So this?

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 65412640cfc7..deb73aea2c06 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -3086,6 +3086,15 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s)
                kvm_x86_call(hwapic_irr_update)(vcpu,
                                                apic_find_highest_irr(apic));
                kvm_x86_call(hwapic_isr_update)(apic_find_highest_isr(apic));
+       } else {
+               /*
+                * Note, kvm_apic_update_apicv() is responsible for updating
+                * isr_count and highest_isr_cache.  irr_pending is somewhat
+                * special because it mustn't be cleared when APICv is disabled
+                * at runtime, and only state restore can cause an IRR bit to
+                * be set without also refreshing irr_pending.
+                */
+               apic->irr_pending = apic_search_irr(apic) != -1;
        }
        kvm_make_request(KVM_REQ_EVENT, vcpu);
        if (ioapic_in_kernel(vcpu->kvm))

> +		apic->irr_pending = true;
>  	kvm_make_request(KVM_REQ_EVENT, vcpu);
>  	if (ioapic_in_kernel(vcpu->kvm))
>  		kvm_rtc_eoi_tracking_restore_one(vcpu);
> -- 
> 2.43.5
>