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

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

> On Jul 6, 2026, at 13:04, Vivian Wang <wangruikang@iscas.ac.cn> wrote:
> 
> [ powerpc maintainers: Thoughts on replacing the ptesync in
> vmemmap_set_pmd() with a flush_cache_vmap() post-vmemmap-hotplug in the
> generic code? ]
> 
> On 7/6/26 11:00, Muchun Song wrote:
>>> 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.
> 
> Good point. I had initially avoided flush_cache_vmap() because, to be
> honest, it looked scary. But the combination of "have vmemmap" and "have
> flush_cache_vmap()" is much less scary.
> 
> I'd need to drop the address range check in RISC-V's flush_cache_vmap(),
> but might as well.

No. Please a new check for vmemmap address range.

> 
>> 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
> 
> At least on the face of it, it makes sense. The main thing is I don't
> know much about powerpc. So I'm also hoping powerpc maintainers could
> chime in. 

You do not need to know about powerpc since the change of powerpc is not
a part of this patch. It should be a separate code clean up for powerpc.

> 
> I also don't know how this would affect new archs, but if they need
> flush_cache_vmap() they *probably* also want this treatment for vmemmap
> hotplug? I guess we can deal with that when it happens.
> 
> Thanks,
> Vivian "dramforever" Wang