[PATCH 2/2] arm64: debug: Always update the IRQ tracing in debug_exception_enter()

From: Kevin Hao <haokexin@gmail.com>
To: linux-arm-kernel@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>
Subject: [PATCH 2/2] arm64: debug: Always update the IRQ tracing in debug_exception_enter()
Date: Fri, 17 Apr 2020 18:32:12 +0800	[thread overview]
Message-ID: <20200417103212.45812-3-haokexin@gmail.com> (raw)
In-Reply-To: <20200417103212.45812-1-haokexin@gmail.com>

When running the kgdb test suite, we get the following call trace:
  # echo kgdbts=V1 > /sys/module/kgdbts/parameters/kgdbts

  DEBUG_LOCKS_WARN_ON(current->hardirqs_enabled)
  WARNING: CPU: 10 PID: 697 at kernel/locking/lockdep.c:4793 check_flags.part.0+0x15c/0x180
  Modules linked in:
  CPU: 10 PID: 697 Comm: sh Not tainted 5.6.0-next-20200331-yoctodev-standard+ #341
  Hardware name: Marvell OcteonTX CN96XX board (DT)
  pstate: 604003c9 (nZCv DAIF +PAN -UAO)
  pc : check_flags.part.0+0x15c/0x180
  lr : check_flags.part.0+0x15c/0x180
  sp : ffff800017d0f830
  x29: ffff800017d0f830 x28: ffff000b936c0000
  x27: 00000000c28f5c29 x26: 00000000ffffffff
  x25: 00000000000003c0 x24: ffff800011af8dd0
  x23: 0000000000000000 x22: ffff8000119afdc0
  x21: ffff80001198bbe8 x20: ffff800011400018
  x19: ffff800012655000 x18: 0000000000000001
  x17: 0000000000000000 x16: 0000000000000000
  x15: ffff000b936c0470 x14: ffffffffffffffff
  x13: 0000000000000000 x12: ffff800012655000
  x11: ffff800017d0f830 x10: ffff800017d0f830
  x9 : 00000000000003c0 x8 : 6e655f7371726964
  x7 : 7261683e2d746e65 x6 : ffff8000126552fe
  x5 : 0000000000000000 x4 : 0000000000000000
  x3 : 00000000ffffffff x2 : 0000000000000000
  x1 : bbf8ef1cf7dda200 x0 : 0000000000000000
  Call trace:
   check_flags.part.0+0x15c/0x180
   lock_is_held_type+0xf0/0x120
   rcu_read_lock_sched_held+0x74/0x98
   trace_rcu_dyntick+0x1b8/0x1e0
   rcu_nmi_enter+0x7c/0xb8
   debug_exception_enter+0x68/0xe8
   do_debug_exception+0x60/0x150
   el1_sync_handler+0xd8/0xf8
   el1_sync+0x7c/0x100
   el1_irq+0x78/0x180
   kgdbts_break_test+0x0/0x40
   param_set_kgdbts_var+0x68/0xe8
   param_attr_store+0xb8/0x120
   module_attr_store+0x2c/0x48
   sysfs_kf_write+0x54/0x80
   kernfs_fop_write+0x154/0x248
   __vfs_write+0x24/0x50
   vfs_write+0xec/0x1d8
   ksys_write+0x74/0x100
   __arm64_sys_write+0x24/0x30
   do_el0_svc+0x8c/0x1e8
   el0_sync_handler+0x11c/0x198
   el0_sync+0x158/0x180
  irq event stamp: 76505
  hardirqs last  enabled at (76505): [<ffff80001009f9ec>] debug_exception_exit+0x54/0x68
  hardirqs last disabled at (76504): [<ffff80001009fb04>] debug_exception_enter+0xac/0xe8
  softirqs last  enabled at (76498): [<ffff8000100817b4>] __do_softirq+0x5a4/0x5ec
  softirqs last disabled at (76439): [<ffff8000100b6ff4>] irq_exit+0x13c/0x150

The reason is that an IRQ is emitted when doing the single step debug,
but in the IRQ handler we would enable the debug exception before
updating the IRQ tracing flags. This will cause the debug exception
running in an context which the IRQ state and IRQ tracing flags are
mismatched. And the debug exception handler only update the IRQ tracing
flag to off when it thinks that the debug emit in a IRQ enabled context.
Then we would get the above call trace if any code like check_flags()
is called in the debug exception handler's path. We can't fix this issue
by just shuffling the enable_da_f in el1_irq. As indicated by
Mark Rutland, we also can run into the same issue when we set a
breakpoint in the middle of local_irq_disable(). So we should invoke the
trace_hardirqs_off() unconditionally in debug_exception_enter().
Also the reason why we conditionally invoke trace_hardirqs_off() seems
invalid since the redundant invoking of trace_hardirqs_off() would not
overwrite the last IRQ disabled address tracked by IRQ tracing.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/arm64/mm/fault.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c9cedc0432d2..b3c6a2e9232e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -789,12 +789,8 @@ void __init hook_debug_fault_code(int nr,
  */
 static void debug_exception_enter(struct pt_regs *regs)
 {
-	/*
-	 * Tell lockdep we disabled irqs in entry.S. Do nothing if they were
-	 * already disabled to preserve the last enabled/disabled addresses.
-	 */
-	if (interrupts_enabled(regs))
-		trace_hardirqs_off();
+	/* Tell lockdep we disabled irqs in entry.S. */
+	trace_hardirqs_off();
 
 	if (user_mode(regs)) {
 		RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
-- 
2.26.0


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