Re: [PATCH] LoongArch: Enhance robust of kprobe

[Huacai Chen]

Hi, Tiezhu,

On Fri, Apr 11, 2025 at 7:37 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:
>
> On 04/11/2025 12:46 PM, Jinyang He wrote:
> >
> > On 2025-04-11 10:48, Tiezhu Yang wrote:
> >> On 04/09/2025 10:17 AM, Jinyang He wrote:
> >>> On 2025-04-08 17:27, Tiezhu Yang wrote:
>
> ...
>
> > I have just explained the previous status may be broken by IRQ.
>
> The initial aim is to make sure the irq is disabled at the end of
> do_bp(), so let us narrow down the scope.
>
> > On the other hand, have you tried only fix do_bp weather cause hang?
> >
> > bool this_bp_ie = regs->csr_prmd & CSR_PRMD_PIE;
> > if (this_bp_ie)
> >   local_irq_enable();
> > ...
> > if (this_bp_ie)
> >   local_irq_disable();
>
> This is a good idea, thank you.
>
> I will test the following change, if it works well and no more comments,
> I will send v2 in the next week.
If it works, I think there is a more fundamental problem, all do_xyz()
should be fixed because PRMD may change during handling. And you can
use
bool pie = regs_irqs_disabled(regs)
instead of
bool pie = regs->csr_prmd & CSR_PRMD_PIE

Huacai

>
> ---8<---
> diff --git a/arch/loongarch/kernel/traps.c b/arch/loongarch/kernel/traps.c
> index 2ec3106c0da3..68cc4165578a 100644
> --- a/arch/loongarch/kernel/traps.c
> +++ b/arch/loongarch/kernel/traps.c
> @@ -710,11 +710,12 @@ asmlinkage void noinstr do_bce(struct pt_regs *regs)
>   asmlinkage void noinstr do_bp(struct pt_regs *regs)
>   {
>          bool user = user_mode(regs);
> +       bool pie = regs->csr_prmd & CSR_PRMD_PIE;
>          unsigned int opcode, bcode;
>          unsigned long era = exception_era(regs);
>          irqentry_state_t state = irqentry_enter(regs);
>
> -       if (regs->csr_prmd & CSR_PRMD_PIE)
> +       if (pie)
>                  local_irq_enable();
>
>          if (__get_inst(&opcode, (u32 *)era, user))
> @@ -780,7 +781,7 @@ asmlinkage void noinstr do_bp(struct pt_regs *regs)
>          }
>
>   out:
> -       if (regs->csr_prmd & CSR_PRMD_PIE)
> +       if (pie)
>                  local_irq_disable();
>
>          irqentry_exit(regs, state);
>
> Thanks,
> Tiezhu
>
>