[PATCH v6 0/3] mm: make persistent huge zero folio read-only

[PATCH v6 0/3] mm: make persistent huge zero folio read-only

[Xueyuan Chen]

The persistent huge zero folio is shared globally and should stay zero
after initialization. As Jann Horn pointed out[1], kernel bugs have ended
up writing to pages that were meant to be read-only, including in
security-sensitive cases. Making the folio read-only in the direct map
turns such writes into faults instead of silent zero-page corruption.

This series adds set_direct_map_ro_noflush() so mm code can make a
direct-map range read-only, then uses it for the persistent huge zero
folio. The helper is direct-map specific, takes an address-based range as
discussed for set_direct_map* helpers[2], and leaves TLB invalidation to
the caller.

The folio is allocated and zeroed through the writable direct map before
thp_shrinker_init() changes its permissions. thp_shrinker_init() is called
from hugepage_init(), which is registered as a subsys_initcall and runs
after SMP initialization. Writable TLB entries may therefore already be
cached when the page-table permissions change. Patch 1 flushes the exact
direct-map range immediately after the noflush page-table update. Keeping
the flush at the call site preserves the helper's explicit noflush contract
and follows existing direct-map helper users such as secretmem and
hibernation.

GFP_TRANSHUGE can allocate from high memory on 32-bit systems. Since a
highmem folio has no permanent direct-map mapping, patch 1 skips both the
permission change and TLB flush in that case.

Patches 2 and 3 add arm64 and x86 implementations.

Link: https://lore.kernel.org/linux-mm/20260508-ro-zeropage-v1-1-9808abc20b49@google.com/ [1]
Link: https://lore.kernel.org/linux-mm/0e5b23a6-4895-454a-9dfa-6dc21adc2991@kernel.org/ [2]
Link: https://lore.kernel.org/linux-mm/CAHbLzkrXXe7r3n3jXgDKtwZhRqj=jDx9E6dLOULohnhBguvi9A@mail.gmail.com/ [3]

v5 -> v6:
- Patch #01: Skip the direct-map permission change and TLB flush for
  highmem folios, which have no permanent direct-map mapping.
Link: https://lore.kernel.org/all/20260727143426.1077133-1-xueyuan.chen21@gmail.com/

RFC v4 -> v5:
- Drop the RFC tag.
- No code changes.
Link: https://lore.kernel.org/all/20260718095647.182592-1-xueyuan.chen21@gmail.com/

RFC v3 -> RFC v4:
- Patch #01: Flush the direct-map range after changing it read-only, since
  the folio was cleared through writable mappings after SMP initialization
  (per Usama, thanks!).
- Patch #01: Keep the flush in the caller to preserve the
  set_direct_map_ro_noflush() contract and make the flushed range explicit.
- Patch #01: Clarify the noflush API contract and the reason stale writable
  translations must be invalidated.
Link: https://lore.kernel.org/linux-mm/20260706130440.9295-1-xueyuan.chen21@gmail.com/

RFC v2 -> RFC v3:
- Patch #01: Replace arch_make_pages_readonly() with
  set_direct_map_ro_noflush() in the existing set_direct_map* family
  (per Mike and David, thanks!).
- Patch #01: Use a direct-map address and number of pages, and document the
  direct-map-only and no-TLB-flush semantics (per David, thanks!).
- Patch #02 and #03: Update the arm64 and x86 implementations for
  set_direct_map_ro_noflush().
Link: https://lore.kernel.org/linux-mm/20260609143801.7917-1-xueyuan.chen21@gmail.com/

RFC v1 -> RFC v2:
- Patch #01: Drop the READONLY_HUGE_ZERO_FOLIO Kconfig option
  (per Dave, thanks!).
- Patch #01: Replace the huge-zero-folio-specific hook with a generic
  page-range hook (per David, thanks!).
- Patch #02 and #03: Update the arm64 and x86 implementations for the new
  hook.
Link: https://lore.kernel.org/linux-mm/20260527035607.14919-1-xueyuan.chen21@gmail.com/

Xueyuan Chen (3):
  mm: make persistent huge zero folio read-only
  arm64/mm: add set_direct_map_ro_noflush()
  x86/mm: add set_direct_map_ro_noflush()

 arch/arm64/include/asm/set_memory.h |  2 ++
 arch/arm64/mm/pageattr.c            | 10 ++++++++++
 arch/x86/include/asm/set_memory.h   |  2 ++
 arch/x86/mm/pat/set_memory.c        | 15 +++++++++++++++
 include/linux/set_memory.h          | 29 +++++++++++++++++++++++++++++
 mm/huge_memory.c                    | 20 +++++++++++++++++++-
 6 files changed, 77 insertions(+), 1 deletion(-)

-- 
2.47.3

[PATCH v6 1/3] mm: make persistent huge zero folio read-only

[Xueyuan Chen]

The persistent huge zero folio is shared globally and should stay zero
after initialization. As Jann Horn pointed out[1], kernel bugs have ended
up writing to pages that were meant to be read-only, including in
security-sensitive cases. Making the persistent huge zero folio read-only
in the direct map turns such writes into faults instead of silent zero-page
corruption.

Add set_direct_map_ro_noflush() so mm code can make a direct-map range
read-only. Use an address-based signature to match ongoing direct-map
helper work[2], where existing page-based helpers may move the same way.
The helper is direct-map specific and leaves TLB invalidation to its
caller. Architectures without direct-map permission support keep existing
behavior through the generic stub.

The folio is allocated and zeroed through the writable direct map before
thp_shrinker_init() changes its permissions. thp_shrinker_init() is called
from hugepage_init(), which is registered as a subsys_initcall and runs
after SMP initialization. Stale writable kernel TLB entries may therefore
exist. Flush the direct-map range immediately after the page-table update
so they cannot bypass the read-only mapping.

GFP_TRANSHUGE includes __GFP_HIGHMEM. On 32-bit systems, the persistent
folio may therefore reside in high memory, where folio_address() returns
NULL. Such a folio has no permanent direct-map mapping to protect, so skip
the permission change and TLB flush.

Treat the direct-map permission change as best-effort. Architectures that
do not implement the helper keep the existing behavior via the generic
stub.

Inspired by Jann Horn's read-only zero page work[1] and follow-up
discussion[3] with Yang Shi.

Link: https://lore.kernel.org/r/20260508-ro-zeropage-v1-1-9808abc20b49@google.com [1]
Link: https://lore.kernel.org/r/0e5b23a6-4895-454a-9dfa-6dc21adc2991@kernel.org [2]
Link: https://lore.kernel.org/r/CAHbLzkrXXe7r3n3jXgDKtwZhRqj=jDx9E6dLOULohnhBguvi9A@mail.gmail.com [3]
Link: https://lore.kernel.org/r/20260727113530.5cd347581a66b3279490a604@linux-foundation.org
Suggested-by: David Hildenbrand <david@kernel.org>
Suggested-by: Usama Arif <usama.arif@linux.dev>
Co-developed-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
---
 include/linux/set_memory.h | 29 +++++++++++++++++++++++++++++
 mm/huge_memory.c           | 20 +++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
index 3030d9245f5a..e83ced6a3827 100644
--- a/include/linux/set_memory.h
+++ b/include/linux/set_memory.h
@@ -40,6 +40,24 @@ static inline int set_direct_map_valid_noflush(struct page *page,
 	return 0;
 }
 
+/**
+ * set_direct_map_ro_noflush - make a direct-map range read-only
+ * @addr: start address in the direct map
+ * @nr_pages: number of pages starting at @addr
+ *
+ * Make the direct-map range starting at @addr read-only without invalidating
+ * TLBs. Callers must either ensure that no stale writable translations can
+ * be used, or treat the permission change as a best-effort hardening step.
+ *
+ * Return: 0 on success or when direct-map permission changes are unsupported,
+ * or a negative errno on failure.
+ */
+static inline int set_direct_map_ro_noflush(const void *addr,
+					    unsigned long nr_pages)
+{
+	return 0;
+}
+
 static inline bool kernel_page_present(struct page *page)
 {
 	return true;
@@ -56,6 +74,17 @@ static inline bool can_set_direct_map(void)
 }
 #define can_set_direct_map can_set_direct_map
 #endif
+
+#ifndef set_direct_map_ro_noflush
+/* See the comment above the generic fallback for the _noflush contract. */
+static inline int set_direct_map_ro_noflush(const void *addr,
+					    unsigned long nr_pages)
+{
+	return 0;
+}
+
+#define set_direct_map_ro_noflush set_direct_map_ro_noflush
+#endif
 #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */
 
 #ifdef CONFIG_X86_64
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 970e077019b7..2dfc7a4218c3 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -40,8 +40,10 @@
 #include <linux/pgalloc.h>
 #include <linux/pgalloc_tag.h>
 #include <linux/pagewalk.h>
+#include <linux/set_memory.h>
 
 #include <asm/tlb.h>
+#include <asm/tlbflush.h>
 #include "internal.h"
 #include "swap.h"
 
@@ -932,6 +934,8 @@ static int __init thp_shrinker_init(void)
 	shrinker_register(deferred_split_shrinker);
 
 	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO)) {
+		unsigned long addr;
+
 		/*
 		 * Bump the reference of the huge_zero_folio and do not
 		 * initialize the shrinker.
@@ -940,8 +944,22 @@ static int __init thp_shrinker_init(void)
 		 * that get_huge_zero_folio() will most likely not fail as
 		 * thp_shrinker_init() is invoked early on during boot.
 		 */
-		if (!get_huge_zero_folio())
+		if (!get_huge_zero_folio()) {
 			pr_warn("Allocating persistent huge zero folio failed\n");
+			return 0;
+		}
+
+		/* Highmem folios have no permanent direct-map mapping to protect. */
+		if (folio_test_highmem(huge_zero_folio))
+			return 0;
+
+		addr = (unsigned long)folio_address(huge_zero_folio);
+		/*
+		 * The folio was zeroed through the writable direct map. Flush
+		 * after the page-table update to invalidate stale translations.
+		 */
+		set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);
+		flush_tlb_kernel_range(addr, addr + HPAGE_PMD_SIZE);
 		return 0;
 	}
 
-- 
2.47.3

Re: [PATCH v6 1/3] mm: make persistent huge zero folio read-only

[David Hildenbrand (Arm)]

On 7/30/26 11:06, Xueyuan Chen wrote:
> The persistent huge zero folio is shared globally and should stay zero
> after initialization. As Jann Horn pointed out[1], kernel bugs have ended
> up writing to pages that were meant to be read-only, including in
> security-sensitive cases. Making the persistent huge zero folio read-only
> in the direct map turns such writes into faults instead of silent zero-page
> corruption.
> 
> Add set_direct_map_ro_noflush() so mm code can make a direct-map range
> read-only. Use an address-based signature to match ongoing direct-map
> helper work[2], where existing page-based helpers may move the same way.
> The helper is direct-map specific and leaves TLB invalidation to its
> caller. Architectures without direct-map permission support keep existing
> behavior through the generic stub.
> 
> The folio is allocated and zeroed through the writable direct map before
> thp_shrinker_init() changes its permissions. thp_shrinker_init() is called
> from hugepage_init(), which is registered as a subsys_initcall and runs
> after SMP initialization. Stale writable kernel TLB entries may therefore
> exist. Flush the direct-map range immediately after the page-table update
> so they cannot bypass the read-only mapping.
> 
> GFP_TRANSHUGE includes __GFP_HIGHMEM. On 32-bit systems, the persistent
> folio may therefore reside in high memory, where folio_address() returns
> NULL. Such a folio has no permanent direct-map mapping to protect, so skip
> the permission change and TLB flush.
> 
> Treat the direct-map permission change as best-effort. Architectures that
> do not implement the helper keep the existing behavior via the generic
> stub.
> 
> Inspired by Jann Horn's read-only zero page work[1] and follow-up
> discussion[3] with Yang Shi.
> 
> Link: https://lore.kernel.org/r/20260508-ro-zeropage-v1-1-9808abc20b49@google.com [1]
> Link: https://lore.kernel.org/r/0e5b23a6-4895-454a-9dfa-6dc21adc2991@kernel.org [2]
> Link: https://lore.kernel.org/r/CAHbLzkrXXe7r3n3jXgDKtwZhRqj=jDx9E6dLOULohnhBguvi9A@mail.gmail.com [3]
> Link: https://lore.kernel.org/r/20260727113530.5cd347581a66b3279490a604@linux-foundation.org
> Suggested-by: David Hildenbrand <david@kernel.org>
> Suggested-by: Usama Arif <usama.arif@linux.dev>
> Co-developed-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  include/linux/set_memory.h | 29 +++++++++++++++++++++++++++++
>  mm/huge_memory.c           | 20 +++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
> index 3030d9245f5a..e83ced6a3827 100644
> --- a/include/linux/set_memory.h
> +++ b/include/linux/set_memory.h
> @@ -40,6 +40,24 @@ static inline int set_direct_map_valid_noflush(struct page *page,
>  	return 0;
>  }
>  
> +/**
> + * set_direct_map_ro_noflush - make a direct-map range read-only
> + * @addr: start address in the direct map
> + * @nr_pages: number of pages starting at @addr
> + *
> + * Make the direct-map range starting at @addr read-only without invalidating
> + * TLBs. Callers must either ensure that no stale writable translations can
> + * be used, or treat the permission change as a best-effort hardening step.
> + *
> + * Return: 0 on success or when direct-map permission changes are unsupported,
> + * or a negative errno on failure.
> + */
> +static inline int set_direct_map_ro_noflush(const void *addr,
> +					    unsigned long nr_pages)

Two tabs here in MM land.

> +{
> +	return 0;
> +}

Nobody checks the return value, so how helpful is it to return it? Do we expect
other users to check for the return code?

I guess we just try to keep consistency with the other functions?

[...]


> @@ -932,6 +934,8 @@ static int __init thp_shrinker_init(void)
>  	shrinker_register(deferred_split_shrinker);
>  
>  	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO)) {
> +		unsigned long addr;
> +
>  		/*
>  		 * Bump the reference of the huge_zero_folio and do not
>  		 * initialize the shrinker.
> @@ -940,8 +944,22 @@ static int __init thp_shrinker_init(void)
>  		 * that get_huge_zero_folio() will most likely not fail as
>  		 * thp_shrinker_init() is invoked early on during boot.
>  		 */
> -		if (!get_huge_zero_folio())
> +		if (!get_huge_zero_folio()) {
>  			pr_warn("Allocating persistent huge zero folio failed\n");
> +			return 0;
> +		}
> +
> +		/* Highmem folios have no permanent direct-map mapping to protect. */
> +		if (folio_test_highmem(huge_zero_folio))
> +			return 0;
> +
> +		addr = (unsigned long)folio_address(huge_zero_folio);
> +		/*
> +		 * The folio was zeroed through the writable direct map. Flush
> +		 * after the page-table update to invalidate stale translations.
> +		 */
> +		set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);
> +		flush_tlb_kernel_range(addr, addr + HPAGE_PMD_SIZE);
>  		return 0;
>  	}
>  

The code changed upstream in the meantime. There is now the huge_zero_init() we
can hook into instead.

-- 
Cheers,

David

Re: [PATCH v6 1/3] mm: make persistent huge zero folio read-only

[Dave Hansen]

On 7/30/26 02:06, Xueyuan Chen wrote:
> + * Return: 0 on success or when direct-map permission changes are unsupported,
> + * or a negative errno on failure.
> + */
> +static inline int set_direct_map_ro_noflush(const void *addr,
> +					    unsigned long nr_pages)
> +{
> +	return 0;
> +}

I think this is probably not the best API. First, it isn't consistent
with the other set_direct_map*() functions. Second, it's unusable for
things that matter. This would be a buggy function:

int this_must_succeed_for_security(unsigned long addr)
{
	return set_direct_map_ro_noflush(addr, 1);
}

... despite looking just fine.

The best thing, IMNHO, is to return an actual error when the stub gets
used and have the callers deal with it (or not deal with it if they
don't care).

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

[Xueyuan Chen]

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@linux.dev>
Signed-off-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
---
 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
-- 
2.47.3

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

[David Hildenbrand (Arm)]

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@linux.dev>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  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.

-- 
Cheers,

David

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

[Will Deacon]

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@linux.dev>
> > Signed-off-by: Lance Yang <lance.yang@linux.dev>
> > Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> > ---
> >  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 :)

Will

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

[David Hildenbrand (Arm)]

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@linux.dev>
>>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
>>> Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
>>> ---
>>>  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.

So agreed, the interface should just flush internally.

-- 
Cheers,

David

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@kernel.org> 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@linux.dev>
> >>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> >>> Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> >>> ---
> >>>  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

[PATCH v6 3/3] x86/mm: add set_direct_map_ro_noflush()

[Xueyuan Chen]

Implement set_direct_map_ro_noflush() for x86 using CPA directly on the
passed direct-map address. Clear _PAGE_RW and _PAGE_DIRTY, keep alias
checks disabled like the existing direct-map _noflush helpers, and leave
TLB invalidation to the caller.

Co-developed-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
---
 arch/x86/include/asm/set_memory.h |  2 ++
 arch/x86/mm/pat/set_memory.c      | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 4362c26aa992..bd3817e06052 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -89,6 +89,8 @@ int set_pages_rw(struct page *page, int numpages);
 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);
 
 extern int kernel_set_to_readonly;
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index d023a40a1e03..5987f4c84f6f 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2662,6 +2662,21 @@ int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid)
 	return __set_pages_np(page, nr);
 }
 
+int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
+{
+	unsigned long tempaddr = (unsigned long)addr;
+	struct cpa_data cpa = {
+		.vaddr = &tempaddr,
+		.pgd = NULL,
+		.numpages = nr_pages,
+		.mask_set = __pgprot(0),
+		.mask_clr = __pgprot(_PAGE_RW | _PAGE_DIRTY),
+		.flags = CPA_NO_CHECK_ALIAS,
+	};
+
+	return __change_page_attr_set_clr(&cpa, 1);
+}
+
 #ifdef CONFIG_DEBUG_PAGEALLOC
 void __kernel_map_pages(struct page *page, int numpages, int enable)
 {
-- 
2.47.3

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

[David Hildenbrand (Arm)]

On 7/30/26 11:06, Xueyuan Chen wrote:
> Implement set_direct_map_ro_noflush() for x86 using CPA directly on the
> passed direct-map address. Clear _PAGE_RW and _PAGE_DIRTY, keep alias
> checks disabled like the existing direct-map _noflush helpers, and leave
> TLB invalidation to the caller.
> 
> Co-developed-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> ---
>  arch/x86/include/asm/set_memory.h |  2 ++
>  arch/x86/mm/pat/set_memory.c      | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
> index 4362c26aa992..bd3817e06052 100644
> --- a/arch/x86/include/asm/set_memory.h
> +++ b/arch/x86/include/asm/set_memory.h
> @@ -89,6 +89,8 @@ int set_pages_rw(struct page *page, int numpages);
>  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);
>  
>  extern int kernel_set_to_readonly;
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index d023a40a1e03..5987f4c84f6f 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -2662,6 +2662,21 @@ int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid)
>  	return __set_pages_np(page, nr);
>  }
>  
> +int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
> +{
> +	unsigned long tempaddr = (unsigned long)addr;
> +	struct cpa_data cpa = {
> +		.vaddr = &tempaddr,
> +		.pgd = NULL,
> +		.numpages = nr_pages,
> +		.mask_set = __pgprot(0),
> +		.mask_clr = __pgprot(_PAGE_RW | _PAGE_DIRTY),
> +		.flags = CPA_NO_CHECK_ALIAS,
> +	};
> +
> +	return __change_page_attr_set_clr(&cpa, 1);
> +}
> +
>  #ifdef CONFIG_DEBUG_PAGEALLOC
>  void __kernel_map_pages(struct page *page, int numpages, int enable)
>  {

We need an ack from x86 folks, to me this looks good.

-- 
Cheers,

David

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

[Dave Hansen]

On 7/30/26 02:06, Xueyuan Chen wrote:
> +int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
> +{
> +	unsigned long tempaddr = (unsigned long)addr;
> +	struct cpa_data cpa = {
> +		.vaddr = &tempaddr,
> +		.pgd = NULL,
> +		.numpages = nr_pages,
> +		.mask_set = __pgprot(0),
> +		.mask_clr = __pgprot(_PAGE_RW | _PAGE_DIRTY),
> +		.flags = CPA_NO_CHECK_ALIAS,
> +	};
> +
> +	return __change_page_attr_set_clr(&cpa, 1);
> +}

A couple of concerns here.

First, why the "_noflush"? Sure, the "this is a best effort hardening"
function argument can be made, so it doesn't need to be correct. But the
result is a function that's called once and also has some sharp corners
and relatively high potential for misuse. Let's just do the flush.

Second, I see that the other set_direct_map*() callers use
CPA_NO_CHECK_ALIAS. The reasoning behind it dates back to 2008 and I'm
not 100% sure what it is referring to. On one hand, it would be nice to
have all the set_direct_map*() callers be consistent. On the other hand,
there shouldn't *be* any aliases of a 2M page that came out of the page
allocator. We almost want a CPA_ASSERT_NO_ALIASES that goes out and
checks for aliases more than we want to ignore them. (Note: I don't
expect you to fix this, but a simple comment saying that no aliases are
expected would be nice)

Third, what's with the 'tempaddr'? Are you working around the 'const'?
Honestly, I'd rather have no const than have it and subvert it with
casting trickery.

Last:

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, ...
int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)

Which one of these things is not like the other, despite being named
just like them?

Imagine the fun if someone did:

	set_direct_map_ro_noflush(page, 1);
and
	set_direct_map_default_noflush(page)

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

[David Hildenbrand (Arm)]

On 8/25/26 18:18, Dave Hansen wrote:
> On 7/30/26 02:06, Xueyuan Chen wrote:
>> +int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
>> +{
>> +	unsigned long tempaddr = (unsigned long)addr;
>> +	struct cpa_data cpa = {
>> +		.vaddr = &tempaddr,
>> +		.pgd = NULL,
>> +		.numpages = nr_pages,
>> +		.mask_set = __pgprot(0),
>> +		.mask_clr = __pgprot(_PAGE_RW | _PAGE_DIRTY),
>> +		.flags = CPA_NO_CHECK_ALIAS,
>> +	};
>> +
>> +	return __change_page_attr_set_clr(&cpa, 1);
>> +}
> 
> A couple of concerns here.
> 
> First, why the "_noflush"? Sure, the "this is a best effort hardening"
> function argument can be made, so it doesn't need to be correct. But the
> result is a function that's called once and also has some sharp corners
> and relatively high potential for misuse. Let's just do the flush.
> 
> Second, I see that the other set_direct_map*() callers use
> CPA_NO_CHECK_ALIAS. The reasoning behind it dates back to 2008 and I'm
> not 100% sure what it is referring to. On one hand, it would be nice to
> have all the set_direct_map*() callers be consistent. On the other hand,
> there shouldn't *be* any aliases of a 2M page that came out of the page
> allocator. We almost want a CPA_ASSERT_NO_ALIASES that goes out and
> checks for aliases more than we want to ignore them. (Note: I don't
> expect you to fix this, but a simple comment saying that no aliases are
> expected would be nice)
> 
> Third, what's with the 'tempaddr'? Are you working around the 'const'?
> Honestly, I'd rather have no const than have it and subvert it with
> casting trickery.
> 
> Last:
> 
> 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, ...
> int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
> 
> Which one of these things is not like the other, despite being named
> just like them?
> 

That's called out in the cover letter:

"
This series adds set_direct_map_ro_noflush() so mm code can make a
direct-map range read-only, then uses it for the persistent huge zero
folio. The helper is direct-map specific, takes an address-based range as
discussed for set_direct_map* helpers[2], and leaves TLB invalidation to
the caller.
"

and patch #1

"
Use an address-based signature to match ongoing direct-map
helper work[2], where existing page-based helpers may move the same way.
The helper is direct-map specific and leaves TLB invalidation to its
caller. Architectures without direct-map permission support keep existing
behavior through the generic stub.
"

Currently it looks like this series would go in first, though, so it would be
better to keep the existing style.

-- 
Cheers,

David

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

[Dave Hansen]

On 8/25/26 09:43, David Hildenbrand (Arm) wrote:
>> 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, ...
>> int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
>>
>> Which one of these things is not like the other, despite being named
>> just like them?
>>
> That's called out in the cover letter:
> 
> "
> This series adds set_direct_map_ro_noflush() so mm code can make a
> direct-map range read-only, then uses it for the persistent huge zero
> folio. The helper is direct-map specific, takes an address-based range as
> discussed for set_direct_map* helpers[2], and leaves TLB invalidation to
> the caller.
> "

My concern is not so much what the function is doing or whether or how
the specific function is documented. It's more about whether the new
function is consistent across all functions with a similar purpose and
name. Also, if it is _not_ consistent there needs to be reasoning behind
the inconsistency. I think that is missing here.

In this case, look at the call site:

	addr = (unsigned long)folio_address(huge_zero_folio);
	set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);

It *has* a folio. But it does a folio_address() and two casts to massage
it into the type for set_direct_map_ro_noflush().

If set_direct_map_ro_noflush() just took a 'struct page *', there would
be one folio=>page conversion, no casting, and complete consistency with
the other set_direct_map*() functions.

I'd also be OK with set_direct_map_ro_noflush() taking a folio, with the
implication being that it might eventually make sense to convert the
other set_direct_map*() functions to folios. But page vs. folio
confusion is much less likely to cause bugs than a void* versus another
pointer.

IOW, what I think I want is:

int set_direct_map_ro_noflush(struct page *page, unsigned long nr_pages)

Or _maybe_:

int set_direct_map_ro_noflush(struct folio *folio)

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

[David Hildenbrand (Arm)]

On 8/25/26 18:57, Dave Hansen wrote:
> On 8/25/26 09:43, David Hildenbrand (Arm) wrote:
>>> 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, ...
>>> int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
>>>
>>> Which one of these things is not like the other, despite being named
>>> just like them?
>>>
>> That's called out in the cover letter:
>>
>> "
>> This series adds set_direct_map_ro_noflush() so mm code can make a
>> direct-map range read-only, then uses it for the persistent huge zero
>> folio. The helper is direct-map specific, takes an address-based range as
>> discussed for set_direct_map* helpers[2], and leaves TLB invalidation to
>> the caller.
>> "
> 
> My concern is not so much what the function is doing or whether or how
> the specific function is documented. It's more about whether the new
> function is consistent across all functions with a similar purpose and
> name. Also, if it is _not_ consistent there needs to be reasoning behind
> the inconsistency. I think that is missing here.

Yes, the intend from the submitter was to prepare for the interface change to
keep them consistent.

I agree that for now it should just consume pages instead of an address and keep
the interface consistent in this series.

> 
> In this case, look at the call site:
> 
> 	addr = (unsigned long)folio_address(huge_zero_folio);
> 	set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);
> 
> It *has* a folio. But it does a folio_address() and two casts to massage
> it into the type for set_direct_map_ro_noflush().
> 
> If set_direct_map_ro_noflush() just took a 'struct page *', there would
> be one folio=>page conversion, no casting, and complete consistency with
> the other set_direct_map*() functions.

Again, I agree with that, and the idea was to keep the interface consistent as
it gets converted; it's just that the series that does the conversion stalled.

> 
> I'd also be OK with set_direct_map_ro_noflush() taking a folio, with the
> implication being that it might eventually make sense to convert the
> other set_direct_map*() functions to folios. But page vs. folio
> confusion is much less likely to cause bugs than a void* versus another
> pointer.
> 
> IOW, what I think I want is:
> 
> int set_direct_map_ro_noflush(struct page *page, unsigned long nr_pages)

Yes, or as will said

	int set_direct_map_ro(struct page *page, unsigned long nr_pages)

performing the flush internally.

> 
> Or _maybe_:
> 
> int set_direct_map_ro_noflush(struct folio *folio)

I think I raised it before, but using folios on this interface is not a good
idea. Primarily because

a) Once we decouple struct folio from struct page this interface would only be
available for folios and ...

b) ... the huge zero page (currently huge zero folio) is on of the examples that
*currently* is a folio but very likely won't be a folio in the future. Only
anon+pagecache that really need mapcounts and all that will be folios.

c) I don't expect any further real folio users (IOW, anonymous folios or
pagecache folios), but could imagine non-folio users.

-- 
Cheers,

David

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

[Xueyuan Chen]

Hi David, Dave,

Thanks for the review and the discussion.

I'll switch the helper to a page-based signature and do the flush
internally:

    int set_direct_map_ro(struct page *page, unsigned long nr_pages)

I originally chose the address-based signature to prepare for the
interface conversion, but since that work stalled, I'll keep the
helper consistent with the other set_direct_map*() helpers.

The remaining comments will be addressed in v7, which
will be based on the latest mm-unstable.

Thanks,
Xueyuan

On Wed, Aug 26, 2026 at 1:31 AM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> On 8/25/26 18:57, Dave Hansen wrote:
> > On 8/25/26 09:43, David Hildenbrand (Arm) wrote:
> >>> 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, ...
> >>> int set_direct_map_ro_noflush(const void *addr, unsigned long nr_pages)
> >>>
> >>> Which one of these things is not like the other, despite being named
> >>> just like them?
> >>>
> >> That's called out in the cover letter:
> >>
> >> "
> >> This series adds set_direct_map_ro_noflush() so mm code can make a
> >> direct-map range read-only, then uses it for the persistent huge zero
> >> folio. The helper is direct-map specific, takes an address-based range as
> >> discussed for set_direct_map* helpers[2], and leaves TLB invalidation to
> >> the caller.
> >> "
> >
> > My concern is not so much what the function is doing or whether or how
> > the specific function is documented. It's more about whether the new
> > function is consistent across all functions with a similar purpose and
> > name. Also, if it is _not_ consistent there needs to be reasoning behind
> > the inconsistency. I think that is missing here.
>
> Yes, the intend from the submitter was to prepare for the interface change to
> keep them consistent.
>
> I agree that for now it should just consume pages instead of an address and keep
> the interface consistent in this series.
>
> >
> > In this case, look at the call site:
> >
> >       addr = (unsigned long)folio_address(huge_zero_folio);
> >       set_direct_map_ro_noflush((void *)addr, HPAGE_PMD_NR);
> >
> > It *has* a folio. But it does a folio_address() and two casts to massage
> > it into the type for set_direct_map_ro_noflush().
> >
> > If set_direct_map_ro_noflush() just took a 'struct page *', there would
> > be one folio=>page conversion, no casting, and complete consistency with
> > the other set_direct_map*() functions.
>
> Again, I agree with that, and the idea was to keep the interface consistent as
> it gets converted; it's just that the series that does the conversion stalled.
>
> >
> > I'd also be OK with set_direct_map_ro_noflush() taking a folio, with the
> > implication being that it might eventually make sense to convert the
> > other set_direct_map*() functions to folios. But page vs. folio
> > confusion is much less likely to cause bugs than a void* versus another
> > pointer.
> >
> > IOW, what I think I want is:
> >
> > int set_direct_map_ro_noflush(struct page *page, unsigned long nr_pages)
>
> Yes, or as will said
>
>         int set_direct_map_ro(struct page *page, unsigned long nr_pages)
>
> performing the flush internally.
>
> >
> > Or _maybe_:
> >
> > int set_direct_map_ro_noflush(struct folio *folio)
>
> I think I raised it before, but using folios on this interface is not a good
> idea. Primarily because
>
> a) Once we decouple struct folio from struct page this interface would only be
> available for folios and ...
>
> b) ... the huge zero page (currently huge zero folio) is on of the examples that
> *currently* is a folio but very likely won't be a folio in the future. Only
> anon+pagecache that really need mapcounts and all that will be folios.
>
> c) I don't expect any further real folio users (IOW, anonymous folios or
> pagecache folios), but could imagine non-folio users.
>
> --
> Cheers,
>
> David