All of lore.kernel.org
 help / color / mirror / Atom feed
From: Puranjay Mohan <puranjay@kernel.org>
To: Eduard Zingerman <eddyz87@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
	daniel@iogearbox.net, martin.lau@linux.dev, kernel-team@fb.com,
	yonghong.song@linux.dev, jose.marchesi@oracle.com
Subject: Re: [RFC bpf-next v1 3/8] bpf, x86: no_caller_saved_registers for bpf_get_smp_processor_id()
Date: Thu, 04 Jul 2024 11:19:20 +0000	[thread overview]
Message-ID: <mb61ptth5be13.fsf@kernel.org> (raw)
In-Reply-To: <f9f7326c570b6163279a991d71ed0a354ef6f80e.camel@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3080 bytes --]

Eduard Zingerman <eddyz87@gmail.com> writes:

> On Wed, 2024-07-03 at 11:27 +0000, Puranjay Mohan wrote:
>
> [...]
>
>> > > > > @@ -16030,7 +16030,14 @@ static u8 get_helper_reg_mask(const struct bpf_func_proto *fn)
>> > > > >   */
>> > > > >  static bool verifier_inlines_helper_call(struct bpf_verifier_env *env, s32 imm)
>> > > > >  {
>> > > > > -       return false;
>> > > > > +       switch (imm) {
>> > > > > +#ifdef CONFIG_X86_64
>> > > > > +       case BPF_FUNC_get_smp_processor_id:
>> > > > > +               return env->prog->jit_requested && bpf_jit_supports_percpu_insn();
>> > > > > +#endif
>> > > > 
>> > > > please see bpf_jit_inlines_helper_call(), arm64 and risc-v inline it
>> > > > in JIT, so we need to validate they don't assume any of R1-R5 register
>> > > > to be a scratch register
>> 
>> They don't assume any register to be scratch (except R0) so we can
>> enable this on arm64 and riscv.
>
> Puranjay, just out of curiosity and tangential to this patch-set,
> I see that get_smp_processor_id is implemented as follows in riscv jit:
>
>   emit_ld(bpf_to_rv_reg(BPF_REG_0, ctx), offsetof(struct thread_info, cpu),
> 	  RV_REG_TP, ctx);
>
> Where bpf_to_rv_reg() refers to regmap, which in turn has the following line:
>
>   static const int regmap[] = {
> 	[BPF_REG_0] =	RV_REG_A5,
> 	...
>   }
>
> At the same time, [1] says:
>
>> 18.2 RVG Calling Convention
>> ...
>> Values are returned from functions in integer registers a0 and a1 and
>> floating-point registers fa0 and fa1.
>
> [1] https://riscv.org/wp-content/uploads/2015/01/riscv-calling.pdf
>
> So, I would expect r0 to be mapped to a0, do you happen to know why is it a5?

I had the same question when I started working with the JITs. This is
seen on both risc-v and arm64, where as you said on risc-v R0 should be
mapped to A0 but is mapped to A5. Similarly, on ARM64, BPF_R0 should be
mapped to ARM64_R0 but is mapped to ARM64_R7.

Here is my understanding of this:

The reason for this quirk is the usage of BPF register R0 as defined by
BPF Registers and calling convention [1]

[1] says:

```
* R0: return value from function calls, and exit value for BPF programs
* R1 - R5: arguments for function calls
```

On arm64 and risc-v the first argument and the return value are
passed/returned in the same register, A0 on risc-v and R0 on arm64.

In BPF, the first argument to a function is passed in R1 and not in R0.
So when we map these registers to riscv or arm64 calling convention, we
have to map BPF_R1 to A0 on risc-v and to R0 on ARM64. This is to make
argument passing easy. Therefore BPF_R0 is mapped to A5 on risc-v and
ARM64_R7 on arm64.

And when we JIT the 'BPF_JMP | BPF_CALL' we add a mov instruction at the
end to move A0 to A5 on risc-v and R0 to R7 on arm64.

But when inlining the call we can directly put the result in A5 or R7. 

Thanks,
Puranjay

[1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next/+/refs/heads/master/Documentation/bpf/standardization/abi.rst

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

  reply	other threads:[~2024-07-04 11:19 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-29  9:47 [RFC bpf-next v1 0/8] no_caller_saved_registers attribute for helper calls Eduard Zingerman
2024-06-29  9:47 ` [RFC bpf-next v1 1/8] bpf: add a get_helper_proto() utility function Eduard Zingerman
2024-07-02  0:41   ` Andrii Nakryiko
2024-07-02 20:07     ` Eduard Zingerman
2024-06-29  9:47 ` [RFC bpf-next v1 2/8] bpf: no_caller_saved_registers attribute for helper calls Eduard Zingerman
2024-07-01 19:01   ` Eduard Zingerman
2024-07-02  0:41   ` Andrii Nakryiko
2024-07-02 20:38     ` Eduard Zingerman
2024-07-02 21:09       ` Andrii Nakryiko
2024-07-02 21:19         ` Eduard Zingerman
2024-07-02 21:22           ` Andrii Nakryiko
2024-07-03 11:57   ` Puranjay Mohan
2024-07-03 16:13     ` Eduard Zingerman
2024-07-04 10:55       ` Puranjay Mohan
2024-06-29  9:47 ` [RFC bpf-next v1 3/8] bpf, x86: no_caller_saved_registers for bpf_get_smp_processor_id() Eduard Zingerman
2024-07-02  0:41   ` Andrii Nakryiko
2024-07-02 20:43     ` Eduard Zingerman
2024-07-02 21:11       ` Andrii Nakryiko
2024-07-02 21:25         ` Eduard Zingerman
2024-07-03 11:27         ` Puranjay Mohan
2024-07-03 23:14           ` Eduard Zingerman
2024-07-04 11:19             ` Puranjay Mohan [this message]
2024-07-04 16:39               ` Eduard Zingerman
2024-07-04 17:00           ` Eduard Zingerman
2024-07-04 17:24             ` Puranjay Mohan
2024-07-04 17:39               ` Eduard Zingerman
2024-06-29  9:47 ` [RFC bpf-next v1 4/8] selftests/bpf: extract utility function for BPF disassembly Eduard Zingerman
2024-07-02  0:41   ` Andrii Nakryiko
2024-07-02 20:59     ` Eduard Zingerman
2024-07-02 21:16       ` Andrii Nakryiko
2024-07-02 21:23         ` Eduard Zingerman
2024-06-29  9:47 ` [RFC bpf-next v1 5/8] selftests/bpf: no need to track next_match_pos in struct test_loader Eduard Zingerman
2024-07-02  0:41   ` Andrii Nakryiko
2024-07-02 21:05     ` Eduard Zingerman
2024-07-02 21:18       ` Andrii Nakryiko
2024-06-29  9:47 ` [RFC bpf-next v1 6/8] selftests/bpf: extract test_loader->expect_msgs as a data structure Eduard Zingerman
2024-07-02  0:42   ` Andrii Nakryiko
2024-07-02 21:06     ` Eduard Zingerman
2024-06-29  9:47 ` [RFC bpf-next v1 7/8] selftests/bpf: allow checking xlated programs in verifier_* tests Eduard Zingerman
2024-07-02  0:42   ` Andrii Nakryiko
2024-07-02 21:07     ` Eduard Zingerman
2024-07-02 21:19       ` Andrii Nakryiko
2024-06-29  9:47 ` [RFC bpf-next v1 8/8] selftests/bpf: test no_caller_saved_registers spill/fill removal Eduard Zingerman
2024-07-02  0:42   ` Andrii Nakryiko
2024-07-02 21:12     ` Eduard Zingerman
2024-07-02 21:20       ` Andrii Nakryiko
2024-07-02  0:41 ` [RFC bpf-next v1 0/8] no_caller_saved_registers attribute for helper calls Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mb61ptth5be13.fsf@kernel.org \
    --to=puranjay@kernel.org \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=jose.marchesi@oracle.com \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.