arch/riscv/kernel/traps.c | 36 ++++++++++++++++++++++++++++++------ arch/riscv/mm/fault.c | 4 ---- 2 files changed, 30 insertions(+), 10 deletions(-)
force_sig_fault() takes a spinlock, which is a sleeping lock with
CONFIG_PREEMPT_RT=y. However, exception handling calls force_sig_fault()
with interrupt disabled, causing a sleeping in atomic context warning.
This can be reproduced using userspace programs such as:
int main() { asm ("ebreak"); }
or
int main() { asm ("unimp"); }
There is no reason that interrupt must be disabled during exception
handling. Considering the previous struggle with a similar bug [1][2], fix
this problem once for all by enabling interrupt during exception handling
whenever possible:
- If exception comes from user (interrupt handling was for sure enabled)
- If exception comes from kernel and interrupt handling was enabled
This also has the added benefit of avoiding unnecessary delays in interrupt
handling.
This patch mimics x86's implementation.
Link: https://lore.kernel.org/linux-riscv/20250411073850.3699180-3-nylon.chen@sifive.com [1]
Link: https://lore.kernel.org/linux-riscv/20250422162324.956065-3-cleger@rivosinc.com [2]
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
arch/riscv/kernel/traps.c | 36 ++++++++++++++++++++++++++++++------
arch/riscv/mm/fault.c | 4 ----
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 9c83848797a78..f7d2372dc0eb6 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -6,6 +6,7 @@
#include <linux/cpu.h>
#include <linux/kernel.h>
#include <linux/init.h>
+#include <linux/irqflags.h>
#include <linux/randomize_kstack.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
@@ -72,6 +73,18 @@ static void dump_instr(const char *loglvl, struct pt_regs *regs)
printk("%sCode: %s\n", loglvl, str);
}
+static void cond_local_irq_enable(struct pt_regs *regs)
+{
+ if (!regs_irqs_disabled(regs))
+ local_irq_enable();
+}
+
+static void cond_local_irq_disable(struct pt_regs *regs)
+{
+ if (!regs_irqs_disabled(regs))
+ local_irq_disable();
+}
+
void die(struct pt_regs *regs, const char *str)
{
static int die_counter;
@@ -151,11 +164,15 @@ asmlinkage __visible __trap_section void name(struct pt_regs *regs) \
{ \
if (user_mode(regs)) { \
irqentry_enter_from_user_mode(regs); \
+ local_irq_enable(); \
do_trap_error(regs, signo, code, regs->epc, "Oops - " str); \
+ local_irq_disable(); \
irqentry_exit_to_user_mode(regs); \
} else { \
irqentry_state_t state = irqentry_nmi_enter(regs); \
+ cond_local_irq_enable(regs); \
do_trap_error(regs, signo, code, regs->epc, "Oops - " str); \
+ cond_local_irq_disable(regs); \
irqentry_nmi_exit(regs, state); \
} \
}
@@ -173,24 +190,23 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
if (user_mode(regs)) {
irqentry_enter_from_user_mode(regs);
-
local_irq_enable();
handled = riscv_v_first_use_handler(regs);
-
- local_irq_disable();
-
if (!handled)
do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc,
"Oops - illegal instruction");
+ local_irq_disable();
irqentry_exit_to_user_mode(regs);
} else {
irqentry_state_t state = irqentry_nmi_enter(regs);
+ cond_local_irq_enable(regs);
do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc,
"Oops - illegal instruction");
+ cond_local_irq_disable(regs);
irqentry_nmi_exit(regs, state);
}
}
@@ -225,6 +241,7 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
local_irq_enable();
} else {
state = irqentry_nmi_enter(regs);
+ cond_local_irq_enable(regs);
}
if (misaligned_handler[type].handler(regs))
@@ -235,6 +252,7 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
local_irq_disable();
irqentry_exit_to_user_mode(regs);
} else {
+ cond_local_irq_disable(regs);
irqentry_nmi_exit(regs, state);
}
}
@@ -308,15 +326,19 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
{
if (user_mode(regs)) {
irqentry_enter_from_user_mode(regs);
+ local_irq_enable();
handle_break(regs);
+ local_irq_disable();
irqentry_exit_to_user_mode(regs);
} else {
irqentry_state_t state = irqentry_nmi_enter(regs);
+ cond_local_irq_enable(regs);
handle_break(regs);
+ cond_local_irq_disable(regs);
irqentry_nmi_exit(regs, state);
}
}
@@ -355,10 +377,12 @@ void do_trap_ecall_u(struct pt_regs *regs)
syscall_exit_to_user_mode(regs);
} else {
irqentry_state_t state = irqentry_nmi_enter(regs);
+ cond_local_irq_enable(regs);
do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
"Oops - environment call from U-mode");
+ cond_local_irq_disable(regs);
irqentry_nmi_exit(regs, state);
}
@@ -368,11 +392,11 @@ void do_trap_ecall_u(struct pt_regs *regs)
asmlinkage __visible noinstr void do_page_fault(struct pt_regs *regs)
{
irqentry_state_t state = irqentry_enter(regs);
+ cond_local_irq_enable(regs);
handle_page_fault(regs);
- local_irq_disable();
-
+ cond_local_irq_disable(regs);
irqentry_exit(regs, state);
}
#endif
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 0194324a0c506..6d23ed0ce8a28 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -306,10 +306,6 @@ void handle_page_fault(struct pt_regs *regs)
return;
}
- /* Enable interrupts if they were enabled in the parent context. */
- if (!regs_irqs_disabled(regs))
- local_irq_enable();
-
/*
* If we're in an interrupt, have no user context, or are running
* in an atomic region, then we must not take the fault.
--
2.39.5On Fri, 20 Jun 2025 04:43:46 PDT (-0700), namcao@linutronix.de wrote:
> force_sig_fault() takes a spinlock, which is a sleeping lock with
> CONFIG_PREEMPT_RT=y. However, exception handling calls force_sig_fault()
> with interrupt disabled, causing a sleeping in atomic context warning.
>
> This can be reproduced using userspace programs such as:
> int main() { asm ("ebreak"); }
> or
> int main() { asm ("unimp"); }
>
> There is no reason that interrupt must be disabled during exception
> handling.
Looks like they used to and we lost it as of f0bddf50586d ("riscv:
entry: Convert to generic entry"), where we'd check the PIE bit and
quickly re-enable interrupts if they were enabled before.
This way of doing it also seems fine, and it's kind of nice that we can
just read the code to figure out how the interrupt contexts are managed.
So I think it's fine to keep it this way, even though it's more code.
I'm kind of split on a Fixes tag here. One could argue it's a
regression, as having interrupts disabled during exceptions is going to
cause all sorts of performance issues for users. Seems a bit risk to
just backport, though...
That said, if nobody noticed then it's probably a good sign nobody is
really paying attention and we should just backport it before anyone
notices...
> Considering the previous struggle with a similar bug [1][2], fix
> this problem once for all by enabling interrupt during exception handling
> whenever possible:
> - If exception comes from user (interrupt handling was for sure enabled)
> - If exception comes from kernel and interrupt handling was enabled
>
> This also has the added benefit of avoiding unnecessary delays in interrupt
> handling.
>
> This patch mimics x86's implementation.
>
> Link: https://lore.kernel.org/linux-riscv/20250411073850.3699180-3-nylon.chen@sifive.com [1]
> Link: https://lore.kernel.org/linux-riscv/20250422162324.956065-3-cleger@rivosinc.com [2]
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> arch/riscv/kernel/traps.c | 36 ++++++++++++++++++++++++++++++------
> arch/riscv/mm/fault.c | 4 ----
> 2 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 9c83848797a78..f7d2372dc0eb6 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -6,6 +6,7 @@
> #include <linux/cpu.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/irqflags.h>
> #include <linux/randomize_kstack.h>
> #include <linux/sched.h>
> #include <linux/sched/debug.h>
> @@ -72,6 +73,18 @@ static void dump_instr(const char *loglvl, struct pt_regs *regs)
> printk("%sCode: %s\n", loglvl, str);
> }
>
> +static void cond_local_irq_enable(struct pt_regs *regs)
> +{
> + if (!regs_irqs_disabled(regs))
> + local_irq_enable();
> +}
> +
> +static void cond_local_irq_disable(struct pt_regs *regs)
> +{
> + if (!regs_irqs_disabled(regs))
> + local_irq_disable();
> +}
> +
> void die(struct pt_regs *regs, const char *str)
> {
> static int die_counter;
> @@ -151,11 +164,15 @@ asmlinkage __visible __trap_section void name(struct pt_regs *regs) \
> { \
> if (user_mode(regs)) { \
> irqentry_enter_from_user_mode(regs); \
> + local_irq_enable(); \
> do_trap_error(regs, signo, code, regs->epc, "Oops - " str); \
> + local_irq_disable(); \
> irqentry_exit_to_user_mode(regs); \
> } else { \
> irqentry_state_t state = irqentry_nmi_enter(regs); \
> + cond_local_irq_enable(regs); \
> do_trap_error(regs, signo, code, regs->epc, "Oops - " str); \
> + cond_local_irq_disable(regs); \
> irqentry_nmi_exit(regs, state); \
> } \
> }
> @@ -173,24 +190,23 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>
> if (user_mode(regs)) {
> irqentry_enter_from_user_mode(regs);
> -
> local_irq_enable();
>
> handled = riscv_v_first_use_handler(regs);
> -
> - local_irq_disable();
> -
> if (!handled)
> do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc,
> "Oops - illegal instruction");
>
> + local_irq_disable();
> irqentry_exit_to_user_mode(regs);
> } else {
> irqentry_state_t state = irqentry_nmi_enter(regs);
> + cond_local_irq_enable(regs);
>
> do_trap_error(regs, SIGILL, ILL_ILLOPC, regs->epc,
> "Oops - illegal instruction");
>
> + cond_local_irq_disable(regs);
> irqentry_nmi_exit(regs, state);
> }
> }
> @@ -225,6 +241,7 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
> local_irq_enable();
> } else {
> state = irqentry_nmi_enter(regs);
> + cond_local_irq_enable(regs);
> }
>
> if (misaligned_handler[type].handler(regs))
> @@ -235,6 +252,7 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
> local_irq_disable();
> irqentry_exit_to_user_mode(regs);
> } else {
> + cond_local_irq_disable(regs);
> irqentry_nmi_exit(regs, state);
> }
> }
> @@ -308,15 +326,19 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
> {
> if (user_mode(regs)) {
> irqentry_enter_from_user_mode(regs);
> + local_irq_enable();
>
> handle_break(regs);
>
> + local_irq_disable();
> irqentry_exit_to_user_mode(regs);
> } else {
> irqentry_state_t state = irqentry_nmi_enter(regs);
> + cond_local_irq_enable(regs);
>
> handle_break(regs);
>
> + cond_local_irq_disable(regs);
> irqentry_nmi_exit(regs, state);
> }
> }
> @@ -355,10 +377,12 @@ void do_trap_ecall_u(struct pt_regs *regs)
> syscall_exit_to_user_mode(regs);
> } else {
> irqentry_state_t state = irqentry_nmi_enter(regs);
> + cond_local_irq_enable(regs);
>
> do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
> "Oops - environment call from U-mode");
>
> + cond_local_irq_disable(regs);
> irqentry_nmi_exit(regs, state);
> }
>
> @@ -368,11 +392,11 @@ void do_trap_ecall_u(struct pt_regs *regs)
> asmlinkage __visible noinstr void do_page_fault(struct pt_regs *regs)
> {
> irqentry_state_t state = irqentry_enter(regs);
> + cond_local_irq_enable(regs);
>
> handle_page_fault(regs);
>
> - local_irq_disable();
> -
> + cond_local_irq_disable(regs);
> irqentry_exit(regs, state);
> }
> #endif
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 0194324a0c506..6d23ed0ce8a28 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -306,10 +306,6 @@ void handle_page_fault(struct pt_regs *regs)
> return;
> }
>
> - /* Enable interrupts if they were enabled in the parent context. */
> - if (!regs_irqs_disabled(regs))
> - local_irq_enable();
> -
> /*
> * If we're in an interrupt, have no user context, or are running
> * in an atomic region, then we must not take the fault.
> --
> 2.39.5
On Mon, 23 Jun 2025, Palmer Dabbelt wrote: > I'm kind of split on a Fixes tag here. One could argue it's a regression, as > having interrupts disabled during exceptions is going to cause all sorts of > performance issues for users. Seems a bit risk to just backport, though... > > That said, if nobody noticed then it's probably a good sign nobody is really > paying attention and we should just backport it before anyone notices... Oh, someone did notice and it's not only performance, cf. <https://lore.kernel.org/r/alpine.DEB.2.21.2501070143250.18889@angie.orcam.me.uk/>. Maciej
On 24/06/2025 04:09, Maciej W. Rozycki wrote: > On Mon, 23 Jun 2025, Palmer Dabbelt wrote: > >> I'm kind of split on a Fixes tag here. One could argue it's a regression, as >> having interrupts disabled during exceptions is going to cause all sorts of >> performance issues for users. Seems a bit risk to just backport, though... >> >> That said, if nobody noticed then it's probably a good sign nobody is really >> paying attention and we should just backport it before anyone notices... > > Oh, someone did notice and it's not only performance, cf. > <https://lore.kernel.org/r/alpine.DEB.2.21.2501070143250.18889@angie.orcam.me.uk/>. I also had a series which was doing so for misaligned accesses handling, but after discussion, it was not ok to do so.: https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ So this series should probably be modified to only reenable interrupts when explicitly needed, ie page faults and for force_sig_fault(). Thanks, Clément > > Maciej > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jun 24, 2025 at 01:37:13PM +0200, Clément Léger wrote: > On 24/06/2025 04:09, Maciej W. Rozycki wrote: > > On Mon, 23 Jun 2025, Palmer Dabbelt wrote: > >> I'm kind of split on a Fixes tag here. One could argue it's a regression, as > >> having interrupts disabled during exceptions is going to cause all sorts of > >> performance issues for users. Seems a bit risk to just backport, though... > >> > >> That said, if nobody noticed then it's probably a good sign nobody is really > >> paying attention and we should just backport it before anyone notices... > > > > Oh, someone did notice and it's not only performance, cf. > > <https://lore.kernel.org/r/alpine.DEB.2.21.2501070143250.18889@angie.orcam.me.uk/>. > > I also had a series which was doing so for misaligned accesses handling, > but after discussion, it was not ok to do so.: > > https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ If I understand that right, exceptions from kernel should be treated as NMI, so that lockdep can tell us if exception handlers touch locks. But (conditionally) enabling interrupts does not lose us that benefit. It is still considered NMI by lockdep. Unless I miss something, the patch is fine as is. Best regards, Nam
On 24/06/2025 15:17, Nam Cao wrote: > On Tue, Jun 24, 2025 at 01:37:13PM +0200, Clément Léger wrote: >> On 24/06/2025 04:09, Maciej W. Rozycki wrote: >>> On Mon, 23 Jun 2025, Palmer Dabbelt wrote: >>>> I'm kind of split on a Fixes tag here. One could argue it's a regression, as >>>> having interrupts disabled during exceptions is going to cause all sorts of >>>> performance issues for users. Seems a bit risk to just backport, though... >>>> >>>> That said, if nobody noticed then it's probably a good sign nobody is really >>>> paying attention and we should just backport it before anyone notices... >>> >>> Oh, someone did notice and it's not only performance, cf. >>> <https://lore.kernel.org/r/alpine.DEB.2.21.2501070143250.18889@angie.orcam.me.uk/>. >> >> I also had a series which was doing so for misaligned accesses handling, >> but after discussion, it was not ok to do so.: >> >> https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ > > If I understand that right, exceptions from kernel should be treated as > NMI, so that lockdep can tell us if exception handlers touch locks. > > But (conditionally) enabling interrupts does not lose us that benefit. It > is still considered NMI by lockdep. > > Unless I miss something, the patch is fine as is. Hi Nam, Yeah indeed, providing that all traps handlers really are reentrant, I think it's fine. Thanks, Clément > > Best regards, > Nam
On Tue, Jun 24, 2025 at 03:48:21PM +0200, Clément Léger wrote: > > > On 24/06/2025 15:17, Nam Cao wrote: > > On Tue, Jun 24, 2025 at 01:37:13PM +0200, Clément Léger wrote: > >> On 24/06/2025 04:09, Maciej W. Rozycki wrote: > >>> On Mon, 23 Jun 2025, Palmer Dabbelt wrote: > >>>> I'm kind of split on a Fixes tag here. One could argue it's a regression, as > >>>> having interrupts disabled during exceptions is going to cause all sorts of > >>>> performance issues for users. Seems a bit risk to just backport, though... > >>>> > >>>> That said, if nobody noticed then it's probably a good sign nobody is really > >>>> paying attention and we should just backport it before anyone notices... > >>> > >>> Oh, someone did notice and it's not only performance, cf. > >>> <https://lore.kernel.org/r/alpine.DEB.2.21.2501070143250.18889@angie.orcam.me.uk/>. > >> > >> I also had a series which was doing so for misaligned accesses handling, > >> but after discussion, it was not ok to do so.: > >> > >> https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ > > > > If I understand that right, exceptions from kernel should be treated as > > NMI, so that lockdep can tell us if exception handlers touch locks. > > > > But (conditionally) enabling interrupts does not lose us that benefit. It > > is still considered NMI by lockdep. > > > > Unless I miss something, the patch is fine as is. I'm confused, you're wanting to conditionally enable interrupts from a kernel exception while its NMI like? *WHY* ?!
On 2025-06-24 16:08:15 [+0200], Peter Zijlstra wrote: > > >> > > >> I also had a series which was doing so for misaligned accesses handling, > > >> but after discussion, it was not ok to do so.: > > >> > > >> https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ > > > > > > If I understand that right, exceptions from kernel should be treated as > > > NMI, so that lockdep can tell us if exception handlers touch locks. > > > > > > But (conditionally) enabling interrupts does not lose us that benefit. It > > > is still considered NMI by lockdep. > > > > > > Unless I miss something, the patch is fine as is. > > I'm confused, you're wanting to conditionally enable interrupts from a > kernel exception while its NMI like? *WHY* ?! What we want is to enable interrupt handling if it was enabled before the exception occured. So we can send a proper signal on PREEMPT_RT without chocking on spinlock_t/ sighand. Sebastian
On Tue, Jun 24, 2025 at 04:11:30PM +0200, Sebastian Andrzej Siewior wrote: > On 2025-06-24 16:08:15 [+0200], Peter Zijlstra wrote: > > > >> > > > >> I also had a series which was doing so for misaligned accesses handling, > > > >> but after discussion, it was not ok to do so.: > > > >> > > > >> https://lore.kernel.org/linux-riscv/20250422094419.GC14170@noisy.programming.kicks-ass.net/ > > > > > > > > If I understand that right, exceptions from kernel should be treated as > > > > NMI, so that lockdep can tell us if exception handlers touch locks. > > > > > > > > But (conditionally) enabling interrupts does not lose us that benefit. It > > > > is still considered NMI by lockdep. > > > > > > > > Unless I miss something, the patch is fine as is. > > > > I'm confused, you're wanting to conditionally enable interrupts from a > > kernel exception while its NMI like? *WHY* ?! > > What we want is to enable interrupt handling if it was enabled before > the exception occured. So we can send a proper signal on PREEMPT_RT > without chocking on spinlock_t/ sighand. I'm confused, sending signals is for exception from userspace. That has nothing to do with exceptions from kernelspace being NMI like.
On 2025-06-24 16:18:01 [+0200], Peter Zijlstra wrote: > I'm confused, sending signals is for exception from userspace. That has > nothing to do with exceptions from kernelspace being NMI like. Yes. See the original submission https://lore.kernel.org/linux-riscv/20250620114346.1740512-1-namcao@linutronix.de/ Sebastian
On Tue, Jun 24, 2025 at 04:23:50PM +0200, Sebastian Andrzej Siewior wrote: > On 2025-06-24 16:18:01 [+0200], Peter Zijlstra wrote: > > I'm confused, sending signals is for exception from userspace. That has > > nothing to do with exceptions from kernelspace being NMI like. > > Yes. See the original submission > https://lore.kernel.org/linux-riscv/20250620114346.1740512-1-namcao@linutronix.de/ I'm still confused, that code is trying to enable IRQs in the from-kernel part. That's insane. Can some Risc-V person explain why a from-kernel exception would ever result in a signal?!?!
On Tue, Jun 24, 2025 at 04:34:58PM +0200, Peter Zijlstra wrote: > On Tue, Jun 24, 2025 at 04:23:50PM +0200, Sebastian Andrzej Siewior wrote: > > On 2025-06-24 16:18:01 [+0200], Peter Zijlstra wrote: > > > I'm confused, sending signals is for exception from userspace. That has > > > nothing to do with exceptions from kernelspace being NMI like. > > > > Yes. See the original submission > > https://lore.kernel.org/linux-riscv/20250620114346.1740512-1-namcao@linutronix.de/ > > I'm still confused, that code is trying to enable IRQs in the > from-kernel part. That's insane. > > Can some Risc-V person explain why a from-kernel exception would ever > result in a signal?!?! Exceptions from kernel do not raise signals. Enabling irqs there is not necessary, I can remove that part. But for my curiousity, do you mind elaborating why it is insane to enable irqs in from-kernel exception handling? For "NMI-like" exceptions, (I think) I get it, the context would be messed up. But what about the others, e.g. kernel page faults? Best regards, Nam
On Tue, Jun 24, 2025 at 05:33:14PM +0200, Nam Cao wrote: > On Tue, Jun 24, 2025 at 04:34:58PM +0200, Peter Zijlstra wrote: > > On Tue, Jun 24, 2025 at 04:23:50PM +0200, Sebastian Andrzej Siewior wrote: > > > On 2025-06-24 16:18:01 [+0200], Peter Zijlstra wrote: > > > > I'm confused, sending signals is for exception from userspace. That has > > > > nothing to do with exceptions from kernelspace being NMI like. > > > > > > Yes. See the original submission > > > https://lore.kernel.org/linux-riscv/20250620114346.1740512-1-namcao@linutronix.de/ > > > > I'm still confused, that code is trying to enable IRQs in the > > from-kernel part. That's insane. > > > > Can some Risc-V person explain why a from-kernel exception would ever > > result in a signal?!?! > > Exceptions from kernel do not raise signals. Enabling irqs there is not > necessary, I can remove that part. > > But for my curiousity, do you mind elaborating why it is insane to enable > irqs in from-kernel exception handling? > > For "NMI-like" exceptions, (I think) I get it, the context would be messed > up. But what about the others, e.g. kernel page faults? The non-NMI exceptions like page-fault are okay. In fact, we hard rely on it when kernel space accesses user-space data in a preemptible context, it is expected to handle the fault. My concern was about the NMI like ones -- enabling IRQs for those is quite mad.
© 2016 - 2025 Red Hat, Inc.