Re: [PATCH v4] riscv: mm: Avoid spurious fault after hotplugging vmemmap

[Muchun Song <muchun.song@linux.dev>]

> On Jun 30, 2026, at 15:51, Vivian Wang <wangruikang@iscas.ac.cn> wrote:
> 
> section_activate() does not flush TLB after populating new vmemmap
> pages. On most architectures, this is okay. However it is a problem on
> RISC-V since there the TLB caching non-present entries is permitted,
> which causes spurious faults on some hardwares.
> 
> This seems to be most easily reproduced with DEBUG_VM=y and
> PAGE_POISONING=y, which causes these newly mapped struct pages to be
> poisoned i.e. written to immediately after mapping.
> 
> Add a hook vmemmap_populate_finalize() in __populate_section_memmap()
> after population, to allow architectures to handle such situations as
> needed. Then implement it on RISC-V to arrange for the existing
> exception handler code to deal with these faults if they happen.
> 
> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> ---
> Changes in v4:
> - Rebase on v7.2-rc1, drop dependencies
> - (No code changes otherwise)
> - (A concurrency fix for mark_new_valid_map was sent independently)
>  https://lore.kernel.org/linux-riscv/20260629-riscv-mm-new-valid-map-ordering-v1-1-60d8c10c6292@iscas.ac.cn/
> - Link to v3: https://patch.msgid.link/20260605-mark-after-vmemmap-populate-v3-1-a06001ac9264@iscas.ac.cn
> 
> Changes in v3:
> - Merged back into one patch (Mike)
> - (No code changes otherwise.)
> - Link to v2: https://patch.msgid.link/20260604-mark-after-vmemmap-populate-v2-0-ab6a7d03b434@iscas.ac.cn
> 
> Changes in v2:
> - Split patch in two, hook point and riscv hook 
> - Explain hook necessity in patch 1 message (Mike)
> - Make hook #define based (Mike)
> - Call finalize hook only on populate success
> - Link to v1: https://patch.msgid.link/20260525-mark-after-vmemmap-populate-v1-1-e698d859ba16@iscas.ac.cn
> ---
> arch/riscv/include/asm/pgtable.h | 4 ++++
> arch/riscv/mm/init.c             | 6 ++++++
> mm/sparse-vmemmap.c              | 8 ++++++++
> 3 files changed, 18 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 5d5756bda82e..6b000c990ba7 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -1253,6 +1253,10 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
> #define TASK_SIZE FIXADDR_START
> #endif
> 
> +/* Needed on SPARSEMEM_VMEMMAP */
> +#define vmemmap_populate_finalize vmemmap_populate_finalize
> +void __meminit vmemmap_populate_finalize(void);
> +
> #else /* CONFIG_MMU */
> 
> #define PAGE_SHARED __pgprot(0)
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 5b1b3c88b4d1..800cb5c007d1 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1372,6 +1372,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
> */
> 	return vmemmap_populate_hugepages(start, end, node, altmap);
> }
> +
> +void __meminit vmemmap_populate_finalize(void)
> +{
> + 	/* Avoid faults on cached non-present TLB entries. */
> + 	mark_new_valid_map();
> +}
> #endif
> 
> #if defined(CONFIG_MMU) && defined(CONFIG_64BIT)
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 99e2be39671b..290cafcfd723 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -544,6 +544,12 @@ static int __meminit vmemmap_populate_compound_pages(unsigned long start_pfn,
> 
> #endif
> 
> +#ifndef vmemmap_populate_finalize
> +static void __meminit vmemmap_populate_finalize(void)
> +{
> +}
> +#endif
> +
> struct page * __meminit __populate_section_memmap(unsigned long pfn,
> unsigned long nr_pages, int nid, struct vmem_altmap *altmap,
> struct dev_pagemap *pgmap)
> @@ -564,6 +570,8 @@ struct page * __meminit __populate_section_memmap(unsigned long pfn,
> 	if (r < 0)
> 		return NULL;
> 
> + 	vmemmap_populate_finalize();

Does the hook of flush_cache_vmap() work for you?  From the document of
cachetlb.rst, it said: 

	6) ``void flush_cache_vmap(unsigned long start, unsigned long end)``
	   ``void flush_cache_vunmap(unsigned long start, unsigned long end)``

		Here in these two interfaces we are flushing a specific range
		of (kernel) virtual addresses from the cache. After running,
		there will be no entries in the cache for the kernel address
		space for virtual addresses in the range 'start' to 'end-1'.

It seems that flush_cache_vmap() is supposed to be called after a new kernel
mapping has been installed, but before this address range is accessed.

Therefore, invoking flush_cache_vmap() here aligns perfectly with its interface
definition. But how would this change impact other architectures? Currently,
among architectures where CONFIG_SPARSEMEM=y, only RISC-V and PowerPC provide
a concrete implementation for flush_cache_vmap().

For PowerPC, the implementation is as follows:

	static inline void flush_cache_vmap(unsigned long start, unsigned long end)
	{
		asm volatile("ptesync" ::: "memory");
	}

Here, flush_cache_vmap() is also intended to supply the ptesync instruction
that the kernel address mapping lacks. However, flush_cache_vmap() is not
called in the __populate_section_memmap() path. So, how does PowerPC ensure
ptesync is executed there? The answer lies in the architecture-specific
vmemmap_set_pmd(), which injects the necessary ptesync.

If we choose to reuse flush_cache_vmap() in the generic path, it yields the
following benefits:

- Avoids architectural churn: We do not need to introduce a new hook
  like vmemmap_populate_finalize().
- Cleans up arch code: PowerPC can deprecate its custom vmemmap_set_pmd()
  and migrate to the generic implementation.

Let me know if this rationale makes sense, or if there are any hidden edge
cases I might have overlooked regarding other architectures.

Thanks,
Muchun

		
> +
> 	return pfn_to_page(pfn);
> }
> 
> 
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260525-mark-after-vmemmap-populate-68bd790839c9
> 
> Best regards,
> --  
> Vivian "dramforever" Wang
>