public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer
@ 2026-02-03 18:20 isaku.yamahata
  2026-02-03 18:30 ` Sean Christopherson
  0 siblings, 1 reply; 4+ messages in thread
From: isaku.yamahata @ 2026-02-03 18:20 UTC (permalink / raw)
  To: kvm; +Cc: isaku.yamahata, isaku.yamahata, Paolo Bonzini,
	Sean Christopherson

From: Isaku Yamahata <isaku.yamahata@intel.com>

Fix apic and vmexit test cases to pass with APIC timer virtualization
by replacing nop instruction with serialize instruction.

apic and vmexit test cases use "wrmsr(TSCDEADLINE); nop" sequence to cause
deadline timer to fire immediately.  It's not guaranteed because
wrmsr(TSCDEADLINE) isn't serializing instruction according to SDM [1].
With APIC timer virtualization enabled, those test can fail.  It worked
before because KVM intercepts wrmsr(TSCDEADLINE).  KVM doesn't intercept it
with apic timer virtualization.

[1] From SDM 3a Serializing intructions
  An execution of WRMSR to any non-serializing MSR is not
  serializing. Non-serializing MSRs include the following: IA32_SPEC_CTRL
  MSR (MSR index 48H), IA32_PRED_CMD MSR (MSR index 49H), IA32_TSX_CTRL MSR
  (MSR index 122H), IA32_TSC_DEADLINE MSR (MSR index 6E0H), IA32_PKRS MSR
  (MSR index 6E1H), IA32_HWP_REQUEST MSR (MSR index 774H), or any of the
  x2APIC MSRs (MSR indices 802H to 83FH).

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
 lib/x86/processor.h | 6 ++++++
 x86/apic.c          | 2 +-
 x86/vmexit.c        | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 42dd2d2a4787..ec91c9c2f87d 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -1086,6 +1086,12 @@ static inline void wrtsc(u64 tsc)
 }
 
 
+static inline void serialize(void)
+{
+	/* serialize instruction. It needs binutils >= 2.35. */
+	asm_safe(".byte 0x0f, 0x01, 0xe8");
+}
+
 static inline void invlpg(volatile void *va)
 {
 	asm volatile("invlpg (%0)" ::"r" (va) : "memory");
diff --git a/x86/apic.c b/x86/apic.c
index 0a52e9a45f1c..0ee788594499 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -35,7 +35,7 @@ static void __test_tsc_deadline_timer(void)
 	handle_irq(TSC_DEADLINE_TIMER_VECTOR, tsc_deadline_timer_isr);
 
 	wrmsr(MSR_IA32_TSCDEADLINE, rdmsr(MSR_IA32_TSC));
-	asm volatile ("nop");
+	serialize();
 	report(tdt_count == 1, "tsc deadline timer");
 	report(rdmsr(MSR_IA32_TSCDEADLINE) == 0, "tsc deadline timer clearing");
 }
diff --git a/x86/vmexit.c b/x86/vmexit.c
index 5296ed38aa34..6e3f4442f2f3 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -437,7 +437,7 @@ static int has_tscdeadline(void)
 static void tscdeadline_immed(void)
 {
 	wrmsr(MSR_IA32_TSCDEADLINE, rdtsc());
-	asm volatile("nop");
+	serialize();
 }
 
 static void tscdeadline(void)

base-commit: 86e53277ac80dabb04f4fa5fa6a6cc7649392bdc
-- 
2.45.2


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

* Re: [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer
  2026-02-03 18:20 [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer isaku.yamahata
@ 2026-02-03 18:30 ` Sean Christopherson
  2026-02-03 18:59   ` Isaku Yamahata
  0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2026-02-03 18:30 UTC (permalink / raw)
  To: isaku.yamahata; +Cc: kvm, isaku.yamahata, Paolo Bonzini

On Tue, Feb 03, 2026, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Fix apic and vmexit test cases to pass with APIC timer virtualization
> by replacing nop instruction with serialize instruction.
> 
> apic and vmexit test cases use "wrmsr(TSCDEADLINE); nop" sequence to cause
> deadline timer to fire immediately.  It's not guaranteed because
> wrmsr(TSCDEADLINE) isn't serializing instruction according to SDM [1].
> With APIC timer virtualization enabled, those test can fail.  It worked
> before because KVM intercepts wrmsr(TSCDEADLINE).  KVM doesn't intercept it
> with apic timer virtualization.
> 
> [1] From SDM 3a Serializing intructions
>   An execution of WRMSR to any non-serializing MSR is not
>   serializing. Non-serializing MSRs include the following: IA32_SPEC_CTRL
>   MSR (MSR index 48H), IA32_PRED_CMD MSR (MSR index 49H), IA32_TSX_CTRL MSR
>   (MSR index 122H), IA32_TSC_DEADLINE MSR (MSR index 6E0H), IA32_PKRS MSR
>   (MSR index 6E1H), IA32_HWP_REQUEST MSR (MSR index 774H), or any of the
>   x2APIC MSRs (MSR indices 802H to 83FH).
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  lib/x86/processor.h | 6 ++++++
>  x86/apic.c          | 2 +-
>  x86/vmexit.c        | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 42dd2d2a4787..ec91c9c2f87d 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -1086,6 +1086,12 @@ static inline void wrtsc(u64 tsc)
>  }
>  
>  
> +static inline void serialize(void)
> +{
> +	/* serialize instruction. It needs binutils >= 2.35. */

And a CPU that supports it...  I don't see any point in using SERIALIZE.  To check
for support, this code would need to do CPUID to query X86_FEATURE_SERIALIZE, and
CPUID itself is serializing (the big reason to favor SERIALIZE over CPUID is to
avoid a VM-Exit for performance reasons).


> +	asm_safe(".byte 0x0f, 0x01, 0xe8");
> +}
> +
>  static inline void invlpg(volatile void *va)
>  {
>  	asm volatile("invlpg (%0)" ::"r" (va) : "memory");
> diff --git a/x86/apic.c b/x86/apic.c
> index 0a52e9a45f1c..0ee788594499 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -35,7 +35,7 @@ static void __test_tsc_deadline_timer(void)
>  	handle_irq(TSC_DEADLINE_TIMER_VECTOR, tsc_deadline_timer_isr);
>  
>  	wrmsr(MSR_IA32_TSCDEADLINE, rdmsr(MSR_IA32_TSC));
> -	asm volatile ("nop");
> +	serialize();
>  	report(tdt_count == 1, "tsc deadline timer");
>  	report(rdmsr(MSR_IA32_TSCDEADLINE) == 0, "tsc deadline timer clearing");
>  }
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index 5296ed38aa34..6e3f4442f2f3 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -437,7 +437,7 @@ static int has_tscdeadline(void)
>  static void tscdeadline_immed(void)
>  {
>  	wrmsr(MSR_IA32_TSCDEADLINE, rdtsc());
> -	asm volatile("nop");
> +	serialize();
>  }
>  
>  static void tscdeadline(void)
> 
> base-commit: 86e53277ac80dabb04f4fa5fa6a6cc7649392bdc
> -- 
> 2.45.2
> 

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

* Re: [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer
  2026-02-03 18:30 ` Sean Christopherson
@ 2026-02-03 18:59   ` Isaku Yamahata
  2026-02-04 14:21     ` Sean Christopherson
  0 siblings, 1 reply; 4+ messages in thread
From: Isaku Yamahata @ 2026-02-03 18:59 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: isaku.yamahata, kvm, isaku.yamahata, Paolo Bonzini,
	isaku.yamahata

On Tue, Feb 03, 2026 at 10:30:06AM -0800,
Sean Christopherson <seanjc@google.com> wrote:

> > +static inline void serialize(void)
> > +{
> > +	/* serialize instruction. It needs binutils >= 2.35. */
> 
> And a CPU that supports it...  I don't see any point in using SERIALIZE.  To check
> for support, this code would need to do CPUID to query X86_FEATURE_SERIALIZE, and
> CPUID itself is serializing (the big reason to favor SERIALIZE over CPUID is to
> avoid a VM-Exit for performance reasons).

Thank you for pointing it out. I'll replace it with raw_cpuid(0, 0).
Or do you want to opencode cpuid() in each places?
-- 
Isaku Yamahata <isaku.yamahata@intel.com>

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

* Re: [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer
  2026-02-03 18:59   ` Isaku Yamahata
@ 2026-02-04 14:21     ` Sean Christopherson
  0 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-02-04 14:21 UTC (permalink / raw)
  To: Isaku Yamahata; +Cc: kvm, isaku.yamahata, Paolo Bonzini, isaku.yamahata

On Tue, Feb 03, 2026, Isaku Yamahata wrote:
> On Tue, Feb 03, 2026 at 10:30:06AM -0800,
> Sean Christopherson <seanjc@google.com> wrote:
> 
> > > +static inline void serialize(void)
> > > +{
> > > +	/* serialize instruction. It needs binutils >= 2.35. */
> > 
> > And a CPU that supports it...  I don't see any point in using SERIALIZE.  To check
> > for support, this code would need to do CPUID to query X86_FEATURE_SERIALIZE, and
> > CPUID itself is serializing (the big reason to favor SERIALIZE over CPUID is to
> > avoid a VM-Exit for performance reasons).
> 
> Thank you for pointing it out. I'll replace it with raw_cpuid(0, 0).
> Or do you want to opencode cpuid() in each places?

It probably makes sense to add a helper to arm the deadline timer, and deal with
the serialization there.  E.g. start_tsc_deadline_timer() also has a nop() of
dubious value.

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

end of thread, other threads:[~2026-02-04 14:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-03 18:20 [kvm-unit-tests PATCH] x86: apic, vmexit: replace nop with serialize to wait for deadline timer isaku.yamahata
2026-02-03 18:30 ` Sean Christopherson
2026-02-03 18:59   ` Isaku Yamahata
2026-02-04 14:21     ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox