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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incomplete `CONFIG_PGTABLE_LEVELS` check in `collapse_pud_page()` fails to skip x86 3-level PAE configurations, exposing them to latent compile-time errors and potential runtime hardware exceptions (RSVD #PF).

Pre-existing issues:
- [High] In `populate_pud()`, an error path during the 'trailing leftover' phase silently masks resource allocation failures, skipping necessary cleanup and causing a system crash (`BUG_ON`) instead of gracefully propagating the error.
--

--- Patch [14]: [PATCH RFC v2 14/20] x86: mm: skip pud setup when using generic compile-time folded pagetable ---
commit 163a3c6228e774c7dcf45c2630763c72ef0ffe3a
Author: Yeoreum Yun <yeoreum.yun@arm.com>

x86: mm: skip pud setup when using generic compile-time folded pagetable

We want to rework how set_pXd() behaves for generic compile-time folded
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.

> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 078689aa7206f..91b33a265a431 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)

[Severity: High]
Does this check skip 3-level PAE configurations correctly?

If a user boots a 32-bit PAE kernel on a CPU with 1GB page support and
gbpages is enabled, direct_gbpages becomes 1. collapse_pud_page() would then
execute on a folded PUD (PDPTE in PAE) and write a PDPTE entry with the
_PAGE_PSE (bit 7) flag copied from the PMD.

In PAE, bit 7 of the PDPTE is strictly reserved, and setting it causes a
hardware exception (kernel panic).

Should this condition be checking if CONFIG_PGTABLE_LEVELS <= 3 to match
the logic added to populate_pud() below?

>  		return 0;
>  
>  	addr &= PUD_MASK;

[ ... ]

> @@ -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))));
>  

[Severity: High]
This is a pre-existing issue, but further down in populate_pud() under the
"Map trailing leftover" section, does the error handler silently mask
allocation failures?

arch/x86/mm/pat/set_memory.c:populate_pud() {
    ...
		tmp = populate_pmd(cpa, start, end, cpa->numpages - cur_pages,
				   pud, pgprot);
		if (tmp < 0)
			return cur_pages;
    ...
}

If populate_pmd() fails (e.g., due to OOM) while mapping trailing leftovers,
it returns a negative error code in tmp. Returning cur_pages (which can be 0)
instead of the error code causes populate_pgd() to assume success.

This skips unmap_pud_range() cleanup, and returns 0 to __change_page_attr().
The top-level __change_page_attr_set_clr() loop updates cpa->numpages = 0
and hits BUG_ON(!cpa->numpages), causing a kernel panic.

Should this return tmp instead to properly propagate the error code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dummy_ptxp3-v2-0-d9e4bad31e0a@arm.com?part=14