On 7/6/26 13:38, Muchun Song wrote:
>
>> On Jul 6, 2026, at 13:04, Vivian Wang <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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-60d8c10c6...@iscas.ac.cn/
>>>> - Link to v3:
>>>> https://patch.msgid.link/20260605-mark-after-vmemmap-populate-v3-1-a06001ac9...@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-ab6a7d03b...@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-e698d859b...@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.
Okay, I'll do that in v5.
>
>>> 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.
Ah, good point. I'll just use flush_cache_vmap() in v5, and, as you
said, the powerpc stuff can come later.
Thanks,
Vivian "dramforever" Wang
