2020-10-10 23:16:05

by Arvind Sankar

[permalink] [raw]
Subject: Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> Commit
> ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> started using a new set of pagetables even without KASLR.
>
> After that commit, initialize_identity_maps() is called before the
> 5-level paging variables are setup in choose_random_location(), which
> will not work if 5-level paging is actually enabled.

Note that I don't have hardware that supports 5-level paging, so this
is not actually tested with 5-level, but based on code inspection, it
shouldn't work.

>
> Fix this by moving the initialization of __pgtable_l5_enabled,
> pgdir_shift and ptrs_per_p4d into cleanup_trampoline(), which is called
> immediately after the finalization of whether the kernel is executing
> with 4- or 5-level paging. This will be earlier than anything that might
> require those variables, and keeps the 4- vs 5-level paging code all in
> one place.
>
> Signed-off-by: Arvind Sankar <[email protected]>
> ---
> arch/x86/boot/compressed/ident_map_64.c | 6 ------
> arch/x86/boot/compressed/kaslr.c | 8 --------
> arch/x86/boot/compressed/pgtable_64.c | 16 ++++++++++++++++
> 3 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
> index a3613857c532..505d6299b76e 100644
> --- a/arch/x86/boot/compressed/ident_map_64.c
> +++ b/arch/x86/boot/compressed/ident_map_64.c
> @@ -34,12 +34,6 @@
> #define __PAGE_OFFSET __PAGE_OFFSET_BASE
> #include "../../mm/ident_map.c"
>
> -#ifdef CONFIG_X86_5LEVEL
> -unsigned int __pgtable_l5_enabled;
> -unsigned int pgdir_shift = 39;
> -unsigned int ptrs_per_p4d = 1;
> -#endif
> -
> /* Used by PAGE_KERN* macros: */
> pteval_t __default_kernel_pte_mask __read_mostly = ~0;
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 489fabc051d7..9eabd8bc7673 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -835,14 +835,6 @@ void choose_random_location(unsigned long input,
> return;
> }
>
> -#ifdef CONFIG_X86_5LEVEL
> - if (__read_cr4() & X86_CR4_LA57) {
> - __pgtable_l5_enabled = 1;
> - pgdir_shift = 48;
> - ptrs_per_p4d = 512;
> - }
> -#endif
> -
> boot_params->hdr.loadflags |= KASLR_FLAG;
>
> if (IS_ENABLED(CONFIG_X86_32))
> diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> index 46d761f7faa6..0976c2d2ab2f 100644
> --- a/arch/x86/boot/compressed/pgtable_64.c
> +++ b/arch/x86/boot/compressed/pgtable_64.c
> @@ -9,6 +9,13 @@
> #define BIOS_START_MIN 0x20000U /* 128K, less than this is insane */
> #define BIOS_START_MAX 0x9f000U /* 640K, absolute maximum */
>
> +#ifdef CONFIG_X86_5LEVEL
> +/* __pgtable_l5_enabled needs to be in .data to avoid being cleared along with .bss */
> +unsigned int __section(.data) __pgtable_l5_enabled;
> +unsigned int __section(.data) pgdir_shift = 39;
> +unsigned int __section(.data) ptrs_per_p4d = 1;
> +#endif
> +
> struct paging_config {
> unsigned long trampoline_start;
> unsigned long l5_required;
> @@ -195,4 +202,13 @@ void cleanup_trampoline(void *pgtable)
>
> /* Restore trampoline memory */
> memcpy(trampoline_32bit, trampoline_save, TRAMPOLINE_32BIT_SIZE);
> +
> + /* Initialize variables for 5-level paging */
> +#ifdef CONFIG_X86_5LEVEL
> + if (__read_cr4() & X86_CR4_LA57) {
> + __pgtable_l5_enabled = 1;
> + pgdir_shift = 48;
> + ptrs_per_p4d = 512;
> + }
> +#endif
> }
> --
> 2.26.2
>


2020-10-12 14:11:55

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

On Sat, Oct 10, 2020 at 03:26:24PM -0400, Arvind Sankar wrote:
> On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> > Commit
> > ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> > started using a new set of pagetables even without KASLR.
> >
> > After that commit, initialize_identity_maps() is called before the
> > 5-level paging variables are setup in choose_random_location(), which
> > will not work if 5-level paging is actually enabled.
>
> Note that I don't have hardware that supports 5-level paging, so this
> is not actually tested with 5-level, but based on code inspection, it
> shouldn't work.

qemu supports it. -cpu "qemu64,+la57"

--
Kirill A. Shutemov

2020-10-13 06:32:38

by Arvind Sankar

[permalink] [raw]
Subject: Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

On Mon, Oct 12, 2020 at 05:08:30PM +0300, Kirill A. Shutemov wrote:
> On Sat, Oct 10, 2020 at 03:26:24PM -0400, Arvind Sankar wrote:
> > On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> > > Commit
> > > ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> > > started using a new set of pagetables even without KASLR.
> > >
> > > After that commit, initialize_identity_maps() is called before the
> > > 5-level paging variables are setup in choose_random_location(), which
> > > will not work if 5-level paging is actually enabled.
> >
> > Note that I don't have hardware that supports 5-level paging, so this
> > is not actually tested with 5-level, but based on code inspection, it
> > shouldn't work.
>
> qemu supports it. -cpu "qemu64,+la57"
>
> --
> Kirill A. Shutemov

Thanks! On QEMU, it does crash without this patch.

2020-10-13 18:24:54

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

On Mon, Oct 12, 2020 at 11:35:01AM -0400, Arvind Sankar wrote:
> > qemu supports it. -cpu "qemu64,+la57"
>
> Thanks! On QEMU, it does crash without this patch.

Works fine here. I gave this to qemu:

-cpu qemu64,+la57,vendor=GenuineIntel

it said:

qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:ECX.la57 [bit 16]

because host is AMD, probably, and I have

CONFIG_X86_5LEVEL=y

enabled for the guest kernel and tip:x86/seves booted fine.

What am I missing?

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette