lkml.org 
[lkml]   [2022]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] LoongArch: Fix irq enable in exception handlers
From
Date
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
>>
>> 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
>>>>
>>

\
 
 \ /
  Last update: 2023-03-26 23:21    [W:0.043 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site