> 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
