On 7/22/26 08:30, Yeoreum Yun wrote:
> We want to rework how set_pXd() behaves for generic compile-time folded
Please move this all to imperative voice. I think I asked for this
before, but perhaps I didn't. Either way, please fix the whole series.
> page tables by disallowing its use and triggering a compile-time error
> when it is used improperly, ensuring that the actual first-level set_pXd()
> function is used instead.
>
> The behaviour of pXd_page() will change with generic compile-time folded
> page tables by disallowing its use and triggering a compile-time error
> when it's used improperly, ensuring that the actual.
>
> To prepare fot that, skip collapse_pud_page() and populate_pud() according
> to CONFIG_PGTABLE_LEVELS.
>
> There should be no functional change.
If this wasn't done, what would happen? There would be a compile error,
right?
Shouldn't that be said out loud somewhere?
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 078689aa7206..91b33a265a43 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -1346,7 +1346,7 @@ static int collapse_pud_page(pud_t *pud, unsigned long
> addr,
> pmd_t *pmd, first;
> int i;
>
> - if (!direct_gbpages)
> + if (CONFIG_PGTABLE_LEVELS == 2 || !direct_gbpages)
> return 0;
This is subtly wrong. It probably shuts the compiler up, but it's subtly
wrong.
x86 has 4 paging modes. I'll use the SDM/Intel terminology:
32-bit:
* 32-bit paging: 2-level
* PAE: 3-level
64-bit:
* 4-level paging
* 5-level paging
But "gbpages" are not available in PAE paging. It's a hardware
limitation. This doesn't cause a functional problem because the pud
level is not folded. But it is confusing and not really an accurate way
to write this check.
The *right* way to do this is probably something like this in a header:
static inline bool direct_gbpages(void)
{
if (!IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES))
return false;
return __direct_gbpages;
}
Which just happens to point readers over to this Kconfig nugget:
config X86_DIRECT_GBPAGES
def_bool y
depends on X86_64
which, combined with the knowledge that 32-bit has a 3-level mode, would
make the proposed check obviously wrong.
So I'll take this as a suitable alternative:
/* Avoid compiling the below code if PUD-level mappings are impossible:
*/
if (IS_ENABLED(CONFIG_X86_DIRECT_GBPAGES))
return 0;
if (!direct_gbpages)
return 0;
That will compile-time optimize the code away in the exact right
conditions *and* fix the (assumed by me) compile error that you were
chasing.
> @@ -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.
