Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1500618 > unrolled thread

[PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Bin Gao <bin.gao@linux.intel.com> - 2016-10-14 01:20 +0200
    Re: [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Thomas Gleixner <tglx@linutronix.de> - 2016-10-20 12:10 +0200
      Re: [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Peter Zijlstra <peterz@infradead.org> - 2016-10-20 12:20 +0200
        Re: [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Thomas Gleixner <tglx@linutronix.de> - 2016-10-20 21:50 +0200
          Re: [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Peter Zijlstra <peterz@infradead.org> - 2016-10-21 07:50 +0200
            Re: [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag Thomas Gleixner <tglx@linutronix.de> - 2016-10-21 10:10 +0200

#1500618 — [PATCH v3] x86/tsc: add X86_FEATURE_TSC_KNOWN_FREQ flag

The X86_FEATURE_TSC_RELIABLE flag in Linux kernel implies both reliable
(at runtime) and trustable (at calibration). But reliable running and
trustable calibration are logically irrelevant. Per Thomas Gleixner's
suggestion we would like to split this flag into two separate flags:
X86_FEATURE_TSC_RELIABLE - running reliably
X86_FEATURE_TSC_KNOWN_FREQ - frequency is known (no calibration required)
These two flags allow Linux kernel to act differently based on
processor/SoC's capability, i.e. no watchdog on TSC if TSC is reliable,
and no calibration if TSC frequency is known.

Current Linux kernel already gurantees calibration is skipped for
processors that can report TSC frequency by CPUID or MSR. However, the
delayed calibration is still not skipped for these CPUID/MSR capable
processors. The new flag X86_FEATURE_TSC_KNOWN_FREQ added by this patch
will gurantee the delayed calibration is skipped.

Signed-off-by: Bin Gao <bin.gao@intel.com>
---
 arch/x86/include/asm/cpufeatures.h  |  1 +
 arch/x86/kernel/tsc.c               | 11 ++++++++++-
 arch/x86/kernel/tsc_msr.c           |  6 ++++++
 arch/x86/platform/intel-mid/mfld.c  |  7 +++++--
 arch/x86/platform/intel-mid/mrfld.c |  6 ++++--
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 1188bc8..2df6e86 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -106,6 +106,7 @@
 #define X86_FEATURE_APERFMPERF	( 3*32+28) /* APERFMPERF */
 #define X86_FEATURE_EAGER_FPU	( 3*32+29) /* "eagerfpu" Non lazy FPU restore */
 #define X86_FEATURE_NONSTOP_TSC_S3 ( 3*32+30) /* TSC doesn't stop in S3 state */
+#define X86_FEATURE_TSC_KNOWN_FREQ ( 3*32+31) /* TSC has known frequency */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	( 4*32+ 0) /* "pni" SSE-3 */
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 46b2f41..aed2dc3 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -702,6 +702,15 @@ unsigned long native_calibrate_tsc(void)
 		}
 	}
 
+	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
+
+	/*
+	 * For Atom SoCs TSC is the only reliable clocksource.
+	 * Mark TSC reliable so no watchdog on it.
+	 */
+	if (boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT)
+		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
+
 	return crystal_khz * ebx_numerator / eax_denominator;
 }
 
@@ -1286,7 +1295,7 @@ static int __init init_tsc_clocksource(void)
 	 * Trust the results of the earlier calibration on systems
 	 * exporting a reliable TSC.
 	 */
-	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE)) {
+	if (boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)) {
 		clocksource_register_khz(&clocksource_tsc, tsc_khz);
 		return 0;
 	}
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 0fe720d..8c33292 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -100,5 +100,11 @@ unsigned long cpu_khz_from_msr(void)
 #ifdef CONFIG_X86_LOCAL_APIC
 	lapic_timer_frequency = (freq * 1000) / HZ;
 #endif
+
+	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
+
+	/* Mark TSC reliable */
+	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
+
 	return res;
 }
diff --git a/arch/x86/platform/intel-mid/mfld.c b/arch/x86/platform/intel-mid/mfld.c
index 1eb47b6..6724ab9b 100644
--- a/arch/x86/platform/intel-mid/mfld.c
+++ b/arch/x86/platform/intel-mid/mfld.c
@@ -49,8 +49,11 @@ static unsigned long __init mfld_calibrate_tsc(void)
 	fast_calibrate = ratio * fsb;
 	pr_debug("read penwell tsc %lu khz\n", fast_calibrate);
 	lapic_timer_frequency = fsb * 1000 / HZ;
-	/* mark tsc clocksource as reliable */
-	set_cpu_cap(&boot_cpu_data, X86_FEATURE_TSC_RELIABLE);
+
+	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
+
+	/* Mark TSC reliable */
+	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
 
 	return fast_calibrate;
 }
diff --git a/arch/x86/platform/intel-mid/mrfld.c b/arch/x86/platform/intel-mid/mrfld.c
index 59253db..c8b9870 100644
--- a/arch/x86/platform/intel-mid/mrfld.c
+++ b/arch/x86/platform/intel-mid/mrfld.c
@@ -78,8 +78,10 @@ static unsigned long __init tangier_calibrate_tsc(void)
 	pr_debug("Setting lapic_timer_frequency = %d\n",
 			lapic_timer_frequency);
 
-	/* mark tsc clocksource as reliable */
-	set_cpu_cap(&boot_cpu_data, X86_FEATURE_TSC_RELIABLE);
+	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
+
+	/* Mark TSC reliable */
+	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
 
 	return fast_calibrate;
 }
-- 
1.9.1

[toc] | [next] | [standalone]

#1504737

On Thu, 13 Oct 2016, Bin Gao wrote:
> @@ -702,6 +702,15 @@ unsigned long native_calibrate_tsc(void)
>  		}
>  	}
>  
> +	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> +
> +	/*
> +	 * For Atom SoCs TSC is the only reliable clocksource.
> +	 * Mark TSC reliable so no watchdog on it.
> +	 */
> +	if (boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT)
> +		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> +

Right. That's what I wanted to see, but please split this into two patches:

  #1 Split the TSC flags
  #2 Set the flag for Goldmont

We do not mix design changes with hw support changes.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1504742

On Thu, Oct 20, 2016 at 11:57:03AM +0200, Thomas Gleixner wrote:
> On Thu, 13 Oct 2016, Bin Gao wrote:
> > @@ -702,6 +702,15 @@ unsigned long native_calibrate_tsc(void)
> >  		}
> >  	}
> >  
> > +	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> > +
> > +	/*
> > +	 * For Atom SoCs TSC is the only reliable clocksource.
> > +	 * Mark TSC reliable so no watchdog on it.
> > +	 */
> > +	if (boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT)
> > +		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > +

AFAICT setting TSC_RELIABLE also skips the check_tsc_warp() tests in
tsc_sync.c.

This means that if someone does a Goldmont BIOS with 'features', we'll
never detect the wreckage :-/

[toc] | [prev] | [next] | [standalone]

#1505212

On Thu, 20 Oct 2016, Peter Zijlstra wrote:
> On Thu, Oct 20, 2016 at 11:57:03AM +0200, Thomas Gleixner wrote:
> > On Thu, 13 Oct 2016, Bin Gao wrote:
> > > @@ -702,6 +702,15 @@ unsigned long native_calibrate_tsc(void)
> > >  		}
> > >  	}
> > >  
> > > +	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> > > +
> > > +	/*
> > > +	 * For Atom SoCs TSC is the only reliable clocksource.
> > > +	 * Mark TSC reliable so no watchdog on it.
> > > +	 */
> > > +	if (boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT)
> > > +		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > > +
> 
> AFAICT setting TSC_RELIABLE also skips the check_tsc_warp() tests in
> tsc_sync.c.
> 
> This means that if someone does a Goldmont BIOS with 'features', we'll
> never detect the wreckage :-/

Well, we have the same issue on other platforms/models which set the
reliable flag.

So one sanity check we can do is to read the IA32_TSC_ADJUST MSR on all
cores. They should all have the same value (usually 0) or at least have a
very minimal delta. If that's off by more than 1us then something is fishy
especially on single socket systems. We could at least WARN about it.

We could do this in idle occasionally as well, so we can detect the dreaded
"SMI wants to hide the cycles" crapola.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1505475

On Thu, Oct 20, 2016 at 09:37:50PM +0200, Thomas Gleixner wrote:

> Well, we have the same issue on other platforms/models which set the
> reliable flag.

I was not aware we had other platforms doing this, git grep tells me
intel-mid does this as well..

> So one sanity check we can do is to read the IA32_TSC_ADJUST MSR on all
> cores. They should all have the same value (usually 0) or at least have a
> very minimal delta. If that's off by more than 1us then something is fishy
> especially on single socket systems. We could at least WARN about it.
> 
> We could do this in idle occasionally as well, so we can detect the dreaded
> "SMI wants to hide the cycles" crapola.

Indeed, that sounds like the best we can; and probably should; do.

[toc] | [prev] | [next] | [standalone]

#1505553

On Fri, 21 Oct 2016, Peter Zijlstra wrote:
> On Thu, Oct 20, 2016 at 09:37:50PM +0200, Thomas Gleixner wrote:
> 
> > Well, we have the same issue on other platforms/models which set the
> > reliable flag.
> 
> I was not aware we had other platforms doing this, git grep tells me
> intel-mid does this as well..
> 
> > So one sanity check we can do is to read the IA32_TSC_ADJUST MSR on all
> > cores. They should all have the same value (usually 0) or at least have a
> > very minimal delta. If that's off by more than 1us then something is fishy
> > especially on single socket systems. We could at least WARN about it.
> > 
> > We could do this in idle occasionally as well, so we can detect the dreaded
> > "SMI wants to hide the cycles" crapola.
> 
> Indeed, that sounds like the best we can; and probably should; do.

I'll have a look at that in the next days.

Thanks,

	tglx
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web