Re: [PATCH bpf-next 10/12] selftests/bpf: Add inline-asm and subprog tests for R0:R2 returns

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2026-07-08 at 13:10 -0700, Yonghong Song wrote:
> Add inline-asm and subprogram tests for more coverage, including
> BPF-to-BPF cases, kfunc cases, backtracking and liveness. Both positive
> and negative tests are added.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

[...]

> @@ -93,4 +97,25 @@ void test_aggregate_ret(void)
>  		run_prog(skel_union_c->progs.aggregate_ret_union_c_test, true);
>  
>  	aggregate_ret_union_c__destroy(skel_union_c);
> +
> +	skel_run = aggregate_ret_run__open_and_load();
> +	if (!ASSERT_OK_PTR(skel_run, "skel_run_open_load"))
> +		return;
> +
> +	/* Inline-asm variant: compiler-version independent, always runs. */
> +	if (test__start_subtest("asm"))
> +		run_prog(skel_run->progs.aggregate_ret_asm_test, false);
> +
> +	/* Struct-by-value kfunc returns, also compiler-version independent. */
> +	if (test__start_subtest("struct"))
> +		run_prog(skel_run->progs.aggregate_ret_struct_test, false);
> +
> +	/* Union-by-value kfunc return, compiler-version independent. */
> +	if (test__start_subtest("union"))
> +		run_prog(skel_run->progs.aggregate_ret_union_test, false);
> +
> +	aggregate_ret_run__destroy(skel_run);
> +
> +	RUN_TESTS(aggregate_ret_func);
> +	RUN_TESTS(aggregate_ret_kfunc);

This can be folded into tools/testing/selftests/bpf/prog_tests/verifier.c.

>  }
> diff --git a/tools/testing/selftests/bpf/progs/aggregate_ret_func.c b/tools/testing/selftests/bpf/progs/aggregate_ret_func.c
> new file mode 100644
> index 000000000000..31e176103374
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/aggregate_ret_func.c
> @@ -0,0 +1,361 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +typedef unsigned __int128 u128;
> +
> +__naked u128 global_agg_good(void)
> +{
> +	asm volatile (
> +	"r0 = 0x1234;"	/* low 64 bits */
> +	"r2 = 0x5678;"	/* high 64 bits */
> +	"exit;"
> +	);
> +}
> +
> +__naked u128 global_agg_bad(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"exit;"
> +	);
> +}

C version of tests from a previous patch already checks global vs.
local functions. I suggest dropping that version from C tests and keep
in assembly only to reduce the duplication.

[...]

> +SEC("tc")
> +__success __retval(0)
> +int aggregate_ret_global_union(void *ctx)
> +{
> +	__u64 lo, hi;
> +
> +	asm volatile (
> +	"call %[global_ret_union];"
> +	"%[lo] = r0;"
> +	"%[hi] = r2;"
> +	: [lo]"=r"(lo), [hi]"=r"(hi)
> +	: __imm(global_ret_union)
> +	: "r0", "r1", "r2", "r3", "r4", "r5");
> +	if (lo != 0x1234)
> +		return 1;
> +	if (hi != 0x5678)
> +		return 2;
> +	return 0;
> +}

And here I'd only keep the C version.

> +__naked struct with_ptr global_ret_struct_ptr(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"r2 = 0;"
> +	"exit;"
> +	);
> +}

[...]

> +static __naked u128 agg_callee(void)
> +{
> +	asm volatile (
> +	"r0 = 1;"
> +	"r2 = 2;"
> +	"exit;"
> +	);
> +}
> +
> +SEC("tc")
> +__log_level(2)
> +__msg("Live regs before insn:")
> +__msg("0: .12345.... (85) call pc+2")
> +__msg("1: ..2....... (bf) r0 = r2")
> +__msg("2: 0......... (95) exit")
> +__msg("3: .......... (b7) r0 = 1")
> +__msg("4: 0......... (b7) r2 = 2")
> +__msg("5: 0.2....... (95) exit")
> +__naked int aggregate_ret_live(void)
> +{
> +	asm volatile (
> +	"call %[agg_callee];"
> +	"r0 = r2;"
> +	"exit;"
> +	:
> +	: [agg_callee]"i"(agg_callee)
> +	: __clobber_all);
> +}

Nit: usually we add such tests to compute_live_registers.c.

[...]

> diff --git a/tools/testing/selftests/bpf/progs/aggregate_ret_run.c b/tools/testing/selftests/bpf/progs/aggregate_ret_run.c

I think the test cases in this file are by a large degree redundant.
A single test case calling a kfunc returning a small struct and
checking returned values would suffice.

[...]