Messages in this thread | | | From | Huacai Chen <> | Date | Tue, 27 Dec 2022 17:52:02 +0800 | Subject | Re: [PATCH] LoongArch: Fix irq enable in exception handlers |
| |
On Tue, Dec 27, 2022 at 5:30 PM Jinyang He <hejinyang@loongson.cn> wrote: > > > On 2022-12-27 15:37, Huacai Chen wrote: > > Hi, Jinyang, > > > > Move die_if_kernel to irq disabled context to solve what? > > For more strict logical. If the code flow go to die in die_if_kernel(), > its interrupt state is enable, that means it may cause schedule. > So I think it is better to call die_if_kernel() firstly. die_if_kernel is called with irq enabled in old kernels for several years, and has no problems.
> > > > And LBT is > > surely allowed to be triggered in kernel context. > > I'm not familar with lbt, I just not see any lbt codes in kernel. Plz, > how lbt exception triggered, and how kernel trigger lbt exception? You can ask Huqi for more details, and this was discussed publicly last week.
Huacai > > > Thanks, > > Jinyang > > > > > > Huacai > > > > On Wed, Dec 21, 2022 at 3:43 PM Jinyang He <hejinyang@loongson.cn> wrote: > >> The interrupt state can be got by regs->csr_prmd. Once previous > >> interrupt state is disable, we shouldn't enable interrupt if we > >> triggered exception which can be triggered in kernel mode. So > >> conditionally enable interrupt. For those do_\exception which > >> can not triggered in kernel mode but need enable interrupt, call > >> die_if_kernel() firstly. And for do_lsx, do_lasx and do_lbt cannot > >> triggered in kernel mode, too. > >> > >> Signed-off-by: Jinyang He <hejinyang@loongson.cn> > >> --- > >> arch/loongarch/kernel/traps.c | 19 ++++++++++--------- > >> 1 file changed, 10 insertions(+), 9 deletions(-) > >> > >> diff --git a/arch/loongarch/kernel/traps.c b/arch/loongarch/kernel/traps.c > >> index 1ea14f6c18d3..3ac7b32d1e15 100644 > >> --- a/arch/loongarch/kernel/traps.c > >> +++ b/arch/loongarch/kernel/traps.c > >> @@ -340,9 +340,9 @@ asmlinkage void noinstr do_fpe(struct pt_regs *regs, unsigned long fcsr) > >> > >> /* Clear FCSR.Cause before enabling interrupts */ > >> write_fcsr(LOONGARCH_FCSR0, fcsr & ~mask_fcsr_x(fcsr)); > >> - local_irq_enable(); > >> > >> die_if_kernel("FP exception in kernel code", regs); > >> + local_irq_enable(); > >> > >> sig = SIGFPE; > >> fault_addr = (void __user *) regs->csr_era; > >> @@ -432,7 +432,8 @@ asmlinkage void noinstr do_bp(struct pt_regs *regs) > >> unsigned long era = exception_era(regs); > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> - local_irq_enable(); > >> + if (regs->csr_prmd & CSR_PRMD_PIE) > >> + local_irq_enable(); > >> current->thread.trap_nr = read_csr_excode(); > >> if (__get_inst(&opcode, (u32 *)era, user)) > >> goto out_sigsegv; > >> @@ -514,7 +515,8 @@ asmlinkage void noinstr do_ri(struct pt_regs *regs) > >> unsigned int __user *era = (unsigned int __user *)exception_era(regs); > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> - local_irq_enable(); > >> + if (regs->csr_prmd & CSR_PRMD_PIE) > >> + local_irq_enable(); > >> current->thread.trap_nr = read_csr_excode(); > >> > >> if (notify_die(DIE_RI, "RI Fault", regs, 0, current->thread.trap_nr, > >> @@ -606,8 +608,8 @@ asmlinkage void noinstr do_fpu(struct pt_regs *regs) > >> { > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> - local_irq_enable(); > >> die_if_kernel("do_fpu invoked from kernel context!", regs); > >> + local_irq_enable(); > >> BUG_ON(is_lsx_enabled()); > >> BUG_ON(is_lasx_enabled()); > >> > >> @@ -623,13 +625,13 @@ asmlinkage void noinstr do_lsx(struct pt_regs *regs) > >> { > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> + die_if_kernel("do_lsx invoked from kernel context!", regs); > >> local_irq_enable(); > >> if (!cpu_has_lsx) { > >> force_sig(SIGILL); > >> goto out; > >> } > >> > >> - die_if_kernel("do_lsx invoked from kernel context!", regs); > >> BUG_ON(is_lasx_enabled()); > >> > >> preempt_disable(); > >> @@ -645,14 +647,13 @@ asmlinkage void noinstr do_lasx(struct pt_regs *regs) > >> { > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> + die_if_kernel("do_lasx invoked from kernel context!", regs); > >> local_irq_enable(); > >> if (!cpu_has_lasx) { > >> force_sig(SIGILL); > >> goto out; > >> } > >> > >> - die_if_kernel("do_lasx invoked from kernel context!", regs); > >> - > >> preempt_disable(); > >> init_restore_lasx(); > >> preempt_enable(); > >> @@ -666,6 +667,7 @@ asmlinkage void noinstr do_lbt(struct pt_regs *regs) > >> { > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> + die_if_kernel("do_lbt invoked from kernel context!", regs); > >> local_irq_enable(); > >> force_sig(SIGILL); > >> local_irq_disable(); > >> @@ -677,7 +679,6 @@ asmlinkage void noinstr do_reserved(struct pt_regs *regs) > >> { > >> irqentry_state_t state = irqentry_enter(regs); > >> > >> - local_irq_enable(); > >> /* > >> * Game over - no way to handle this if it ever occurs. Most probably > >> * caused by a fatal error after another hardware/software error. > >> @@ -685,8 +686,8 @@ asmlinkage void noinstr do_reserved(struct pt_regs *regs) > >> pr_err("Caught reserved exception %u on pid:%d [%s] - should not happen\n", > >> read_csr_excode(), current->pid, current->comm); > >> die_if_kernel("do_reserved exception", regs); > >> + local_irq_enable(); > >> force_sig(SIGUNUSED); > >> - > >> local_irq_disable(); > >> > >> irqentry_exit(regs, state); > >> -- > >> 2.34.3 > >> > >
| |