* [PATCH 0/2] Refine the logic of uprobe on LoongArch
@ 2025-05-13 9:21 Tiezhu Yang
2025-05-13 9:21 ` [PATCH 1/2] LoongArch: uprobe: Remove user_{en,dis}able_single_step() Tiezhu Yang
2025-05-13 9:21 ` [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era Tiezhu Yang
0 siblings, 2 replies; 6+ messages in thread
From: Tiezhu Yang @ 2025-05-13 9:21 UTC (permalink / raw)
To: Huacai Chen; +Cc: loongarch, linux-kernel
Tiezhu Yang (2):
LoongArch: uprobe: Remove user_{en,dis}able_single_step()
LoongArch: uprobe: Remove redundant code about resume_era
arch/loongarch/include/asm/uprobes.h | 1 -
arch/loongarch/kernel/uprobes.c | 11 +----------
2 files changed, 1 insertion(+), 11 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] LoongArch: uprobe: Remove user_{en,dis}able_single_step() 2025-05-13 9:21 [PATCH 0/2] Refine the logic of uprobe on LoongArch Tiezhu Yang @ 2025-05-13 9:21 ` Tiezhu Yang 2025-05-13 9:21 ` [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era Tiezhu Yang 1 sibling, 0 replies; 6+ messages in thread From: Tiezhu Yang @ 2025-05-13 9:21 UTC (permalink / raw) To: Huacai Chen; +Cc: loongarch, linux-kernel When executing "perf probe" and "perf stat" test case about cryptographic algorithm, the output shows that "Trace/breakpoint trap", this is because it uses the software singlestep breakpoint for uprobe on LoongArch, so no need to use the hardware singlestep, just remove the related function call user_{en,dis}able_single_step() for uprobe on LoongArch. How to reproduce: Please make sure CONFIG_UPROBE_EVENTS is set and openssl supports algorithm sm2, then execute the following command. cd tools/perf && make sudo ./perf probe -x /usr/lib64/libcrypto.so BN_mod_mul_montgomery sudo ./perf stat -e probe_libcrypto:BN_mod_mul_montgomery openssl speed sm2 Fixes: 19bc6cb64092 ("LoongArch: Add uprobes support") Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/loongarch/kernel/uprobes.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/loongarch/kernel/uprobes.c b/arch/loongarch/kernel/uprobes.c index 87abc7137b73..0ab9d8d631c4 100644 --- a/arch/loongarch/kernel/uprobes.c +++ b/arch/loongarch/kernel/uprobes.c @@ -42,7 +42,6 @@ int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) utask->autask.saved_trap_nr = current->thread.trap_nr; current->thread.trap_nr = UPROBE_TRAP_NR; instruction_pointer_set(regs, utask->xol_vaddr); - user_enable_single_step(current); return 0; } @@ -59,8 +58,6 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) else instruction_pointer_set(regs, utask->vaddr + LOONGARCH_INSN_SIZE); - user_disable_single_step(current); - return 0; } @@ -70,7 +67,6 @@ void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) current->thread.trap_nr = utask->autask.saved_trap_nr; instruction_pointer_set(regs, utask->vaddr); - user_disable_single_step(current); } bool arch_uprobe_xol_was_trapped(struct task_struct *t) -- 2.42.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era 2025-05-13 9:21 [PATCH 0/2] Refine the logic of uprobe on LoongArch Tiezhu Yang 2025-05-13 9:21 ` [PATCH 1/2] LoongArch: uprobe: Remove user_{en,dis}able_single_step() Tiezhu Yang @ 2025-05-13 9:21 ` Tiezhu Yang 2025-05-13 15:13 ` Huacai Chen 1 sibling, 1 reply; 6+ messages in thread From: Tiezhu Yang @ 2025-05-13 9:21 UTC (permalink / raw) To: Huacai Chen; +Cc: loongarch, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era 2025-05-13 9:21 ` [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era Tiezhu Yang @ 2025-05-13 15:13 ` Huacai Chen 2025-05-14 9:32 ` Tiezhu Yang 0 siblings, 1 reply; 6+ messages in thread From: Huacai Chen @ 2025-05-13 15:13 UTC (permalink / raw) To: Tiezhu Yang; +Cc: loongarch, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era 2025-05-13 15:13 ` Huacai Chen @ 2025-05-14 9:32 ` Tiezhu Yang 2025-05-14 14:51 ` Huacai Chen 0 siblings, 1 reply; 6+ messages in thread From: Tiezhu Yang @ 2025-05-14 9:32 UTC (permalink / raw) To: Huacai Chen; +Cc: loongarch, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era 2025-05-14 9:32 ` Tiezhu Yang @ 2025-05-14 14:51 ` Huacai Chen 0 siblings, 0 replies; 6+ messages in thread From: Huacai Chen @ 2025-05-14 14:51 UTC (permalink / raw) To: Tiezhu Yang; +Cc: loongarch, linux-kernel 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-14 14:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-13 9:21 [PATCH 0/2] Refine the logic of uprobe on LoongArch Tiezhu Yang
2025-05-13 9:21 ` [PATCH 1/2] LoongArch: uprobe: Remove user_{en,dis}able_single_step() Tiezhu Yang
2025-05-13 9:21 ` [PATCH 2/2] LoongArch: uprobe: Remove redundant code about resume_era Tiezhu Yang
2025-05-13 15:13 ` Huacai Chen
2025-05-14 9:32 ` Tiezhu Yang
2025-05-14 14:51 ` Huacai Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®