[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAhV-H7maW1YWhfvCzYVix=aCMMFbx0KkGodaBjka4ThunBO5Q@mail.gmail.com>
Date: Tue, 27 Dec 2022 17:52:02 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Jinyang He <hejinyang@...ngson.cn>
Cc: WANG Xuerui <kernel@...0n.name>,
Tiezhu Yang <yangtiezhu@...ngson.cn>,
loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: Fix irq enable in exception handlers
On Tue, Dec 27, 2022 at 5:30 PM Jinyang He <hejinyang@...ngson.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@...ngson.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@...ngson.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
> >>
>
>
Powered by blists - more mailing lists