Re: [PATCH] ARM: enable interrupts when arm_notify_die() is handling user mode errors

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-06-25 10:05:52 [+0100], Russell King wrote:
> > for this but actual breakpoint handling might be broken or is it just
> > me? But then your stack trace looks like mine so :/
> 
> ARM Linux doesn't use BKPT. BKPT was an instruction introduced by Arm
> Ltd in ARMv5TE. Prior to this, we use a UDF instruction instead (we
> had to pick something!) and gdb and other tools use that as a
> breapoint.
> 
> Moreover, BKPT isn't guaranteed to trap to the kernel, especially when
> there is a hardware debugger connected. In that case, DDI0100E states
> that use of BKPT must be according to the instructions provided with
> the hardware debugger. This makes BKPT unsuitable for use.

So you are saying this:

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index e62cc4be5adf6..11ac69113eca2 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -595,6 +595,16 @@ do_bad(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	return 1;
 }
 
+static int do_debug_event(unsigned long addr, unsigned int fsr,
+			  struct pt_regs *regs)
+{
+	if (!user_mode(regs))
+		return 1;
+	local_irq_enable();
+	ptrace_break(regs);
+	return 0;
+}
+
 struct fsr_info {
 	int	(*fn)(unsigned long addr, unsigned int fsr, struct pt_regs *regs);
 	int	sig;
diff --git a/arch/arm/mm/fsr-2level.c b/arch/arm/mm/fsr-2level.c
index f2be95197265d..bfd718f64020c 100644
--- a/arch/arm/mm/fsr-2level.c
+++ b/arch/arm/mm/fsr-2level.c
@@ -46,7 +46,7 @@ static struct fsr_info fsr_info[] = {
 static struct fsr_info ifsr_info[] = {
 	{ do_bad,		SIGBUS,  0,		"unknown 0"			   },
 	{ do_bad,		SIGBUS,  0,		"unknown 1"			   },
-	{ do_bad,		SIGBUS,  0,		"debug event"			   },
+	{ do_debug_event,	SIGBUS,  0,		"debug event"			   },
 	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"section access flag fault"	   },
 	{ do_bad,		SIGBUS,  0,		"unknown 4"			   },
 	{ do_translation_fault,	SIGSEGV, SEGV_MAPERR,	"section translation fault"	   },
diff --git a/arch/arm/mm/fsr-3level.c b/arch/arm/mm/fsr-3level.c
index d0ae2963656a6..96c1d45d20d9e 100644
--- a/arch/arm/mm/fsr-3level.c
+++ b/arch/arm/mm/fsr-3level.c
@@ -34,7 +34,7 @@ static struct fsr_info fsr_info[] = {
 	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk" },
 	{ do_bad,		SIGBUS,  0,		"unknown 32"			},
 	{ do_bad,		SIGBUS,  BUS_ADRALN,	"alignment fault"		},
-	{ do_bad,		SIGBUS,  0,		"debug event"			},
+	{ do_debug_event,	SIGBUS,  0,		"debug event"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 35"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 36"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 37"			},

is not worth doing it? With this I can my little testcase working.

> Consequently, ARM Linux has not supported the use of BKPT - no code
> was added to support this instruction, hence why the kernel prints an
> Alert level message stating that the fault was unhandled.
> 
> In addition, when a hardware debugger is not being used, with the
> addition of hw_breakpoint.c, what userland sees in response depends on
> kernel configuration. If hw_breakpoint.c is not built (when PERF_EVENTS
> is disabled), then a SIGBUS signal will be raised. If it is built, and
> prior to a recent commit by LinusW, it will raise a SIGTRAP. After
> LinusW's commit, it won't raise a signal, but userspace will spin on
> the BKPT instruction.
> 
> The path we go through in the above case is very much an "oh damn,
> we aren't handling this exception, let's try to do something that
> might save the day".

Okay.

> Rather than throwing local_irq_enable() in random places, it would
> be better to do it earlier, but I would want to review what x86 does
> when it gets an exception that the kernel doesn't handle - not sure
> when I'll get around to doing that though.

That would be exc_int3() from arch/x86/kernel/traps.c.
Besides doing "notify_die(DIE_INT3, "int3", regs, 0, X86_TRAP_BP, SIGTRAP);"

it does cond_local_irq_enable() which enables the interrupts if they
were enabled by the "caller", sends the signal (SIGTRAP).

Sebastian