From: Jiri Olsa <olsajiri@gmail.com>
To: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com
Subject: Re: [PATCH bpf-next v2 3/6] bpf: x86: Support in-register struct arguments
Date: Sun, 14 Aug 2022 22:24:24 +0200 [thread overview]
Message-ID: <YvlZ+ETaaTD3hwrM@krava> (raw)
In-Reply-To: <20220812052435.523068-1-yhs@fb.com>
On Thu, Aug 11, 2022 at 10:24:35PM -0700, Yonghong Song wrote:
SNIP
> }
>
> static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
> @@ -2020,6 +2081,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY];
> struct bpf_tramp_links *fexit = &tlinks[BPF_TRAMP_FEXIT];
> struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN];
> + int struct_val_off, extra_nregs = 0;
> u8 **branches = NULL;
> u8 *prog;
> bool save_ret;
> @@ -2028,6 +2090,20 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> if (nr_args > 6)
> return -ENOTSUPP;
>
> + for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
> + if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG) {
> + /* Only support up to 16 bytes struct which should keep
> + * values in registers.
> + */
it seems that if the struct contains 'double' field, it's passed in
SSE register, which we don't support is save/restore
we should probably check struct's BTF in btf_distill_func_proto and
fail if we found anything else than regular regs types?
> + if (m->arg_size[i] > 16)
> + return -ENOTSUPP;
> +
> + extra_nregs += (m->arg_size[i] + 7) / 8 - 1;
> + }
> + }
> + if (nr_args + extra_nregs > 6)
should this value be minus the number of actually found struct arguments?
> + return -ENOTSUPP;
> +
> /* Generated trampoline stack layout:
> *
> * RBP + 8 [ return address ]
> @@ -2066,6 +2142,13 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> stack_size += (sizeof(struct bpf_tramp_run_ctx) + 7) & ~0x7;
> run_ctx_off = stack_size;
>
> + /* For structure argument */
> + for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
> + if (m->arg_flags[i] & BTF_FMODEL_STRUCT_ARG)
> + stack_size += (m->arg_size[i] + 7) & ~0x7;
> + }
> + struct_val_off = stack_size;
could you please update the 'Generated trampoline stack layout' table
above with this offset
thanks,
jirka
> +
> if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> /* skip patched call instruction and point orig_call to actual
> * body of the kernel function.
> @@ -2101,7 +2184,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -ip_off);
> }
>
> - save_regs(m, &prog, nr_args, regs_off);
> + save_regs(m, &prog, nr_args, regs_off, struct_val_off);
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> /* arg1: mov rdi, im */
> @@ -2131,7 +2214,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> }
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> - restore_regs(m, &prog, nr_args, regs_off);
> + restore_regs(m, &prog, nr_args, regs_off, struct_val_off);
>
> if (flags & BPF_TRAMP_F_ORIG_STACK) {
> emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, 8);
> @@ -2172,7 +2255,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> }
>
> if (flags & BPF_TRAMP_F_RESTORE_REGS)
> - restore_regs(m, &prog, nr_args, regs_off);
> + restore_regs(m, &prog, nr_args, regs_off, struct_val_off);
>
> /* This needs to be done regardless. If there were fmod_ret programs,
> * the return value is only updated on the stack and still needs to be
> --
> 2.30.2
>
next prev parent reply other threads:[~2022-08-14 20:24 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-12 5:24 [PATCH bpf-next v2 0/6] bpf: Support struct argument for trampoline base progs Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 1/6] bpf: Add struct argument info in btf_func_model Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 2/6] bpf: x86: Rename stack_size to regs_off in {save,restore}_regs() Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 3/6] bpf: x86: Support in-register struct arguments Yonghong Song
2022-08-14 20:24 ` Jiri Olsa [this message]
2022-08-15 5:29 ` Yonghong Song
2022-08-15 7:29 ` Jiri Olsa
2022-08-15 15:25 ` Yonghong Song
2022-08-15 22:44 ` Alexei Starovoitov
2022-08-18 4:56 ` Yonghong Song
2022-08-18 20:44 ` Alexei Starovoitov
2022-08-24 19:04 ` Yonghong Song
2022-08-24 22:35 ` Alexei Starovoitov
2022-08-25 4:10 ` Yonghong Song
2022-08-24 19:05 ` Andrii Nakryiko
2022-08-25 4:04 ` Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 4/6] bpf: arm64: No support of struct argument Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 5/6] bpf: Populate struct argument info in btf_func_model Yonghong Song
2022-08-12 5:24 ` [PATCH bpf-next v2 6/6] selftests/bpf: Add struct argument tests with fentry/fexit programs Yonghong Song
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=YvlZ+ETaaTD3hwrM@krava \
--to=olsajiri@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=yhs@fb.com \
/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.