Re: [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 4, 2022 at 4:30 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
>
> On Thu, 2022-02-03 at 21:23 -0800, Andrii Nakryiko wrote:
> > On Thu, Feb 3, 2022 at 9:22 PM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> > >
> > > On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > > wrote:
> > > >
> > > > Some architectures pass a pointer to struct pt_regs to syscall
> > > > handlers, others unpack it into individual function parameters.
> > > > Introduce a macro to describe what a particular arch does, using
> > > > `passing pt_regs *` as a default.
> > > >
> > > > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > > > ---
> > > >  tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > > b/tools/lib/bpf/bpf_tracing.h
> > > > index 30f0964f8c9e..400a4f002f77 100644
> > > > --- a/tools/lib/bpf/bpf_tracing.h
> > > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > > @@ -334,6 +334,15 @@ struct pt_regs;
> > > >
> > > >  #endif /* defined(bpf_target_defined) */
> > > >
> > > > +/*
> > > > + * When invoked from a syscall handler kprobe, returns a pointer
> > > > to a
> > > > + * struct pt_regs containing syscall arguments and suitable for
> > > > passing to
> > > > + * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
> > > > + */
> > > > +#ifndef PT_REGS_SYSCALL
> > > > +#define PT_REGS_SYSCALL(ctx) ((struct pt_regs
> > > > *)PT_REGS_PARM1(ctx))
> > > > +#endif
> > >
> > > maybe PT_REGS_SYSCALL_REGS? It returns regs, not the "syscall".
> > > PT_REGS prefix is for consistency with all other pt_regs macros,
> > > but
> > > "SYSCALL_REGS" is specifying what is actually returned by the macro
> > >
> >
> > Oh, and instead of casting to `struct pt_regs *` directly, maybe use
> > __PT_REGS_CAST() instead? For some architectures it probably should
> > stay user_pt_regs (or whatever it is there).
> >
> > > > +
> > > >  #ifndef ___bpf_concat
> > > >  #define ___bpf_concat(a, b) a ## b
> > > >  #endif
> > > > --
> > > > 2.34.1
> > > >
>
> I think it's better to keep this as struct pt_regs *, so that in
> bpf progs we can do
>
>         struct pt_regs *real_regs = PT_REGS_SYSCALL(ctx);
>
> without having to worry about which arch we are on, or using the
> opaque void *.

Makes sense, sounds good to me.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux