linux-kernel - Re: [PATCH] x86, setup: add __flush

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, 25 Sep 2014 22:32:44 -0700
From:	Dave Hansen <dave.hansen@...el.com>
To:	Ong Boon Leong <boon.leong.ong@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>
CC:	linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Bryan O'Donoghue <pure.logic@...us-software.ie>
Subject: Re: [PATCH] x86, setup: add __flush_tlb() for Intel Quark X1000

On 09/25/2014 09:30 PM, Ong Boon Leong wrote:
> Intel Quark X1000 advertises PGE (bit 13 of EDX) in CPUID.
> In reality, it does not implement PGE in current silicon.
> 
> This is an issue because __flush_tlb_all() depends on cpu_has_pge
> which has following dependency:
>  - cpu_has_pge --> boot_cpu_data --> new_cpu_data obtained from CPUID
>    in head_32.S.

Can I suggest a better description?

__flush_tlb_all() looks at the CPUID PGE bit.  If it finds it, the TLB
is flushed by clearing and then setting the PGE bit in CR4.  However,
since the Intel Quark X1000 does not _actually_ have PGE, this bit in
CR4 is ignored and the  attempt to flush the TLB does nothing.

Normally, we can fix up these kinds of CPUID mishaps in software.
However, this is earlier than that fixup code actually runs.

To work around this, we simply use an unconditional __flush_tlb() which
uses a cr3 write instead of the PGE bit twiddling in CR4.

We considered attempting to reorder the CPUID fixup code, but decided
that we were more likely to cause collateral damage if we tried this.
The cost of the extra TLB flush is minimal and is unlikely to break
anything else.

> As this is early stage of kernel boot-up and  adding
> __flush_tlb() is not going to hurt much in CPU cycles, we add it
> here and together with the explanation for future reference.
> 
> Signed-off-by: Ong Boon Leong <boon.leong.ong@...el.com>
> ---
>  arch/x86/kernel/setup.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 41ead8d..90e0b85 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -880,6 +880,13 @@ void __init setup_arch(char **cmdline_p)
>  
>  	load_cr3(swapper_pg_dir);
>  	__flush_tlb_all();
> +	/*
> +	 * Quark X1000 wrongly advertises PGE, add __flush_tlb()
> +	 * below to make sure TLB is flushed correctly in the early stage
> +	 * of setup_arch() for Quark X1000.
> +	 * X86_FEATURE_PGE flag is only setup later stage at early_cpu_init();
> +	 */
> +	__flush_tlb();

I'd just say:

Quark X1000 wrongly advertises PGE.  Work around this with an
unconditional full flush using a CR3 write instead of CR4.PGE:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/