Re: [PATCH bpf-next v2 2/2] selftests/bpf: Enable stack argument tests for RV64

[bot+bpf-ci@kernel.org]

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

> diff --git a/tools/testing/selftests/bpf/progs/exceptions.c b/tools/testing/selftests/bpf/progs/exceptions.c
> index c8d716fbd4191..bae20f0f247eb 100644
> --- a/tools/testing/selftests/bpf/progs/exceptions.c
> +++ b/tools/testing/selftests/bpf/progs/exceptions.c
> @@ -379,8 +379,9 @@ int exception_bad_assert_range_with(struct __sk_buff *ctx)
>  	return 1;
>  }
>
> -#if (defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64)) \
> -	&& defined(__BPF_FEATURE_STACK_ARGUMENT)
> +#if (defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64) || \
> +	(defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64)) && \
> +	defined(__BPF_FEATURE_STACK_ARGUMENT)
>
>  const volatile bool has_stack_arg = true;

Can the code following this guard actually run on RV64?

The block enabled by this guard contains four bpf_throw()-based programs
(exception_throw_stack_arg, exception_throw_after_stack_arg,
exception_throw_subprog_stack_arg, exception_throw_subprog_after_stack_arg)
plus the has_stack_arg flag. bpf_throw is only accepted by the verifier
when the target JIT supports exceptions:

kernel/bpf/verifier.c
    if (!bpf_jit_supports_exceptions()) {

RISC-V implements neither bpf_jit_supports_exceptions() nor
arch_bpf_stack_walk() (only arch/x86, arm64, s390 and powerpc do), so both
fall back to the __weak definitions in kernel/bpf/core.c:

    bool __weak bpf_jit_supports_exceptions(void) { return false; }
    void __weak arch_bpf_stack_walk(...) { }

Consequently exceptions.bpf.o cannot be loaded at all on RV64 - the rest
of exceptions.c is full of bpf_throw() outside this guard - and the test
is already excluded from RV64 runs:

tools/testing/selftests/bpf/DENYLIST.riscv64
    # riscv64 deny list for BPF CI and local vmtest
    exceptions                     # JIT does not support exceptions

prog_tests/exceptions.c only consumes has_stack_arg after
exceptions__load() succeeds:

    ret = exceptions__load(skel);
    if (!ASSERT_OK(ret, "exceptions__load"))
            goto done;
    ...
    if (skel->rodata->has_stack_arg) {
            RUN_SUCCESS(exception_throw_stack_arg, 56);

so on RV64 the flag is never read. The hunk therefore changes nothing
observable on riscv64 and cannot have been validated there, which
conflicts with the commit subject 'Enable the stack argument selftests for
RV64'. The other seven hunks are genuinely effective; only this one is
not.

Either drop the exceptions.c hunk from this patch (and add it when the
RV64 JIT gains arch_bpf_stack_walk()/bpf_jit_supports_exceptions()), or
state in the changelog that it is deliberately pre-enabled for a future
RV64 exception implementation and remains untested for now.


---
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/31677823910