Messages in this thread | | | From | Ard Biesheuvel <> | Date | Fri, 9 Feb 2024 13:55:02 +0000 | Subject | Re: [PATCH v3 06/19] x86/startup_64: Drop global variables keeping track of LA57 state |
| |
On Wed, 7 Feb 2024 at 13:29, Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Jan 29, 2024 at 07:05:09PM +0100, Ard Biesheuvel wrote: > > static inline bool pgtable_l5_enabled(void) > > { > > return __pgtable_l5_enabled; > > } > > #else > > -#define pgtable_l5_enabled() cpu_feature_enabled(X86_FEATURE_LA57) > > +#define pgtable_l5_enabled() !!(native_read_cr4() & X86_CR4_LA57) > > #endif /* USE_EARLY_PGTABLE_L5 */ > > Can we drop this ifdeffery and simply have __pgtable_l5_enabled always > present and contain the correct value? >
I was trying to get rid of global variable assignments and accesses from the 1:1 mapping, but since we cannot get rid of those entirely, we might just keep __pgtable_l5_enabled but use RIP_REL_REF() in the accessors, and move the assignment to the asm startup code.
> So that we don't have an expensive CR4 read hidden in > pgtable_l5_enabled()? >
Yeah, I didn't realize it was expensive. Alternatively, we might do something like
static __always_inline bool pgtable_l5_enabled(void) { unsigned long r; bool ret;
asm(ALTERNATIVE_TERNARY( "movq %%cr4, %[reg] \n\t btl %[la57], %k[reg]" CC_SET(c), %P[feat], "stc", "clc") : [reg] "=r" (r), CC_OUT(c) (ret) : [feat] "i" (X86_FEATURE_LA57), [la57] "i" (X86_CR4_LA57_BIT) : "cc"); return ret; }
but we'd still have two versions in that case.
> For the sake of simplicity, pgtable_l5_enabled() can be defined outside > of CONFIG_X86_5LEVEL and since both vendors support 5level now, might as > well start dropping the CONFIG ifdeffery slowly... > > Other than that - a nice cleanup! >
Thanks.
| |