Re: [PATCH RFC v2 14/20] x86: mm: skip pud setup when using generic compile-time folded pagetable

[Yeoreum Yun <yeoreum.yun@arm.com>]

> On 7/22/26 10:18, Yeoreum Yun wrote:
> >>> @@ -1730,7 +1730,8 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, p4d_t *p4d,
> >>>  	/*
> >>>  	 * Map everything starting from the Gb boundary, possibly with 1G pages
> >>>  	 */
> >>> -	while (boot_cpu_has(X86_FEATURE_GBPAGES) && end - start >= PUD_SIZE) {
> >>> +	while (CONFIG_PGTABLE_LEVELS > 3 && boot_cpu_has(X86_FEATURE_GBPAGES) &&
> >>> +	       end - start >= PUD_SIZE) {
> >>>  		set_pud(pud, pud_mkhuge(pfn_pud(cpa->pfn,
> >>>  				   canon_pgprot(pud_pgprot))));
> >> This is an OK approach. But there's a way to fix this site *and*
> >> optimize a non-zero amount of other code at the same time. Add this hunk
> >> to arch/x86/Kconfig.cpufeatures:
> >>
> >> config X86_DISABLED_FEATURE_GBPAGES
> >>         def_bool y
> >>         depends on X86_32
> >>
> >> That will turn the boot_cpu_has() check in to something that can be
> >> resolved at compile time. It has the added advantage of compiling out
> >> all of the code under X86_FEATURE_GBPAGES everywhere else in the tree.
> > Does it? when I glimpse check, this wouldn't be compiled since
> > there is no bit for X86_DISABLED_FEATURE_GBPAGES and defining the
> > DISABLED bit for FEATURE_GBPAGES seems odd since bit X86_FEATURE_GBPAGES
> > is already defined.
> 
> x86 is a special snowflake here and all the similarly-named things glued
> together with magic makes them hard to grok.
> 
> Our X86_FEATURE_* bits normally compile down to a bit in a bitmap in
> memory. But, there are also some optimizations in the helpers that
> access those bits. The optimizations turn bit checks like:
> 
> 	if (bitmap[N] & bit)
> 		...
> 
> into a compile-time check:
> 
> 	if (__builtin_constant_p(bit) && DISABLED_MASK_BIT_SET(bit) ? 0:
> 	    (bitmap[N] & bit))
> 		...
> 
> The DISABLED_MASK_BIT_SET() macro magic check if (for instance)
> X86_DISABLED_FEATURE_GBPAGES is around. Some processing of the Kconfig
> variables produces a header with X86_DISABLED_FEATURE_GBPAGES defined.

Sorry. But do you mean check with cpu_feature_enabled() not with
boot_cpu_has()?
IIRC, the boot_cpu_has() doesn't use the DISABLE_MASK_BIT_SET() but
uses REQUIRED_MASK_BIT_SET() and I don't expect it wouldn't make
an optimisation to constant check which is contrast to DISABLE_MASK_BIT_SET().

> > Instead of CONFIG_PGTABLE_LEVEL > 3, as above, would it be better to
> > add check IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES)?
> 
> That would definitely change the behavior. It honestly might be the
> _right_ change, but I'm ignoring that for the moment because even if it
> is best it is fodder for another patch, not this one.
> 
> Please just keep the check against X86_FEATURE_GBPAGES for now.

Okay. In round, we can remove CONFIG_PGTABLE_LEVEL check too since
it wouldn't make a compile error right now.

-- 
Sincerely,
Yeoreum Yun