Re: [PATCH] arm kgdb: fix breakpoint for thumb2

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Feb 14, 2022 at 12:05:56PM +0100, Johannes Stezenbach wrote:
> On Mon, Feb 14, 2022 at 10:13:43AM +0000, Russell King (Oracle) wrote:
> > On Mon, Feb 14, 2022 at 10:59:49AM +0100, Johannes Stezenbach wrote:
> > > Entering kdb via SysRq-G with CONFIG_THUMB2_KERNEL=y
> > > on Cortex-A7 in Qemu results in an Ooops, and it is
> > > not possible to continue because of "Catastrophic error detected".
> > > The root cause is using an arm breakpoint instruction in
> > > thumb code.
> > 
> > This sounds like a bug in qemu.
> > 
> > 0xe7ffdeXX is two 16-bit instructions:
> > 
> >    0:   e7ff            b.n     2 <.text+0x2>
> >    2:   def1            udf     #241    ; 0xf1
> > 
> > 0xe7ff is a branch to the UDF instruction.
> > 
> > 0xdeXX is a UDF (Permanently undefined) instruction which should raise a
> > undefined instruction trap. As per the Arm ARM on UDF: "Permanently
> > Undefined generates an Undefined Instruction exception."
> > 
> > The encoding is also a 32-bit UDF instruction:
> > 
> >    0:   e7ffdef1        udf     #64993  ; 0xfde1
> > 
> > which is exactly why these opcodes were chosen - so we can instrument
> > both ARM and Thumb code without caring which it is.
> > 
> > Qemu needs fixing if it complains about this.
> 
> My apologies for the incomplete problem description.
> 
> Qemu is not complaining at all and correctly raises the invalid
> instruction exception. But the kgdb_compiled_brkpt_hook undef_hook
> only sees the 0xdef1 part and thus does not match.

The kgdb code needs to register an undef hook for the Thumb UDF
instruction that will fault, in addition to the ARM version. Probably
something like this (untested). Please let me know if this works.

diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index 7bd30c0a4280..a3602cacda0b 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -154,22 +154,38 @@ static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int instr)
 	return 0;
 }
 
-static struct undef_hook kgdb_brkpt_hook = {
+static struct undef_hook kgdb_brkpt_arm_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_BREAKINST,
-	.cpsr_mask		= MODE_MASK,
+	.cpsr_mask		= PSR_T_BIT | MODE_MASK,
 	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_brk_fn
 };
 
-static struct undef_hook kgdb_compiled_brkpt_hook = {
+static struct undef_hook kgdb_brkpt_thumb_hook = {
+	.instr_mask		= 0xffff,
+	.instr_val		= KGDB_BREAKINST & 0xffff,
+	.cpsr_mask		= PSR_T_BIT | MODE_MASK,
+	.cpsr_val		= PSR_T_BIT | SVC_MODE,
+	.fn			= kgdb_brk_fn
+};
+
+static struct undef_hook kgdb_compiled_brkpt_arm_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_COMPILED_BREAK,
-	.cpsr_mask		= MODE_MASK,
+	.cpsr_mask		= PSR_T_BIT | MODE_MASK,
 	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_compiled_brk_fn
 };
 
+static struct undef_hook kgdb_compiled_brkpt_thumb_hook = {
+	.instr_mask		= 0xffff,
+	.instr_val		= KGDB_COMPILED_BREAK & 0xffff,
+	.cpsr_mask		= PSR_T_BIT | MODE_MASK,
+	.cpsr_val		= PSR_T_BIT | SVC_MODE,
+	.fn			= kgdb_compiled_brk_fn
+};
+
 static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 {
 	struct pt_regs *regs = args->regs;
@@ -210,8 +226,10 @@ int kgdb_arch_init(void)
 	if (ret != 0)
 		return ret;
 
-	register_undef_hook(&kgdb_brkpt_hook);
-	register_undef_hook(&kgdb_compiled_brkpt_hook);
+	register_undef_hook(&kgdb_brkpt_arm_hook);
+	register_undef_hook(&kgdb_brkpt_thumb_hook);
+	register_undef_hook(&kgdb_compiled_brkpt_arm_hook);
+	register_undef_hook(&kgdb_compiled_brkpt_thumb_hook);
 
 	return 0;
 }

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel