Messages in this thread Patch in this message | | | Subject | Re: [PATCH] LoongArch: Fix irq enable in exception handlers | From | Jinyang He <> | Date | Thu, 29 Dec 2022 14:13:51 +0800 |
| |
On 2022-12-29 00:51, Qi Hu wrote:
> > 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 > > Hi,
That's helpful. Thanks!
But I still wonder if SXD or ASXD have the same possibility of being triggered in the kernel mode by sc_save_{lsx, lasx} or other place. Do we need remove these die_if_kernel codes in do_lasx() and do_lsx()?
Jinyang
> 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 >>>>>> >>>> >> >
| |