[PATCH] Allow Intel platforms to declare unsynchronized TSC to kernel

[PATCH] Allow Intel platforms to declare unsynchronized TSC to kernel

[Andi Kleen]

Allow Intel platforms to declare unsynchronized TSC to kernel

[This came out of a private discussion with Linus last week]

Nehalem class processors support the CONSTANT_TSC bit in the 8000_0007 CPUID
leaf to declare CONSTANT_TSC. One of the ideas behind this bit was to allow the 
platform to clear it when it cannot guarantee a constant TSC.

Unfortunately this doesn't work right now because the Intel CPU
initialization also checks the model number and assumes that
all CPUs newer than Yonah have CONSTANT_TSC too.

Terminate the model number check before Nehalem and rely on the 8000_0007
bit only on newer CPUs. This way the platform can actually turn it off.

Atom CPUs might also have newer model numbers and do not necessarily
have that bit, but they are all single socket and expected to always
have synchronous TSCs.

Open: the two checks still differ in setting sched_clock_stable. 

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 arch/x86/kernel/cpu/intel.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Index: linux-2.6.34-rc1-ak/arch/x86/kernel/cpu/intel.c
===================================================================
--- linux-2.6.34-rc1-ak.orig/arch/x86/kernel/cpu/intel.c	2010-03-03 02:01:27.000000000 +0100
+++ linux-2.6.34-rc1-ak/arch/x86/kernel/cpu/intel.c	2010-03-24 06:16:42.000000000 +0100
@@ -43,8 +43,16 @@
 		}
 	}
 
+	/*
+	 * On Nehalem+ class we trust the CONSTANT_TSC bit in 8000_0007 edx,
+	 * so that a platform that doesn't have synchronized TSCs can clear it.
+ 	 * Older CPUs didn't set that bit, so we keep hard coding the model
+	 * numbers for those.
+ 	 * Note this matches Atom class too, but there the TSCs are always
+	 * synchronized anyways.
+ 	 */
 	if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
-		(c->x86 == 0x6 && c->x86_model >= 0x0e))
+		(c->x86 == 0x6 && c->x86_model >= 0x0e && c->x86_model < 0x1a))
 		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
 
 #ifdef CONFIG_X86_64

Re: [PATCH] Allow Intel platforms to declare unsynchronized TSC to kernel

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:

> Allow Intel platforms to declare unsynchronized TSC to kernel
>
> [This came out of a private discussion with Linus last week]

Ping?  Please review this patch.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] Allow Intel platforms to declare unsynchronized TSC to kernel

[Hidetoshi Seto]

(2010/03/29 16:46), Andi Kleen wrote:
> Andi Kleen <andi@firstfloor.org> writes:
> 
>> Allow Intel platforms to declare unsynchronized TSC to kernel
>>
>> [This came out of a private discussion with Linus last week]
> 
> Ping?  Please review this patch.
> 
> -Andi

Does "constant" implicitly means "synchronized"?
IMHO the answer is no.

Following commit tells us that there could be
"constant-but-unsynchronized TSC":
 6c56ccecf05fafe100ab4ea94f6fccbf5ff00db7
> So, reenable TSC sync test even on constant and non-stop TSC systems.

Quote from the patch description of commit:
 40fb17152c50a69dc304dd632131c2f41281ce44
> Add support for CPUID_0x80000007_Bit8 on Intel CPUs as well. This bit means
> that the TSC is invariant with C/P/T states and always runs at constant
> frequency.
>
> With Intel CPUs, we have 3 classes
> * CPUs where TSC runs at constant rate and does not stop n C-states
> * CPUs where TSC runs at constant rate, but will stop in deep C-states
> * CPUs where TSC rate will vary based on P/T-states and TSC will stop in
>   deep C-states.

It looks like that "invariant" also means "constant."

I suppose your change is intended to allow newer platform to declare
"not invariant", instead of "not invariant but constant."
Humm, but how things change...?  Still newer platform cannot declare
one of 3 classes.  Which class is "never on newer platform"?
Or we need to have 4th new class? 

At least I think your patch description needs some update,
to get more comments without confusing reviewers.


Thanks,
H.Seto

Re: [PATCH] Allow Intel platforms to declare unsynchronized TSC to kernel

[Andi Kleen]

Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> writes:

> (2010/03/29 16:46), Andi Kleen wrote:
>> Andi Kleen <andi@firstfloor.org> writes:
>> 
>>> Allow Intel platforms to declare unsynchronized TSC to kernel
>>>
>>> [This came out of a private discussion with Linus last week]
>> 
>> Ping?  Please review this patch.
>> 
>> -Andi
>
> Does "constant" implicitly means "synchronized"?

Yes it does for the CPUID bit.

> Following commit tells us that there could be
> "constant-but-unsynchronized TSC":
>  6c56ccecf05fafe100ab4ea94f6fccbf5ff00db7
>> So, reenable TSC sync test even on constant and non-stop TSC systems.

First the platform is supposed to clear in this case and when it
doesn't the later check catches it anyways.

BTW all the things you're mentioning are not actually changed by my
patch. In this regard it behaves the same as before. I don't think
the patch description has to describe things it does not change
or be a general tutorial on TSCs.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.