Hi Alexei,
> > +// New helper added
> > +static long (*bpf_access_process_vm)(
> > + struct task_struct *tsk,
> > + unsigned long addr,
> > + void *buf,
> > + int len,
> > + unsigned int gup_flags) = (void *)186;
>
> This shouldn't be needed.
> Since patch 1 updates tools/include/uapi/linux/bpf.h
> it will be in bpf_helper_defs.h automatically.
I will fix. This is my first time writing selftests, so I am not too familiar
with how these are built and run. For my understanding, are these tests
meant to be built and run after booting the new kernel?
> > +
> > +// Copied from include/linux/mm.h
> > +#define FOLL_REMOTE 0x2000 /* we are working on non-current tsk/mm */
>
> Please use C style comments only.
I will fix.
> > + numread = bpf_access_process_vm(task,
> > + (unsigned long)ptr,
> > + (void *)&user_data,
> > + sizeof(uint32_t),
> > + FOLL_REMOTE);
>
> We probably would need to hide flags like FOLL_REMOTE
> inside the helper otherwise prog might confuse the kernel.
> In this case I'm not even sure that FOLL_REMOTE is needed.
> I suspect gup_flags=0 in all cases will work fine.
> We're not doing write here and not pining anything.
> fast_gup is not necessary either.
Thanks for the suggestion! I'll remove the flag argument from the helper
to simplify the API for bpf programs. This means that the helper will have
the following signature:
bpf_access_process_vm(struct task_struct *tsk,
unsigned long addr,
void *buf,
int len);
Thanks for the feedback!
Kenny
