Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755406Ab1CKVY2 (ORCPT ); Fri, 11 Mar 2011 16:24:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18733 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754244Ab1CKVYZ (ORCPT ); Fri, 11 Mar 2011 16:24:25 -0500 Date: Fri, 11 Mar 2011 16:24:19 -0500 From: Dave Jones To: x86@kernel.org Cc: Linux Kernel Subject: RFC: Bogus setting of CONSTANT TSC ? Message-ID: <20110311212418.GA14784@redhat.com> Mail-Followup-To: Dave Jones , x86@kernel.org, Linux Kernel MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2921 Lines: 71 [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 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/