On Sat, Dec 4, 2021 at 6:07 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> As suggested by Andrii, adding variables for registers and ip
> address offsets, which makes the code more clear, rather than
> abusing single stack_size variable for everything.
>
> Also describing the stack layout in the comment.
>
> There is no function change.
>
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> arch/x86/net/bpf_jit_comp.c | 42 ++++++++++++++++++++++++-------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 1d7b0c69b644..b106e80e8d9c 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1941,7 +1941,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> void *orig_call)
> {
> int ret, i, nr_args = m->nr_args;
> - int stack_size = nr_args * 8;
> + int regs_off, ip_off, stack_size = nr_args * 8;
> struct bpf_tramp_progs *fentry = &tprogs[BPF_TRAMP_FENTRY];
> struct bpf_tramp_progs *fexit = &tprogs[BPF_TRAMP_FEXIT];
> struct bpf_tramp_progs *fmod_ret = &tprogs[BPF_TRAMP_MODIFY_RETURN];
> @@ -1956,14 +1956,33 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> if (!is_valid_bpf_tramp_flags(flags))
> return -EINVAL;
>
> + /* Generated trampoline stack layout:
> + *
> + * RBP + 8 [ return address ]
> + * RBP + 0 [ RBP ]
> + *
> + * RBP - 8 [ return value ] BPF_TRAMP_F_CALL_ORIG or
> + * BPF_TRAMP_F_RET_FENTRY_RET flags
> + *
> + * [ reg_argN ] always
> + * [ ... ]
> + * RBP - regs_off [ reg_arg1 ]
> + *
I think it's also worth mentioning that context passed into
fentry/fexit programs are pointing here (makes it a bit easier to
track those ctx[-1] and ctx[-2] in the next patch.
> + * RBP - ip_off [ traced function ] BPF_TRAMP_F_IP_ARG flag
> + */
> +
> /* room for return value of orig_call or fentry prog */
> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> if (save_ret)
> stack_size += 8;
>
> + regs_off = stack_size;
> +
> if (flags & BPF_TRAMP_F_IP_ARG)
> stack_size += 8; /* room for IP address argument */
>
> + ip_off = stack_size;
> +
[...]
