1. Patchew
  2. linux
  3. [PATCH 0/2] Refine the logic of uprobe on LoongArch
  4. View patch

[PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era

Tiezhu Yang posted 2 patches 9 months ago
[PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era
Posted by Tiezhu Yang 9 months ago 
arch_uprobe_skip_sstep() returns true if instruction was emulated,
that is to say, there is no need to single step for the emulated
instructions, it will point to the destination address directly
after the exception, so the resume_era related code is redundant,
just remove them.

Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/include/asm/uprobes.h | 1 -
 arch/loongarch/kernel/uprobes.c      | 7 +------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/loongarch/include/asm/uprobes.h b/arch/loongarch/include/asm/uprobes.h
index 99a0d198927f..025fc3f0a102 100644
--- a/arch/loongarch/include/asm/uprobes.h
+++ b/arch/loongarch/include/asm/uprobes.h
@@ -15,7 +15,6 @@ typedef u32 uprobe_opcode_t;
 #define UPROBE_XOLBP_INSN	__emit_break(BRK_UPROBE_XOLBP)
 
 struct arch_uprobe {
-	unsigned long	resume_era;
 	u32	insn[2];
 	u32	ixol[2];
 	bool	simulate;
diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c
index 0ab9d8d631c4..6022eb0f71db 100644
--- a/arch/loongarch/kernel/uprobes.c
+++ b/arch/loongarch/kernel/uprobes.c
@@ -52,11 +52,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 
 	WARN_ON_ONCE(current->thread.trap_nr != UPROBE_TRAP_NR);
 	current->thread.trap_nr = utask->autask.saved_trap_nr;
-
-	if (auprobe->simulate)
-		instruction_pointer_set(regs, auprobe->resume_era);
-	else
-		instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
+	instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
 
 	return 0;
 }
@@ -86,7 +82,6 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
 
 	insn.word = auprobe->insn[0];
 	arch_simulate_insn(insn, regs);
-	auprobe->resume_era = regs->csr_era;
 
 	return true;
 }
-- 
2.42.0
Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era
Posted by Huacai Chen 9 months ago 
Hi, Tiezhu,

On Tue, May 13, 2025 at 5:21 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> arch_uprobe_skip_sstep() returns true if instruction was emulated,
> that is to say, there is no need to single step for the emulated
> instructions, it will point to the destination address directly
> after the exception, so the resume_era related code is redundant,
> just remove them.
>
> Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/include/asm/uprobes.h | 1 -
>  arch/loongarch/kernel/uprobes.c      | 7 +------
>  2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/uprobes.h b/arch/loongarch/include/asm/uprobes.h
> index 99a0d198927f..025fc3f0a102 100644
> --- a/arch/loongarch/include/asm/uprobes.h
> +++ b/arch/loongarch/include/asm/uprobes.h
> @@ -15,7 +15,6 @@ typedef u32 uprobe_opcode_t;
>  #define UPROBE_XOLBP_INSN      __emit_break(BRK_UPROBE_XOLBP)
>
>  struct arch_uprobe {
> -       unsigned long   resume_era;
>         u32     insn[2];
>         u32     ixol[2];
>         bool    simulate;
> diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c
> index 0ab9d8d631c4..6022eb0f71db 100644
> --- a/arch/loongarch/kernel/uprobes.c
> +++ b/arch/loongarch/kernel/uprobes.c
> @@ -52,11 +52,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
>
>         WARN_ON_ONCE(current->thread.trap_nr != UPROBE_TRAP_NR);
>         current->thread.trap_nr = utask->autask.saved_trap_nr;
> -
> -       if (auprobe->simulate)
> -               instruction_pointer_set(regs, auprobe->resume_era);
> -       else
> -               instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
> +       instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
This seems wrong. If in the simulate case, regs->csr_era has already
pointed to the correct destination address, then here we should only
handle the non-simulate case.

Huacai

>
>         return 0;
>  }
> @@ -86,7 +82,6 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
>
>         insn.word = auprobe->insn[0];
>         arch_simulate_insn(insn, regs);
> -       auprobe->resume_era = regs->csr_era;
>
>         return true;
>  }
> --
> 2.42.0
>
>
Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era
Posted by Tiezhu Yang 9 months ago 
On 05/13/2025 11:13 PM, Huacai Chen wrote:
> Hi, Tiezhu,
>
> On Tue, May 13, 2025 at 5:21 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> arch_uprobe_skip_sstep() returns true if instruction was emulated,
>> that is to say, there is no need to single step for the emulated
>> instructions, it will point to the destination address directly
>> after the exception, so the resume_era related code is redundant,
>> just remove them.
>>
>> Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support")
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>  arch/loongarch/include/asm/uprobes.h | 1 -
>>  arch/loongarch/kernel/uprobes.c      | 7 +------
>>  2 files changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/uprobes.h b/arch/loongarch/include/asm/uprobes.h
>> index 99a0d198927f..025fc3f0a102 100644
>> --- a/arch/loongarch/include/asm/uprobes.h
>> +++ b/arch/loongarch/include/asm/uprobes.h
>> @@ -15,7 +15,6 @@ typedef u32 uprobe_opcode_t;
>>  #define UPROBE_XOLBP_INSN      __emit_break(BRK_UPROBE_XOLBP)
>>
>>  struct arch_uprobe {
>> -       unsigned long   resume_era;
>>         u32     insn[2];
>>         u32     ixol[2];
>>         bool    simulate;
>> diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c
>> index 0ab9d8d631c4..6022eb0f71db 100644
>> --- a/arch/loongarch/kernel/uprobes.c
>> +++ b/arch/loongarch/kernel/uprobes.c
>> @@ -52,11 +52,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
>>
>>         WARN_ON_ONCE(current->thread.trap_nr != UPROBE_TRAP_NR);
>>         current->thread.trap_nr = utask->autask.saved_trap_nr;
>> -
>> -       if (auprobe->simulate)
>> -               instruction_pointer_set(regs, auprobe->resume_era);
>> -       else
>> -               instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
>> +       instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
> This seems wrong. If in the simulate case, regs->csr_era has already
> pointed to the correct destination address, then here we should only
> handle the non-simulate case.

What is wrong with this code? AFAICT, the code is right.

Here are the call chains in the generic code of uprobe:

handle_swbp()
   arch_uprobe_skip_sstep()
   pre_ssout()
     arch_uprobe_pre_xol()
handle_singlestep()
   arch_uprobe_post_xol()

arch_uprobe_post_xol() only handles the instruction that is not emulated
because if arch_uprobe_skip_sstep() returns true, arch_uprobe_post_xol()
will not be called, it will be called only if arch_uprobe_skip_sstep()
returns false.

Thanks,
Tiezhu
Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era
Posted by Huacai Chen 9 months ago 
On Wed, May 14, 2025 at 5:33 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 05/13/2025 11:13 PM, Huacai Chen wrote:
> > Hi, Tiezhu,
> >
> > On Tue, May 13, 2025 at 5:21 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>
> >> arch_uprobe_skip_sstep() returns true if instruction was emulated,
> >> that is to say, there is no need to single step for the emulated
> >> instructions, it will point to the destination address directly
> >> after the exception, so the resume_era related code is redundant,
> >> just remove them.
> >>
> >> Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support")
> >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> >> ---
> >>  arch/loongarch/include/asm/uprobes.h | 1 -
> >>  arch/loongarch/kernel/uprobes.c      | 7 +------
> >>  2 files changed, 1 insertion(+), 7 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/uprobes.h b/arch/loongarch/include/asm/uprobes.h
> >> index 99a0d198927f..025fc3f0a102 100644
> >> --- a/arch/loongarch/include/asm/uprobes.h
> >> +++ b/arch/loongarch/include/asm/uprobes.h
> >> @@ -15,7 +15,6 @@ typedef u32 uprobe_opcode_t;
> >>  #define UPROBE_XOLBP_INSN      __emit_break(BRK_UPROBE_XOLBP)
> >>
> >>  struct arch_uprobe {
> >> -       unsigned long   resume_era;
> >>         u32     insn[2];
> >>         u32     ixol[2];
> >>         bool    simulate;
> >> diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c
> >> index 0ab9d8d631c4..6022eb0f71db 100644
> >> --- a/arch/loongarch/kernel/uprobes.c
> >> +++ b/arch/loongarch/kernel/uprobes.c
> >> @@ -52,11 +52,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> >>
> >>         WARN_ON_ONCE(current->thread.trap_nr != UPROBE_TRAP_NR);
> >>         current->thread.trap_nr = utask->autask.saved_trap_nr;
> >> -
> >> -       if (auprobe->simulate)
> >> -               instruction_pointer_set(regs, auprobe->resume_era);
> >> -       else
> >> -               instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
> >> +       instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE);
> > This seems wrong. If in the simulate case, regs->csr_era has already
> > pointed to the correct destination address, then here we should only
> > handle the non-simulate case.
>
> What is wrong with this code? AFAICT, the code is right.
>
> Here are the call chains in the generic code of uprobe:
>
> handle_swbp()
>    arch_uprobe_skip_sstep()
>    pre_ssout()
>      arch_uprobe_pre_xol()
> handle_singlestep()
>    arch_uprobe_post_xol()
>
> arch_uprobe_post_xol() only handles the instruction that is not emulated
> because if arch_uprobe_skip_sstep() returns true, arch_uprobe_post_xol()
> will not be called, it will be called only if arch_uprobe_skip_sstep()
> returns false.
OK, I know.

Huacai

>
> Thanks,
> Tiezhu
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.