Skip to content

Commit 9cfec6d

Browse files
Taogle2018sean-jc
authored andcommitted
KVM: x86: Fix lapic timer interrupt lost after loading a snapshot.
When running android emulator (which is based on QEMU 2.12) on certain Intel hosts with kernel version 6.3-rc1 or above, guest will freeze after loading a snapshot. This is almost 100% reproducible. By default, the android emulator will use snapshot to speed up the next launching of the same android guest. So this breaks the android emulator badly. I tested QEMU 8.0.4 from Debian 12 with an Ubuntu 22.04 guest by running command "loadvm" after "savevm". The same issue is observed. At the same time, none of our AMD platforms is impacted. More experiments show that loading the KVM module with "enable_apicv=false" can workaround it. The issue started to show up after commit 8e6ed96 ("KVM: x86: fire timer when it is migrated and expired, and in oneshot mode"). However, as is pointed out by Sean Christopherson, it is introduced by commit 967235d ("KVM: vmx: clear pending interrupts on KVM_SET_LAPIC"). commit 8e6ed96 ("KVM: x86: fire timer when it is migrated and expired, and in oneshot mode") just makes it easier to hit the issue. Having both commits, the oneshot lapic timer gets fired immediately inside the KVM_SET_LAPIC call when loading the snapshot. On Intel platforms with APIC virtualization and posted interrupt processing, this eventually leads to setting the corresponding PIR bit. However, the whole PIR bits get cleared later in the same KVM_SET_LAPIC call by apicv_post_state_restore. This leads to timer interrupt lost. The fix is to move vmx_apicv_post_state_restore to the beginning of the KVM_SET_LAPIC call and rename to vmx_apicv_pre_state_restore. What vmx_apicv_post_state_restore does is actually clearing any former apicv state and this behavior is more suitable to carry out in the beginning. Fixes: 967235d ("KVM: vmx: clear pending interrupts on KVM_SET_LAPIC") Cc: [email protected] Suggested-by: Sean Christopherson <[email protected]> Signed-off-by: Haitao Shan <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]>
1 parent 5804c19 commit 9cfec6d

File tree

4 files changed

+8
-2
lines changed

4 files changed

+8
-2
lines changed

arch/x86/include/asm/kvm-x86-ops.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ KVM_X86_OP_OPTIONAL(vcpu_blocking)
108108
KVM_X86_OP_OPTIONAL(vcpu_unblocking)
109109
KVM_X86_OP_OPTIONAL(pi_update_irte)
110110
KVM_X86_OP_OPTIONAL(pi_start_assignment)
111+
KVM_X86_OP_OPTIONAL(apicv_pre_state_restore)
111112
KVM_X86_OP_OPTIONAL(apicv_post_state_restore)
112113
KVM_X86_OP_OPTIONAL_RET0(dy_apicv_has_pending_interrupt)
113114
KVM_X86_OP_OPTIONAL(set_hv_timer)

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,6 +1709,7 @@ struct kvm_x86_ops {
17091709
int (*pi_update_irte)(struct kvm *kvm, unsigned int host_irq,
17101710
uint32_t guest_irq, bool set);
17111711
void (*pi_start_assignment)(struct kvm *kvm);
1712+
void (*apicv_pre_state_restore)(struct kvm_vcpu *vcpu);
17121713
void (*apicv_post_state_restore)(struct kvm_vcpu *vcpu);
17131714
bool (*dy_apicv_has_pending_interrupt)(struct kvm_vcpu *vcpu);
17141715

arch/x86/kvm/lapic.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2670,6 +2670,8 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
26702670
u64 msr_val;
26712671
int i;
26722672

2673+
static_call_cond(kvm_x86_apicv_pre_state_restore)(vcpu);
2674+
26732675
if (!init_event) {
26742676
msr_val = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE;
26752677
if (kvm_vcpu_is_reset_bsp(vcpu))
@@ -2977,6 +2979,8 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s)
29772979
struct kvm_lapic *apic = vcpu->arch.apic;
29782980
int r;
29792981

2982+
static_call_cond(kvm_x86_apicv_pre_state_restore)(vcpu);
2983+
29802984
kvm_lapic_set_base(vcpu, vcpu->arch.apic_base);
29812985
/* set SPIV separately to get count of SW disabled APICs right */
29822986
apic_set_spiv(apic, *((u32 *)(s->regs + APIC_SPIV)));

arch/x86/kvm/vmx/vmx.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6912,7 +6912,7 @@ static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
69126912
vmcs_write64(EOI_EXIT_BITMAP3, eoi_exit_bitmap[3]);
69136913
}
69146914

6915-
static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu)
6915+
static void vmx_apicv_pre_state_restore(struct kvm_vcpu *vcpu)
69166916
{
69176917
struct vcpu_vmx *vmx = to_vmx(vcpu);
69186918

@@ -8286,7 +8286,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
82868286
.set_apic_access_page_addr = vmx_set_apic_access_page_addr,
82878287
.refresh_apicv_exec_ctrl = vmx_refresh_apicv_exec_ctrl,
82888288
.load_eoi_exitmap = vmx_load_eoi_exitmap,
8289-
.apicv_post_state_restore = vmx_apicv_post_state_restore,
8289+
.apicv_pre_state_restore = vmx_apicv_pre_state_restore,
82908290
.required_apicv_inhibits = VMX_REQUIRED_APICV_INHIBITS,
82918291
.hwapic_irr_update = vmx_hwapic_irr_update,
82928292
.hwapic_isr_update = vmx_hwapic_isr_update,

0 commit comments

Comments
 (0)