KVM: x86: fire timer when it is migrated and expired, and in oneshot mode
Subsystems (2)
PATCH 1/1 KVM: x86: fire timer when it is migrated and expired, and in oneshot mode
From: Li RongQing <[email protected]>
[ Upstream Commit 8e6ed96cdd5001c55fccc80a17f651741c1ca7d2]
when the vCPU was migrated, if its timer is expired, KVM _should_ fire the timer ASAP, zeroing the deadline here will cause the timer to immediately fire on the destination
(cherry picked from commit 8e6ed96cdd5001c55fccc80a17f651741c1ca7d2) The code was able to compile without errors or warnings.
--- arch/x86/kvm/lapic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
arch/x86/kvm/lapic.c +6 -2
@@ -1843,8 +1843,12 @@ static bool set_target_expiration(struct kvm_lapic *apic, u32 count_reg)if (unlikely (count_reg !=APIC_TMICT )) { deadline =tmict_to_ns (apic,kvm_lapic_get_reg (apic, count_reg)); -if (unlikely (deadline <=0 )) - deadline = apic->lapic_timer.period; +if (unlikely (deadline <=0 )) { +if (apic_lvtt_period (apic)) + deadline = apic->lapic_timer.period; +else + deadline =0 ; + }else if (unlikely (deadline > apic->lapic_timer.period)) {pr_info_ratelimited ( "kvm: vcpu %i: requested lapic timer restore with "
Signature · 1 line
2.43.0
Quoted 9 lines
From: Li RongQing <[email protected]>
[ Upstream Commit
8e6ed96cdd5001c55fccc80a17f651741c1ca7d2]when the vCPU was migrated, if its timer is expired, KVM _should_ fire the timer ASAP, zeroing the deadline here will cause the timer to immediately fire on the destination
This patch increased the reproduce ratio of lapic timer interrupt losing, which has been fixed by the following patch; so I think patch should not merge it into 6.1
commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2
Author: Haitao Shan <[email protected]> Date: Tue Sep 12 16:55:45 2023 -0700
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 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.
Quoted 39 lines
Cc Sean Christopherson <[email protected]>Cc Peter Shier <[email protected]>Cc Jim Mattson <[email protected]>Cc Wanpeng Li <[email protected]>Cc Paolo Bonzini <[email protected]>Signed-off-by Li RongQing <[email protected]>Link:
https://lore.kernel.org/r/[email protected]Signed-off-by Sean Christopherson <[email protected]>(cherry picked from commit
8e6ed96cdd5001c55fccc80a17f651741c1ca7d2) The code was able to compile without errors or warnings.Signed-off-by David Hunter <[email protected]>--- arch/x86/kvm/lapic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)arch/x86/kvm/lapic.c index renamed from arch/x86/kvm/lapic.c +0
-0
c90fef0258c5..3cd590ace95a100644arch/x86/kvm/lapic.c +6
-2@@ -1843,8 +1843,12 @@ static bool set_target_expiration(struct kvm_lapic *apic, u32 count_reg)if (unlikely (count_reg !=APIC_TMICT )) { deadline =tmict_to_ns (apic,kvm_lapic_get_reg (apic, count_reg)); -if (unlikely (deadline <=0 )) - deadline = apic->lapic_timer.period; +if (unlikely (deadline <=0 )) { +if (apic_lvtt_period (apic)) + deadline = apic->lapic_timer.period; +else + deadline =0 ; + }else if (unlikely (deadline > apic->lapic_timer.period)) { pr_info_ratelimited("kvm: vcpu %i: requested lapic timer restore with "
--
2.43.0
Sean Christopherson
reviewerQuoted 11 lines · Li,Rongqing
Quoted 9 lines
From: Li RongQing <[email protected]>
[ Upstream Commit
8e6ed96cdd5001c55fccc80a17f651741c1ca7d2]when the vCPU was migrated, if its timer is expired, KVM _should_ fire the timer ASAP, zeroing the deadline here will cause the timer to immediately fire on the destination
This patch increased the reproduce ratio of lapic timer interrupt losing,
Yep, this caused a painful amount of fallout in our environment.
Quoted 2 lines
which has been fixed by the following patch; so I think patch should not merge it into 6.1
David, can you prep a small series with both this patch and the fix below?
Thanks!
Quoted 3 lines
commit
9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2Author: Haitao Shan <[email protected]> Date: Tue Sep 12 16:55:45 2023 -0700
v2 cover KVM: x86: fire timer when it is migrated
Hello,
I'm sending you two this first because this will be my first time sending a series patch. Is this okay?
Backport for 6.1.y. These two commits should be backported together to fix an issue that arrises from commit 967235d320329e4a7a2bd1a36b04293063e985ae
-Subject: 'VM: vmx: clear pending interrupts on KVM_SET_LAPIC'
[ Upstream Commit 8e6ed96cdd5001c55fccc80a17f651741c1ca7d2 ] Haitao Shan (1):
KVM: x86: Fix lapic timer interrupt lost after loading a snapshot.
[ Upstream Commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2 ] Li RongQing (1):
KVM: x86: fire timer when it is migrated and expired, and in oneshot
mode
---
V1 --> V2
- changed to a series patch to fix an issue with the patch
arch/x86/kvm/lapic.c | 8 ++++++-- arch/x86/kvm/vmx/vmx.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/2 KVM: x86: fire timer when it is migrated and expired, and in oneshot mode
[Upstream Commit 8e6ed96cdd5001c55fccc80a17f651741c1ca7d2
From: Li RongQing <[email protected]>
when the vCPU was migrated, if its timer is expired, KVM _should_ fire the timer ASAP, zeroing the deadline here will cause the timer to immediately fire on the destination
Cherry-picked from commit 8e6ed96cdd5001c55fccc80a17f651741c1ca7d2]
--- arch/x86/kvm/lapic.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
arch/x86/kvm/lapic.c +6 -2
@@ -1843,8 +1843,12 @@ static bool set_target_expiration(struct kvm_lapic *apic, u32 count_reg)if (unlikely (count_reg !=APIC_TMICT )) { deadline =tmict_to_ns (apic,kvm_lapic_get_reg (apic, count_reg)); -if (unlikely (deadline <=0 )) - deadline = apic->lapic_timer.period; +if (unlikely (deadline <=0 )) { +if (apic_lvtt_period (apic)) + deadline = apic->lapic_timer.period; +else + deadline =0 ; + }else if (unlikely (deadline > apic->lapic_timer.period)) {pr_info_ratelimited ( "kvm: vcpu %i: requested lapic timer restore with "
Signature · 1 line
2.43.0
PATCH v2 2/2 KVM: x86: Fix lapic timer interrupt lost after loading a snapshot.
In reply to David Hunter[ Upstream Commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2]
From: Haitao Shan <[email protected]>
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.
(Cherry-Picked from commit 9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2)
--- arch/x86/kvm/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+)
arch/x86/kvm/vmx/vmx.c +1 -0
@@ -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,
Signature · 1 line
2.43.0
Greg KH
maintainerRe: [PATCH 6.1.y 2/2 V2] KVM: x86: Fix lapic timer interrupt lost after loading a snapshot.
Quoted 2 lines · David Hunter
[ Upstream Commit
9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2]
This is already in the 6.1.66 release, so do you want it applied again?
Quoted 55 lines
From: Haitao Shan <[email protected]>
Date: Tue Sep 12 16:55:45 2023 -0700When 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 commit967235d32032("KVM: vmx: clear pending interrupts onKVM_SET_LAPIC"). commit8e6ed96cdd50("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_LAPICcall 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 sameKVM_SET_LAPICcall byapicv_post_state_restore. This leads to timer interrupt lost.The fix is to move
vmx_apicv_post_state_restoreto the beginning of theKVM_SET_LAPICcall and rename tovmx_apicv_pre_state_restore. Whatvmx_apicv_post_state_restoredoes 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")Suggested-by Sean Christopherson <[email protected]>Signed-off-by Haitao Shan <[email protected]>Signed-off-by Sean Christopherson <[email protected]>(Cherry-Picked from commit
9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2)Signed-off-by David Hunter <[email protected]>--- arch/x86/kvm/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+)arch/x86/kvm/vmx/vmx.c +0
-0
arch/x86/kvm/vmx/vmx.c static struct kvm_x86_ops vmx_x86_ops __initdata = { · line 8209
Quoted 7 lines
arch/x86/kvm/vmx/vmx.c +1
-0@@ -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?
thanks,
greg k-h
Sean Christopherson
reviewerRe: [PATCH 6.1.y 2/2 V2] KVM: x86: Fix lapic timer interrupt lost after loading a snapshot.
Quoted 56 lines · Greg KH
Quoted 2 lines · David Hunter
[ Upstream Commit
9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2]This is already in the 6.1.66 release, so do you want it applied again?
Quoted 50 lines
From: Haitao Shan <[email protected]>
Date: Tue Sep 12 16:55:45 2023 -0700When 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 commit967235d32032("KVM: vmx: clear pending interrupts onKVM_SET_LAPIC"). commit8e6ed96cdd50("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_LAPICcall 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 sameKVM_SET_LAPICcall byapicv_post_state_restore. This leads to timer interrupt lost.The fix is to move
vmx_apicv_post_state_restoreto the beginning of theKVM_SET_LAPICcall and rename tovmx_apicv_pre_state_restore. Whatvmx_apicv_post_state_restoredoes 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")Suggested-by Sean Christopherson <[email protected]>Signed-off-by Haitao Shan <[email protected]>Signed-off-by Sean Christopherson <[email protected]>(Cherry-Picked from commit
9cfec6d097c607e36199cf0cfbb8cf5acbd8e9b2)Signed-off-by David Hunter <[email protected]>--- arch/x86/kvm/vmx/vmx.c | 1 + 1 file changed, 1 insertion(+)
Quoted 16 lines
arch/x86/kvm/vmx/vmx.c static struct kvm_x86_ops vmx_x86_ops __initdata = { · line 8209
Quoted 12 lines
arch/x86/kvm/vmx/vmx.c +1
-0@@ -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.