[PATCH] arm64: kprobes: Use BRK instead of single-step when executing instructions out-of-line

[Masami Hiramatsu]

On Tue, 17 Nov 2020 18:28:12 +0100
Jean-Philippe Brucker <jean-philippe at linaro.org> wrote:

> On Tue, Nov 03, 2020 at 09:23:16AM +0000, Will Deacon wrote:
> > Yes, let's just set all of DAIF during the trampoline. Also, while I've got
> > you, If you get a chance, I'd appreciate any feedback on my proposal for
> > reworking our debug exception handling altogether:
> > 
> > https://lore.kernel.org/r/20200626095551.GA9312@willie-the-truck
> 
> Well, I stared at this for a while... It looks fine to me, but I don't
> have a full picture of the trap infrastructure (not sure whether you were
> asking me). I could help with testing if you get around to reworking it.
> 
> > On taking an interrupt from EL1, stash MDSCR_EL1.SS in a pcpu variable and
> > clear the register bit if it was set. Then unmask only D and leave I set. On
> > return from the exception, set D and restore MDSCR_EL1.SS. If we decide to
> > reschedule, unmask D (i.e. we only step into interrupts if we need a
> > reschedule. Alternatively, we could skip the reschedule if we were
> > stepping.)
> 
> Any specific reason to treat reschedule differently, or just to keep
> things simple? I'm asking because that could be a problem with the
> current code: when taking an interrupt while stepping EL1, we keep
> MDSCR_EL1.SS set and unmask D before calling the IRQ handler. The step
> exception might only be taken after the next context synchronization event
> (on QEMU it happens at an isb in the timer handler). If the IRQ handler
> doesn't happen to do any context synchronization and we reschedule, I
> guess the step exception could happen after the next eret?

Will, would you have any comment on this?

I guess if SS on EL1 enabled with kprobes, this may happen (SS means SS exception happens)

[orig-code(SS)]->[1st BRK](D set)->[copied insn(!SS)]->[2nd BRK](D unmask)->[orig-code(SS)]

What if we unmask D flag (current code), this may happen

[orig-code(SS)]->[1st BRK](D unmask)->[copied insn(SS)]->[2nd BRK](D unmask)->[orig-code(SS)]

But as you pointed, this may make things complex. (maybe debugger also needs
to care about this case, because the single-stepped address is far from the
original code, they may not be able to find correct source line.)

So keeping D flag set in this phase is good to me, until we solve the debugger-side support.

Thank you,

-- 
Masami Hiramatsu <mhiramat at kernel.org>