Re: [RFC PATCH] LoongArch: mm: avoid backing vmemmap beyond max_pfn

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] loongarch/mm: leaf PMD dereferenced as PTE table in vmemmap_populate()
- [Medium] LoongArch: mm: vmemmap_populate() forces basepages for hotplugged memory above boot max_pfn
- [Medium] LoongArch: mm: unnecessary basepage vmemmap fallback in vmemmap_populate()
--

commit 6672ecaf8ad946ad85463c3a10b46da193101efe
Author: Wentao Guan <guanwentao@uniontech.com>

LoongArch: mm: avoid backing vmemmap beyond max_pfn

This commit changes vmemmap_populate() on LoongArch to fall back to
vmemmap_populate_basepages() when !altmap and the PMD covering start
extends beyond pfn_to_page(max_pfn), avoiding PMD hugepage mappings that
extend past max_pfn.

> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> index f801f7097f037..8c33744b93fae 100644
> --- a/arch/loongarch/mm/init.c
> +++ b/arch/loongarch/mm/init.c
> @@ -146,6 +146,11 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end,
>  #if CONFIG_PGTABLE_LEVELS == 2
>  	return vmemmap_populate_basepages(start, end, node, NULL);
>  #else
> +	unsigned long vmemmap_end = (unsigned long)pfn_to_page(max_pfn);

[Severity: Medium]
How does this check behave when memory is hot-added above initial boot DRAM?

On LoongArch, max_pfn is only initialized during early boot from
PFN_DOWN(memblock_end_of_DRAM()) in arch/loongarch/kernel/mem.c:

void __init memblock_init(void)
{
	...
	max_pfn = PFN_DOWN(memblock_end_of_DRAM());

and in arch/loongarch/kernel/setup.c:

static void __init arch_mem_init(char **cmdline_p)
{
	/* Recalculate max_low_pfn for "mem=xxx" */
	max_pfn = PFN_DOWN(memblock_end_of_DRAM());

When memory is hot-added at runtime via:

  arch_add_memory()
    __add_pages()
      sparse_add_section()
        section_activate()
          populate_section_memmap()
            __populate_section_memmap()
              vmemmap_populate()

or via memremap_pages() without an altmap at physical addresses above boot
DRAM (pfn >= max_pfn), max_pfn is never updated, so start >= vmemmap_end and
(!altmap && (start & PMD_MASK) + PMD_SIZE > vmemmap_end) is unconditionally
true.

Since get_order(PMD_SIZE) <= MAX_PAGE_ORDER on 64-bit LoongArch across 4KiB,
16KiB, and 64KiB page sizes, vmemmap_populate_hugepages() can allocate
PMD_SIZE blocks at runtime. Won't comparing against max_pfn unintentionally
force all non-altmap hotplugged memory above boot DRAM to fall back to
vmemmap_populate_basepages()?

[Severity: Medium]
Should vmemmap_end be aligned up to PAGES_PER_SECTION and gated on PMD_SIZE
exceeding the section vmemmap size?

During early boot, sparse_metadata_init_nid() in mm/sparse.c always populates
a full PAGES_PER_SECTION range for every present section:

	for_each_early_section_nr(start_section_nr, section_nr) {
		...
		mem_map = __populate_section_memmap(pfn, PAGES_PER_SECTION, nid,
						    NULL, NULL);

So falling back to vmemmap_populate_basepages(start, end, node, NULL) still
populates the entire section up to SECTION_ALIGN_UP(max_pfn). However, in
memblock_init() in arch/loongarch/kernel/mem.c, EFI_RESERVED_TYPE and
EFI_RUNTIME_SERVICES_* regions at the top of DRAM are only passed to
memblock_reserve() (not memblock_add()), or DRAM may be capped with mem=,
leaving max_pfn unaligned inside the final section of a PMD. If all 16
sections of a 32MiB PMD on a 16KiB page kernel are present, but max_pfn is
unaligned within the 16th section, (start & PMD_MASK) + PMD_SIZE > vmemmap_end
is true for all 16 sections even though the entire 32MiB PMD is populated,
allocating an extra PTE page table page and losing the PMD mapping while
saving zero vmemmap pages.

Additionally, on 4KiB page kernels (CONFIG_4KB_3LEVEL and CONFIG_4KB_4LEVEL),
SECTION_SIZE_BITS in arch/loongarch/include/asm/sparsemem.h is 29 (512MiB, or
131072 pages), so one section's vmemmap is 8MiB, spanning four 2MiB PMDs.
vmemmap_populate_hugepages() never maps past the section end on 4KiB page
kernels, yet if max_pfn falls within the first 2MiB PMD of the last section,
(start & PMD_MASK) + PMD_SIZE > vmemmap_end forces the entire 8MiB section to
use 4KiB base pages instead of four 2MiB PMDs.

> +
> +	if (!altmap && (start & PMD_MASK) + PMD_SIZE > vmemmap_end)
> +		return vmemmap_populate_basepages(start, end, node, NULL);
> +
>  	return vmemmap_populate_hugepages(start, end, node, NULL);

[Severity: High]
Can calling vmemmap_populate_basepages() here dereference a leaf huge PMD as a
PTE page table pointer and crash the kernel?

In arch/loongarch/mm/init.c, vmemmap_set_pmd() installs a leaf huge PMD
containing a shifted physical PFN and hardware page flags:

void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
			       unsigned long addr, unsigned long next)
{
	pmd_t entry;

	entry = pfn_pmd(virt_to_pfn(p), PAGE_KERNEL);
	pmd_val(entry) |= _PAGE_HUGE | _PAGE_HGLOBAL;
	set_pmd_at(&init_mm, addr, pmd, entry);
}

whereas a non-leaf PMD on LoongArch stores the kernel virtual address of the
PTE table (arch/loongarch/include/asm/pgtable.h):

static inline int pmd_none(pmd_t pmd)
{
	return pmd_val(pmd) == (unsigned long)invalid_pte_table;
}
...
#define pmd_page_vaddr(pmd)	pmd_val(pmd)

While vmemmap_populate_hugepages() checks vmemmap_check_pmd() before reusing
an existing PMD, vmemmap_populate_basepages() in mm/sparse-vmemmap.c calls
vmemmap_pmd_populate() and then vmemmap_pte_populate():

static pmd_t * __meminit vmemmap_pmd_populate(pud_t *pud, unsigned long addr, int node)
{
	pmd_t *pmd = pmd_offset(pud, addr);
	if (pmd_none(*pmd)) {
		...
	}
	return pmd;
}

static pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
		struct vmem_altmap *altmap)
{
	pte_t *pte = pte_offset_kernel(pmd, addr);
	unsigned long pfn = page_to_pfn((struct page *)addr);

	if (pte_none(ptep_get(pte))) {

If *pmd is already a leaf huge PMD, pmd_none(*pmd) is false, so
vmemmap_pmd_populate() returns the leaf PMD unchanged and
vmemmap_pte_populate() treats pmd_val(*pmd) as a kernel virtual address in
pte_offset_kernel() and dereferences it in ptep_get(pte).

Because vmemmap_populate() passes NULL for altmap to both helpers and
vmemmap_free() in arch/loongarch/mm/init.c is a no-op, a leaf huge PMD can
already be present when vmemmap_populate_basepages() runs in two ways:

1. In a PMD straddling or above max_pfn, populating a range with
   altmap != NULL (for example via memremap_pages() -> arch_add_memory() ->
   __add_pages() -> sparse_add_section()) skips the !altmap branch and calls
   vmemmap_populate_hugepages(start, end, node, NULL), which installs a leaf
   huge PMD covering the entire PMD (up to 16 sections on 16KiB pages). A
   subsequent population in the same PMD with altmap == NULL (via add_memory()
   or memremap_pages() without altmap, or after memunmap_pages() since
   vmemmap_free() is a no-op) then takes the vmemmap_populate_basepages() path
   on that existing leaf huge PMD.

2. On 4KiB page kernels, a 512MiB section's vmemmap is 8MiB (four 2MiB PMDs).
   When max_pfn falls in the 2nd, 3rd, or 4th PMD of the last section,
   (start & PMD_MASK) + PMD_SIZE > vmemmap_end only tests the first 2MiB PMD
   (which is <= vmemmap_end), so vmemmap_populate_hugepages() installs leaf
   huge PMDs for all four PMDs (also still mapping vmemmap beyond max_pfn in
   the trailing PMDs). If that section is later hot-removed (leaving the four
   leaf huge PMDs in place because vmemmap_free() is empty) and a range
   starting in one of the trailing PMDs above vmemmap_end is populated with
   altmap == NULL, (start & PMD_MASK) + PMD_SIZE > vmemmap_end is true and
   vmemmap_populate_basepages() runs against the existing leaf huge PMD.

>  #endif
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021062115.116922-1-guanwentao@uniontech.com?part=1