On Tue, Apr 12, 2022 at 9:56 AM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
>
> BPF trampolines will create a bpf_tramp_run_ctx, a bpf_run_ctx, on
> stacks and set/reset the current bpf_run_ctx before/after calling a
> bpf_prog.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> ---
> arch/x86/net/bpf_jit_comp.c | 55 +++++++++++++++++++++++++++++++++++++
> include/linux/bpf.h | 17 +++++++++---
> kernel/bpf/syscall.c | 7 +++--
> kernel/bpf/trampoline.c | 20 +++++++++++---
> 4 files changed, 89 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 4dcc0b1ac770..0f521be68f7b 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1766,10 +1766,26 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
> {
> u8 *prog = *pprog;
> u8 *jmp_insn;
> + int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx, bpf_cookie);
> struct bpf_prog *p = l->link.prog;
>
> + /* mov rdi, 0 */
> + emit_mov_imm64(&prog, BPF_REG_1, 0, 0);
> +
> + /* Prepare struct bpf_tramp_run_ctx.
> + *
> + * bpf_tramp_run_ctx is already preserved by
> + * arch_prepare_bpf_trampoline().
> + *
> + * mov QWORD PTR [rsp + ctx_cookie_off], rdi
> + */
> + EMIT4(0x48, 0x89, 0x7C, 0x24); EMIT1(ctx_cookie_off);
> +
> /* arg1: mov rdi, progs[i] */
> emit_mov_imm64(&prog, BPF_REG_1, (long) p >> 32, (u32) (long) p);
> + /* arg2: mov rsi, rsp (struct bpf_run_ctx *) */
> + EMIT3(0x48, 0x89, 0xE6);
> +
> if (emit_call(&prog,
> p->aux->sleepable ? __bpf_prog_enter_sleepable :
> __bpf_prog_enter, prog))
> @@ -1815,6 +1831,8 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
> emit_mov_imm64(&prog, BPF_REG_1, (long) p >> 32, (u32) (long) p);
> /* arg2: mov rsi, rbx <- start time in nsec */
> emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_6);
> + /* arg3: mov rdx, rsp (struct bpf_run_ctx *) */
> + EMIT3(0x48, 0x89, 0xE2);
> if (emit_call(&prog,
> p->aux->sleepable ? __bpf_prog_exit_sleepable :
> __bpf_prog_exit, prog))
> @@ -2079,6 +2097,16 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> }
> }
>
> + if (nr_args < 3 && (fentry->nr_links || fexit->nr_links || fmod_ret->nr_links))
> + EMIT1(0x52); /* push rdx */
this nr_args < 3 condition is new, maybe leave a comment on why we
need this? Also instead of repeating this whole (fentry->nr_links ||
... || ...) check, why not move if (nr_args < 3) inside the if right
below?
> +
> + if (fentry->nr_links || fexit->nr_links || fmod_ret->nr_links) {
if (nr_args > 3) here?
> + /* Prepare struct bpf_tramp_run_ctx.
> + * sub rsp, sizeof(struct bpf_tramp_run_ctx)
> + */
> + EMIT4(0x48, 0x83, 0xEC, sizeof(struct bpf_tramp_run_ctx));
> + }
> +
> if (fentry->nr_links)
> if (invoke_bpf(m, &prog, fentry, regs_off,
> flags & BPF_TRAMP_F_RET_FENTRY_RET))
[...]
> if (fmod_ret->nr_links) {
> @@ -2133,6 +2179,15 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> goto cleanup;
> }
>
> + /* pop struct bpf_tramp_run_ctx
> + * add rsp, sizeof(struct bpf_tramp_run_ctx)
> + */
> + if (fentry->nr_links || fexit->nr_links || fmod_ret->nr_links)
well, actually, can it ever be that this condition doesn't hold? That
would mean we are generating empty trampoline for some reason, no? Do
we do that? Checking bpf_trampoline_update() and
bpf_struct_ops_prepare_trampoline() doesn't seem like we ever do this.
So seems like all these checks can be dropped?
> + EMIT4(0x48, 0x83, 0xC4, sizeof(struct bpf_tramp_run_ctx));
> +
> + if (nr_args < 3 && (fentry->nr_links || fexit->nr_links || fmod_ret->nr_links))
> + EMIT1(0x5A); /* pop rdx */
same, move it inside if above?
> +
> if (flags & BPF_TRAMP_F_RESTORE_REGS)
> restore_regs(m, &prog, nr_args, regs_off);
>
[...]
