Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Jul 30, 2020 at 1:38 PM Jerry Cruntime <jerry.c.t@web.de> wrote:
>
> Hi,
>
>  > I've quickly looked up some doc on MIPS calling convention, doesn't
>  > seem like regs[8] is actually used for 5th input argument (the doc I
>  > found documented only the use of $4 through $7 for first 4 args).
>  > Should we drop PT_REGS_PARM5() for MIPS, while at it?
>
> My understanding is that with o32 only 4 arguments can be passed in
> registers ($4-$7). But n32 and n64 extended it to pass 8 arguments in
> registers ($4-$11).
>
> My source is "MIPS Run, Second Edition" from Dominic Sweetman table 11.2
> on page 327. It is also described here:
>
> https://en.wikipedia.org/wiki/Calling_convention#MIPS
>

Oh, right, I should have used Wikipedia instead. :)

Acked-by: Andrii Nakryiko <andriin@fb.com>

>
> On 7/30/20 9:55 PM, Andrii Nakryiko wrote:
> > On Thu, Jul 30, 2020 at 4:45 AM Jerry Cruntime <jerry.c.t@web.de> wrote:
> >>
> >> The o32, n32 and n64 calling conventions require the return
> >> value to be stored in $v0 which maps to $2 register, i.e.,
> >> the second register.
> >>
> >> Fixes: c1932cd ("bpf: Add MIPS support to samples/bpf.")
> >> ---
> >>    tools/lib/bpf/bpf_tracing.h | 2 +-
> >>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> >> index 58eceb884..ae205dcf8 100644
> >> --- a/tools/lib/bpf/bpf_tracing.h
> >> +++ b/tools/lib/bpf/bpf_tracing.h
> >> @@ -215,7 +215,7 @@ struct pt_regs;
> >>    #define PT_REGS_PARM5(x) ((x)->regs[8])
> >
> > I've quickly looked up some doc on MIPS calling convention, doesn't
> > seem like regs[8] is actually used for 5th input argument (the doc I
> > found documented only the use of $4 through $7 for first 4 args).
> > Should we drop PT_REGS_PARM5() for MIPS, while at it?
> >
> >>    #define PT_REGS_RET(x) ((x)->regs[31])
> >>    #define PT_REGS_FP(x) ((x)->regs[30]) /* Works only with
> >> CONFIG_FRAME_POINTER */
> >> -#define PT_REGS_RC(x) ((x)->regs[1])
> >> +#define PT_REGS_RC(x) ((x)->regs[2])
> >
> > This looks good, though.
> >
> >>    #define PT_REGS_SP(x) ((x)->regs[29])
> >>    #define PT_REGS_IP(x) ((x)->cp0_epc)
> >>
> >> --
> >> 2.17.1