All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN
@ 2026-07-15 23:42 Venkatesh Srinivas
  2026-07-16  1:14 ` James Houghton
  2026-07-20  8:37 ` Chao Gao
  0 siblings, 2 replies; 4+ messages in thread
From: Venkatesh Srinivas @ 2026-07-15 23:42 UTC (permalink / raw)
  To: kvm
  Cc: stable, Venkatesh Srinivas, David Matlack, Sean Christopherson,
	Jim Mattson, James Houghton

Fixes a lost timer interrupt issue if:
1) We are on an Intel platform w/ a VMX preemption timer && APICv
2) A VMM calls KVM_GET_LAPIC before KVM_GET_MSRS to save vCPU state
3) The thread executing the two calls migrates CPUs between the two calls

After migration across CPUs, KVM_GET_MSRS calls vcpu_load, posting the
interrupt and clearing the MSR:
vcpu_load() ->
  kvm_arch_vcpu_load() ->
    kvm_lapic_restart_hv_timer() ->
      start_hv_timer() ->
        apic_timer_expired() ->
          kvm_apic_inject_pending_timer_irqs()
            . post interrupt into the LAPIC state
            . clear IA32_TSCDEADLINE

The saved LAPIC state will be missing the pending interrupt and the saved
MSR will be zero. Oops.

Fix by only posting an interrupt when we're attempting to enter the guest
(vcpu->wants_to_run == true), not for vcpu_load from other paths.

Assisted-by: gemini:gemini-3.1-pro-preview
Debugged-by: David Matlack <dmatlack@google.com>
Debugged-by: Sean Christopherson <seanjc@google.com>
Debugged-by: Jim Mattson <jmattson@google.com>
Debugged-by: James Houghton <jthoughton@google.com>
Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>
---
 arch/x86/kvm/lapic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6f30bbdddb5a..74dfdbab4ab9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2052,7 +2052,7 @@ static void apic_timer_expired(struct kvm_lapic *apic, bool from_timer_fn)
 	if (apic_lvtt_tscdeadline(apic) || ktimer->hv_timer_in_use)
 		ktimer->expired_tscdeadline = ktimer->tscdeadline;
 
-	if (!from_timer_fn && apic->apicv_active) {
+	if (!from_timer_fn && apic->apicv_active && vcpu->wants_to_run) {
 		WARN_ON(kvm_get_running_vcpu() != vcpu);
 		kvm_apic_inject_pending_timer_irqs(apic);
 		return;
-- 
2.55.0.141.g00534a21ce-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN
  2026-07-15 23:42 [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN Venkatesh Srinivas
@ 2026-07-16  1:14 ` James Houghton
  2026-07-20 13:51   ` Paolo Bonzini
  2026-07-20  8:37 ` Chao Gao
  1 sibling, 1 reply; 4+ messages in thread
From: James Houghton @ 2026-07-16  1:14 UTC (permalink / raw)
  To: Venkatesh Srinivas
  Cc: kvm, stable, David Matlack, Sean Christopherson, Jim Mattson

Hi Venkatesh,

On Wed, Jul 15, 2026 at 4:42 PM Venkatesh Srinivas
<venkateshs@chromium.org> wrote:
>
> Fixes a lost timer interrupt issue if:
> 1) We are on an Intel platform w/ a VMX preemption timer && APICv
> 2) A VMM calls KVM_GET_LAPIC before KVM_GET_MSRS to save vCPU state
> 3) The thread executing the two calls migrates CPUs between the two calls

I think the paragraph above should be rephrased, something like this:

"On Intel platforms with a VMX preemption timer and APICv, if a VMM
calls KVM_GET_LAPIC before KVM_GET_MSRS to save the vCPU state, it is
possible to lose a pending timer interrupt.

If the thread running these ioctls is migrated to another core after
calling KVM_GET_LAPIC but before KVM_GET_MSRS and the guest is using
their LAPIC timer in TSC-deadline mode, not only does the save LAPIC
state not carry the pending interrupt, the TSCDEADLINE MSR will be
zeroed."

Sean doesn't like pronouns (like "we"); he'll probably have similar
style feedback.

>
> After migration across CPUs, KVM_GET_MSRS calls vcpu_load, posting the
> interrupt and clearing the MSR:
> vcpu_load() ->
>   kvm_arch_vcpu_load() ->
>     kvm_lapic_restart_hv_timer() ->
>       start_hv_timer() ->
>         apic_timer_expired() ->
>           kvm_apic_inject_pending_timer_irqs()
>             . post interrupt into the LAPIC state
>             . clear IA32_TSCDEADLINE
>
> The saved LAPIC state will be missing the pending interrupt and the saved
> MSR will be zero. Oops.
>
> Fix by only posting an interrupt when we're attempting to enter the guest
> (vcpu->wants_to_run == true), not for vcpu_load from other paths.
>
> Assisted-by: gemini:gemini-3.1-pro-preview
> Debugged-by: David Matlack <dmatlack@google.com>
> Debugged-by: Sean Christopherson <seanjc@google.com>
> Debugged-by: Jim Mattson <jmattson@google.com>
> Debugged-by: James Houghton <jthoughton@google.com>
> Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>

You have included cc:stable (on the CC line in this email); could you
include it here in the patch as well?

Also, what is the appropriate Fixes?

> ---
>  arch/x86/kvm/lapic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 6f30bbdddb5a..74dfdbab4ab9 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2052,7 +2052,7 @@ static void apic_timer_expired(struct kvm_lapic *apic, bool from_timer_fn)
>         if (apic_lvtt_tscdeadline(apic) || ktimer->hv_timer_in_use)
>                 ktimer->expired_tscdeadline = ktimer->tscdeadline;
>
> -       if (!from_timer_fn && apic->apicv_active) {
> +       if (!from_timer_fn && apic->apicv_active && vcpu->wants_to_run) {
>                 WARN_ON(kvm_get_running_vcpu() != vcpu);
>                 kvm_apic_inject_pending_timer_irqs(apic);
>                 return;
> --

As far as this change goes, it makes sense to me.

Reviewed-by: James Houghton <jthoughton@google.com>

Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN
  2026-07-15 23:42 [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN Venkatesh Srinivas
  2026-07-16  1:14 ` James Houghton
@ 2026-07-20  8:37 ` Chao Gao
  1 sibling, 0 replies; 4+ messages in thread
From: Chao Gao @ 2026-07-20  8:37 UTC (permalink / raw)
  To: Venkatesh Srinivas
  Cc: kvm, stable, David Matlack, Sean Christopherson, Jim Mattson,
	James Houghton

On Wed, Jul 15, 2026 at 11:42:35PM +0000, Venkatesh Srinivas wrote:
>Fixes a lost timer interrupt issue if:
>1) We are on an Intel platform w/ a VMX preemption timer && APICv
>2) A VMM calls KVM_GET_LAPIC before KVM_GET_MSRS to save vCPU state
>3) The thread executing the two calls migrates CPUs between the two calls
>
>After migration across CPUs, KVM_GET_MSRS calls vcpu_load, posting the
>interrupt and clearing the MSR:
>vcpu_load() ->
>  kvm_arch_vcpu_load() ->
>    kvm_lapic_restart_hv_timer() ->
>      start_hv_timer() ->
>        apic_timer_expired() ->
>          kvm_apic_inject_pending_timer_irqs()
>            . post interrupt into the LAPIC state
>            . clear IA32_TSCDEADLINE
>
>The saved LAPIC state will be missing the pending interrupt and the saved
>MSR will be zero. Oops.
>
>Fix by only posting an interrupt when we're attempting to enter the guest
>(vcpu->wants_to_run == true), not for vcpu_load from other paths.
>
>Assisted-by: gemini:gemini-3.1-pro-preview
>Debugged-by: David Matlack <dmatlack@google.com>
>Debugged-by: Sean Christopherson <seanjc@google.com>
>Debugged-by: Jim Mattson <jmattson@google.com>
>Debugged-by: James Houghton <jthoughton@google.com>
>Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>

Reviewed-by: Chao Gao <chao.gao@intel.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN
  2026-07-16  1:14 ` James Houghton
@ 2026-07-20 13:51   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2026-07-20 13:51 UTC (permalink / raw)
  To: James Houghton, Venkatesh Srinivas
  Cc: kvm, stable, David Matlack, Sean Christopherson, Jim Mattson

On 7/16/26 03:14, James Houghton wrote:
> Hi Venkatesh,
> 
> On Wed, Jul 15, 2026 at 4:42 PM Venkatesh Srinivas
> <venkateshs@chromium.org> wrote:
>>
>> Fixes a lost timer interrupt issue if:
>> 1) We are on an Intel platform w/ a VMX preemption timer && APICv
>> 2) A VMM calls KVM_GET_LAPIC before KVM_GET_MSRS to save vCPU state
>> 3) The thread executing the two calls migrates CPUs between the two calls
> 
> I think the paragraph above should be rephrased, something like this:
> 
> "On Intel platforms with a VMX preemption timer and APICv, if a VMM
> calls KVM_GET_LAPIC before KVM_GET_MSRS to save the vCPU state, it is
> possible to lose a pending timer interrupt.
> 
> If the thread running these ioctls is migrated to another core after
> calling KVM_GET_LAPIC but before KVM_GET_MSRS and the guest is using
> their LAPIC timer in TSC-deadline mode, not only does the save LAPIC
> state not carry the pending interrupt, the TSCDEADLINE MSR will be
> zeroed."
> 
> Sean doesn't like pronouns (like "we"); he'll probably have similar
> style feedback.

Yes, I sometimes use "we" as "the code", i.e. collective past 
developers, but not for runtime situations.

>>
>> After migration across CPUs, KVM_GET_MSRS calls vcpu_load, posting the
>> interrupt and clearing the MSR:
>> vcpu_load() ->
>>    kvm_arch_vcpu_load() ->
>>      kvm_lapic_restart_hv_timer() ->
>>        start_hv_timer() ->
>>          apic_timer_expired() ->
>>            kvm_apic_inject_pending_timer_irqs()
>>              . post interrupt into the LAPIC state
>>              . clear IA32_TSCDEADLINE
>>
>> The saved LAPIC state will be missing the pending interrupt and the saved
>> MSR will be zero. Oops.
>>
>> Fix by only posting an interrupt when we're attempting to enter the guest
>> (vcpu->wants_to_run == true), not for vcpu_load from other paths.
>>
>> Assisted-by: gemini:gemini-3.1-pro-preview
>> Debugged-by: David Matlack <dmatlack@google.com>
>> Debugged-by: Sean Christopherson <seanjc@google.com>
>> Debugged-by: Jim Mattson <jmattson@google.com>
>> Debugged-by: James Houghton <jthoughton@google.com>
>> Signed-off-by: Venkatesh Srinivas <venkateshs@chromium.org>
> 
> You have included cc:stable (on the CC line in this email); could you
> include it here in the patch as well?
> 
> Also, what is the appropriate Fixes?

Fixes: ae95f566b3d2 ("KVM: X86: TSCDEADLINE MSR emulation fastpath")
Cc: stable@vger.kernel.org

> As far as this change goes, it makes sense to me.
> 
> Reviewed-by: James Houghton <jthoughton@google.com>
Queued, thanks!

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-20 13:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 23:42 [PATCH] KVM: x86: Only reset TSC Deadline Timer in apic_timer_expired on KVM_RUN Venkatesh Srinivas
2026-07-16  1:14 ` James Houghton
2026-07-20 13:51   ` Paolo Bonzini
2026-07-20  8:37 ` Chao Gao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.