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

Brian Gerst
2024-02-21T14:53:06Z

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