linux-kernel - Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 10 Mar 2016 16:17:36 -0700
From:	Toshi Kani <toshi.kani@....com>
To:	Borislav Petkov <bp@...e.de>
Cc:	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Richard Purdie <richard.purdie@...uxfoundation.org>,
	Toshi Kani <toshi.kani@...com>,
	Bruce Ashfield <bruce.ashfield@...driver.com>,
	"Hart, Darren" <darren.hart@...el.com>,
	"saul.wold" <saul.wold@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
 disabled"

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
Download attachment "01-pat-disable" of type "message/rfc822" (4444 bytes)

Download attachment "02-mtrr" of type "message/rfc822" (2806 bytes)