linux-kernel - RFC: Bogus setting of CONSTANT TSC ?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 11 Mar 2011 16:24:19 -0500
From:	Dave Jones <davej@...hat.com>
To:	x86@...nel.org
Cc:	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: RFC: Bogus setting of CONSTANT TSC ?

[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 <davej@...hat.com>

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);
+		}
 	}

 	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/