* [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-11 22:31 ` Andrew Bresticker
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-11 22:31 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou
Cc: Atish Patra, Guo Ren, linux-riscv, linux-kernel, Andrew Bresticker
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-11 22:31 ` Andrew Bresticker
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-11 22:31 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou
Cc: Atish Patra, Guo Ren, linux-riscv, linux-kernel, Andrew Bresticker
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-11 22:31 ` Andrew Bresticker
@ 2022-11-12 23:52 ` Conor Dooley
-1 siblings, 0 replies; 22+ messages in thread
From: Conor Dooley @ 2022-11-12 23:52 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra, Guo Ren,
linux-riscv, linux-kernel
On Fri, Nov 11, 2022 at 05:31:08PM -0500, Andrew Bresticker wrote:
> 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.
Heh.. In case this makes the cut instead - what commit does this fix?
> ---
> 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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-12 23:52 ` Conor Dooley
0 siblings, 0 replies; 22+ messages in thread
From: Conor Dooley @ 2022-11-12 23:52 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra, Guo Ren,
linux-riscv, linux-kernel
On Fri, Nov 11, 2022 at 05:31:08PM -0500, Andrew Bresticker wrote:
> 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.
Heh.. In case this makes the cut instead - what commit does this fix?
> ---
> 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
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-12 23:52 ` Conor Dooley
@ 2022-11-14 14:31 ` Andrew Bresticker
-1 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 14:31 UTC (permalink / raw)
To: Conor Dooley
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra, Guo Ren,
linux-riscv, linux-kernel
On Sat, Nov 12, 2022 at 6:52 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Nov 11, 2022 at 05:31:08PM -0500, Andrew Bresticker wrote:
> > 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.
>
> Heh.. In case this makes the cut instead - what commit does this fix?
Ah, right --
Fixes: 3c4697982982 ("riscv: Enable LOCKDEP_SUPPORT & fixup
TRACE_IRQFLAGS_SUPPORT")
-Andrew
>
> > ---
> > 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
> >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 14:31 ` Andrew Bresticker
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 14:31 UTC (permalink / raw)
To: Conor Dooley
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra, Guo Ren,
linux-riscv, linux-kernel
On Sat, Nov 12, 2022 at 6:52 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Nov 11, 2022 at 05:31:08PM -0500, Andrew Bresticker wrote:
> > 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.
>
> Heh.. In case this makes the cut instead - what commit does this fix?
Ah, right --
Fixes: 3c4697982982 ("riscv: Enable LOCKDEP_SUPPORT & fixup
TRACE_IRQFLAGS_SUPPORT")
-Andrew
>
> > ---
> > 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
> >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-11 22:31 ` Andrew Bresticker
@ 2022-11-14 14:42 ` Guo Ren
-1 siblings, 0 replies; 22+ messages in thread
From: Guo Ren @ 2022-11-14 14:42 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
>
> 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.
Could generic_entry solve your problem? please try:
https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>
> [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);
> }
The more graceful code has been written in kernel/entry/common.c.
Let's base it on that:
static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
unsigned long ti_work)
{
/*
* Before returning to user space ensure that all pending work
* items have been completed.
*/
while (ti_work & EXIT_TO_USER_MODE_WORK) {
local_irq_enable_exit_to_user(ti_work);
if (ti_work & _TIF_NEED_RESCHED)
schedule();
if (ti_work & _TIF_UPROBE)
uprobe_notify_resume(regs);
if (ti_work & _TIF_PATCH_PENDING)
klp_update_patch_state(current);
if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
arch_do_signal_or_restart(regs);
if (ti_work & _TIF_NOTIFY_RESUME)
resume_user_mode_work(regs);
/* Architecture specific TIF work */
arch_exit_to_user_mode_work(regs, ti_work);
/*
* Disable interrupts and reevaluate the work flags as they
* might have changed while interrupts and preemption was
* enabled above.
*/
local_irq_disable_exit_to_user();
/* Check if any of the above work has queued a
deferred wakeup */
tick_nohz_user_enter_prepare();
ti_work = read_thread_flags();
}
/* Return the latest work state for arch_exit_to_user_mode() */
return ti_work;
}
> --
> 2.25.1
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 14:42 ` Guo Ren
0 siblings, 0 replies; 22+ messages in thread
From: Guo Ren @ 2022-11-14 14:42 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
>
> 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.
Could generic_entry solve your problem? please try:
https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>
> [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);
> }
The more graceful code has been written in kernel/entry/common.c.
Let's base it on that:
static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
unsigned long ti_work)
{
/*
* Before returning to user space ensure that all pending work
* items have been completed.
*/
while (ti_work & EXIT_TO_USER_MODE_WORK) {
local_irq_enable_exit_to_user(ti_work);
if (ti_work & _TIF_NEED_RESCHED)
schedule();
if (ti_work & _TIF_UPROBE)
uprobe_notify_resume(regs);
if (ti_work & _TIF_PATCH_PENDING)
klp_update_patch_state(current);
if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
arch_do_signal_or_restart(regs);
if (ti_work & _TIF_NOTIFY_RESUME)
resume_user_mode_work(regs);
/* Architecture specific TIF work */
arch_exit_to_user_mode_work(regs, ti_work);
/*
* Disable interrupts and reevaluate the work flags as they
* might have changed while interrupts and preemption was
* enabled above.
*/
local_irq_disable_exit_to_user();
/* Check if any of the above work has queued a
deferred wakeup */
tick_nohz_user_enter_prepare();
ti_work = read_thread_flags();
}
/* Return the latest work state for arch_exit_to_user_mode() */
return ti_work;
}
> --
> 2.25.1
>
--
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-14 14:42 ` Guo Ren
@ 2022-11-14 15:20 ` Andrew Bresticker
-1 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 15:20 UTC (permalink / raw)
To: Guo Ren
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
>
> On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> >
> > 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.
> Could generic_entry solve your problem? please try:
> https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
Indeed it does, as I mentioned below, however it wasn't clear to me
how close your series was to landing so I'v emailed this small fix for
the existing bug in case your series does not make it into 6.2
-Andrew
>
> >
> > [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);
> > }
> The more graceful code has been written in kernel/entry/common.c.
> Let's base it on that:
>
> static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> unsigned long ti_work)
> {
> /*
> * Before returning to user space ensure that all pending work
> * items have been completed.
> */
> while (ti_work & EXIT_TO_USER_MODE_WORK) {
>
> local_irq_enable_exit_to_user(ti_work);
>
> if (ti_work & _TIF_NEED_RESCHED)
> schedule();
>
> if (ti_work & _TIF_UPROBE)
> uprobe_notify_resume(regs);
>
> if (ti_work & _TIF_PATCH_PENDING)
> klp_update_patch_state(current);
>
> if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> arch_do_signal_or_restart(regs);
>
> if (ti_work & _TIF_NOTIFY_RESUME)
> resume_user_mode_work(regs);
>
> /* Architecture specific TIF work */
> arch_exit_to_user_mode_work(regs, ti_work);
>
> /*
> * Disable interrupts and reevaluate the work flags as they
> * might have changed while interrupts and preemption was
> * enabled above.
> */
> local_irq_disable_exit_to_user();
>
> /* Check if any of the above work has queued a
> deferred wakeup */
> tick_nohz_user_enter_prepare();
>
> ti_work = read_thread_flags();
> }
>
> /* Return the latest work state for arch_exit_to_user_mode() */
> return ti_work;
> }
>
> > --
> > 2.25.1
> >
>
>
> --
> Best Regards
> Guo Ren
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 15:20 ` Andrew Bresticker
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 15:20 UTC (permalink / raw)
To: Guo Ren
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
>
> On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> >
> > 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.
> Could generic_entry solve your problem? please try:
> https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
Indeed it does, as I mentioned below, however it wasn't clear to me
how close your series was to landing so I'v emailed this small fix for
the existing bug in case your series does not make it into 6.2
-Andrew
>
> >
> > [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);
> > }
> The more graceful code has been written in kernel/entry/common.c.
> Let's base it on that:
>
> static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> unsigned long ti_work)
> {
> /*
> * Before returning to user space ensure that all pending work
> * items have been completed.
> */
> while (ti_work & EXIT_TO_USER_MODE_WORK) {
>
> local_irq_enable_exit_to_user(ti_work);
>
> if (ti_work & _TIF_NEED_RESCHED)
> schedule();
>
> if (ti_work & _TIF_UPROBE)
> uprobe_notify_resume(regs);
>
> if (ti_work & _TIF_PATCH_PENDING)
> klp_update_patch_state(current);
>
> if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> arch_do_signal_or_restart(regs);
>
> if (ti_work & _TIF_NOTIFY_RESUME)
> resume_user_mode_work(regs);
>
> /* Architecture specific TIF work */
> arch_exit_to_user_mode_work(regs, ti_work);
>
> /*
> * Disable interrupts and reevaluate the work flags as they
> * might have changed while interrupts and preemption was
> * enabled above.
> */
> local_irq_disable_exit_to_user();
>
> /* Check if any of the above work has queued a
> deferred wakeup */
> tick_nohz_user_enter_prepare();
>
> ti_work = read_thread_flags();
> }
>
> /* Return the latest work state for arch_exit_to_user_mode() */
> return ti_work;
> }
>
> > --
> > 2.25.1
> >
>
>
> --
> Best Regards
> Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-14 15:20 ` Andrew Bresticker
@ 2022-11-14 15:30 ` Conor Dooley
-1 siblings, 0 replies; 22+ messages in thread
From: Conor Dooley @ 2022-11-14 15:30 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Guo Ren, Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > >
> > > 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.
> > Could generic_entry solve your problem? please try:
> > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>
> Indeed it does, as I mentioned below, however it wasn't clear to me
> how close your series was to landing so I'v emailed this small fix for
> the existing bug in case your series does not make it into 6.2
The backportablilty of this 26+, 26- change has to be considered too.
The generic entry patchset has 20x the changes and would not ordinarily
be stable material. How much of that series would be required to solve
the problem?
> >
> > >
> > > [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);
> > > }
> > The more graceful code has been written in kernel/entry/common.c.
> > Let's base it on that:
> >
> > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > unsigned long ti_work)
> > {
> > /*
> > * Before returning to user space ensure that all pending work
> > * items have been completed.
> > */
> > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> >
> > local_irq_enable_exit_to_user(ti_work);
> >
> > if (ti_work & _TIF_NEED_RESCHED)
> > schedule();
> >
> > if (ti_work & _TIF_UPROBE)
> > uprobe_notify_resume(regs);
> >
> > if (ti_work & _TIF_PATCH_PENDING)
> > klp_update_patch_state(current);
> >
> > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > arch_do_signal_or_restart(regs);
> >
> > if (ti_work & _TIF_NOTIFY_RESUME)
> > resume_user_mode_work(regs);
> >
> > /* Architecture specific TIF work */
> > arch_exit_to_user_mode_work(regs, ti_work);
> >
> > /*
> > * Disable interrupts and reevaluate the work flags as they
> > * might have changed while interrupts and preemption was
> > * enabled above.
> > */
> > local_irq_disable_exit_to_user();
> >
> > /* Check if any of the above work has queued a
> > deferred wakeup */
> > tick_nohz_user_enter_prepare();
> >
> > ti_work = read_thread_flags();
> > }
> >
> > /* Return the latest work state for arch_exit_to_user_mode() */
> > return ti_work;
> > }
> >
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 15:30 ` Conor Dooley
0 siblings, 0 replies; 22+ messages in thread
From: Conor Dooley @ 2022-11-14 15:30 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Guo Ren, Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > >
> > > 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.
> > Could generic_entry solve your problem? please try:
> > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>
> Indeed it does, as I mentioned below, however it wasn't clear to me
> how close your series was to landing so I'v emailed this small fix for
> the existing bug in case your series does not make it into 6.2
The backportablilty of this 26+, 26- change has to be considered too.
The generic entry patchset has 20x the changes and would not ordinarily
be stable material. How much of that series would be required to solve
the problem?
> >
> > >
> > > [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);
> > > }
> > The more graceful code has been written in kernel/entry/common.c.
> > Let's base it on that:
> >
> > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > unsigned long ti_work)
> > {
> > /*
> > * Before returning to user space ensure that all pending work
> > * items have been completed.
> > */
> > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> >
> > local_irq_enable_exit_to_user(ti_work);
> >
> > if (ti_work & _TIF_NEED_RESCHED)
> > schedule();
> >
> > if (ti_work & _TIF_UPROBE)
> > uprobe_notify_resume(regs);
> >
> > if (ti_work & _TIF_PATCH_PENDING)
> > klp_update_patch_state(current);
> >
> > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > arch_do_signal_or_restart(regs);
> >
> > if (ti_work & _TIF_NOTIFY_RESUME)
> > resume_user_mode_work(regs);
> >
> > /* Architecture specific TIF work */
> > arch_exit_to_user_mode_work(regs, ti_work);
> >
> > /*
> > * Disable interrupts and reevaluate the work flags as they
> > * might have changed while interrupts and preemption was
> > * enabled above.
> > */
> > local_irq_disable_exit_to_user();
> >
> > /* Check if any of the above work has queued a
> > deferred wakeup */
> > tick_nohz_user_enter_prepare();
> >
> > ti_work = read_thread_flags();
> > }
> >
> > /* Return the latest work state for arch_exit_to_user_mode() */
> > return ti_work;
> > }
> >
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-14 15:30 ` Conor Dooley
@ 2022-11-14 15:42 ` Andrew Bresticker
-1 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 15:42 UTC (permalink / raw)
To: Conor Dooley
Cc: Guo Ren, Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
<conor.dooley@microchip.com> wrote:
>
> On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > > >
> > > > 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.
> > > Could generic_entry solve your problem? please try:
> > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
> >
> > Indeed it does, as I mentioned below, however it wasn't clear to me
> > how close your series was to landing so I'v emailed this small fix for
> > the existing bug in case your series does not make it into 6.2
>
> The backportablilty of this 26+, 26- change has to be considered too.
> The generic entry patchset has 20x the changes and would not ordinarily
> be stable material. How much of that series would be required to solve
> the problem?
We'd need up to patch 6, "riscv: convert to generic entry":
https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
-Andrew
>
> > >
> > > >
> > > > [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);
> > > > }
> > > The more graceful code has been written in kernel/entry/common.c.
> > > Let's base it on that:
> > >
> > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > > unsigned long ti_work)
> > > {
> > > /*
> > > * Before returning to user space ensure that all pending work
> > > * items have been completed.
> > > */
> > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> > >
> > > local_irq_enable_exit_to_user(ti_work);
> > >
> > > if (ti_work & _TIF_NEED_RESCHED)
> > > schedule();
> > >
> > > if (ti_work & _TIF_UPROBE)
> > > uprobe_notify_resume(regs);
> > >
> > > if (ti_work & _TIF_PATCH_PENDING)
> > > klp_update_patch_state(current);
> > >
> > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > > arch_do_signal_or_restart(regs);
> > >
> > > if (ti_work & _TIF_NOTIFY_RESUME)
> > > resume_user_mode_work(regs);
> > >
> > > /* Architecture specific TIF work */
> > > arch_exit_to_user_mode_work(regs, ti_work);
> > >
> > > /*
> > > * Disable interrupts and reevaluate the work flags as they
> > > * might have changed while interrupts and preemption was
> > > * enabled above.
> > > */
> > > local_irq_disable_exit_to_user();
> > >
> > > /* Check if any of the above work has queued a
> > > deferred wakeup */
> > > tick_nohz_user_enter_prepare();
> > >
> > > ti_work = read_thread_flags();
> > > }
> > >
> > > /* Return the latest work state for arch_exit_to_user_mode() */
> > > return ti_work;
> > > }
> > >
> > > > --
> > > > 2.25.1
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 15:42 ` Andrew Bresticker
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Bresticker @ 2022-11-14 15:42 UTC (permalink / raw)
To: Conor Dooley
Cc: Guo Ren, Palmer Dabbelt, Paul Walmsley, Albert Ou, Atish Patra,
linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
<conor.dooley@microchip.com> wrote:
>
> On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > > >
> > > > 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.
> > > Could generic_entry solve your problem? please try:
> > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
> >
> > Indeed it does, as I mentioned below, however it wasn't clear to me
> > how close your series was to landing so I'v emailed this small fix for
> > the existing bug in case your series does not make it into 6.2
>
> The backportablilty of this 26+, 26- change has to be considered too.
> The generic entry patchset has 20x the changes and would not ordinarily
> be stable material. How much of that series would be required to solve
> the problem?
We'd need up to patch 6, "riscv: convert to generic entry":
https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
-Andrew
>
> > >
> > > >
> > > > [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);
> > > > }
> > > The more graceful code has been written in kernel/entry/common.c.
> > > Let's base it on that:
> > >
> > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > > unsigned long ti_work)
> > > {
> > > /*
> > > * Before returning to user space ensure that all pending work
> > > * items have been completed.
> > > */
> > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> > >
> > > local_irq_enable_exit_to_user(ti_work);
> > >
> > > if (ti_work & _TIF_NEED_RESCHED)
> > > schedule();
> > >
> > > if (ti_work & _TIF_UPROBE)
> > > uprobe_notify_resume(regs);
> > >
> > > if (ti_work & _TIF_PATCH_PENDING)
> > > klp_update_patch_state(current);
> > >
> > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > > arch_do_signal_or_restart(regs);
> > >
> > > if (ti_work & _TIF_NOTIFY_RESUME)
> > > resume_user_mode_work(regs);
> > >
> > > /* Architecture specific TIF work */
> > > arch_exit_to_user_mode_work(regs, ti_work);
> > >
> > > /*
> > > * Disable interrupts and reevaluate the work flags as they
> > > * might have changed while interrupts and preemption was
> > > * enabled above.
> > > */
> > > local_irq_disable_exit_to_user();
> > >
> > > /* Check if any of the above work has queued a
> > > deferred wakeup */
> > > tick_nohz_user_enter_prepare();
> > >
> > > ti_work = read_thread_flags();
> > > }
> > >
> > > /* Return the latest work state for arch_exit_to_user_mode() */
> > > return ti_work;
> > > }
> > >
> > > > --
> > > > 2.25.1
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-14 15:42 ` Andrew Bresticker
@ 2022-11-14 15:54 ` Guo Ren
-1 siblings, 0 replies; 22+ messages in thread
From: Guo Ren @ 2022-11-14 15:54 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Conor Dooley, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Atish Patra, linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 11:42 PM Andrew Bresticker
<abrestic@rivosinc.com> wrote:
>
> On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
> <conor.dooley@microchip.com> wrote:
> >
> > On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> > > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > > > >
> > > > > 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.
> > > > Could generic_entry solve your problem? please try:
> > > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
> > >
> > > Indeed it does, as I mentioned below, however it wasn't clear to me
> > > how close your series was to landing so I'v emailed this small fix for
> > > the existing bug in case your series does not make it into 6.2
> >
> > The backportablilty of this 26+, 26- change has to be considered too.
> > The generic entry patchset has 20x the changes and would not ordinarily
> > be stable material. How much of that series would be required to solve
> > the problem?
>
> We'd need up to patch 6, "riscv: convert to generic entry":
> https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
Yes, but 6 needs 1 and 5, and they are very small patches.
>
> -Andrew
>
> >
> > > >
> > > > >
> > > > > [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);
> > > > > }
> > > > The more graceful code has been written in kernel/entry/common.c.
> > > > Let's base it on that:
> > > >
> > > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > > > unsigned long ti_work)
> > > > {
> > > > /*
> > > > * Before returning to user space ensure that all pending work
> > > > * items have been completed.
> > > > */
> > > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> > > >
> > > > local_irq_enable_exit_to_user(ti_work);
> > > >
> > > > if (ti_work & _TIF_NEED_RESCHED)
> > > > schedule();
> > > >
> > > > if (ti_work & _TIF_UPROBE)
> > > > uprobe_notify_resume(regs);
> > > >
> > > > if (ti_work & _TIF_PATCH_PENDING)
> > > > klp_update_patch_state(current);
> > > >
> > > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > > > arch_do_signal_or_restart(regs);
> > > >
> > > > if (ti_work & _TIF_NOTIFY_RESUME)
> > > > resume_user_mode_work(regs);
> > > >
> > > > /* Architecture specific TIF work */
> > > > arch_exit_to_user_mode_work(regs, ti_work);
> > > >
> > > > /*
> > > > * Disable interrupts and reevaluate the work flags as they
> > > > * might have changed while interrupts and preemption was
> > > > * enabled above.
> > > > */
> > > > local_irq_disable_exit_to_user();
> > > >
> > > > /* Check if any of the above work has queued a
> > > > deferred wakeup */
> > > > tick_nohz_user_enter_prepare();
> > > >
> > > > ti_work = read_thread_flags();
> > > > }
> > > >
> > > > /* Return the latest work state for arch_exit_to_user_mode() */
> > > > return ti_work;
> > > > }
> > > >
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-11-14 15:54 ` Guo Ren
0 siblings, 0 replies; 22+ messages in thread
From: Guo Ren @ 2022-11-14 15:54 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Conor Dooley, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Atish Patra, linux-riscv, linux-kernel
On Mon, Nov 14, 2022 at 11:42 PM Andrew Bresticker
<abrestic@rivosinc.com> wrote:
>
> On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
> <conor.dooley@microchip.com> wrote:
> >
> > On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
> > > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
> > > > >
> > > > > 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.
> > > > Could generic_entry solve your problem? please try:
> > > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
> > >
> > > Indeed it does, as I mentioned below, however it wasn't clear to me
> > > how close your series was to landing so I'v emailed this small fix for
> > > the existing bug in case your series does not make it into 6.2
> >
> > The backportablilty of this 26+, 26- change has to be considered too.
> > The generic entry patchset has 20x the changes and would not ordinarily
> > be stable material. How much of that series would be required to solve
> > the problem?
>
> We'd need up to patch 6, "riscv: convert to generic entry":
> https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
Yes, but 6 needs 1 and 5, and they are very small patches.
>
> -Andrew
>
> >
> > > >
> > > > >
> > > > > [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);
> > > > > }
> > > > The more graceful code has been written in kernel/entry/common.c.
> > > > Let's base it on that:
> > > >
> > > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> > > > unsigned long ti_work)
> > > > {
> > > > /*
> > > > * Before returning to user space ensure that all pending work
> > > > * items have been completed.
> > > > */
> > > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
> > > >
> > > > local_irq_enable_exit_to_user(ti_work);
> > > >
> > > > if (ti_work & _TIF_NEED_RESCHED)
> > > > schedule();
> > > >
> > > > if (ti_work & _TIF_UPROBE)
> > > > uprobe_notify_resume(regs);
> > > >
> > > > if (ti_work & _TIF_PATCH_PENDING)
> > > > klp_update_patch_state(current);
> > > >
> > > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> > > > arch_do_signal_or_restart(regs);
> > > >
> > > > if (ti_work & _TIF_NOTIFY_RESUME)
> > > > resume_user_mode_work(regs);
> > > >
> > > > /* Architecture specific TIF work */
> > > > arch_exit_to_user_mode_work(regs, ti_work);
> > > >
> > > > /*
> > > > * Disable interrupts and reevaluate the work flags as they
> > > > * might have changed while interrupts and preemption was
> > > > * enabled above.
> > > > */
> > > > local_irq_disable_exit_to_user();
> > > >
> > > > /* Check if any of the above work has queued a
> > > > deferred wakeup */
> > > > tick_nohz_user_enter_prepare();
> > > >
> > > > ti_work = read_thread_flags();
> > > > }
> > > >
> > > > /* Return the latest work state for arch_exit_to_user_mode() */
> > > > return ti_work;
> > > > }
> > > >
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-11 22:31 ` Andrew Bresticker
@ 2022-12-08 23:43 ` Palmer Dabbelt
-1 siblings, 0 replies; 22+ messages in thread
From: Palmer Dabbelt @ 2022-12-08 23:43 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrew Bresticker
Cc: linux-kernel, Atish Patra, Guo Ren, linux-riscv
On Fri, 11 Nov 2022 17:31:08 -0500, Andrew Bresticker wrote:
> 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.
>
> [...]
Applied, thanks!
[1/1] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
https://git.kernel.org/palmer/c/e1ceb0964163
Best regards,
--
Palmer Dabbelt <palmer@rivosinc.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-12-08 23:43 ` Palmer Dabbelt
0 siblings, 0 replies; 22+ messages in thread
From: Palmer Dabbelt @ 2022-12-08 23:43 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Andrew Bresticker
Cc: linux-kernel, Atish Patra, Guo Ren, linux-riscv
On Fri, 11 Nov 2022 17:31:08 -0500, Andrew Bresticker wrote:
> 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.
>
> [...]
Applied, thanks!
[1/1] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
https://git.kernel.org/palmer/c/e1ceb0964163
Best regards,
--
Palmer Dabbelt <palmer@rivosinc.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-14 15:54 ` Guo Ren
@ 2022-12-08 23:43 ` Palmer Dabbelt
-1 siblings, 0 replies; 22+ messages in thread
From: Palmer Dabbelt @ 2022-12-08 23:43 UTC (permalink / raw)
To: guoren
Cc: abrestic, Conor Dooley, Paul Walmsley, aou, Atish Patra,
linux-riscv, linux-kernel
On Mon, 14 Nov 2022 07:54:47 PST (-0800), guoren@kernel.org wrote:
> On Mon, Nov 14, 2022 at 11:42 PM Andrew Bresticker
> <abrestic@rivosinc.com> wrote:
>>
>> On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
>> <conor.dooley@microchip.com> wrote:
>> >
>> > On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
>> > > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
>> > > >
>> > > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
>> > > > >
>> > > > > 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.
>> > > > Could generic_entry solve your problem? please try:
>> > > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>> > >
>> > > Indeed it does, as I mentioned below, however it wasn't clear to me
>> > > how close your series was to landing so I'v emailed this small fix for
>> > > the existing bug in case your series does not make it into 6.2
>> >
>> > The backportablilty of this 26+, 26- change has to be considered too.
>> > The generic entry patchset has 20x the changes and would not ordinarily
>> > be stable material. How much of that series would be required to solve
>> > the problem?
>>
>> We'd need up to patch 6, "riscv: convert to generic entry":
>> https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
> Yes, but 6 needs 1 and 5, and they are very small patches.
I'm just going to take this one, for the ease of backporting. The
generic entry one keeps getting re-spun and I'm not sure it'll be in
time for 6.2, so at least this way we can get the fix in now.
I've merge th is just on top of 6.1-rc1, so it's easier to deal with any
conflicts later.
Thanks!
>
>>
>> -Andrew
>>
>> >
>> > > >
>> > > > >
>> > > > > [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);
>> > > > > }
>> > > > The more graceful code has been written in kernel/entry/common.c.
>> > > > Let's base it on that:
>> > > >
>> > > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
>> > > > unsigned long ti_work)
>> > > > {
>> > > > /*
>> > > > * Before returning to user space ensure that all pending work
>> > > > * items have been completed.
>> > > > */
>> > > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
>> > > >
>> > > > local_irq_enable_exit_to_user(ti_work);
>> > > >
>> > > > if (ti_work & _TIF_NEED_RESCHED)
>> > > > schedule();
>> > > >
>> > > > if (ti_work & _TIF_UPROBE)
>> > > > uprobe_notify_resume(regs);
>> > > >
>> > > > if (ti_work & _TIF_PATCH_PENDING)
>> > > > klp_update_patch_state(current);
>> > > >
>> > > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
>> > > > arch_do_signal_or_restart(regs);
>> > > >
>> > > > if (ti_work & _TIF_NOTIFY_RESUME)
>> > > > resume_user_mode_work(regs);
>> > > >
>> > > > /* Architecture specific TIF work */
>> > > > arch_exit_to_user_mode_work(regs, ti_work);
>> > > >
>> > > > /*
>> > > > * Disable interrupts and reevaluate the work flags as they
>> > > > * might have changed while interrupts and preemption was
>> > > > * enabled above.
>> > > > */
>> > > > local_irq_disable_exit_to_user();
>> > > >
>> > > > /* Check if any of the above work has queued a
>> > > > deferred wakeup */
>> > > > tick_nohz_user_enter_prepare();
>> > > >
>> > > > ti_work = read_thread_flags();
>> > > > }
>> > > >
>> > > > /* Return the latest work state for arch_exit_to_user_mode() */
>> > > > return ti_work;
>> > > > }
>> > > >
>> > > > > --
>> > > > > 2.25.1
>> > > > >
>> > > >
>> > > >
>> > > > --
>> > > > Best Regards
>> > > > Guo Ren
>> > >
>> > > _______________________________________________
>> > > linux-riscv mailing list
>> > > linux-riscv@lists.infradead.org
>> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-12-08 23:43 ` Palmer Dabbelt
0 siblings, 0 replies; 22+ messages in thread
From: Palmer Dabbelt @ 2022-12-08 23:43 UTC (permalink / raw)
To: guoren
Cc: abrestic, Conor Dooley, Paul Walmsley, aou, Atish Patra,
linux-riscv, linux-kernel
On Mon, 14 Nov 2022 07:54:47 PST (-0800), guoren@kernel.org wrote:
> On Mon, Nov 14, 2022 at 11:42 PM Andrew Bresticker
> <abrestic@rivosinc.com> wrote:
>>
>> On Mon, Nov 14, 2022 at 10:31 AM Conor Dooley
>> <conor.dooley@microchip.com> wrote:
>> >
>> > On Mon, Nov 14, 2022 at 10:20:30AM -0500, Andrew Bresticker wrote:
>> > > On Mon, Nov 14, 2022 at 9:43 AM Guo Ren <guoren@kernel.org> wrote:
>> > > >
>> > > > On Sat, Nov 12, 2022 at 6:31 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
>> > > > >
>> > > > > 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.
>> > > > Could generic_entry solve your problem? please try:
>> > > > https://lore.kernel.org/linux-riscv/20221103075047.1634923-1-guoren@kernel.org/
>> > >
>> > > Indeed it does, as I mentioned below, however it wasn't clear to me
>> > > how close your series was to landing so I'v emailed this small fix for
>> > > the existing bug in case your series does not make it into 6.2
>> >
>> > The backportablilty of this 26+, 26- change has to be considered too.
>> > The generic entry patchset has 20x the changes and would not ordinarily
>> > be stable material. How much of that series would be required to solve
>> > the problem?
>>
>> We'd need up to patch 6, "riscv: convert to generic entry":
>> https://lore.kernel.org/linux-riscv/20221103075047.1634923-7-guoren@kernel.org/
> Yes, but 6 needs 1 and 5, and they are very small patches.
I'm just going to take this one, for the ease of backporting. The
generic entry one keeps getting re-spun and I'm not sure it'll be in
time for 6.2, so at least this way we can get the fix in now.
I've merge th is just on top of 6.1-rc1, so it's easier to deal with any
conflicts later.
Thanks!
>
>>
>> -Andrew
>>
>> >
>> > > >
>> > > > >
>> > > > > [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);
>> > > > > }
>> > > > The more graceful code has been written in kernel/entry/common.c.
>> > > > Let's base it on that:
>> > > >
>> > > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
>> > > > unsigned long ti_work)
>> > > > {
>> > > > /*
>> > > > * Before returning to user space ensure that all pending work
>> > > > * items have been completed.
>> > > > */
>> > > > while (ti_work & EXIT_TO_USER_MODE_WORK) {
>> > > >
>> > > > local_irq_enable_exit_to_user(ti_work);
>> > > >
>> > > > if (ti_work & _TIF_NEED_RESCHED)
>> > > > schedule();
>> > > >
>> > > > if (ti_work & _TIF_UPROBE)
>> > > > uprobe_notify_resume(regs);
>> > > >
>> > > > if (ti_work & _TIF_PATCH_PENDING)
>> > > > klp_update_patch_state(current);
>> > > >
>> > > > if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
>> > > > arch_do_signal_or_restart(regs);
>> > > >
>> > > > if (ti_work & _TIF_NOTIFY_RESUME)
>> > > > resume_user_mode_work(regs);
>> > > >
>> > > > /* Architecture specific TIF work */
>> > > > arch_exit_to_user_mode_work(regs, ti_work);
>> > > >
>> > > > /*
>> > > > * Disable interrupts and reevaluate the work flags as they
>> > > > * might have changed while interrupts and preemption was
>> > > > * enabled above.
>> > > > */
>> > > > local_irq_disable_exit_to_user();
>> > > >
>> > > > /* Check if any of the above work has queued a
>> > > > deferred wakeup */
>> > > > tick_nohz_user_enter_prepare();
>> > > >
>> > > > ti_work = read_thread_flags();
>> > > > }
>> > > >
>> > > > /* Return the latest work state for arch_exit_to_user_mode() */
>> > > > return ti_work;
>> > > > }
>> > > >
>> > > > > --
>> > > > > 2.25.1
>> > > > >
>> > > >
>> > > >
>> > > > --
>> > > > Best Regards
>> > > > Guo Ren
>> > >
>> > > _______________________________________________
>> > > linux-riscv mailing list
>> > > linux-riscv@lists.infradead.org
>> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
2022-11-11 22:31 ` Andrew Bresticker
@ 2022-12-08 23:50 ` patchwork-bot+linux-riscv
-1 siblings, 0 replies; 22+ messages in thread
From: patchwork-bot+linux-riscv @ 2022-12-08 23:50 UTC (permalink / raw)
To: Andrew Bresticker
Cc: linux-riscv, palmer, paul.walmsley, aou, atishp, guoren, linux-kernel
Hello:
This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:
On Fri, 11 Nov 2022 17:31:08 -0500 you wrote:
> 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.
>
> [...]
Here is the summary with links:
- RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
https://git.kernel.org/riscv/c/b0f4c74eadbf
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
@ 2022-12-08 23:50 ` patchwork-bot+linux-riscv
0 siblings, 0 replies; 22+ messages in thread
From: patchwork-bot+linux-riscv @ 2022-12-08 23:50 UTC (permalink / raw)
To: Andrew Bresticker
Cc: linux-riscv, palmer, paul.walmsley, aou, atishp, guoren, linux-kernel
Hello:
This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:
On Fri, 11 Nov 2022 17:31:08 -0500 you wrote:
> 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.
>
> [...]
Here is the summary with links:
- RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path
https://git.kernel.org/riscv/c/b0f4c74eadbf
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-12-08 23:50 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11 22:31 [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path Andrew Bresticker
2022-11-11 22:31 ` Andrew Bresticker
2022-11-12 23:52 ` Conor Dooley
2022-11-12 23:52 ` Conor Dooley
2022-11-14 14:31 ` Andrew Bresticker
2022-11-14 14:31 ` Andrew Bresticker
2022-11-14 14:42 ` Guo Ren
2022-11-14 14:42 ` Guo Ren
2022-11-14 15:20 ` Andrew Bresticker
2022-11-14 15:20 ` Andrew Bresticker
2022-11-14 15:30 ` Conor Dooley
2022-11-14 15:30 ` Conor Dooley
2022-11-14 15:42 ` Andrew Bresticker
2022-11-14 15:42 ` Andrew Bresticker
2022-11-14 15:54 ` Guo Ren
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:43 ` Palmer Dabbelt
2022-12-08 23:43 ` Palmer Dabbelt
2022-12-08 23:50 ` patchwork-bot+linux-riscv
2022-12-08 23:50 ` patchwork-bot+linux-riscv
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.