linux-kernel - Re: [PATCH v5 07/16] x86/startup_64: Simplify CR4 handling in startup code

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]

Message-ID: <CAMzpN2iu52yJWv3w1SgDXXGdvbfGi8cHRkip_qqrAO_89-4+sQ@mail.gmail.com>
Date: Wed, 21 Feb 2024 09:52:53 -0500
From: Brian Gerst <brgerst@...il.com>
To: Ard Biesheuvel <ardb+git@...gle.com>
Cc: linux-kernel@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>, 
	Kevin Loughlin <kevinloughlin@...gle.com>, Tom Lendacky <thomas.lendacky@....com>, 
	Dionna Glaze <dionnaglaze@...gle.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Andy Lutomirski <luto@...nel.org>, 
	Arnd Bergmann <arnd@...db.de>, Nathan Chancellor <nathan@...nel.org>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Kees Cook <keescook@...omium.org>, linux-arch@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v5 07/16] x86/startup_64: Simplify CR4 handling in startup code

On Wed, Feb 21, 2024 at 6:35 AM Ard Biesheuvel <ardb+git@...gle.com> wrote:
>
> From: Ard Biesheuvel <ardb@...nel.org>
>
> When paging is enabled, the CR4.PAE and CR4.LA57 control bits cannot be
> changed, and so they can simply be preserved rather than reason about
> whether or not they need to be set. CR4.MCE should be preserved unless
> the kernel was built without CONFIG_X86_MCE, in which case it must be
> cleared.
>
> CR4.PSE should be set explicitly, regardless of whether or not it was
> set before.
>
> CR4.PGE is set explicitly, and then cleared and set again after
> programming CR3 in order to flush TLB entries based on global
> translations. This makes the first assignment redundant, and can
> therefore be omitted.
>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---
>  arch/x86/kernel/head_64.S | 24 +++++++-------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index fb2a98c29094..426f6fdc0075 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -185,6 +185,8 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
>         addq    $(init_top_pgt - __START_KERNEL_map), %rax
>  1:
>
> +       /* Create a mask of CR4 bits to preserve */
> +       movl    $(X86_CR4_PAE | X86_CR4_LA57), %edx
>  #ifdef CONFIG_X86_MCE
>         /*
>          * Preserve CR4.MCE if the kernel will enable #MC support.
> @@ -193,20 +195,13 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
>          * configured will crash the system regardless of the CR4.MCE value set
>          * here.
>          */
> -       movq    %cr4, %rcx
> -       andl    $X86_CR4_MCE, %ecx
> -#else
> -       movl    $0, %ecx
> +       orl     $X86_CR4_MCE, %edx
>  #endif
> +       movq    %cr4, %rcx
> +       andl    %edx, %ecx
>
> -       /* Enable PAE mode, PSE, PGE and LA57 */
> -       orl     $(X86_CR4_PAE | X86_CR4_PSE | X86_CR4_PGE), %ecx
> -#ifdef CONFIG_X86_5LEVEL
> -       testb   $1, __pgtable_l5_enabled(%rip)
> -       jz      1f
> -       orl     $X86_CR4_LA57, %ecx
> -1:
> -#endif
> +       /* Even if ignored in long mode, set PSE uniformly on all logical CPUs. */
> +       btsl    $X86_CR4_PSE_BIT, %ecx
>         movq    %rcx, %cr4

This CR4 write now does the global flush - see below.

>
>         /* Setup early boot stage 4-/5-level pagetables. */
> @@ -226,11 +221,8 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
>          * Do a global TLB flush after the CR3 switch to make sure the TLB
>          * entries from the identity mapping are flushed.
>          */

This comment is misleading now since it's the first CR4 write above
(with PGE clear) that actually does the global flush.

> -       movq    %cr4, %rcx
> -       movq    %rcx, %rax
> -       xorq    $X86_CR4_PGE, %rcx
> +       btsl    $X86_CR4_PGE_BIT, %ecx
>         movq    %rcx, %cr4
> -       movq    %rax, %cr4
>
>         /* Ensure I am executing from virtual addresses */
>         movq    $1f, %rax
> --
> 2.44.0.rc0.258.g7320e95886-goog
>

Brian Gerst