From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754611AbaCCT3r (ORCPT ); Mon, 3 Mar 2014 14:29:47 -0500 Received: from mail.skyhub.de ([78.46.96.112]:53070 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753741AbaCCT3p (ORCPT ); Mon, 3 Mar 2014 14:29:45 -0500 Date: Mon, 3 Mar 2014 20:29:39 +0100 From: Borislav Petkov To: Chris Bainbridge Cc: Andreas Mohr , Dennis Mungai , "H. Peter Anvin" , x86@kernel.org, Dave Jones , linux-kernel@vger.kernel.org, devzero@web.de Subject: Re: Re: [PATCH] x86: set Pentium M as PAE capable Message-ID: <20140303192939.GD31265@pd.tnic> References: <20140228140043.GA12157@debian.local> <20140302205619.GA8676@rhlx01.hs-esslingen.de> <20140303080432.GA25489@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140303080432.GA25489@localhost> 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 On Mon, Mar 03, 2014 at 03:04:35PM +0700, Chris Bainbridge wrote: > On 3 March 2014 02:05, Roland Kletzing wrote: > > i would recommend adding the newly introduced param to > > Documentation/kernel- > > parameters.txt , though. > > Done. > > Signed-off-by: Chris Bainbridge > --- > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index b9e9bd8..388b5e9 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -962,6 +962,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted. > parameter will force ia64_sal_cache_flush to call > ia64_pal_cache_flush instead of SAL_CACHE_FLUSH. > > + forcepae [X86-32] > + Forcefully enable Physical Address Extension (PAE). > + Many Pentium M systems disable PAE but may have a > + functionally usable PAE implementation. > + Note: This parameter is unsupported, may cause unknown What does "unsupported" mean here exactly? > + problems, and will taint the kernel. > + > ftrace=[tracer] > [FTRACE] will set and start the specified tracer > as early as possible in order to facilitate early > diff --git a/arch/x86/boot/cpucheck.c b/arch/x86/boot/cpucheck.c > index 4d3ff03..93ba160 100644 > --- a/arch/x86/boot/cpucheck.c > +++ b/arch/x86/boot/cpucheck.c > @@ -69,6 +69,13 @@ static int is_transmeta(void) > cpu_vendor[2] == A32('M', 'x', '8', '6'); > } > > +static int is_intel(void) > +{ > + return cpu_vendor[0] == A32('G', 'e', 'n', 'u') && > + cpu_vendor[1] == A32('i', 'n', 'e', 'I') && > + cpu_vendor[2] == A32('n', 't', 'e', 'l'); > +} > + > static int has_fpu(void) > { > u16 fcw = -1, fsw = -1; > @@ -239,6 +246,24 @@ int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr) > asm("wrmsr" : : "a" (eax), "d" (edx), "c" (ecx)); > > err = check_flags(); > + } else if (err == 0x01 && > + !(err_flags[0] & ~(1 << X86_FEATURE_PAE)) && > + is_intel() && cpu.level == 6 && > + (cpu.model == 9 || cpu.model == 13)) { > + /* PAE is disabled on this Pentium M but can be forced */ > + if (cmdline_find_option_bool("forcepae")) { > + puts("WARNING: Forcing PAE in CPU flags\n"); > + set_bit(X86_FEATURE_PAE, cpu.flags); > + err = check_flags(); This function is called check_cpuflags() now. You probably want to redo your patch against tip/master, i.e.: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git#master > + } > + else { > + puts("ERROR: PAE is disabled on this Pentium M\n" > + "(PAE can potentially be enabled with " > + "kernel parameter\n" > + "\"forcepae\" - this is unsupported, may " > + "cause unknown\n" > + "problems, and will taint the kernel)\n"); This string could definitely violate the 80 cols rule so that it is much more readable: } else puts("WARNING: PAE disabled. Use \"forcepae\" to enable at your own risk!\n"); I've shortened it to the most relevant info only. No need to say we're tainting the kernel because LOCKDEP_NOW_UNRELIABLE will cause that anyway below. > + } > } > > if (err_flags_ptr) > diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c > index bbe1b8b..271686d 100644 > --- a/arch/x86/kernel/cpu/intel.c > +++ b/arch/x86/kernel/cpu/intel.c > @@ -196,6 +196,14 @@ static void intel_smp_check(struct cpuinfo_x86 *c) > } > } > > +static int forcepae; > +static int __init forcepae_setup(char *__unused) > +{ > + forcepae = 1; > + return 1; > +} > +__setup("forcepae", forcepae_setup); Yeah, why not simply call it "pae"? It is smaller and the letter combination is not used yet and it means the same. > + > static void intel_workarounds(struct cpuinfo_x86 *c) > { > unsigned long lo, hi; > @@ -226,6 +234,17 @@ static void intel_workarounds(struct cpuinfo_x86 *c) > clear_cpu_cap(c, X86_FEATURE_SEP); > > /* > + * PAE CPUID issue: many Pentium M report no PAE but may have a > + * functionally usable PAE implementation. > + * Forcefully enable PAE if kernel parameter "forcepae" is present. > + */ > + if (forcepae) { > + printk(KERN_WARNING "PAE forced!\n"); > + set_cpu_cap(c, X86_FEATURE_PAE); > + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_NOW_UNRELIABLE); Right, this implies Dave's patch is preceding yours. I guess hpa can fish it out from the thread when applying. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --