Messages in this thread Patch in this message | | | Date | Thu, 29 Dec 2022 00:51:33 +0800 | Subject | Re: [PATCH] LoongArch: Fix irq enable in exception handlers | From | Qi Hu <> |
| |
On 2022/12/27 18:10, Jinyang He wrote: > On 2022-12-27 17:52, Huacai Chen wrote: > >> 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. > > > I think because it never call die() in die_if_kernel(). What I do > emphasize is that there needs to be more strict logic here than > it worked well in the past. I bet if die_if_kernel() was removed, > it will still work well in the future. > > >> >>> >>>> 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. > > To: Qi Hu > > > Hi, > > > We really need some help. Can you give us some ideas? > > > Thanks, > > Jinyang > Huacai is correct. The LBT disable exception (BTD) can be triggered in kernel context.
If the CSR.ENEU.BTE == 0 [^1], the LBT instructions (these [^2] will be used in the kernel) will trigger the exception.
Unfortunately, when you want to do some fpu_{save, restore}, you need to use some LBT instructions [^3] [^4]. So if FPD is triggered, LBT might still not be enabled, and the 'do_lbt' will be called in the kernel context.
Hope the information can help. Thanks.
[1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#extended-component-unit-enable
[2] https://github.com/loongson/linux/pull/4/files#diff-381d03cf86e2796d280e2fc82c005409d5e44b4bbbf90dd0dc17f5f0fa5553f1R140-R184
[3] https://github.com/loongson/linux/pull/4/files#diff-381d03cf86e2796d280e2fc82c005409d5e44b4bbbf90dd0dc17f5f0fa5553f1R218-R230
[4] https://github.com/loongson/linux/pull/4/files#diff-381d03cf86e2796d280e2fc82c005409d5e44b4bbbf90dd0dc17f5f0fa5553f1R236-R263
Qi
> >> 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 >>>>> >>> >
| |