On Mon, Mar 03, 2014 at 03:04:35PM +0700, Chris Bainbridge wrote:
> On 3 March 2014 02:05, Roland Kletzing <devz...@web.de> wrote:
> > i would recommend adding the newly introduced param to
> > Documentation/kernel-
> > parameters.txt , though.
> 
> Done.
> 
> Signed-off-by: Chris Bainbridge <chris.bainbri...@gmail.com>
> ---
> 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.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to