Skip to content

Commit 20a004e

Browse files
wildea01ctmarinas
wildea01
authored and
ctmarinas
committed
arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables
In many cases, page tables can be accessed concurrently by either another CPU (due to things like fast gup) or by the hardware page table walker itself, which may set access/dirty bits. In such cases, it is important to use READ_ONCE/WRITE_ONCE when accessing page table entries so that entries cannot be torn, merged or subject to apparent loss of coherence due to compiler transformations. Whilst there are some scenarios where this cannot happen (e.g. pinned kernel mappings for the linear region), the overhead of using READ_ONCE /WRITE_ONCE everywhere is minimal and makes the code an awful lot easier to reason about. This patch consistently uses these macros in the arch code, as well as explicitly namespacing pointers to page table entries from the entries themselves by using adopting a 'p' suffix for the former (as is sometimes used elsewhere in the kernel source). Tested-by: Yury Norov <ynorov@caviumnetworks.com> Tested-by: Richard Ruigrok <rruigrok@codeaurora.org> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> Signed-off-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
1 parent 2ce77f6 commit 20a004e

13 files changed

Lines changed: 426 additions & 399 deletions

File tree

‎arch/arm64/include/asm/hugetlb.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
static inline pte_t huge_ptep_get(pte_t *ptep)
2424
{
25-
return *ptep;
25+
return READ_ONCE(*ptep);
2626
}
2727

2828

‎arch/arm64/include/asm/kvm_mmu.h‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,42 +185,42 @@ static inline pmd_t kvm_s2pmd_mkexec(pmd_t pmd)
185185
return pmd;
186186
}
187187

188-
static inline void kvm_set_s2pte_readonly(pte_t *pte)
188+
static inline void kvm_set_s2pte_readonly(pte_t *ptep)
189189
{
190190
pteval_t old_pteval, pteval;
191191

192-
pteval = READ_ONCE(pte_val(*pte));
192+
pteval = READ_ONCE(pte_val(*ptep));
193193
do {
194194
old_pteval = pteval;
195195
pteval &= ~PTE_S2_RDWR;
196196
pteval |= PTE_S2_RDONLY;
197-
pteval = cmpxchg_relaxed(&pte_val(*pte), old_pteval, pteval);
197+
pteval = cmpxchg_relaxed(&pte_val(*ptep), old_pteval, pteval);
198198
} while (pteval != old_pteval);
199199
}
200200

201-
static inline bool kvm_s2pte_readonly(pte_t *pte)
201+
static inline bool kvm_s2pte_readonly(pte_t *ptep)
202202
{
203-
return (pte_val(*pte) & PTE_S2_RDWR) == PTE_S2_RDONLY;
203+
return (READ_ONCE(pte_val(*ptep)) & PTE_S2_RDWR) == PTE_S2_RDONLY;
204204
}
205205

206-
static inline bool kvm_s2pte_exec(pte_t *pte)
206+
static inline bool kvm_s2pte_exec(pte_t *ptep)
207207
{
208-
return !(pte_val(*pte) & PTE_S2_XN);
208+
return !(READ_ONCE(pte_val(*ptep)) & PTE_S2_XN);
209209
}
210210

211-
static inline void kvm_set_s2pmd_readonly(pmd_t *pmd)
211+
static inline void kvm_set_s2pmd_readonly(pmd_t *pmdp)
212212
{
213-
kvm_set_s2pte_readonly((pte_t *)pmd);
213+
kvm_set_s2pte_readonly((pte_t *)pmdp);
214214
}
215215

216-
static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
216+
static inline bool kvm_s2pmd_readonly(pmd_t *pmdp)
217217
{
218-
return kvm_s2pte_readonly((pte_t *)pmd);
218+
return kvm_s2pte_readonly((pte_t *)pmdp);
219219
}
220220

221-
static inline bool kvm_s2pmd_exec(pmd_t *pmd)
221+
static inline bool kvm_s2pmd_exec(pmd_t *pmdp)
222222
{
223-
return !(pmd_val(*pmd) & PMD_S2_XN);
223+
return !(READ_ONCE(pmd_val(*pmdp)) & PMD_S2_XN);
224224
}
225225

226226
static inline bool kvm_page_empty(void *ptr)

‎arch/arm64/include/asm/mmu_context.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,13 @@ static inline void cpu_install_idmap(void)
141141
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
142142
* avoiding the possibility of conflicting TLB entries being allocated.
143143
*/
144-
static inline void cpu_replace_ttbr1(pgd_t *pgd)
144+
static inline void cpu_replace_ttbr1(pgd_t *pgdp)
145145
{
146146
typedef void (ttbr_replace_func)(phys_addr_t);
147147
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
148148
ttbr_replace_func *replace_phys;
149149

150-
phys_addr_t pgd_phys = virt_to_phys(pgd);
150+
phys_addr_t pgd_phys = virt_to_phys(pgdp);
151151

152152
replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
153153

‎arch/arm64/include/asm/pgalloc.h‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,23 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
3636
return (pmd_t *)__get_free_page(PGALLOC_GFP);
3737
}
3838

39-
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
39+
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
4040
{
41-
BUG_ON((unsigned long)pmd & (PAGE_SIZE-1));
42-
free_page((unsigned long)pmd);
41+
BUG_ON((unsigned long)pmdp & (PAGE_SIZE-1));
42+
free_page((unsigned long)pmdp);
4343
}
4444

45-
static inline void __pud_populate(pud_t *pud, phys_addr_t pmd, pudval_t prot)
45+
static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
4646
{
47-
set_pud(pud, __pud(__phys_to_pud_val(pmd) | prot));
47+
set_pud(pudp, __pud(__phys_to_pud_val(pmdp) | prot));
4848
}
4949

50-
static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
50+
static inline void pud_populate(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp)
5151
{
52-
__pud_populate(pud, __pa(pmd), PMD_TYPE_TABLE);
52+
__pud_populate(pudp, __pa(pmdp), PMD_TYPE_TABLE);
5353
}
5454
#else
55-
static inline void __pud_populate(pud_t *pud, phys_addr_t pmd, pudval_t prot)
55+
static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
5656
{
5757
BUILD_BUG();
5858
}
@@ -65,30 +65,30 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
6565
return (pud_t *)__get_free_page(PGALLOC_GFP);
6666
}
6767

68-
static inline void pud_free(struct mm_struct *mm, pud_t *pud)
68+
static inline void pud_free(struct mm_struct *mm, pud_t *pudp)
6969
{
70-
BUG_ON((unsigned long)pud & (PAGE_SIZE-1));
71-
free_page((unsigned long)pud);
70+
BUG_ON((unsigned long)pudp & (PAGE_SIZE-1));
71+
free_page((unsigned long)pudp);
7272
}
7373

74-
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pud, pgdval_t prot)
74+
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pudp, pgdval_t prot)
7575
{
76-
set_pgd(pgdp, __pgd(__phys_to_pgd_val(pud) | prot));
76+
set_pgd(pgdp, __pgd(__phys_to_pgd_val(pudp) | prot));
7777
}
7878

79-
static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)
79+
static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, pud_t *pudp)
8080
{
81-
__pgd_populate(pgd, __pa(pud), PUD_TYPE_TABLE);
81+
__pgd_populate(pgdp, __pa(pudp), PUD_TYPE_TABLE);
8282
}
8383
#else
84-
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pud, pgdval_t prot)
84+
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pudp, pgdval_t prot)
8585
{
8686
BUILD_BUG();
8787
}
8888
#endif /* CONFIG_PGTABLE_LEVELS > 3 */
8989

9090
extern pgd_t *pgd_alloc(struct mm_struct *mm);
91-
extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
91+
extern void pgd_free(struct mm_struct *mm, pgd_t *pgdp);
9292

9393
static inline pte_t *
9494
pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
@@ -114,10 +114,10 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
114114
/*
115115
* Free a PTE table.
116116
*/
117-
static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
117+
static inline void pte_free_kernel(struct mm_struct *mm, pte_t *ptep)
118118
{
119-
if (pte)
120-
free_page((unsigned long)pte);
119+
if (ptep)
120+
free_page((unsigned long)ptep);
121121
}
122122

123123
static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
@@ -126,10 +126,10 @@ static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
126126
__free_page(pte);
127127
}
128128

129-
static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
129+
static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t ptep,
130130
pmdval_t prot)
131131
{
132-
set_pmd(pmdp, __pmd(__phys_to_pmd_val(pte) | prot));
132+
set_pmd(pmdp, __pmd(__phys_to_pmd_val(ptep) | prot));
133133
}
134134

135135
/*

‎arch/arm64/include/asm/pgtable.h‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ static inline pmd_t pmd_mkcont(pmd_t pmd)
218218

219219
static inline void set_pte(pte_t *ptep, pte_t pte)
220220
{
221-
*ptep = pte;
221+
WRITE_ONCE(*ptep, pte);
222222

223223
/*
224224
* Only if the new pte is valid and kernel, otherwise TLB maintenance
@@ -250,6 +250,8 @@ extern void __sync_icache_dcache(pte_t pteval, unsigned long addr);
250250
static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
251251
pte_t *ptep, pte_t pte)
252252
{
253+
pte_t old_pte;
254+
253255
if (pte_present(pte) && pte_user_exec(pte) && !pte_special(pte))
254256
__sync_icache_dcache(pte, addr);
255257

@@ -258,14 +260,15 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
258260
* hardware updates of the pte (ptep_set_access_flags safely changes
259261
* valid ptes without going through an invalid entry).
260262
*/
261-
if (IS_ENABLED(CONFIG_DEBUG_VM) && pte_valid(*ptep) && pte_valid(pte) &&
263+
old_pte = READ_ONCE(*ptep);
264+
if (IS_ENABLED(CONFIG_DEBUG_VM) && pte_valid(old_pte) && pte_valid(pte) &&
262265
(mm == current->active_mm || atomic_read(&mm->mm_users) > 1)) {
263266
VM_WARN_ONCE(!pte_young(pte),
264267
"%s: racy access flag clearing: 0x%016llx -> 0x%016llx",
265-
__func__, pte_val(*ptep), pte_val(pte));
266-
VM_WARN_ONCE(pte_write(*ptep) && !pte_dirty(pte),
268+
__func__, pte_val(old_pte), pte_val(pte));
269+
VM_WARN_ONCE(pte_write(old_pte) && !pte_dirty(pte),
267270
"%s: racy dirty state clearing: 0x%016llx -> 0x%016llx",
268-
__func__, pte_val(*ptep), pte_val(pte));
271+
__func__, pte_val(old_pte), pte_val(pte));
269272
}
270273

271274
set_pte(ptep, pte);
@@ -431,7 +434,7 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
431434

432435
static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
433436
{
434-
*pmdp = pmd;
437+
WRITE_ONCE(*pmdp, pmd);
435438
dsb(ishst);
436439
isb();
437440
}
@@ -482,7 +485,7 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
482485

483486
static inline void set_pud(pud_t *pudp, pud_t pud)
484487
{
485-
*pudp = pud;
488+
WRITE_ONCE(*pudp, pud);
486489
dsb(ishst);
487490
isb();
488491
}
@@ -500,7 +503,7 @@ static inline phys_addr_t pud_page_paddr(pud_t pud)
500503
/* Find an entry in the second-level page table. */
501504
#define pmd_index(addr) (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))
502505

503-
#define pmd_offset_phys(dir, addr) (pud_page_paddr(*(dir)) + pmd_index(addr) * sizeof(pmd_t))
506+
#define pmd_offset_phys(dir, addr) (pud_page_paddr(READ_ONCE(*(dir))) + pmd_index(addr) * sizeof(pmd_t))
504507
#define pmd_offset(dir, addr) ((pmd_t *)__va(pmd_offset_phys((dir), (addr))))
505508

506509
#define pmd_set_fixmap(addr) ((pmd_t *)set_fixmap_offset(FIX_PMD, addr))
@@ -535,7 +538,7 @@ static inline phys_addr_t pud_page_paddr(pud_t pud)
535538

536539
static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
537540
{
538-
*pgdp = pgd;
541+
WRITE_ONCE(*pgdp, pgd);
539542
dsb(ishst);
540543
}
541544

@@ -552,7 +555,7 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
552555
/* Find an entry in the frst-level page table. */
553556
#define pud_index(addr) (((addr) >> PUD_SHIFT) & (PTRS_PER_PUD - 1))
554557

555-
#define pud_offset_phys(dir, addr) (pgd_page_paddr(*(dir)) + pud_index(addr) * sizeof(pud_t))
558+
#define pud_offset_phys(dir, addr) (pgd_page_paddr(READ_ONCE(*(dir))) + pud_index(addr) * sizeof(pud_t))
556559
#define pud_offset(dir, addr) ((pud_t *)__va(pud_offset_phys((dir), (addr))))
557560

558561
#define pud_set_fixmap(addr) ((pud_t *)set_fixmap_offset(FIX_PUD, addr))

‎arch/arm64/kernel/efi.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ static int __init set_permissions(pte_t *ptep, pgtable_t token,
9090
unsigned long addr, void *data)
9191
{
9292
efi_memory_desc_t *md = data;
93-
pte_t pte = *ptep;
93+
pte_t pte = READ_ONCE(*ptep);
9494

9595
if (md->attribute & EFI_MEMORY_RO)
9696
pte = set_pte_bit(pte, __pgprot(PTE_RDONLY));

0 commit comments

Comments
 (0)