[PATCH] riscv: Enable interrupt during exception handling

[PATCH] riscv: Enable interrupt during exception handling

[Nam Cao]

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.5

Re: [PATCH] riscv: Enable interrupt during exception handling

[Palmer Dabbelt]

On 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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Maciej W. Rozycki]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Clément Léger]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Nam Cao]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Clément Léger]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Peter Zijlstra]

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* ?!

Re: [PATCH] riscv: Enable interrupt during exception handling

[Sebastian Andrzej Siewior]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Peter Zijlstra]

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.

Re: [PATCH] riscv: Enable interrupt during exception handling

[Sebastian Andrzej Siewior]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Peter Zijlstra]

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?!?!

Re: [PATCH] riscv: Enable interrupt during exception handling

[Nam Cao]

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

Re: [PATCH] riscv: Enable interrupt during exception handling

[Peter Zijlstra]

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.