Re: [PATCH v6 2/3] arm64/mm: add set_direct_map_ro_noflush()

[Xueyuan Chen]

On Wed, Aug 26, 2026 at 12:46 AM David Hildenbrand (Arm)
<david@xxxxxxxxxx> wrote:
>
> On 8/25/26 18:44, Will Deacon wrote:
> > On Tue, Aug 25, 2026 at 05:52:20PM +0200, David Hildenbrand (Arm) wrote:
> >> On 7/30/26 11:06, Xueyuan Chen wrote:
> >>> Implement set_direct_map_ro_noflush() for arm64 with update_range_prot() on
> >>> the linear map, setting PTE_RDONLY and clearing PTE_WRITE. Keep the
> >>> existing can_set_direct_map() guard and leave TLB invalidation to the
> >>> caller.
> >>>
> >>> Co-developed-by: Lance Yang <lance.yang@xxxxxxxxx>
> >>> Signed-off-by: Lance Yang <lance.yang@xxxxxxxxx>
> >>> Signed-off-by: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>
> >>> ---
> >>>  arch/arm64/include/asm/set_memory.h |  2 ++
> >>>  arch/arm64/mm/pageattr.c            | 10 ++++++++++
> >>>  2 files changed, 12 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/include/asm/set_memory.h b/arch/arm64/include/asm/set_memory.h
> >>> index 90f61b17275e..7083260303c3 100644
> >>> --- a/arch/arm64/include/asm/set_memory.h
> >>> +++ b/arch/arm64/include/asm/set_memory.h
> >>> @@ -14,6 +14,8 @@ int set_memory_valid(unsigned long addr, int numpages, int enable);
> >>>  int set_direct_map_invalid_noflush(struct page *page);
> >>>  int set_direct_map_default_noflush(struct page *page);
> >>>  int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid);
> >>> +int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages);
> >>> +#define set_direct_map_ro_noflush set_direct_map_ro_noflush
> >>>  bool kernel_page_present(struct page *page);
> >>>
> >>>  int set_memory_encrypted(unsigned long addr, int numpages);
> >>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> >>> index ce035e1b4eaf..c51236b61651 100644
> >>> --- a/arch/arm64/mm/pageattr.c
> >>> +++ b/arch/arm64/mm/pageattr.c
> >>> @@ -365,6 +365,16 @@ int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid)
> >>>     return set_memory_valid(addr, nr, valid);
> >>>  }
> >>>
> >>> +int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
> >>> +{
> >>> +   if (!can_set_direct_map())
> >>> +           return 0;
> >>> +
> >>> +   return update_range_prot((unsigned long)addr, PAGE_SIZE * nr_pages,
> >>> +                            __pgprot(PTE_RDONLY),
> >>> +                            __pgprot(PTE_WRITE));
> >>> +}
> >>> +
> >>>  #ifdef CONFIG_DEBUG_PAGEALLOC
> >>>  /*
> >>>   * This is - apart from the return value - doing the same
> >>
> >> We'll need an ACK from arm64 maintaines. To me this looks good.
> >
> > I generally dislike deferring critical things like TLB invalidation to
> > the caller, so it would be good to see an example of a caller that really
> > needs that. The caller in patch 1 unconditionally  calls
> > flush_tlb_kernel_range() immediately after calling
> > set_direct_map_ro_noflush().
> >
> > In fact, that means we have a pointless invalidation if
> > !can_set_direct_map(). See exhibit A :)
>
> Ack. I recall that the patch set originally didn't do any flushes, but we really
> have to flush even though we are early during boot.

Hi David,

Right, the original assumption was that the TLB might not hold a
writable mapping for these pages at all. But the folio is allocated
with __GFP_ZERO, so the allocation itself zeroes the 2 MiB through
the writable direct map and leaves writable TLB entries behind.
That's why skipping the flush is not safe even this early during
boot. (Usama Arif pointed this out.)

Thanks,
Xueyuan
>
> So agreed, the interface should just flush internally.
>
> --
> Cheers,
>
> David