Re: [RFT PATCH v2 03/23] x86/boot: Drop global variables keeping track of LA57 state

Ingo Molnar
2025-05-05T21:25:02Z

* Ingo Molnar <mingo@kernel.org> wrote:

> Anyway, with these limitations in mind, we can see that the top 5
> usecases cover about 80% of all uses:
>
> - MAX_PHYSMEM_BITS: (inlined 179 times)
>
> arch/x86/include/asm/sparsemem.h:# define MAX_PHYSMEM_BITS (pgtable_l5_enabled() ? 52 : 46)
>
> This could be implemented via a precomputed, constant percpu value
> (per_cpu__x86_MAX_PHYSMEM_BITS) of 52 vs. 46, eliminating not just
> the CR4 access, but also a branch, at the cost of a percpu memory
> access. (Which should still be a win on all microarchitectures IMO.)
>
> Alternatively, since this value is a semi-constant of 52 vs. 46, we
> could also, I suspect, ALTERNATIVES-patch MAX_PHYSMEM_BITS in as an
> immediate constant value? Any reason this shouldn't work:
>
> static inline unsigned int __MAX_PHYSMEM_BITS(void)
> {
> unsigned int bits;
>
> asm_inline (ALTERNATIVE("movl $46, %0", "movl $52, %0", X86_FEATURE_LA57) :"=g" (bits));
>
> return bits;
> }
> #define MAX_PHYSMEM_BITS __MAX_PHYSMEM_BITS()
>
> ... or something like that? This would result in the best code
> generation IMO, by far. (It would even make use of the
> zero-extension property of a 32-bit MOVL, further compressing the
> opcode to only 5 bytes or so.)
>
> We'd even create a secondary helper macro for this, something like:
>
> #define ALTERNATIVES_CONST_U32(__val1, __val2, __feature) \
> ({ \
> u32 __val; \
> \
> asm_inline (ALTERNATIVE("movl $" #__val1 ", %0", "movl $" __val2 ", %0", __feature) :"=g" (__val)); \
> \
> __val; \
> })
>
> ...
>
> #define MAX_PHYSMEM_BITS ALTERNATIVE_CONST_U32(46, 52, X86_FEATURE_LA57)
>
> (Or so. Totally untested.)

BTW., I keep comparing it to the CR4 access, which is a bit unfair,
since that's only one out of the 3 variants of ALTERNATIVE_TERNARY():

static __always_inline __pure bool pgtable_l5_enabled(void)
{
unsigned long r;
bool ret;

if (!IS_ENABLED(CONFIG_X86_5LEVEL))
return false;

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;
}

The STC and CLC variants will probably be the more common outcomes on
modern CPUs.

But I still think the ALTERNATIVE_CONST_U32() approach I outline above
generates superior code for the binary-values cases, which covers 3 out
of the top 5 uses of pgtable_l5_enabled().

For non-constant branching uses of pgtable_l5_enabled() I suspect the
STC/CLC approach above is pretty good, although the 'cc' constraint
will clobber all flags I suspect, while ALTERNATIVE_CONST_U32()
doesn't? Ie. with ALTERNATIVE_CONST_U32() we just load the resulting
constant into a register, with no additional branches and with flags
undisturbed.

Also, is STC/CLC always just as fast as the testing of an immediate (or
a static branch), on CPUs we care about?

Thanks,

Ingo