Re: [PATCH bpf-next v3 1/2] bpf, riscv: Add BPF stack arguments support for RV64 JIT

[bot+bpf-ci@kernel.org]

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

> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 2504df1fa1118..62c84de22163f 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c

[ ... ]

> @@ -1891,6 +1916,21 @@ int bpf_jit_emit_insn
>  	case BPF_LDX | BPF_MEM | BPF_H:
>  	case BPF_LDX | BPF_MEM | BPF_W:
>  	case BPF_LDX | BPF_MEM | BPF_DW:
> +		if (insn->src_reg == BPF_REG_PARAMS) {
> +			int idx = off / 8 - 1;
> +
> +			if (is_12b_int(idx * 8)) {
> +				emit_ldx_insn(rd, idx * 8, RV_REG_FP, BPF_SIZE(code), false, ctx);
> +			} else {
> +				emit_imm(RV_REG_T1, idx * 8, ctx);
> +				emit_add(RV_REG_T1, RV_REG_T1, RV_REG_FP, ctx);
> +				emit_ldx_insn(rd, 0, RV_REG_T1, BPF_SIZE(code), false, ctx);
> +			}
> +			if (BPF_SIZE(code) != BPF_DW && insn_is_zext(&insn[1]))
> +				return 1;
> +			break;
> +		}
> +		fallthrough;

Does this new BPF-to-BPF stack argument convention work correctly with
the existing RV64 trampoline code?

The convention here reads incoming stack arguments from FP+0, FP+8, etc
(callee FP == caller SP). But looking at store_args() in this same file,
the trampoline appears to use the native RISC-V ABI instead:

arch/riscv/net/bpf_jit_comp64.c:store_args() {
    for (i = 0; i < nr_arg_slots; i++) {
        if (i < RV_MAX_REG_ARGS)
            emit_sd(RV_REG_FP, -args_off, RV_REG_A0 + i, ctx);
        else
            emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);

where RV_MAX_REG_ARGS is 8, so it expects arguments 6, 7, 8 in a5, a6, a7
and arguments 9+ at FP+16.

For a BPF subprogram using this new convention, arguments 6+ arrive in
memory, not in a5-a7. When an fentry or fexit program attaches to such a
subprogram, wouldn't it read unrelated register values instead of the real
arguments?

Both arm64 and x86-64 appear to avoid this by keeping their BPF stack
argument convention identical to their native ABIs. arm64 uses
`stack_arg_reg[] = { A64_R(5), A64_R(6), A64_R(7) }` and x86-64 keeps arg6
in r9, so their trampolines work without changes.

Can fentry/fexit attach to static BPF subprograms with more than 5
arguments, or is there a verifier check that prevents this?

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31761418445