public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Eduard Zingerman <eddyz87@gmail.com>,
	bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, hffilwlqm@gmail.com
Subject: Re: [PATCH bpf-next 4/4] selftests/bpf: validate jit behaviour for tail calls
Date: Thu, 15 Aug 2024 14:32:48 -0700	[thread overview]
Message-ID: <78d7872d-4644-4a9a-9ef2-f4823fd7944f@linux.dev> (raw)
In-Reply-To: <20240809010518.1137758-5-eddyz87@gmail.com>


On 8/8/24 6:05 PM, Eduard Zingerman wrote:
> A program calling sub-program which does a tail call.
> The idea is to verify instructions generated by jit for tail calls:
> - in program and sub-program prologues;
> - for subprogram call instruction;
> - for tail call itself.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
>   .../selftests/bpf/prog_tests/verifier.c       |   2 +
>   .../bpf/progs/verifier_tailcall_jit.c         | 103 ++++++++++++++++++
>   2 files changed, 105 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index f8f546eba488..cf3662dbd24f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -75,6 +75,7 @@
>   #include "verifier_stack_ptr.skel.h"
>   #include "verifier_subprog_precision.skel.h"
>   #include "verifier_subreg.skel.h"
> +#include "verifier_tailcall_jit.skel.h"
>   #include "verifier_typedef.skel.h"
>   #include "verifier_uninit.skel.h"
>   #include "verifier_unpriv.skel.h"
> @@ -198,6 +199,7 @@ void test_verifier_spin_lock(void)            { RUN(verifier_spin_lock); }
>   void test_verifier_stack_ptr(void)            { RUN(verifier_stack_ptr); }
>   void test_verifier_subprog_precision(void)    { RUN(verifier_subprog_precision); }
>   void test_verifier_subreg(void)               { RUN(verifier_subreg); }
> +void test_verifier_tailcall_jit(void)         { RUN(verifier_tailcall_jit); }
>   void test_verifier_typedef(void)              { RUN(verifier_typedef); }
>   void test_verifier_uninit(void)               { RUN(verifier_uninit); }
>   void test_verifier_unpriv(void)               { RUN(verifier_unpriv); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c b/tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c
> new file mode 100644
> index 000000000000..1a09c76d7be0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +int main(void);
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
> +	__uint(max_entries, 1);
> +	__uint(key_size, sizeof(__u32));
> +	__array(values, void (void));
> +} jmp_table SEC(".maps") = {
> +	.values = {
> +		[0] = (void *) &main,
> +	},
> +};
> +
> +__noinline __auxiliary
> +static __naked int sub(void)
> +{
> +	asm volatile (
> +	"r2 = %[jmp_table] ll;"
> +	"r3 = 0;"
> +	"call 12;"
> +	"exit;"
> +	:
> +	: __imm_addr(jmp_table)
> +	: __clobber_all);
> +}
> +
> +__success
> +/* program entry for main(), regular function prologue */
> +__jit_x86("	endbr64")
> +__jit_x86("	nopl	(%rax,%rax)")
> +__jit_x86("	xorq	%rax, %rax")
> +__jit_x86("	pushq	%rbp")
> +__jit_x86("	movq	%rsp, %rbp")

How do we hanble multi architectures (x86, arm64, riscv64)?

Do we support the following?

__jit_x86(...)
__jit_x86(...)
...

__jit_arm64(...)
__jit_arm64(...)
...

__jit_riscv64(...)
__jit_riscv64(...)
...

Or we can use macro like

#ifdef __TARGET_ARCH_x86
__jit(...)
...
#elif defined(__TARGET_ARCH_arm64)
__jit(...)
...
#elif defined(...)

Or we can have

__arch_x86_64
__jit(...) // code for x86
...

__arch_arm64
__jit(...) // code for arm64
...

__arch_riscv
__jit(...) // code for riscv
...

For xlated, different archs could share the same code.
Bot for jited code, different arch has different encoding,
so we need to figure out a format suitable for multiple
archs.

> +/* tail call prologue for program:
> + * - establish memory location for tail call counter at &rbp[-8];
> + * - spill tail_call_cnt_ptr at &rbp[-16];
> + * - expect tail call counter to be passed in rax;
> + * - for entry program rax is a raw counter, value < 33;
> + * - for tail called program rax is tail_call_cnt_ptr (value > 33).
> + */
> +__jit_x86("	endbr64")
> +__jit_x86("	cmpq	$0x21, %rax")
> +__jit_x86("	ja	L0")
> +__jit_x86("	pushq	%rax")
> +__jit_x86("	movq	%rsp, %rax")
> +__jit_x86("	jmp	L1")
> +__jit_x86("L0:	pushq	%rax")			/* rbp[-8]  = rax         */
> +__jit_x86("L1:	pushq	%rax")			/* rbp[-16] = rax         */
> +/* on subprogram call restore rax to be tail_call_cnt_ptr from rbp[-16]
> + * (cause original rax might be clobbered by this point)
> + */
[...]

  parent reply	other threads:[~2024-08-15 21:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-09  1:05 [PATCH bpf-next 0/4] __jited_x86 test tag to check x86 assembly after jit Eduard Zingerman
2024-08-09  1:05 ` [PATCH bpf-next 1/4] selftests/bpf: less spam in the log for message matching Eduard Zingerman
2024-08-09  1:05 ` [PATCH bpf-next 2/4] selftests/bpf: utility function to get program disassembly after jit Eduard Zingerman
2024-08-13 16:05   ` Yonghong Song
2024-08-13 22:01     ` Eduard Zingerman
2024-08-15 19:27   ` Yonghong Song
2024-08-15 19:34     ` Eduard Zingerman
2024-08-15 21:06   ` Andrii Nakryiko
2024-08-15 21:50     ` Eduard Zingerman
2024-08-15 22:04       ` Andrii Nakryiko
2024-08-19 19:45     ` Eduard Zingerman
2024-08-19 21:05       ` Andrii Nakryiko
2024-08-09  1:05 ` [PATCH bpf-next 3/4] selftests/bpf: __jited_x86 test tag to check x86 assembly " Eduard Zingerman
2024-08-15 21:11   ` Andrii Nakryiko
2024-08-15 21:48     ` Eduard Zingerman
2024-08-09  1:05 ` [PATCH bpf-next 4/4] selftests/bpf: validate jit behaviour for tail calls Eduard Zingerman
2024-08-15 21:15   ` Andrii Nakryiko
2024-08-15 21:42     ` Eduard Zingerman
2024-08-15 22:07       ` Andrii Nakryiko
2024-08-15 22:10         ` Eduard Zingerman
2024-08-15 22:14           ` Andrii Nakryiko
2024-08-15 22:19             ` Eduard Zingerman
2024-08-15 21:32   ` Yonghong Song [this message]
2024-08-15 21:47     ` Eduard Zingerman
2024-08-15 22:09       ` Andrii Nakryiko
2024-08-15 22:16         ` Eduard Zingerman

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=78d7872d-4644-4a9a-9ef2-f4823fd7944f@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=hffilwlqm@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox