On Thu, 2016-03-10 at 22:07 +0100, Borislav Petkov wrote:
> On Thu, Mar 10, 2016 at 01:24:11PM -0700, Toshi Kani wrote:
> > I am not familiar with PPRO_FEATURES,
> 
> That's the feature bits of the "qemu32" model, and others, in qemu.
> 
> > but shouldn't 'flags' in /proc/cpuinfo show "pat" when X86_FEATURE_PAT
> > is set?
> 
> static void early_init_intel(struct cpuinfo_x86 *c)
> ...
> 
>         /*
>          * There is a known erratum on Pentium III and Core Solo
>          * and Core Duo CPUs.
>          * " Page with PAT set to WC while associated MTRR is UC
>          *   may consolidate to UC "
>          * Because of this erratum, it is better to stick with
>          * setting WC in MTRR rather than using PAT on these CPUs.
>          *
>          * Enable PAT WC only on P4, Core 2 or later CPUs.
>          */
>         if (c->x86 == 6 && c->x86_model < 15)
>                 clear_cpu_cap(c, X86_FEATURE_PAT);
> ---
> 
> which also gives a hint as to how we should fix this: pat_enabled()
> needs to look at that feature bit too:

I see.  I will take a look.

> ---
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index faec01e7a17d..359c30d9a78c 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -56,7 +56,7 @@ early_param("nopat", nopat);
>  
>  bool pat_enabled(void)
>  {
> -	return !!__pat_enabled;
> +	return !!__pat_enabled && static_cpu_has(X86_FEATURE_PAT);
>  }
>  EXPORT_SYMBOL_GPL(pat_enabled);
> ---
> 
> Makes sense?

Yes, I agree that pat_enable() needs to check the PAT feature bit.  In some
reason, static_cpu_has(X86_FEATURE_PAT) returns 0 while cpu_has_pat returns
1 in my testing...  I need to check this.

> > pat_init() is being called as part of MTRR setup because PAT
> > initialization requires the same CPU rendezvous operation implemented
> > in the MTRR code.
> 
> ... which means, PAT depends on MTRR being present.

Yes, and we need more changes to handle this dependency since MTRR does not
call pat_init() when it is disabled.

Attached is the changes I am working now.  I will include your changes, and
send them out once I finished testing.  Let me know if you have any
suggestion.

Thanks,
-Toshi