Re: [LSF/MM/BPF TOPIC] faster uprobes

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Mar 11, 2024 at 04:06:59PM +0100, Oleg Nesterov wrote:
> I forgot everything about the low-level x86_64 code, but...
> 
> On 03/11, Jiri Olsa wrote:
> >
> >  #ifdef CONFIG_X86_64
> > +
> > +asm (
> > +       ".pushsection .rodata\n"
> > +       ".global uretprobe_syscall_entry\n"
> > +       "uretprobe_syscall_entry:\n"
> > +       "push %rax\n"
> > +       "mov $462, %rax\n"
> > +       "syscall\n"
> 
> Hmm... I think you need to save/restore more registers clobbered by
> syscall/entry_SYSCALL_64 ?

hum, so the call happens on the function call return, so I thought
we should just preserve callee saved registers which seems to be
taken care of by the entry_SYSCALL_64 path.. I will double check

> 
> > +SYSCALL_DEFINE1(uprobe, unsigned long, cmd)
> > +{
> > +	struct pt_regs *regs = task_pt_regs(current);
> > +	unsigned long ax, err;
> > +
> > +	/*
> > +	 * We get invoked from the trampoline that pushed rax
> > +	 * value on stack, read and restore the value.
> > +	 */
> > +	err = copy_from_user((void*) &ax, (void *) regs->sp, sizeof(ax));
> > +	WARN_ON_ONCE(err);
> > +
> > +	regs->ax = ax;
> 
> probably not strictly needed, we are going to return ax...

it needs to be there for the bpf program to read proper return
value from regs

> 
> > +	regs->orig_ax = ax;
> 
> This doesn't look right. I think you need
> 
> 	regs->orig_ax = -1;
> 
> Say, to avoid the "Did we come from a system call" checks in
> arch_do_signal_or_restart() or handle_signal().

ugh right that's probably wrong, I need check on that

thanks,
jirka