Re: [PATCH] x86/mm: Define the zapped-entry checks only with CONFIG_DEBUG_VM

[Edgecombe, Rick P]

On Mon, 2026-11-02 at 14:39 +0000, Kiryl Shutsemau (Meta) wrote:
> Zapping a range calls arch_check_zapped_pte() for every batch of PTEs,
> which is every page for small folios, and arch_check_zapped_pmd() or
> arch_check_zapped_pud() for every huge leaf. On x86 these are out-of-line
> functions whose only content is a VM_WARN_ON_ONCE(), so without
> CONFIG_DEBUG_VM they are empty, but the call remains. With the SRSO "Safe
> RET" mitigation the return of every such call mispredicts.
> 
> Declare and define the checks only with CONFIG_DEBUG_VM. Otherwise the
> empty inlines from <linux/pgtable.h> apply, as on every other
> architecture. They cannot be inlines in <asm/pgtable.h>: VM_SHADOW_STACK
> is defined in <linux/mm.h> after it includes <linux/pgtable.h>.
> 
> In a KVM guest on Zen 4 with Safe RET, zapping 4K pages takes one branch
> miss per page fewer. MADV_DONTNEED of 1 GiB gets 6% faster on anonymous
> memory and 8% on shmem, munmap() 5%. THP is unchanged.

I guess this is the impact of this fix on Safe RET machines, but it's a little
ambiguous. With the assumption that a mitigation didn't result in the
performance improvement I can parse it.

Do you happen to know if this performance impact is relevant at all in the non
Safe RET case?

> 
> Fixes: e5136e876581 ("mm: Warn on shadow stack memory in wrong vma")
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/pgtable.h | 2 ++
>  arch/x86/mm/pgtable.c          | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 000fc6c1ca37..9b03a316fb75 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1666,6 +1666,7 @@ static inline bool arch_has_pfn_modify_check(void)
>  	return boot_cpu_has_bug(X86_BUG_L1TF);
>  }
>  
> +#ifdef CONFIG_DEBUG_VM
>  #define arch_check_zapped_pte arch_check_zapped_pte
>  void arch_check_zapped_pte(struct vm_area_struct *vma, pte_t pte);
>  
> @@ -1674,6 +1675,7 @@ void arch_check_zapped_pmd(struct vm_area_struct *vma,
> pmd_t pmd);
>  
>  #define arch_check_zapped_pud arch_check_zapped_pud
>  void arch_check_zapped_pud(struct vm_area_struct *vma, pud_t pud);
> +#endif
>  
>  #ifdef CONFIG_XEN_PV
>  #define arch_has_hw_nonleaf_pmd_young arch_has_hw_nonleaf_pmd_young
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 4a105f283cfb..3352bc6904f3 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -798,6 +798,7 @@ pmd_t pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
>  	return pmd_clear_saveddirty(pmd);
>  }
>  
> +#ifdef CONFIG_DEBUG_VM
>  void arch_check_zapped_pte(struct vm_area_struct *vma, pte_t pte)
>  {
>  	/*
> @@ -823,3 +824,4 @@ void arch_check_zapped_pud(struct vm_area_struct *vma,
> pud_t pud)
>  	/* See note in arch_check_zapped_pte() */
>  	VM_WARN_ON_ONCE(!(vma->vm_flags & VM_SHADOW_STACK) &&
> pud_shstk(pud));

I guess all of these don't need to be the "VM_WARN_" varieties if they are
compiled out in the other case.

>  }
> +#endif