[PATCH v3 09/13] arm64: debug: split single stepping exception entry

[Ada Couprie Diaz]

On 18/06/2025 17:02, Mark Rutland wrote:

> This looks good; one comment below.
>
> On Mon, Jun 09, 2025 at 06:34:09PM +0100, Ada Couprie Diaz wrote:
>> +void do_softstep(unsigned long esr, struct pt_regs *regs)
>>   {
>> -	/*
>> -	 * If we are stepping a pending breakpoint, call the hw_breakpoint
>> -	 * handler first.
>> -	 */
>> -	if (try_step_suspended_breakpoints(regs))
>> -		return 0;
>> -
>>   	if (call_step_hook(regs, esr) == DBG_HOOK_HANDLED)
>> -		return 0;
>> +		return;
>>   
>>   	if (user_mode(regs)) {
>>   		send_user_sigtrap(TRAP_TRACE);
>> @@ -219,10 +212,8 @@ static int single_step_handler(unsigned long unused, unsigned long esr,
>>   		 */
>>   		set_regs_spsr_ss(regs);
>>   	}
>> -
>> -	return 0;
>>   }
>> -NOKPROBE_SYMBOL(single_step_handler);
>> +NOKPROBE_SYMBOL(do_softstep);
> With the EL0/EL1 entry paths split up, it would be nice to split this
> into separate do_el{0,1}_softstep() handlers, like we do for
> do_el{0,1}_undef(). With the relevant portions of call_step_hook()
> folded in, that'd leave us with:
>
> | void do_el0_softstep(struct pt_regs *regs, unsigned long esr)
> | {
> | 	if (uprobe_singlestep_handler(regs, esr) == DBG_HOOK_HANDLED)
> | 		return;
> | 	
> | 	send_user_sigtrap(TRAP_TRACE);
> | 	/*
> | 	 * ptrace will disable single step unless explicitly asked to
> | 	 * re-enable it. For other clients, it makes sense to leave it
> | 	 * enabled (i.e. rewind the controls to the active-not-pending
> | 	 * state).
> | 	 */
> | 	 user_rewind_single_step(current);
> | }
> |
> | void do_el1_softstep(struct pt_regs *regs, unsigned long esr)
> | {
> | 	if (kgdb_singlestep_handler(regs, esr) == DBG_HOOK_HANDLED)
> | 		return;
> | 	
> | 	pr_warn("Unexpected kernel single-step exception at EL1\n");
> | 	/*
> | 	 * Re-enable stepping since we know that we will be
> | 	 * returning to regs.
> | 	 */
> | 	set_regs_spsr_ss(regs);
> | }
> | NOKPROBE_SYMBOL(do_el1_softstep):
>
> Doing that would remove some indirection, and make this a bit easier to
> follow. That would also permit the EL0 handler to be kprobed (which is
> safe since there's no risk of recursion in the EL0 handler).
>
> Mark.
Agree, that's really nice and being able to probe a bit more is quite 
useful !

I split the EL0/EL1 handlers for the soft step and BRK64 in v4.

Thanks Mark,
Ada