[RFC because I'm not sure about this. Can any of the Intel people
confirm/deny that there are recent CPUs without constant TSC ?
I was under the belief that all current ones had this, but either
this isn't the case, or cpuid is lying to me.
also: That hardcoded model check looks suspect to me. Shouldn't
that be checking for extended models ? ]
I've got a core2 laptop that I noticed reports all zeros as a result of
cpuid(0x80000007), indicating no constant TSC. The flags field
of /proc/cpuinfo seems to think otherwise however. This looks to
be because of this code in early_init_intel()
if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
(c->x86 == 0x6 && c->x86_model >= 0x0e))
set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
We explicitly set X86_FEATURE_CONSTANT_TSC on certain steppings
when we enter early_init_intel, and then later we check if cpuid
reports this feature, and set it on any other CPUs that were missed.
I think there are two problems here:
1. The code doesn't validate that x86_power was set to something sane before
we trust it (it's only valid if cpuid_level is >= 0x80000007)
On cpu's that don't support this level, it /should/ return zeros, but
I'm not sure that's defined behaviour.
2. If x86_power was valid, but the CONSTANT_TSC bit was 0, we don't clear it
if it was mistakenly set by the hard-coded stepping check.
We need to add the first check, otherwise the fix for the second part would
always clear it on the CPUs that were hardcoded that don't support cpuid 0x80000007
As a follow-up, perhaps we should move the hardcoding code to an } else arm here
where we don't support that cpuid feature to keep all the relevant code that frobs
this stuff in one place.
Signed-off-by: Dave Jones <[email protected]>
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index d16c2c5..02c5357 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -88,11 +88,15 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
* It is also reliable across cores and sockets. (but not across
* cabinets - we turn it off in that case explicitly.)
*/
- if (c->x86_power & (1 << 8)) {
- set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
- set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
- if (!check_tsc_unstable())
- sched_clock_stable = 1;
+ if (c->extended_cpuid_level >= 0x80000007) {
+ if (c->x86_power & (1 << 8)) {
+ set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
+ set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
+ if (!check_tsc_unstable())
+ sched_clock_stable = 1;
+ } else {
+ clear_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
+ }
}
/*
On Fri, 11 Mar 2011, Dave Jones wrote:
> [RFC because I'm not sure about this. Can any of the Intel people
> confirm/deny that there are recent CPUs without constant TSC ?
> I was under the belief that all current ones had this, but either
> this isn't the case, or cpuid is lying to me.
>
> also: That hardcoded model check looks suspect to me. Shouldn't
> that be checking for extended models ? ]
>
>
>
> I've got a core2 laptop that I noticed reports all zeros as a result of
> cpuid(0x80000007), indicating no constant TSC. The flags field
> of /proc/cpuinfo seems to think otherwise however. This looks to
> be because of this code in early_init_intel()
>
> if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> (c->x86 == 0x6 && c->x86_model >= 0x0e))
> set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
This was initially introduced with commit 39b3a7 ([PATCH] i386/x86-64:
Generalize X86_FEATURE_CONSTANT_TSC flag), though the changelog is
utterly useless. Andi ??
> We explicitly set X86_FEATURE_CONSTANT_TSC on certain steppings
> when we enter early_init_intel, and then later we check if cpuid
> reports this feature, and set it on any other CPUs that were missed.
>
> I think there are two problems here:
> 1. The code doesn't validate that x86_power was set to something sane before
> we trust it (it's only valid if cpuid_level is >= 0x80000007)
> On cpu's that don't support this level, it /should/ return zeros, but
> I'm not sure that's defined behaviour.
> 2. If x86_power was valid, but the CONSTANT_TSC bit was 0, we don't clear it
> if it was mistakenly set by the hard-coded stepping check.
>
> We need to add the first check, otherwise the fix for the second part would
> always clear it on the CPUs that were hardcoded that don't support cpuid 0x80000007
>
> As a follow-up, perhaps we should move the hardcoding code to an } else arm here
> where we don't support that cpuid feature to keep all the relevant code that frobs
> this stuff in one place.
>
> Signed-off-by: Dave Jones <[email protected]>
>
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index d16c2c5..02c5357 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -88,11 +88,15 @@ static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
> * It is also reliable across cores and sockets. (but not across
> * cabinets - we turn it off in that case explicitly.)
> */
> - if (c->x86_power & (1 << 8)) {
> - set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> - set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> - if (!check_tsc_unstable())
> - sched_clock_stable = 1;
> + if (c->extended_cpuid_level >= 0x80000007) {
> + if (c->x86_power & (1 << 8)) {
> + set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> + set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
> + if (!check_tsc_unstable())
> + sched_clock_stable = 1;
> + } else {
> + clear_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
> + }
> }
>
> /*
>
> This was initially introduced with commit 39b3a7 ([PATCH] i386/x86-64:
> Generalize X86_FEATURE_CONSTANT_TSC flag), though the changelog is
> utterly useless. Andi ??
Only Nehalem and beyond set the 8000_0007 flag, but Intel has had a constant
TSC since Yonah. That is what this logic implements.
However on Nehalem and beyond if the 8000_0007 is clear the BIOS wants
to tell us that it knows the system doesn't have a synchronized TSC.
This is currently still broken, but the timer watchdog usually catches
it later. It only affects a few relatively obscure systems.
-Andi