public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Wang YanQing <udknight@gmail.com>
To: linux@armlinux.org.uk
Cc: akpm@linux-foundation.org, willy@infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2] arm: lpae: fix non-atomic page table entry update issue
Date: Sun, 15 Mar 2026 08:47:46 +0800	[thread overview]
Message-ID: <20260315004746.GA32062@udknight> (raw)

The ARM Architecture Reference Manual explicitly dictates that writes of 64-bit
translation table descriptors must be single-copy atomic:
ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition (https://developer.arm.com/documentation/ddi0406/latest)
"
...
A3.5.3 Atomicity in the ARM architecture
...
In an implementation that includes the Large Physical Address Extension, LDRD, and STRD accesses to 64-bit aligned
locations are 64-bit single-copy atomic as seen by translation table walks and accesses to translation tables.
Note
The Large Physical Address Extension adds this requirement to avoid the need for complex measures to avoid
atomicity issues when changing translation table entries, without creating a requirement that all locations in the
memory system are 64-bit single-copy atomic. This addition means:
•The system designer must ensure that all writable memory locations that might be used to hold translations,
such as bulk SDRAM, can be accessed with 64-bit single-copy atomicity.
•Software must ensure that translation tables are not held in memory locations that cannot meet this atomicity
requirement, such as peripherals that are typically accessed using a narrow bus.
This requirement places no burden on read-only memory locations for which reads have no side effects, since it is
impossible to detect the size of memory accesses to such locations.
...
"

ARM Architecture Reference Manual for A-profile architecture (https://developer.arm.com/documentation/ddi0487/latest)
"
...
E2.2.1 Requirements for single-copy atomicity
In AArch32 state, the single-copy atomic PE accesses are:
•All byte accesses.
•All halfword accesses to halfword-aligned locations.
•All word accesses to word-aligned locations.
•Memory accesses caused by LDREXD and STREXD instructions to doubleword-aligned locations.
LDM, LDC, LDRD, STM, STC, STRD, PUSH, POP, RFE, SRS, VLDM, VLDR, VSTM, and VSTR instructions are executed as a
sequence of word-aligned word accesses. Each 32-bit word access is guaranteed to be single-copy atomic. The
architecture does not require subsequences of two or more word accesses from the sequence to be single-copy atomic.
LDRD and STRD accesses to 64-bit aligned locations are 64-bit single-copy atomic as seen by translation table walks and
accesses to translation tables.
...
"

To archieve this 64-bit atomicity on a 32-bit architecture, the linux kernel relies on the STRD (Store Register Dual)
instruction, but the copy_pmd() in pgtable-3level.h is C code, then compiler could do very crazy optimization for it
and generate code that break the atomicity, for example, we get below copy_pmd() assembly code with gcc 12.4.0
(Using CC_OPTIMIZE_FOR_PERFORMANCE, it is the default compile option):
"
gdb vmlinux
gdb disassemble do_translation_fault
gdb ...
gdb 0xc020e544 <+136>:    ldr.w    r4, [r0, r1, lsl #3] @load low 32-bit of pmdps
gdb 0xc020e548 <+140>:    ldr      r0, [r6, #4]         @load high 32-bit of pmdps
gdb 0xc020e54a <+142>:    orrs.w   r6, r4, r0           @ pmd_none(pmd_k[index])
gdb 0xc020e54e <+146>:    beq.n    0xc020e586 <do_translation_fault+202>
gdb ...
gdb 0xc020e562 <+166>:    str.w    r4, [r5, r1, lsl #3] @store low 32-bit to pmdpd
gdb 0xc020e566 <+170>:    str      r0, [r2, #4]         @store hight 32-bit to pmdpd

The code breaks the atomicity and valid bit is in the low 32-bit, page table walker
could see and cache the partial write entry, this will cause very strange
translation-related issues when next page table (level3 PTE table) physical address
is larger than 32-bits.

So let's use WRITE_ONCE() to protect the page table entry update functions from crazy
optimization.

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 Changes v1-v2:
 1: Add documentation reference in changelog, suggested by Russell King

 arch/arm/include/asm/pgtable-3level.h | 28 +++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
index 7b71a3d414b7..b077174a4231 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -120,15 +120,15 @@
 						 PMD_TYPE_SECT)
 #define pmd_leaf(pmd)		pmd_sect(pmd)
 
-#define pud_clear(pudp)			\
-	do {				\
-		*pudp = __pud(0);	\
-		clean_pmd_entry(pudp);	\
+#define pud_clear(pudp)				\
+	do {					\
+		WRITE_ONCE(*pudp, __pud(0));	\
+		clean_pmd_entry(pudp);		\
 	} while (0)
 
 #define set_pud(pudp, pud)		\
 	do {				\
-		*pudp = pud;		\
+		WRITE_ONCE(*pudp, pud);	\
 		flush_pmd_entry(pudp);	\
 	} while (0)
 
@@ -139,16 +139,16 @@ static inline pmd_t *pud_pgtable(pud_t pud)
 
 #define pmd_bad(pmd)		(!(pmd_val(pmd) & PMD_TABLE_BIT))
 
-#define copy_pmd(pmdpd,pmdps)		\
-	do {				\
-		*pmdpd = *pmdps;	\
-		flush_pmd_entry(pmdpd);	\
+#define copy_pmd(pmdpd, pmdps)				\
+	do {						\
+		WRITE_ONCE(*pmdpd, READ_ONCE(*pmdps));	\
+		flush_pmd_entry(pmdpd);			\
 	} while (0)
 
-#define pmd_clear(pmdp)			\
-	do {				\
-		*pmdp = __pmd(0);	\
-		clean_pmd_entry(pmdp);	\
+#define pmd_clear(pmdp)				\
+	do {					\
+		WRITE_ONCE(*pmdp, __pmd(0));	\
+		clean_pmd_entry(pmdp);		\
 	} while (0)
 
 /*
@@ -241,7 +241,7 @@ static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,
 	else
 		pmd_val(pmd) |= PMD_SECT_AP2;
 
-	*pmdp = __pmd(pmd_val(pmd) | PMD_SECT_nG);
+	WRITE_ONCE(*pmdp,  __pmd(pmd_val(pmd) | PMD_SECT_nG));
 	flush_pmd_entry(pmdp);
 }
 
-- 
2.34.1


             reply	other threads:[~2026-03-15  0:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-15  0:47 Wang YanQing [this message]
2026-03-15  1:12 ` [PATCH v2] arm: lpae: fix non-atomic page table entry update issue Russell King (Oracle)
2026-03-15  3:29   ` Wang YanQing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260315004746.GA32062@udknight \
    --to=udknight@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox