[kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test

[kvm-unit-tests PATCH 1/3] x86: tsc_adjust: Remove flaky timing test

[Jim Mattson]

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

[kvm-unit-tests PATCH 2/3] x86: tsc_adjust: Use report_skip when skipping test

[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

[kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc)

[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

Re: [kvm-unit-tests PATCH 3/3] x86: Define wrtsc(tsc) as wrmsr(MSR_IA32_TSC, tsc)

[Paolo Bonzini]

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