Re: [PATCH V2 1/2] arm64/mm: Move __check_safe_pte_update()

[Anshuman Khandual]

On Wed, Sep 09, 2026 at 05:12:24PM +0200, David Hildenbrand (Arm) wrote:
> On 9/9/26 06:00, Anshuman Khandual wrote:
> > The page table entry print helpers and related macros which are defined in
> > <linux/pgtable.h> will not be accessible in platform <asm/pgtable.h> which
> > is basically caused by cycling dependency.
> > 
> > Move __check_safe_pte_update() inside arch/arm64/mm/mmu.c as a preparation
> > for subsequent usage of the afore mentioned generic MM helpers. While here
> > drop IS_ENABLED(CONFIG_DEBUG_VM), although wrap __check_safe_pte_update()
> > inside #ifdef CONFIG_DEBUG_VM that preserves the current code optimization
> > which is achieved via the static inline functions.
> > 
> > This does not cause any functional change.
> > 
> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > ---
> > Changes in V2:
> > 
> > - Moved __check_safe_pte_update() inside #ifdef CONFIG_DEBUG_VM
> > - Added empty stub for __check_safe_pte_update() when !CONFIG_DEBUG_VM
> > - Dropped IS_ENABLED(CONFIG_DEBUG_VM) from __check_safe_pte_update()
> > 
> >  arch/arm64/include/asm/pgtable.h | 49 ++++----------------------------
> >  arch/arm64/mm/mmu.c              | 44 ++++++++++++++++++++++++++++
> >  2 files changed, 49 insertions(+), 44 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index e89ec5f4787b..281a512f1fc6 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -387,52 +387,13 @@ static inline pte_t __ptep_get(pte_t *ptep)
> >  extern void __sync_icache_dcache(pte_t pteval);
> >  bool pgattr_change_is_safe(pteval_t old, pteval_t new);
> >  
> > -/*
> > - * PTE bits configuration in the presence of hardware Dirty Bit Management
> > - * (PTE_WRITE == PTE_DBM):
> > - *
> > - * Dirty  Writable | PTE_RDONLY  PTE_WRITE  PTE_DIRTY (sw)
> > - *   0      0      |   1           0          0
> > - *   0      1      |   1           1          0
> > - *   1      0      |   1           0          1
> > - *   1      1      |   0           1          x
> > - *
> > - * When hardware DBM is not present, the software PTE_DIRTY bit is updated via
> > - * the page fault mechanism. Checking the dirty status of a pte becomes:
> > - *
> > - *   PTE_DIRTY || (PTE_WRITE && !PTE_RDONLY)
> > - */
> > -
> > -static inline void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep,
> > -					   pte_t pte)
> > +#ifdef CONFIG_DEBUG_VM
> > +void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte);
> > +#else
> > +static inline void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
> >  {
> > -	pte_t old_pte;
> > -
> > -	if (!IS_ENABLED(CONFIG_DEBUG_VM))
> > -		return;
> > -
> > -	old_pte = __ptep_get(ptep);
> > -
> > -	if (!pte_valid(old_pte) || !pte_valid(pte))
> > -		return;
> > -	if (mm != current->active_mm && atomic_read(&mm->mm_users) <= 1)
> > -		return;
> > -
> > -	/*
> > -	 * Check for potential race with hardware updates of the pte
> > -	 * (__ptep_set_access_flags safely changes valid ptes without going
> > -	 * through an invalid entry).
> > -	 */
> > -	VM_WARN_ONCE(!pte_young(pte),
> > -		     "%s: racy access flag clearing: 0x%016llx -> 0x%016llx",
> > -		     __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(old_pte), pte_val(pte));
> > -	VM_WARN_ONCE(!pgattr_change_is_safe(pte_val(old_pte), pte_val(pte)),
> > -		     "%s: unsafe attribute change: 0x%016llx -> 0x%016llx",
> > -		     __func__, pte_val(old_pte), pte_val(pte));
> >  }
> > +#endif
> 
> [...]
> 
> > +#ifdef CONFIG_DEBUG_VM
> > +void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
> > +{
> > +	pte_t old_pte;
> > +
> > +	old_pte = __ptep_get(ptep);
> > +
> > +	if (!pte_valid(old_pte) || !pte_valid(pte))
> > +		return;
> > +	if (mm != current->active_mm && atomic_read(&mm->mm_users) <= 1)
> > +		return;
> > +
> > +	/*
> > +	 * Check for potential race with hardware updates of the pte
> > +	 * (__ptep_set_access_flags safely changes valid ptes without going
> > +	 * through an invalid entry).
> > +	 */
> > +	VM_WARN_ONCE(!pte_young(pte),
> > +		     "%s: racy access flag clearing: 0x%016llx -> 0x%016llx",
> > +		     __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(old_pte), pte_val(pte));
> > +	VM_WARN_ONCE(!pgattr_change_is_safe(pte_val(old_pte), pte_val(pte)),
> > +		     "%s: unsafe attribute change: 0x%016llx -> 0x%016llx",
> > +		     __func__, pte_val(old_pte), pte_val(pte));
> > +}
> > +#endif
> 
> #endif /* CONFIG_DEBUG_VM */
> 
> Might be helpful here.
> 
> Consider the same in the case above

Sure will update the same both in here and also for the declrations
in the header file as well.

> 
> Reviewed-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks for the review.

> 
> -- 
> Cheers,
> 
> David