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

[Ingo Molnar <mingo@kernel.org>]

* 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