On Wed, Oct 23, 2024, Yong He wrote:
> From: Yong He <alexyonghe@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
> 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
>
