On Mon, Sep 14, 2020 at 05:13:28PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Sep 14, 2020 at 6:02 AM Daniel Thompson > <daniel.thomp...@linaro.org> wrote: > > > > During debug trap execution we expect dbg_deactivate_sw_breakpoints() > > to be paired with an dbg_activate_sw_breakpoint(). Currently although > > the calls are paired correctly they are needlessly smeared across three > > different functions. Worse this also results in code to drive polled I/O > > being called with breakpoints activated which, in turn, needlessly > > increases the set of functions that will recursively trap if breakpointed. > > > > Fix this by moving the activation of breakpoints into the debug core. > > > > Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org> > > --- > > kernel/debug/debug_core.c | 2 ++ > > kernel/debug/gdbstub.c | 1 - > > kernel/debug/kdb/kdb_debugger.c | 2 -- > > 3 files changed, 2 insertions(+), 3 deletions(-) > > I like the idea, but previously the kgdb_arch_handle_exception() was > always called after the SW breakpoints were activated. Are you sure > it's OK to swap those two orders across all architectures?
Pretty sure, yes. However, given the poor attention to detail I demonstrated in patch 2/3, I figure I'd better write out the full chain of reasoning if I want you to trust me ;-) . kgdb_arch_handle_exception() is already called frequently with breakpoints disabled since it is basically a fallback that is called whenever the architecture neutral parts of the gdb packet processing cannot cope. So your question becomes more specific: is it OK to swap orders when the architecture code is handling a (c)ontinue or (s)tep packet[1]? The reason the architecture neutral part cannot cope is because it because *if* gdb has been instructed that the PC must be changed before resuming then the architecture neutral code does not know how to effect this. In other words the call to kgdb_arch_handle_exception() will boil down to doing whatever the architecture equivalent of writing to linux_regs->pc actually is (or return an error). Updating the PC is safe whether or not breakpoints are deactivated and activating the breakpoints if the architecture code actually censors the resume is a bug (which we just fixed). Daniel. [1] The fallthroughs aren't a whole lot of fun to read but if gdb_cmd_exception_pass() provokes a fallthrough then it will have rewritten the packet as a (c)ontinue. _______________________________________________ Kgdb-bugreport mailing list Kgdb-bugreport@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
