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

On 7/31/2026 10:41 PM, Mark Rutland wrote:
> On Mon, Jul 27, 2026 at 12:25:34PM +0000, Hongyan Xia wrote:
>> From: Hongyan Xia <[email protected]>
>>
>> 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 <[email protected]>
> 
> 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.

Okay. I think it's a bit unclear what the scope of this series is. This 
series started out trying to make the whole debug_exception() path 
noinstr, not just Kprobe. With my recent involvement in Kprobe, the 
Kprobe part is handled carefully while other paths are hit with a giant 
instructiontation_{begin,end}() hammer.

I can see this is confusing. I'll limit the series to just Kprobe 
noinstr and make the commit messages clear. This should also drop a few 
patches in this series.

> 
> 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
>>