Re: [RFC PATCH 1/9] arm64/entry: Bound certain debug exception paths in instrumentation windows

[Mark Rutland]

On Mon, Jul 27, 2026 at 12:25:34PM +0000, Hongyan Xia wrote:
> From: Hongyan Xia <hongyan.xia@xxxxxxxxxxxxx>
> 
> The el1 debug exception handlers are noinstr, but their payload calls
> (do_breakpoint(), do_watchpoint(), try_step_suspended_breakpoints()
> and the Cortex-A76 erratum handler) are ordinary instrumentable code.
> 
> Mark the boundaries explicitly with instrumentation_begin()/end(), in
> the same style as x86 exception entries.
> 
> The BRK handlers (do_el1_brk64(), do_el1_softstep()) will be dealt with
> in later patches.
> 
> Signed-off-by: Hongyan Xia <hongyan.xia@xxxxxxxxxxxxx>

Please don't do this.

The instrumentation_{begin,end}() markers don't do anything useful on
arm64 (since we don't use objtoo, etc), and adding them to a subset of
arm64's exception entry code gives the misleading impression that these
cases are different from other exception entry cases.

Mark.

> ---
>  arch/arm64/kernel/entry-common.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index ceb4eb11232a..466529cce1c3 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/context_tracking.h>
> +#include <linux/instrumentation.h>
>  #include <linux/irq-entry-common.h>
>  #include <linux/kasan.h>
>  #include <linux/linkage.h>
> @@ -380,7 +381,9 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
>  
>  	state = arm64_enter_el1_dbg(regs);
>  	debug_exception_enter(regs);
> +	instrumentation_begin();
>  	do_breakpoint(esr, regs);
> +	instrumentation_end();
>  	debug_exception_exit(regs);
>  	arm64_exit_el1_dbg(regs, state);
>  }
> @@ -388,9 +391,15 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
>  static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
>  {
>  	irqentry_state_t state;
> +	bool handled;
>  
>  	state = arm64_enter_el1_dbg(regs);
> -	if (!cortex_a76_erratum_1463225_debug_handler(regs)) {
> +
> +	instrumentation_begin();
> +	handled = cortex_a76_erratum_1463225_debug_handler(regs);
> +	instrumentation_end();
> +
> +	if (!handled) {
>  		debug_exception_enter(regs);
>  		/*
>  		 * After handling a breakpoint, we suspend the breakpoint
> @@ -398,7 +407,11 @@ static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
>  		 * If we are stepping a suspended breakpoint there's nothing more to do:
>  		 * the single-step is complete.
>  		 */
> -		if (!try_step_suspended_breakpoints(regs))
> +		instrumentation_begin();
> +		handled = try_step_suspended_breakpoints(regs);
> +		instrumentation_end();
> +
> +		if (!handled)
>  			do_el1_softstep(esr, regs);
>  		debug_exception_exit(regs);
>  	}
> @@ -413,7 +426,9 @@ static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
>  
>  	state = arm64_enter_el1_dbg(regs);
>  	debug_exception_enter(regs);
> +	instrumentation_begin();
>  	do_watchpoint(far, esr, regs);
> +	instrumentation_end();
>  	debug_exception_exit(regs);
>  	arm64_exit_el1_dbg(regs, state);
>  }
> -- 
> 2.47.3
>