RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
authorAndrew Bresticker <abrestic@rivosinc.com>
Fri, 11 Nov 2022 22:31:08 +0000 (17:31 -0500)
committerPalmer Dabbelt <palmer@rivosinc.com>
Thu, 8 Dec 2022 23:31:41 +0000 (15:31 -0800)
The return to userspace path in entry.S may enable interrupts without the
corresponding lockdep annotation, producing a splat[0] when DEBUG_LOCKDEP
is enabled. Simply calling __trace_hardirqs_on() here gets a bit messy
due to the use of RA to point back to ret_from_exception, so just move
the whole slow-path loop into C. It's more readable and it lets us use
local_irq_{enable,disable}(), avoiding the need for manual annotations
altogether.

[0]:
  ------------[ cut here ]------------
  DEBUG_LOCKS_WARN_ON(!lockdep_hardirqs_enabled())
  WARNING: CPU: 2 PID: 1 at kernel/locking/lockdep.c:5512 check_flags+0x10a/0x1e0
  Modules linked in:
  CPU: 2 PID: 1 Comm: init Not tainted 6.1.0-rc4-00160-gb56b6e2b4f31 #53
  Hardware name: riscv-virtio,qemu (DT)
  epc : check_flags+0x10a/0x1e0
  ra : check_flags+0x10a/0x1e0
  <snip>
   status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
  [<ffffffff808edb90>] lock_is_held_type+0x78/0x14e
  [<ffffffff8003dae2>] __might_resched+0x26/0x22c
  [<ffffffff8003dd24>] __might_sleep+0x3c/0x66
  [<ffffffff80022c60>] get_signal+0x9e/0xa70
  [<ffffffff800054a2>] do_notify_resume+0x6e/0x422
  [<ffffffff80003c68>] ret_from_exception+0x0/0x10
  irq event stamp: 44512
  hardirqs last  enabled at (44511): [<ffffffff808f901c>] _raw_spin_unlock_irqrestore+0x54/0x62
  hardirqs last disabled at (44512): [<ffffffff80008200>] __trace_hardirqs_off+0xc/0x14
  softirqs last  enabled at (44472): [<ffffffff808f9fbe>] __do_softirq+0x3de/0x51e
  softirqs last disabled at (44467): [<ffffffff80017760>] irq_exit+0xd6/0x104
  ---[ end trace 0000000000000000 ]---
  possible reason: unannotated irqs-on.

Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>
Fixes: 3c4697982982 ("riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT")
Link: https://lore.kernel.org/r/20221111223108.1976562-1-abrestic@rivosinc.com
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
arch/riscv/kernel/entry.S
arch/riscv/kernel/signal.c

index b9eda3f..58dfa85 100644 (file)
@@ -263,12 +263,11 @@ ret_from_exception:
 #endif
        bnez s0, resume_kernel
 
-resume_userspace:
        /* Interrupts must be disabled here so flags are checked atomically */
        REG_L s0, TASK_TI_FLAGS(tp) /* current_thread_info->flags */
        andi s1, s0, _TIF_WORK_MASK
-       bnez s1, work_pending
-
+       bnez s1, resume_userspace_slow
+resume_userspace:
 #ifdef CONFIG_CONTEXT_TRACKING_USER
        call user_enter_callable
 #endif
@@ -368,19 +367,12 @@ resume_kernel:
        j restore_all
 #endif
 
-work_pending:
+resume_userspace_slow:
        /* Enter slow path for supplementary processing */
-       la ra, ret_from_exception
-       andi s1, s0, _TIF_NEED_RESCHED
-       bnez s1, work_resched
-work_notifysig:
-       /* Handle pending signals and notify-resume requests */
-       csrs CSR_STATUS, SR_IE /* Enable interrupts for do_notify_resume() */
        move a0, sp /* pt_regs */
        move a1, s0 /* current_thread_info->flags */
-       tail do_notify_resume
-work_resched:
-       tail schedule
+       call do_work_pending
+       j resume_userspace
 
 /* Slow paths for ptrace. */
 handle_syscall_trace_enter:
index 5c59112..bfb2afa 100644 (file)
@@ -313,19 +313,27 @@ static void do_signal(struct pt_regs *regs)
 }
 
 /*
- * notification of userspace execution resumption
- * - triggered by the _TIF_WORK_MASK flags
+ * Handle any pending work on the resume-to-userspace path, as indicated by
+ * _TIF_WORK_MASK. Entered from assembly with IRQs off.
  */
-asmlinkage __visible void do_notify_resume(struct pt_regs *regs,
-                                          unsigned long thread_info_flags)
+asmlinkage __visible void do_work_pending(struct pt_regs *regs,
+                                         unsigned long thread_info_flags)
 {
-       if (thread_info_flags & _TIF_UPROBE)
-               uprobe_notify_resume(regs);
-
-       /* Handle pending signal delivery */
-       if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
-               do_signal(regs);
-
-       if (thread_info_flags & _TIF_NOTIFY_RESUME)
-               resume_user_mode_work(regs);
+       do {
+               if (thread_info_flags & _TIF_NEED_RESCHED) {
+                       schedule();
+               } else {
+                       local_irq_enable();
+                       if (thread_info_flags & _TIF_UPROBE)
+                               uprobe_notify_resume(regs);
+                       /* Handle pending signal delivery */
+                       if (thread_info_flags & (_TIF_SIGPENDING |
+                                                _TIF_NOTIFY_SIGNAL))
+                               do_signal(regs);
+                       if (thread_info_flags & _TIF_NOTIFY_RESUME)
+                               resume_user_mode_work(regs);
+               }
+               local_irq_disable();
+               thread_info_flags = read_thread_flags();
+       } while (thread_info_flags & _TIF_WORK_MASK);
 }