On Fri, Jul 11, 2025 at 10:29:18AM +0200, Jiri Olsa wrote: > +enum { > + OPT_PART, > + OPT_INSN, > + UNOPT_INT3, > + UNOPT_PART, > +}; > + > +struct write_opcode_ctx { > + unsigned long base; > + int update; > +}; > + > +static int is_call_insn(uprobe_opcode_t *insn) > +{ > + return *insn == CALL_INSN_OPCODE; > +} > + > +static int verify_insn(struct page *page, unsigned long vaddr, > uprobe_opcode_t *new_opcode, > + int nbytes, void *data) > +{ > + struct write_opcode_ctx *ctx = data; > + uprobe_opcode_t old_opcode[5]; > + > + uprobe_copy_from_page(page, ctx->base, (uprobe_opcode_t *) &old_opcode, > 5); > + > + switch (ctx->update) { > + case OPT_PART: > + case OPT_INSN: > + if (is_swbp_insn(&old_opcode[0])) > + return 1; > + break; > + case UNOPT_INT3: > + if (is_call_insn(&old_opcode[0])) > + return 1; > + break; > + case UNOPT_PART: > + if (is_swbp_insn(&old_opcode[0])) > + return 1; > + break; > + } > + > + return -1; > +} > + > +static int write_insn(struct arch_uprobe *auprobe, struct vm_area_struct > *vma, unsigned long vaddr, > + uprobe_opcode_t *insn, int nbytes, void *ctx) > +{ > + return uprobe_write(auprobe, vma, vaddr, insn, nbytes, verify_insn, > + true /* is_register */, false /* do_update_ref_ctr > */, ctx); > +} > + > +static void relative_call(void *dest, long from, long to) > +{ > + struct __packed __arch_relative_insn { > + u8 op; > + s32 raddr; > + } *insn; > + > + insn = (struct __arch_relative_insn *)dest; > + insn->raddr = (s32)(to - (from + 5)); > + insn->op = CALL_INSN_OPCODE; > +}
We already have this in asm/text-patching.h, its called __text_gen_insn(). > + > +static int swbp_optimize(struct arch_uprobe *auprobe, struct vm_area_struct > *vma, > + unsigned long vaddr, unsigned long tramp) > +{ > + struct write_opcode_ctx ctx = { > + .base = vaddr, > + }; > + char call[5]; > + int err; > + > + relative_call(call, vaddr, tramp); __text_gen_insn(call, CALL_INSN_OPCODE, vaddr, tramp, CALL_INSN_SIZE); > + > + /* > + * We are in state where breakpoint (int3) is installed on top of first > + * byte of the nop5 instruction. We will do following steps to overwrite > + * this to call instruction: > + * > + * - sync cores > + * - write last 4 bytes of the call instruction > + * - sync cores > + * - update the call instruction opcode The sanctioned text poke sequence has another sync-core at the end. Please also do this. > + */ > + > + smp_text_poke_sync_each_cpu(); > + > + ctx.update = OPT_PART; > + err = write_insn(auprobe, vma, vaddr + 1, call + 1, 4, &ctx); > + if (err) > + return err; > + > + smp_text_poke_sync_each_cpu(); > + > + ctx.update = OPT_INSN; > + return write_insn(auprobe, vma, vaddr, call, 1, &ctx); > +} > + > +static int swbp_unoptimize(struct arch_uprobe *auprobe, struct > vm_area_struct *vma, > + unsigned long vaddr) > +{ > + uprobe_opcode_t int3 = UPROBE_SWBP_INSN; > + struct write_opcode_ctx ctx = { > + .base = vaddr, > + }; > + int err; > + > + /* > + * We need to overwrite call instruction into nop5 instruction with > + * breakpoint (int3) installed on top of its first byte. We will: > + * > + * - overwrite call opcode with breakpoint (int3) > + * - sync cores > + * - write last 4 bytes of the nop5 instruction > + * - sync cores > + */ > + > + ctx.update = UNOPT_INT3; > + err = write_insn(auprobe, vma, vaddr, &int3, 1, &ctx); > + if (err) > + return err; > + > + smp_text_poke_sync_each_cpu(); > + > + ctx.update = UNOPT_PART; > + err = write_insn(auprobe, vma, vaddr + 1, (uprobe_opcode_t *) > auprobe->insn + 1, 4, &ctx); > + > + smp_text_poke_sync_each_cpu(); > + return err; > +} Please unify these two functions; it makes absolutely no sense to have two copies of this logic around.