Skip to content

Commit

Permalink
arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Tested-by: Richard Ruigrok <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Catalin Marinas <[email protected]>
  • Loading branch information
@wildea01 @ctmarinas
wildea01 authored and ctmarinas committed Feb 16, 2018
1 parent 2ce77f6 commit 20a004e
Show file tree
Hide file tree
Showing 13 changed files with 426 additions and 399 deletions.
2 changes: 1 addition & 1 deletion arch/arm64/include/asm/hugetlb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

static inline pte_t huge_ptep_get(pte_t *ptep)
{
return *ptep;
return READ_ONCE(*ptep);
}


Expand Down
26 changes: 13 additions & 13 deletions arch/arm64/include/asm/kvm_mmu.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,42 +185,42 @@ static inline pmd_t kvm_s2pmd_mkexec(pmd_t pmd)
return pmd;
}

static inline void kvm_set_s2pte_readonly(pte_t *pte)
static inline void kvm_set_s2pte_readonly(pte_t *ptep)
{
pteval_t old_pteval, pteval;

pteval = READ_ONCE(pte_val(*pte));
pteval = READ_ONCE(pte_val(*ptep));
do {
old_pteval = pteval;
pteval &= ~PTE_S2_RDWR;
pteval |= PTE_S2_RDONLY;
pteval = cmpxchg_relaxed(&pte_val(*pte), old_pteval, pteval);
pteval = cmpxchg_relaxed(&pte_val(*ptep), old_pteval, pteval);
} while (pteval != old_pteval);
}

static inline bool kvm_s2pte_readonly(pte_t *pte)
static inline bool kvm_s2pte_readonly(pte_t *ptep)
{
return (pte_val(*pte) & PTE_S2_RDWR) == PTE_S2_RDONLY;
return (READ_ONCE(pte_val(*ptep)) & PTE_S2_RDWR) == PTE_S2_RDONLY;
}

static inline bool kvm_s2pte_exec(pte_t *pte)
static inline bool kvm_s2pte_exec(pte_t *ptep)
{
return !(pte_val(*pte) & PTE_S2_XN);
return !(READ_ONCE(pte_val(*ptep)) & PTE_S2_XN);
}

static inline void kvm_set_s2pmd_readonly(pmd_t *pmd)
static inline void kvm_set_s2pmd_readonly(pmd_t *pmdp)
{
kvm_set_s2pte_readonly((pte_t *)pmd);
kvm_set_s2pte_readonly((pte_t *)pmdp);
}

static inline bool kvm_s2pmd_readonly(pmd_t *pmd)
static inline bool kvm_s2pmd_readonly(pmd_t *pmdp)
{
return kvm_s2pte_readonly((pte_t *)pmd);
return kvm_s2pte_readonly((pte_t *)pmdp);
}

static inline bool kvm_s2pmd_exec(pmd_t *pmd)
static inline bool kvm_s2pmd_exec(pmd_t *pmdp)
{
return !(pmd_val(*pmd) & PMD_S2_XN);
return !(READ_ONCE(pmd_val(*pmdp)) & PMD_S2_XN);
}

static inline bool kvm_page_empty(void *ptr)
Expand Down
4 changes: 2 additions & 2 deletions arch/arm64/include/asm/mmu_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@ static inline void cpu_install_idmap(void)
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
* avoiding the possibility of conflicting TLB entries being allocated.
*/
static inline void cpu_replace_ttbr1(pgd_t *pgd)
static inline void cpu_replace_ttbr1(pgd_t *pgdp)
{
typedef void (ttbr_replace_func)(phys_addr_t);
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
ttbr_replace_func *replace_phys;

phys_addr_t pgd_phys = virt_to_phys(pgd);
phys_addr_t pgd_phys = virt_to_phys(pgdp);

replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);

Expand Down
44 changes: 22 additions & 22 deletions arch/arm64/include/asm/pgalloc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,23 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
return (pmd_t *)__get_free_page(PGALLOC_GFP);
}

static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
{
BUG_ON((unsigned long)pmd & (PAGE_SIZE-1));
free_page((unsigned long)pmd);
BUG_ON((unsigned long)pmdp & (PAGE_SIZE-1));
free_page((unsigned long)pmdp);
}

static inline void __pud_populate(pud_t *pud, phys_addr_t pmd, pudval_t prot)
static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
{
set_pud(pud, __pud(__phys_to_pud_val(pmd) | prot));
set_pud(pudp, __pud(__phys_to_pud_val(pmdp) | prot));
}

static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
static inline void pud_populate(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp)
{
__pud_populate(pud, __pa(pmd), PMD_TYPE_TABLE);
__pud_populate(pudp, __pa(pmdp), PMD_TYPE_TABLE);
}
#else
static inline void __pud_populate(pud_t *pud, phys_addr_t pmd, pudval_t prot)
static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
{
BUILD_BUG();
}
Expand All @@ -65,30 +65,30 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
return (pud_t *)__get_free_page(PGALLOC_GFP);
}

static inline void pud_free(struct mm_struct *mm, pud_t *pud)
static inline void pud_free(struct mm_struct *mm, pud_t *pudp)
{
BUG_ON((unsigned long)pud & (PAGE_SIZE-1));
free_page((unsigned long)pud);
BUG_ON((unsigned long)pudp & (PAGE_SIZE-1));
free_page((unsigned long)pudp);
}

static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pud, pgdval_t prot)
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pudp, pgdval_t prot)
{
set_pgd(pgdp, __pgd(__phys_to_pgd_val(pud) | prot));
set_pgd(pgdp, __pgd(__phys_to_pgd_val(pudp) | prot));
}

static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)
static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, pud_t *pudp)
{
__pgd_populate(pgd, __pa(pud), PUD_TYPE_TABLE);
__pgd_populate(pgdp, __pa(pudp), PUD_TYPE_TABLE);
}
#else
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pud, pgdval_t prot)
static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pudp, pgdval_t prot)
{
BUILD_BUG();
}
#endif /* CONFIG_PGTABLE_LEVELS > 3 */

extern pgd_t *pgd_alloc(struct mm_struct *mm);
extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
extern void pgd_free(struct mm_struct *mm, pgd_t *pgdp);

static inline pte_t *
pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
Expand All @@ -114,10 +114,10 @@ pte_alloc_one(struct mm_struct *mm, unsigned long addr)
/*
* Free a PTE table.
*/
static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
static inline void pte_free_kernel(struct mm_struct *mm, pte_t *ptep)
{
if (pte)
free_page((unsigned long)pte);
if (ptep)
free_page((unsigned long)ptep);
}

static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
Expand All @@ -126,10 +126,10 @@ static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
__free_page(pte);
}

static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t pte,
static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t ptep,
pmdval_t prot)
{
set_pmd(pmdp, __pmd(__phys_to_pmd_val(pte) | prot));
set_pmd(pmdp, __pmd(__phys_to_pmd_val(ptep) | prot));
}

/*
Expand Down
23 changes: 13 additions & 10 deletions arch/arm64/include/asm/pgtable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ static inline pmd_t pmd_mkcont(pmd_t pmd)

static inline void set_pte(pte_t *ptep, pte_t pte)
{
*ptep = pte;
WRITE_ONCE(*ptep, pte);

/*
* Only if the new pte is valid and kernel, otherwise TLB maintenance
Expand Down Expand Up @@ -250,6 +250,8 @@ extern void __sync_icache_dcache(pte_t pteval, unsigned long addr);
static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte)
{
pte_t old_pte;

if (pte_present(pte) && pte_user_exec(pte) && !pte_special(pte))
__sync_icache_dcache(pte, addr);

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

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

static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
{
*pmdp = pmd;
WRITE_ONCE(*pmdp, pmd);
dsb(ishst);
isb();
}
Expand Down Expand Up @@ -482,7 +485,7 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)

static inline void set_pud(pud_t *pudp, pud_t pud)
{
*pudp = pud;
WRITE_ONCE(*pudp, pud);
dsb(ishst);
isb();
}
Expand All @@ -500,7 +503,7 @@ static inline phys_addr_t pud_page_paddr(pud_t pud)
/* Find an entry in the second-level page table. */
#define pmd_index(addr) (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))

#define pmd_offset_phys(dir, addr) (pud_page_paddr(*(dir)) + pmd_index(addr) * sizeof(pmd_t))
#define pmd_offset_phys(dir, addr) (pud_page_paddr(READ_ONCE(*(dir))) + pmd_index(addr) * sizeof(pmd_t))
#define pmd_offset(dir, addr) ((pmd_t *)__va(pmd_offset_phys((dir), (addr))))

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

static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
{
*pgdp = pgd;
WRITE_ONCE(*pgdp, pgd);
dsb(ishst);
}

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

#define pud_offset_phys(dir, addr) (pgd_page_paddr(*(dir)) + pud_index(addr) * sizeof(pud_t))
#define pud_offset_phys(dir, addr) (pgd_page_paddr(READ_ONCE(*(dir))) + pud_index(addr) * sizeof(pud_t))
#define pud_offset(dir, addr) ((pud_t *)__va(pud_offset_phys((dir), (addr))))

#define pud_set_fixmap(addr) ((pud_t *)set_fixmap_offset(FIX_PUD, addr))
Expand Down
2 changes: 1 addition & 1 deletion arch/arm64/kernel/efi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static int __init set_permissions(pte_t *ptep, pgtable_t token,
unsigned long addr, void *data)
{
efi_memory_desc_t *md = data;
pte_t pte = *ptep;
pte_t pte = READ_ONCE(*ptep);

if (md->attribute & EFI_MEMORY_RO)
pte = set_pte_bit(pte, __pgprot(PTE_RDONLY));
Expand Down
Loading

0 comments on commit 20a004e

Please sign in to comment.