Thread (4 messages) 4 messages, 3 authors, 2020-05-18

Re: [PATCH] arm64: Call debug_traps_init() from trap_init() to help early kgdb

From: Daniel Thompson <hidden>
Date: 2020-05-16 20:29:30
Also in: lkml

On Fri, May 15, 2020 at 05:23:17PM +0100, Will Deacon wrote:
On Wed, May 13, 2020 at 04:06:37PM -0700, Douglas Anderson wrote:
quoted
A new kgdb feature will soon land (kgdb_earlycon) that lets us run
kgdb much earlier.  In order for everything to work properly it's
important that the break hook is setup by the time we process
"kgdbwait".

Right now the break hook is setup in debug_traps_init() and that's
called from arch_initcall().  That's a bit too late since
kgdb_earlycon really needs things to be setup by the time the system
calls dbg_late_init().

We could fix this by adding call_break_hook() into early_brk64() and
that works fine.  However, it's a little ugly.  Instead, let's just
add a call to debug_traps_init() straight from trap_init().  There's
already a documented dependency between trap_init() and
debug_traps_init() and this makes the dependency more obvious rather
than just relying on a comment.

NOTE: this solution isn't early enough to let us select the
"ARCH_HAS_EARLY_DEBUG" KConfig option that is introduced by the
kgdb_earlycon patch series.  That would only be set if we could do
breakpoints when early params are parsed.  This patch only enables
"late early" breakpoints, AKA breakpoints when dbg_late_init() is
called.  It's expected that this should be fine for most people.

It should also be noted that if you crash you can still end up in kgdb
earlier than debug_traps_init().  Since you don't need breakpoints to
debug a crash that's fine.

Suggested-by: Will Deacon <will@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
This replaces the patch ("arm64: Add call_break_hook() to
early_brk64() for early kgdb") in my recent kgdb series [1].  If I end
up re-posting that series again I'll include this patch as a
replacement, but I'm sending it separately to avoid spamming a pile of
people another time with a 12-patch series.

Note that, because it doesn't select the "ARCH_HAS_EARLY_DEBUG"
KConfig option it could be landed standalone.  However, it's still
probably better to land together with that patch series.

If the kgdb_earlycon patch series lands without this patch then
kgdbwait + kgdb_earlycon won't work well on arm64, but there would be
no other bad side effects.

If this patch lands without the kgdb_earlycon patch series then there
will be no known problems.

[1] https://lore.kernel.org/r/20200507130644.v4.5.I22067ad43e77ddfd4b64c2d49030628480f9e8d9@changeid (local)

 arch/arm64/include/asm/debug-monitors.h | 2 ++
 arch/arm64/kernel/debug-monitors.c      | 4 +---
 arch/arm64/kernel/traps.c               | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
[...]

Acked-by: Will Deacon <will@kernel.org>

I would prefer to take this via arm64, if possible, since we have quite lot
going in for 5.8, although I don't think this conflicts at the moment.

Daniel -- what do you want to do?
I'm very happy for you to take it!

On my side I hope to get the rest of the patchset into linux-next early
next week.


Daniel.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help