[PATCH v2 03/11] arm64: debug: call step handlers statically

[Will Deacon]

On Mon, May 12, 2025 at 06:43:18PM +0100, Ada Couprie Diaz wrote:
> Software stepping checks for the correct handler by iterating over a list
> of dynamically registered handlers and calling all of them until one
> handles the exception.
> 
> This is the only generic way to handle software stepping handlers in arm64
> as the exception does not provide an immediate that could be checked,
> contrary to software breakpoints.
> 
> However, the registration mechanism is not exported and has only
> two current users : the KGDB stepping handler, and the uprobe single step
> handler.
> Given that one comes from user mode and the other from kernel mode, call
> the appropriate one by checking the source EL of the exception, if it
> is enabled.
> 
> Unify the naming of the handler to XXX_singlestep_handler(), making it
> clear they are related.
> 
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz at arm.com>
> ---
>  arch/arm64/include/asm/kgdb.h      |  1 +
>  arch/arm64/include/asm/uprobes.h   |  1 +
>  arch/arm64/kernel/debug-monitors.c | 32 +++++++++++++-----------------
>  arch/arm64/kernel/kgdb.c           | 17 +++-------------
>  arch/arm64/kernel/probes/uprobes.c |  9 +--------
>  5 files changed, 20 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kgdb.h b/arch/arm64/include/asm/kgdb.h
> index 82a76b2102fb..fd287ec38bb7 100644
> --- a/arch/arm64/include/asm/kgdb.h
> +++ b/arch/arm64/include/asm/kgdb.h
> @@ -26,6 +26,7 @@ extern int kgdb_fault_expected;
>  
>  int kgdb_brk_handler(struct pt_regs *regs, unsigned long esr);
>  int kgdb_compiled_brk_handler(struct pt_regs *regs, unsigned long esr);
> +int kgdb_singlestep_handler(struct pt_regs *regs, unsigned long esr);
>  
>  #endif /* !__ASSEMBLY__ */
>  
> diff --git a/arch/arm64/include/asm/uprobes.h b/arch/arm64/include/asm/uprobes.h
> index 3659a79a9f32..e44bbef40eca 100644
> --- a/arch/arm64/include/asm/uprobes.h
> +++ b/arch/arm64/include/asm/uprobes.h
> @@ -29,5 +29,6 @@ struct arch_uprobe {
>  };
>  
>  int uprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
> +int uprobe_singlestep_handler(struct pt_regs *regs, unsigned long esr);
>  
>  #endif
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 4ece4a93b872..81b813e16842 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -200,30 +200,26 @@ void unregister_kernel_step_hook(struct step_hook *hook)
>  }
>  
>  /*
> - * Call registered single step handlers
> + * Call single step handlers
>   * There is no Syndrome info to check for determining the handler.
> - * So we call all the registered handlers, until the right handler is
> - * found which returns zero.
> + * However, there is only one possible handler for user and kernel modes, so
> + * check and call the appropriate one if it is enabled.
>   */
>  static int call_step_hook(struct pt_regs *regs, unsigned long esr)
>  {
> -	struct step_hook *hook;
> -	struct list_head *list;
> -	int retval = DBG_HOOK_ERROR;
> -
> -	list = user_mode(regs) ? &user_step_hook : &kernel_step_hook;
> -
> -	/*
> -	 * Since single-step exception disables interrupt, this function is
> -	 * entirely not preemptible, and we can use rcu list safely here.
> -	 */
> -	list_for_each_entry_rcu(hook, list, node)	{
> -		retval = hook->fn(regs, esr);
> -		if (retval == DBG_HOOK_HANDLED)
> -			break;
> +	if (user_mode(regs)) {
> +#if CONFIG_UPROBES

nit: this should be #ifdef and my compiler complains about that:

  | arch/arm64/kernel/debug-monitors.c:172:5: warning: 'CONFIG_UPROBES' is not defined, evaluates to 0 [-Wundef]

> +		return uprobe_singlestep_handler(regs, esr);
> +#else
> +		return DBG_HOOK_ERROR;
> +#endif

It would probably be cleaner to have a static inline definition of
uprobe_singlestep_handler() when !CONFIG_UPROBES that just returns
DBG_HOOK_ERROR.

>  	}
>  
> -	return retval;
> +#ifdef CONFIG_KGDB
> +	return kgdb_singlestep_handler(regs, esr);
> +#else
> +	return DBG_HOOK_ERROR;
> +#endif

Similarly here.

Will