All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test
@ 2022-01-27 21:55 Jim Mattson
  2022-01-27 21:55 ` [kvm-unit-tests PATCH 2/3] x86: tsc_adjust: Use report_skip when skipping test Jim Mattson
  2022-01-27 21:55 ` [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc) Jim Mattson
  0 siblings, 2 replies; 4+ messages in thread
From: Jim Mattson @ 2022-01-27 21:55 UTC (permalink / raw)
  To: kvm, pbonzini; +Cc: Jim Mattson, Will Auld

The subtest labeled, "MSR_IA32_TSC_ADJUST msr adjustment on tsc
write," sometimes fails. The behavior tested is neither architected
nor guaranteed. Running under qemu/kvm, the 'est_delta_time' has been
observed to be as much as an order of magnitude greater than the
expression that is supposed to be its upper bound.

Remove the flaky subtest, and replace it with some invariants that
actually are architecturally guaranteed (as long as IA32_TSC doesn't
wrap around).

Fixes: 5fecf5d8cad1 ("Added tests for ia32_tsc_adjust funtionality.")
Cc: Will Auld <will.auld.intel@gmail.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 x86/tsc_adjust.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/x86/tsc_adjust.c b/x86/tsc_adjust.c
index 3636b5e082d5..b0d79c499edb 100644
--- a/x86/tsc_adjust.c
+++ b/x86/tsc_adjust.c
@@ -4,7 +4,6 @@
 int main(void)
 {
 	u64 t1, t2, t3, t4, t5;
-	u64 est_delta_time;
 
 	if (this_cpu_has(X86_FEATURE_TSC_ADJUST)) { // MSR_IA32_TSC_ADJUST Feature is enabled?
 		report(rdmsr(MSR_IA32_TSC_ADJUST) == 0x0,
@@ -26,12 +25,9 @@ int main(void)
 		wrtsc(t4);
 		t2 = rdtsc();
 		t5 = rdmsr(MSR_IA32_TSC_ADJUST);
-		// est of time between reading tsc and writing tsc,
-		// (based on MSR_IA32_TSC_ADJUST msr value) should be small
-		est_delta_time = t4 - t5 - t1;
-		// arbitray 2x latency (wrtsc->rdtsc) threshold
-		report(est_delta_time <= (2 * (t2 - t4)),
-		       "MSR_IA32_TSC_ADJUST msr adjustment on tsc write");
+		report(t1 <= t4 - t5,
+		       "Internal TSC advances across write to IA32_TSC");
+		report(t2 >= t4, "IA32_TSC advances after write to IA32_TSC");
 	}
 	else {
 		report_pass("MSR_IA32_TSC_ADJUST feature not enabled");
-- 
2.35.0.rc2.247.g8bbb082509-goog


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

* [kvm-unit-tests PATCH 2/3] x86: tsc_adjust: Use report_skip when skipping test
  2022-01-27 21:55 [kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test Jim Mattson
@ 2022-01-27 21:55 ` Jim Mattson
  2022-01-27 21:55 ` [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc) Jim Mattson
  1 sibling, 0 replies; 4+ messages in thread
From: Jim Mattson @ 2022-01-27 21:55 UTC (permalink / raw)
  To: kvm, pbonzini; +Cc: Jim Mattson

Opportunistically reorder the code to reduce indentation.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 x86/tsc_adjust.c | 52 ++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/x86/tsc_adjust.c b/x86/tsc_adjust.c
index b0d79c499edb..c98c1eacb8dc 100644
--- a/x86/tsc_adjust.c
+++ b/x86/tsc_adjust.c
@@ -5,32 +5,32 @@ int main(void)
 {
 	u64 t1, t2, t3, t4, t5;
 
-	if (this_cpu_has(X86_FEATURE_TSC_ADJUST)) { // MSR_IA32_TSC_ADJUST Feature is enabled?
-		report(rdmsr(MSR_IA32_TSC_ADJUST) == 0x0,
-		       "MSR_IA32_TSC_ADJUST msr initialization");
-		t3 = 100000000000ull;
-		t1 = rdtsc();
-		wrmsr(MSR_IA32_TSC_ADJUST, t3);
-		t2 = rdtsc();
-		report(rdmsr(MSR_IA32_TSC_ADJUST) == t3,
-		       "MSR_IA32_TSC_ADJUST msr read / write");
-		report((t2 - t1) >= t3,
-		       "TSC adjustment for MSR_IA32_TSC_ADJUST value");
-		t3 = 0x0;
-		wrmsr(MSR_IA32_TSC_ADJUST, t3);
-		report(rdmsr(MSR_IA32_TSC_ADJUST) == t3,
-		       "MSR_IA32_TSC_ADJUST msr read / write");
-		t4 = 100000000000ull;
-		t1 = rdtsc();
-		wrtsc(t4);
-		t2 = rdtsc();
-		t5 = rdmsr(MSR_IA32_TSC_ADJUST);
-		report(t1 <= t4 - t5,
-		       "Internal TSC advances across write to IA32_TSC");
-		report(t2 >= t4, "IA32_TSC advances after write to IA32_TSC");
-	}
-	else {
-		report_pass("MSR_IA32_TSC_ADJUST feature not enabled");
+	if (!this_cpu_has(X86_FEATURE_TSC_ADJUST)) {
+		report_skip("MSR_IA32_TSC_ADJUST feature not enabled");
+		return report_summary();
 	}
+
+	report(rdmsr(MSR_IA32_TSC_ADJUST) == 0x0,
+	       "MSR_IA32_TSC_ADJUST msr initialization");
+	t3 = 100000000000ull;
+	t1 = rdtsc();
+	wrmsr(MSR_IA32_TSC_ADJUST, t3);
+	t2 = rdtsc();
+	report(rdmsr(MSR_IA32_TSC_ADJUST) == t3,
+	       "MSR_IA32_TSC_ADJUST msr read / write");
+	report((t2 - t1) >= t3,
+	       "TSC adjustment for MSR_IA32_TSC_ADJUST value");
+	t3 = 0x0;
+	wrmsr(MSR_IA32_TSC_ADJUST, t3);
+	report(rdmsr(MSR_IA32_TSC_ADJUST) == t3,
+	       "MSR_IA32_TSC_ADJUST msr read / write");
+	t4 = 100000000000ull;
+	t1 = rdtsc();
+	wrtsc(t4);
+	t2 = rdtsc();
+	t5 = rdmsr(MSR_IA32_TSC_ADJUST);
+	report(t1 <= t4 - t5, "Internal TSC advances across write to IA32_TSC");
+	report(t2 >= t4, "IA32_TSC advances after write to IA32_TSC");
+
 	return report_summary();
 }
-- 
2.35.0.rc2.247.g8bbb082509-goog


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

* [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc)
  2022-01-27 21:55 [kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test Jim Mattson
  2022-01-27 21:55 ` [kvm-unit-tests PATCH 2/3] x86: tsc_adjust: Use report_skip when skipping test Jim Mattson
@ 2022-01-27 21:55 ` Jim Mattson
  2022-01-28 15:17   ` Paolo Bonzini
  1 sibling, 1 reply; 4+ messages in thread
From: Jim Mattson @ 2022-01-27 21:55 UTC (permalink / raw)
  To: kvm, pbonzini; +Cc: Jim Mattson

Remove some inline assembly code duplication and opportunistically
replace the magic constant, "0x10," with "MSR_IA32_TSC."

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 lib/x86/processor.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index fe5add548261..117032a4895c 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -592,9 +592,7 @@ static inline unsigned long long rdtscp(u32 *aux)
 
 static inline void wrtsc(u64 tsc)
 {
-	unsigned a = tsc, d = tsc >> 32;
-
-	asm volatile("wrmsr" : : "a"(a), "d"(d), "c"(0x10));
+	wrmsr(MSR_IA32_TSC, tsc);
 }
 
 static inline void irq_disable(void)
-- 
2.35.0.rc2.247.g8bbb082509-goog


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

* Re: [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc)
  2022-01-27 21:55 ` [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc) Jim Mattson
@ 2022-01-28 15:17   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2022-01-28 15:17 UTC (permalink / raw)
  To: Jim Mattson, kvm

On 1/27/22 22:55, Jim Mattson wrote:
> Remove some inline assembly code duplication and opportunistically
> replace the magic constant, "0x10," with "MSR_IA32_TSC."
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>   lib/x86/processor.h | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index fe5add548261..117032a4895c 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -592,9 +592,7 @@ static inline unsigned long long rdtscp(u32 *aux)
>   
>   static inline void wrtsc(u64 tsc)
>   {
> -	unsigned a = tsc, d = tsc >> 32;
> -
> -	asm volatile("wrmsr" : : "a"(a), "d"(d), "c"(0x10));
> +	wrmsr(MSR_IA32_TSC, tsc);
>   }
>   
>   static inline void irq_disable(void)

Queued all three, thanks.

Paolo


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

end of thread, other threads:[~2022-01-28 15:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-27 21:55 [kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test Jim Mattson
2022-01-27 21:55 ` [kvm-unit-tests PATCH 2/3] x86: tsc_adjust: Use report_skip when skipping test Jim Mattson
2022-01-27 21:55 ` [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc) Jim Mattson
2022-01-28 15:17   ` Paolo Bonzini

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.