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

Linus Torvalds
2025-05-05T21:26:57Z

On Mon, 5 May 2025 at 14:07, Ingo Molnar <mingo@kernel.org> wrote:
>
> - 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.)

This is literally why I did the "runtime-const" stuff. Exactly for
simple constants that you don't want to load from percpu memory
because it's just annoying.

Now, we only have 64-bit constant values which is very wasteful, and
we could just do a signed byte constant if we cared.

(We also have a "shift 32-bit value right by a constant amount",
which actually does use a signed byte, but it's masked by 0x1f because
that's how 32-bit shifts work).

I doubt we care - I doubt any of this MAX_PHYSMEM_BITS use is actually
performance-critical.

The runtime-const stuff would be trivial to use here if we really want to.

> - PGDIR_SHIFT: (inlined 156 times)

Several of those are actually of the form

#define PGDIR_SIZE (1UL << PGDIR_SHIFT)

so you artificially see PGDIR_SHIFT as the important part, even though
it's often a different constant entirely that just gets generated
using it.

> - p4d_offset(): (inlined 60 times)
> Here pgtable_l5_enabled() is used as a binary flag.

static branch would probably work best, and as Ard says, just using
cpu_feature_enabled() would just fix it..

> - pgd_none(): (inlined 49 times)
> Binary flag use as well, although the compiler might eliminate the
> branch here and replace it with 'AND !native_pgd_val(pgd)'

This could easily be done as runtime-const.

But again, I doubt it's all that performance-critical.

> - PTRS_PER_P4D: (inlined 46 times)
> This too could be implemented via a precomputed constant percpu
> value (per_cpu__x86_PTRS_PER_P4D), eliminating a branch,
> or via an ALTERNATIVE() immediate constant.

Again, we do have that, although the 64-bit constant is a bit wasteful.

The reason runtime-const does a 64-bit constant is that the actual
performance-critical cases were for big constants (TASK_SIZE) and for
pointers (hash table pointers).

Linus