Re: [RFC V2 07/14] arm64/mm: Route all pgtable reads via pxxval_get()

[Ryan Roberts]

On 13/05/2026 05:45, Anshuman Khandual wrote:
> Define arm64 platform specific implementations for new pXdp_get() helpers.
> These resolve into READ_ONCE(), thus ensuring required single copy atomic
> semantics for the page table entry reads.
> 
> In future this infrastructure can be used for D128 to maintain single copy
> atomicity semantics with inline asm blocks.
> 
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> ---
> Changes in RFC V2:
> 
> - Renamed all ptdesc_ instances as pxxval_ instead
> - Moved arm64 pgtable header READ_ONCE() replacements here in this patch
> 
>  arch/arm64/include/asm/pgtable.h | 38 +++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index cefe8ab86acd..72da582e8d12 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -84,6 +84,32 @@ static inline void arch_leave_lazy_mmu_mode(void)
>  	arch_flush_lazy_mmu_mode();
>  }
>  
> +#define pxxval_get(x)		READ_ONCE(x)

I think this would be better as an inline (or __force_inline) function, which
takes a pointer, and dereferences it. READ_ONCE() is special - it's modifying
how the dereference should be done, so it makes sense that the dereference is
done in the caller.

> +
> +#define pmdp_get pmdp_get
> +static inline pmd_t pmdp_get(pmd_t *pmdp)
> +{
> +	return pxxval_get(*pmdp);

This is weird to read. It would be clearer as:

	return pxxval_get(pmdp);

Thanks,
Ryan

> +}
> +
> +#define pudp_get pudp_get
> +static inline pud_t pudp_get(pud_t *pudp)
> +{
> +	return pxxval_get(*pudp);
> +}
> +
> +#define p4dp_get p4dp_get
> +static inline p4d_t p4dp_get(p4d_t *p4dp)
> +{
> +	return pxxval_get(*p4dp);
> +}
> +
> +#define pgdp_get pgdp_get
> +static inline pgd_t pgdp_get(pgd_t *pgdp)
> +{
> +	return pxxval_get(*pgdp);
> +}
> +
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
>  
> @@ -380,7 +406,7 @@ static inline void __set_pte(pte_t *ptep, pte_t pte)
>  
>  static inline pte_t __ptep_get(pte_t *ptep)
>  {
> -	return READ_ONCE(*ptep);
> +	return pxxval_get(*ptep);
>  }
>  
>  extern void __sync_icache_dcache(pte_t pteval);
> @@ -1011,7 +1037,7 @@ static inline phys_addr_t pud_offset_phys(p4d_t *p4dp, unsigned long addr)
>  {
>  	BUG_ON(!pgtable_l4_enabled());
>  
> -	return p4d_page_paddr(READ_ONCE(*p4dp)) + pud_index(addr) * sizeof(pud_t);
> +	return p4d_page_paddr(p4dp_get(p4dp)) + pud_index(addr) * sizeof(pud_t);
>  }
>  
>  static inline
> @@ -1025,7 +1051,7 @@ pud_t *pud_offset_lockless(p4d_t *p4dp, p4d_t p4d, unsigned long addr)
>  
>  static inline pud_t *pud_offset(p4d_t *p4dp, unsigned long addr)
>  {
> -	return pud_offset_lockless(p4dp, READ_ONCE(*p4dp), addr);
> +	return pud_offset_lockless(p4dp, p4dp_get(p4dp), addr);
>  }
>  #define pud_offset	pud_offset
>  
> @@ -1134,7 +1160,7 @@ static inline phys_addr_t p4d_offset_phys(pgd_t *pgdp, unsigned long addr)
>  {
>  	BUG_ON(!pgtable_l5_enabled());
>  
> -	return pgd_page_paddr(READ_ONCE(*pgdp)) + p4d_index(addr) * sizeof(p4d_t);
> +	return pgd_page_paddr(pgdp_get(pgdp)) + p4d_index(addr) * sizeof(p4d_t);
>  }
>  
>  static inline
> @@ -1148,7 +1174,7 @@ p4d_t *p4d_offset_lockless(pgd_t *pgdp, pgd_t pgd, unsigned long addr)
>  
>  static inline p4d_t *p4d_offset(pgd_t *pgdp, unsigned long addr)
>  {
> -	return p4d_offset_lockless(pgdp, READ_ONCE(*pgdp), addr);
> +	return p4d_offset_lockless(pgdp, pgdp_get(pgdp), addr);
>  }
>  
>  static inline p4d_t *p4d_set_fixmap(unsigned long addr)
> @@ -1346,7 +1372,7 @@ static inline bool pmdp_test_and_clear_young(struct vm_area_struct *vma,
>  		unsigned long address, pmd_t *pmdp)
>  {
>  	/* Operation applies to PMD table entry only if FEAT_HAFT is enabled */
> -	VM_WARN_ON(pmd_table(READ_ONCE(*pmdp)) && !system_supports_haft());
> +	VM_WARN_ON(pmd_table(pmdp_get(pmdp)) && !system_supports_haft());
>  	return __ptep_test_and_clear_young(vma, address, (pte_t *)pmdp);
>  }
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_ARCH_HAS_NONLEAF_PMD_YOUNG */