Re: [PATCH RFCv2 08/18] uprobes/x86: Add uprobe syscall to speed up uprobe

On Mon, Feb 24, 2025 at 11:22:42AM -0800, Alexei Starovoitov wrote:

On Mon, Feb 24, 2025 at 6:08 AM Jiri Olsa [off-list ref] wrote:

quoted

+SYSCALL_DEFINE0(uprobe)
+{
+       struct pt_regs *regs = task_pt_regs(current);
+       unsigned long bp_vaddr;
+       int err;
+
+       err = copy_from_user(&bp_vaddr, (void __user *)regs->sp + 3*8, sizeof(bp_vaddr));
+       if (err) {
+               force_sig(SIGILL);
+               return -1;
+       }
+
+       /* Allow execution only from uprobe trampolines. */
+       if (!in_uprobe_trampoline(regs->ip)) {
+               force_sig(SIGILL);
+               return -1;
+       }
+
+       handle_syscall_uprobe(regs, bp_vaddr - 5);
+       return 0;
+}
+
+asm (
+       ".pushsection .rodata\n"
+       ".balign " __stringify(PAGE_SIZE) "\n"
+       "uprobe_trampoline_entry:\n"
+       "endbr64\n"

why endbr is there?
The trampoline is called with a direct call.

ok, that's wrong, will remove that

quoted

+       "push %rcx\n"
+       "push %r11\n"
+       "push %rax\n"
+       "movq $" __stringify(__NR_uprobe) ", %rax\n"

To avoid introducing a new syscall for a very similar operation
can we disambiguate uprobe vs uretprobe via %rdi or
some other way?
imo not too late to change uretprobe api.
Maybe it was discussed already.

yes, I recall discussing that early during uretprobe work with the decision to
have separate syscalls for each uprobe and uretprobe.. however wrt recent seccomp
changes, it might be easier just to add argument to uretprobe syscall to handle
uprobe

too bad it's not the other way around.. uprobe syscall with argument to do uretprobe
would sound better

quoted

+       "syscall\n"
+       "pop %rax\n"
+       "pop %r11\n"
+       "pop %rcx\n"
+       "ret\n"

In later patches I see nop5 is replaced with a call to
uprobe_trampoline_entry, but which part saves
rdi and other regs?
Compiler doesn't automatically spill/fill around USDT's nop/nop5.
Selftest is doing:
+__naked noinline void uprobe_test(void)
so just lucky ?

if you mean registers that would carry usdt arguments, ebpf programs
access those based on assembler operand string stored in usdt record:

  stapsdt              0x00000048       NT_STAPSDT (SystemTap probe descriptors)
    Provider: test
    Name: usdt3
    Location: 0x0000000000712f2f, Base: 0x0000000002f516b0, Semaphore: 0x0000000003348ec2
->  Arguments: -4@-1192(%rbp) -8@-1200(%rbp) 8@%rax

it's up to bpf program to know which register(+offset) to access, libbpf have all
this logic hidden behind usdt_manager_attach_usdt and bpf_usdt_arg bpf call

the trampoline only saves rcx/r11/rax, because they are changed by syscall instruction

but actually I forgot to load these saved values (of rcx/r11/rax) and rsp into regs that
are passed to ebpf program, (like we do in uretprobe syscall) will fix that in next version

I'll add tests for optimized usdt with more arguments

thanks,
jirka