Re: [RFC PATCH v2 2/4] arm64/debug: Make the Kprobe functions noinstr

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Thu, 13 Aug 2026 06:49:42 +0000
Hongyan Xia <hongyan.xia@transsion.com> wrote:

> From: Hongyan Xia <hongyan.xia@transsion.com>
> 
> The Kprobe debug exception path must be run with extra care. NOKPROBE
> isn't sufficient, as other instrumentation like ftrace still opens a can
> of worms that is very complex to deal with.
> 
> Mark the three main Kprobe entry points noinstr, as well as the debug
> exception paths that lead to and exit from these entry points. Note that
> noinstr attribute is stronger and can safely replace NOKPROBE and
> __kprobe modifiers.
> 
> Also mark esr_brk_comment() __always_inline, as clang does not
> reliably inline plain static inline functions into noinline (noinstr)
> callers such as call_el1_break_hook().
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
>  arch/arm64/include/asm/esr.h       | 2 +-
>  arch/arm64/include/asm/kprobes.h   | 9 +++------
>  arch/arm64/kernel/debug-monitors.c | 6 ++----
>  arch/arm64/kernel/probes/kprobes.c | 6 +++---
>  4 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index f816f5d77f1a..a75bfdb7e5fe 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -437,7 +437,7 @@
>  #ifndef __ASSEMBLER__
>  #include <asm/types.h>
>  
> -static inline unsigned long esr_brk_comment(unsigned long esr)
> +static __always_inline unsigned long esr_brk_comment(unsigned long esr)
>  {
>  	return esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
>  }
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index 35ce2c94040e..a694f7d34f45 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -48,11 +48,8 @@ void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
>  
>  #endif /* CONFIG_KPROBES */
>  
> -int __kprobes kprobe_brk_handler(struct pt_regs *regs,
> -				 unsigned long esr);
> -int __kprobes kprobe_ss_brk_handler(struct pt_regs *regs,
> -				 unsigned long esr);
> -int __kprobes kretprobe_brk_handler(struct pt_regs *regs,
> -				 unsigned long esr);
> +int noinstr kprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
> +int noinstr kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr);
> +int noinstr kretprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
>  
>  #endif /* _ARM_KPROBES_H */
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 29307642f4c9..5cf4fb8ddf83 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -207,7 +207,7 @@ void do_el1_softstep(unsigned long esr, struct pt_regs *regs)
>  }
>  NOKPROBE_SYMBOL(do_el1_softstep);
>  
> -static int call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
> +static int noinstr call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
>  {
>  	if (esr_brk_comment(esr) == BUG_BRK_IMM)
>  		return bug_brk_handler(regs, esr);
> @@ -245,7 +245,6 @@ static int call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
>  
>  	return DBG_HOOK_ERROR;
>  }
> -NOKPROBE_SYMBOL(call_el1_break_hook);
>  
>  /*
>   * We have already unmasked interrupts and enabled preemption
> @@ -261,14 +260,13 @@ void do_el0_brk64(unsigned long esr, struct pt_regs *regs)
>  	send_user_sigtrap(TRAP_BRKPT);
>  }
>  
> -void do_el1_brk64(unsigned long esr, struct pt_regs *regs)
> +void noinstr do_el1_brk64(unsigned long esr, struct pt_regs *regs)
>  {
>  	if (call_el1_break_hook(regs, esr) == DBG_HOOK_HANDLED)
>  		return;
>  
>  	die("Oops - BRK", regs, esr);
>  }
> -NOKPROBE_SYMBOL(do_el1_brk64);
>  
>  #ifdef CONFIG_COMPAT
>  void do_bkpt32(unsigned long esr, struct pt_regs *regs)
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 4e0efad5caf2..0e66abf9958e 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -350,7 +350,7 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
>  	return 0;
>  }
>  
> -int __kprobes
> +int noinstr
>  kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
>  {
>  	struct kprobe *p, *cur_kprobe;
> @@ -394,7 +394,7 @@ kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
>  	return DBG_HOOK_HANDLED;
>  }
>  
> -int __kprobes
> +int noinstr
>  kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr)
>  {
>  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> @@ -413,7 +413,7 @@ kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr)
>  	return DBG_HOOK_ERROR;
>  }
>  
> -int __kprobes
> +int noinstr
>  kretprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
>  {
>  	if (regs->pc != (unsigned long)__kretprobe_trampoline)
> -- 
> 2.47.3
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>