* [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
@ 2020-07-30 11:44 Jerry Cruntime
2020-07-30 19:55 ` Andrii Nakryiko
2020-07-30 23:00 ` Daniel Borkmann
0 siblings, 2 replies; 7+ messages in thread
From: Jerry Cruntime @ 2020-07-30 11:44 UTC (permalink / raw)
To: bpf
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])
#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])
#define PT_REGS_SP(x) ((x)->regs[29])
#define PT_REGS_IP(x) ((x)->cp0_epc)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-30 11:44 [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros Jerry Cruntime
@ 2020-07-30 19:55 ` Andrii Nakryiko
2020-07-30 20:38 ` Jerry Cruntime
2020-07-30 23:00 ` Daniel Borkmann
1 sibling, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2020-07-30 19:55 UTC (permalink / raw)
To: Jerry Cruntime; +Cc: bpf
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-30 19:55 ` Andrii Nakryiko
@ 2020-07-30 20:38 ` Jerry Cruntime
2020-07-30 20:43 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Jerry Cruntime @ 2020-07-30 20:38 UTC (permalink / raw)
To: Andrii Nakryiko, Jerry Cruntime; +Cc: bpf
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
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-30 20:38 ` Jerry Cruntime
@ 2020-07-30 20:43 ` Andrii Nakryiko
0 siblings, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2020-07-30 20:43 UTC (permalink / raw)
To: Jerry Cruntime; +Cc: bpf
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-30 11:44 [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros Jerry Cruntime
2020-07-30 19:55 ` Andrii Nakryiko
@ 2020-07-30 23:00 ` Daniel Borkmann
2020-07-31 9:56 ` Jerry Crunchtime
1 sibling, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2020-07-30 23:00 UTC (permalink / raw)
To: Jerry Cruntime, bpf
On 7/30/20 1:44 PM, Jerry Cruntime 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.")
Jerry, your patch is missing a Signed-off-by from you. It should be enough if
you just reply with one in here that I'll add to the commit message and I'll
take it via bpf tree then, thanks.
> ---
> 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])
> #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])
> #define PT_REGS_SP(x) ((x)->regs[29])
> #define PT_REGS_IP(x) ((x)->cp0_epc)
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-30 23:00 ` Daniel Borkmann
@ 2020-07-31 9:56 ` Jerry Crunchtime
2020-07-31 10:31 ` Daniel Borkmann
0 siblings, 1 reply; 7+ messages in thread
From: Jerry Crunchtime @ 2020-07-31 9:56 UTC (permalink / raw)
To: Daniel Borkmann, bpf
Hi,
> Jerry, your patch is missing a Signed-off-by from you.
Signed-off-by: Jerry Crunchtime <jerry.c.t@web.de>
On 7/31/20 1:00 AM, Daniel Borkmann wrote:
> On 7/30/20 1:44 PM, Jerry Crunchtime 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.")
>
> Jerry, your patch is missing a Signed-off-by from you. It should be
> enough if
> you just reply with one in here that I'll add to the commit message and
> I'll
> take it via bpf tree then, thanks.
>
>> ---
>> 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])
>> #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])
>> #define PT_REGS_SP(x) ((x)->regs[29])
>> #define PT_REGS_IP(x) ((x)->cp0_epc)
>>
>> --
>> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros
2020-07-31 9:56 ` Jerry Crunchtime
@ 2020-07-31 10:31 ` Daniel Borkmann
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Borkmann @ 2020-07-31 10:31 UTC (permalink / raw)
To: Jerry Crunchtime; +Cc: bpf, andriin
On 7/31/20 11:56 AM, Jerry Crunchtime wrote:
> Hi,
>
> > Jerry, your patch is missing a Signed-off-by from you.
>
> Signed-off-by: Jerry Crunchtime <jerry.c.t@web.de>
Thanks! One more comment below:
> On 7/31/20 1:00 AM, Daniel Borkmann wrote:
>> On 7/30/20 1:44 PM, Jerry Crunchtime 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.")
>>
>> Jerry, your patch is missing a Signed-off-by from you. It should be
>> enough if
>> you just reply with one in here that I'll add to the commit message and
>> I'll
>> take it via bpf tree then, thanks.
>>
>>> ---
>>> 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])
>>> #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])
>>> #define PT_REGS_SP(x) ((x)->regs[29])
>>> #define PT_REGS_IP(x) ((x)->cp0_epc)
While in process of applying, I noticed that there is one more thing broken; you
fixed the PT_REGS_RC() but by that logic at the same time we would also need to
fix PT_REGS_RC_CORE() given it still points to regs[1] as well:
#define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), regs[1])
Please fix up and resubmit.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-31 10:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 11:44 [PATCH bpf] libbpf: Fix register in PT_REGS MIPS macros Jerry Cruntime
2020-07-30 19:55 ` Andrii Nakryiko
2020-07-30 20:38 ` Jerry Cruntime
2020-07-30 20:43 ` Andrii Nakryiko
2020-07-30 23:00 ` Daniel Borkmann
2020-07-31 9:56 ` Jerry Crunchtime
2020-07-31 10:31 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).