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

[Brian Gerst]

On Wed, Feb 21, 2024 at 6:35 AM Ard Biesheuvel <ardb+git@xxxxxxxxxx> wrote:
>
> From: Ard Biesheuvel <ardb@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
>  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