Messages in this thread | | | Date | Thu, 11 Apr 2024 15:39:39 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v2] x86/boot/64: Clear CR4.PGE to disable global 1:1 mappings |
| |
* Ard Biesheuvel <ardb+git@google.com> wrote:
> From: Ard Biesheuvel <ardb@kernel.org> > > The early 64-bit boot code must be entered with a 1:1 mapping of the > bootable image, but it cannot operate without a 1:1 mapping of all the > assets in memory that it accesses, and therefore, it creates such > mappings for all known assets upfront, and additional ones on demand > when a page fault happens on a memory address. > > These mappings are created with the global bit G set, as the flags used > to create page table descriptors are based on __PAGE_KERNEL_LARGE_EXEC > defined by the core kernel, even though the context where these mappings > are used is very different. > > This means that the TLB maintenance carried out by the decompressor is > not sufficient if it is entered with CR4.PGE enabled, which has been > observed to happen with the stage0 bootloader of project Oak. While this > is a dubious practice if no global mappings are being used to begin > with, the decompressor is clearly at fault here for creating global > mappings and not performing the appropriate TLB maintenance. > > Since commit > > f97b67a773cd84b ("x86/decompressor: Only call the trampoline when changing paging levels") > > CR4 is no longer modified by the decompressor if no change in the number > of paging levels is needed. Before that, CR4 would always be set to a > consistent value with PGE cleared. > > So let's reinstate a simplified version of the original logic to put CR4 > into a known state, and preserve the PAE, MCE and LA57 bits, none of > which can be modified freely at this point (PAE and LA57 cannot be > changed while running in long mode, and MCE cannot be cleared when > running under some hypervisors) > > Cc: Conrad Grobler <grobler@google.com> > Cc: Kevin Loughlin <kevinloughlin@google.com> > Fixes: f97b67a773cd84b ("x86/decompressor: Only call the trampoline when ...") > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > v2: Bring back the original logic rather than only clearing PGE. Note > that this means that the load of CR4 cannot be elided since MCE and > LA57 cannot be set unconditionally. > > arch/x86/boot/compressed/head_64.S | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S > index bf4a10a5794f..d0f2c591f730 100644 > --- a/arch/x86/boot/compressed/head_64.S > +++ b/arch/x86/boot/compressed/head_64.S > @@ -398,6 +398,11 @@ SYM_CODE_START(startup_64) > call sev_enable > #endif > > + /* Preserve only the CR4 bits that must be preserved, and clear the rest */ > + movq %cr4, %rax > + andl $(X86_CR4_PAE | X86_CR4_MCE | X86_CR4_LA57), %eax > + movq %rax, %cr4
Yeah, this is still better IMO than the original patch.
Note that you reused the original title, which isn't entirely accurate anymore:
x86/boot/64: Clear CR4.PGE to disable global 1:1 mappings
I updated it to:
x86/boot/64: Clear most of CR4 in startup_64(), except PAE, MCE and LA57
Thanks,
Ingo
| |