| Message ID | 20240826221336.14023-3-david.hunter.linux@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: fire timer when it is migrated | expand |
On Tue, Aug 27, 2024, Greg KH wrote: > On Mon, Aug 26, 2024 at 06:13:36PM -0400, David Hunter wrote: > > > > [ Upstream Commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2] > > This is already in the 6.1.66 release, so do you want it applied again? > > > From: Haitao Shan <hshan@google.com> > > Date: Tue Sep 12 16:55:45 2023 -0700 > > > > 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 8e6ed96cdd50 ("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 967235d32032 ("KVM: vmx: clear pending interrupts on > > KVM_SET_LAPIC"). commit 8e6ed96cdd50 ("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: 967235d32032 ("KVM: vmx: clear pending interrupts on KVM_SET_LAPIC") > > Cc: stable@vger.kernel.org > > Suggested-by: Sean Christopherson <seanjc@google.com> > > Signed-off-by: Haitao Shan <hshan@google.com> > > Link: https://lore.kernel.org/r/20230913000215.478387-1-hshan@google.com > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > > (Cherry-Picked from commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2) > > Signed-off-by: David Hunter <david.hunter.linux@gmail.com> > > --- > > arch/x86/kvm/vmx/vmx.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > index 87abf4eebf8a..4040075bbd5a 100644 > > --- a/arch/x86/kvm/vmx/vmx.c > > +++ b/arch/x86/kvm/vmx/vmx.c > > @@ -8203,6 +8203,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = { > > .load_eoi_exitmap = vmx_load_eoi_exitmap, > > .apicv_pre_state_restore = vmx_apicv_pre_state_restore, > > .check_apicv_inhibit_reasons = vmx_check_apicv_inhibit_reasons, > > + .required_apicv_inhibits = VMX_REQUIRED_APICV_INHIBITS, > > .hwapic_irr_update = vmx_hwapic_irr_update, > > .hwapic_isr_update = vmx_hwapic_isr_update, > > .guest_apic_has_interrupt = vmx_guest_apic_has_interrupt, > > Wait, this is just one hunk? This feels wrong, you didn't say why you > modfied this from the original commit, or backport, what was wrong with > that? Gah, my bad. I told David[*] that this needed to be paired with patch 1 to avoid creating a regression in 6.1.y, without realizing this commit had already landed in 6.1.y. So yeah, please ignore this patch. [*] https://lore.kernel.org/all/ZsSiQkQVSz0DarYC@google.com
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 87abf4eebf8a..4040075bbd5a 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -8203,6 +8203,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = { .load_eoi_exitmap = vmx_load_eoi_exitmap, .apicv_pre_state_restore = vmx_apicv_pre_state_restore, .check_apicv_inhibit_reasons = vmx_check_apicv_inhibit_reasons, + .required_apicv_inhibits = VMX_REQUIRED_APICV_INHIBITS, .hwapic_irr_update = vmx_hwapic_irr_update, .hwapic_isr_update = vmx_hwapic_isr_update, .guest_apic_has_interrupt = vmx_guest_apic_has_interrupt,