From: Andrew Bresticker <abrestic@rivosinc.com>
To: Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Albert Ou <aou@eecs.berkeley.edu>
Cc: Atish Patra <atishp@rivosinc.com>, Guo Ren <guoren@kernel.org>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Andrew Bresticker <abrestic@rivosinc.com>
Subject: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
Date: Fri, 11 Nov 2022 17:31:08 -0500 [thread overview]
Message-ID: <20221111223108.1976562-1-abrestic@rivosinc.com> (raw)
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>
---
This should also theoretically be fixed by the conversion to generic entry,
but it's not clear how far away that series is from landing.
---
arch/riscv/kernel/entry.S | 18 +++++-------------
arch/riscv/kernel/signal.c | 34 +++++++++++++++++++++-------------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index b9eda3fcbd6d..58dfa8595e19 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -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:
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 5c591123c440..bfb2afa4135f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -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);
}
--
2.25.1
next reply other threads:[~2022-11-11 22:31 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-11 22:31 Andrew Bresticker [this message]
2022-11-12 23:52 ` Conor Dooley
2022-11-14 14:31 ` Andrew Bresticker
2022-11-14 14:42 ` Guo Ren
2022-11-14 15:20 ` Andrew Bresticker
2022-11-14 15:30 ` Conor Dooley
2022-11-14 15:42 ` Andrew Bresticker
2022-11-14 15:54 ` Guo Ren
2022-12-08 23:43 ` Palmer Dabbelt
2022-12-08 23:43 ` Palmer Dabbelt
2022-12-08 23:50 ` patchwork-bot+linux-riscv
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221111223108.1976562-1-abrestic@rivosinc.com \
--to=abrestic@rivosinc.com \
--cc=aou@eecs.berkeley.edu \
--cc=atishp@rivosinc.com \
--cc=guoren@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®