[PATCH bpf-next 0/3] riscv: bpf: Add BPF private stack support

[PATCH bpf-next 0/3] riscv: bpf: Add BPF private stack support

[Chen Pei]

This series adds private-stack support to the RV64 BPF JIT.

The verifier rejects a struct_ops program that requests a private stack
with -EACCES when the JIT does not report
bpf_jit_supports_private_stack(), and sched_ext requests one for its
dispatch callback, so no sched_ext scheduler can load on riscv64 today.

Allocation, the 16-byte guard regions at both ends, the guard value and
the lifetime handling follow arm64, x86-64 and powerpc64. What is
specific to riscv64 is where the pointers end up:

 - Every BPF stack access goes through BPF_REG_FP (S5), so pointing S5
   at the top of the per-CPU area and leaving SP on the kernel stack is
   all the JIT has to do. S5 is callee-saved and each subprog derives
   its own, so bpf2bpf calls need no extra save and restore.
 - The outgoing stack-argument area stays in the kernel frame: callers
   write it relative to SP while callees read it relative to RV_REG_FP.
   Unlike x86-64, no register here is shared with the private-stack
   frame pointer, so no verifier-side exclusion is needed and stack
   arguments keep working together with a private stack. arm64 already
   does the same, and patch 3 measures the three ranges at run time and
   pins them in the disassembly.
 - The per-CPU lookup is emitted after the tail-call entry point. A
   program using a private stack can still be entered as a tail-call
   target, at RV_TAILCALL_OFFSET, so it has to set up its own S5 there;
   and since that offset is a constant added to the target's bpf_func,
   everything in front of it must be the same size in every program.

The base address is materialized with absolute moves, as on arm64,
x86-64 and powerpc64. Those three reach the per-CPU offset through gs:,
TPIDR or r13 and only have to move the base into a register; riscv64 has
no such register, so the address of __per_cpu_offset is loaded here too.
A PC-relative pair is not an option for the base itself: the dynamic
per-CPU area is vmalloc'd, further from the JIT region than auipc
reaches, which is why the whole setup is ten instructions on the sv39
kernel tested here. __per_cpu_offset is a kernel-image symbol and could
use the emit_addr() form that insn_is_mov_percpu_addr() already uses, at
the same two instructions; unifying the two needs an extra_pass argument
and an error return in bpf_jit_build_prologue(), which also touches
bpf_jit_comp32.c, so it is left as is. Both constants are fixed before
the offset-convergence loop, so the prologue length is stable across
passes.

RV32 does not declare support and keeps the existing behaviour; the
prologue signature is unchanged, so bpf_jit_comp32.c needs no edit.

Testing
=======
QEMU virt rv64, an 8 vCPU SMP kernel and a 1 vCPU UP kernel, source tree
clean at the head of this series, selftests built from these patches, no
private-stack related failures. On UP, kfunc_call/subprog,
kfunc_call/subprog_lskel and scx_simple are not run: all three fail the
same way on the unpatched base.

 - verifier_private_stack, struct_ops_private_stack,
   private_stack_runtime
 - tailcalls, stack_arg, subprogs, fentry_fexit, kfunc_call,
   percpu_alloc, async_stack_depth, arena_atomics, arena_ldsx
 - may_goto_priv_stack, may_goto_far, verifier_may_goto_1,
   verifier_may_goto_2
 - scx_simple on SMP

Chen Pei (3):
  riscv: bpf: Add BPF private stack support
  selftests/bpf: Enable private stack tests for riscv64
  selftests/bpf: Add a runtime test for private stack and stack
    arguments

 arch/riscv/net/bpf_jit.h                      |   1 +
 arch/riscv/net/bpf_jit_comp64.c               |  42 +++++-
 arch/riscv/net/bpf_jit_core.c                 |  84 +++++++++--
 .../selftests/bpf/private_stack_runtime.h     |  12 ++
 .../bpf/prog_tests/private_stack_runtime.c    |  83 +++++++++++
 .../bpf/prog_tests/struct_ops_private_stack.c |   3 +-
 .../bpf/progs/private_stack_runtime.c         | 140 ++++++++++++++++++
 .../bpf/progs/verifier_private_stack.c        |  41 ++++-
 8 files changed, 392 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/private_stack_runtime.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
 create mode 100644 tools/testing/selftests/bpf/progs/private_stack_runtime.c

-- 
2.50.1

[PATCH bpf-next 1/3] riscv: bpf: Add BPF private stack support

[Chen Pei]

A program on a private stack keeps its BPF frames in a per-CPU area of
its own rather than on the kernel stack of whichever context entered it.
bpf_enable_priv_stack() grants that mode only once the JIT reports
bpf_jit_supports_private_stack(), which riscv64 did not: programs there
always ran on the kernel stack, and a struct_ops program that requires
one was rejected with -EACCES.

Allocate a per-CPU area of the verifier calculated depth plus a 16 byte
guard region at each end, and point BPF_REG_FP (S5) at the top of it
while leaving SP on the kernel stack. Every BPF stack access already
goes through S5, and S5 is callee-saved with each subprog deriving its
own, so nothing else in the JIT has to know about the private stack.

The outgoing stack argument area stays in the kernel frame: callers
write it relative to SP, callees relative to RV_REG_FP, which the
prologue sets to the caller's SP.

The per-CPU lookup is emitted after the tail call entry point: a program
using a private stack can still be entered as a tail call target, and
RV_TAILCALL_OFFSET is a constant added to the target's bpf_func, so what
is in front of it must be the same size in every program.

The base is materialized with absolute moves, as on arm64, x86-64 and
powerpc64: the dynamic per-CPU area is vmalloc'd, beyond a PC-relative
auipc pair from the JIT region, and riscv64 has no segment or register
holding the per-CPU offset, so __per_cpu_offset needs materializing the
same way. Both constants are fixed before the offset convergence loop,
so the prologue length is stable across passes.

The guard layout and value follow arm64, x86-64 and powerpc64: a 16
byte region at each end holding two copies of the pattern. A jited
program has them checked when it is freed, reporting the first
corrupted CPU only; failure paths release the area unchecked since the
image never ran, which also clears priv_stack_ptr for the later
bpf_jit_free().

RV32 does not declare support and keeps the existing kernel stack
behaviour.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 arch/riscv/net/bpf_jit.h        |  1 +
 arch/riscv/net/bpf_jit_comp64.c | 42 +++++++++++++++--
 arch/riscv/net/bpf_jit_core.c   | 84 +++++++++++++++++++++++++++++----
 3 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index 5c4f53060c91..6d42083829f7 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -14,6 +14,7 @@
 
 #define RV_MAX_REG_ARGS		8
 #define RV_EXTRA_STK_ARGS	(RV_MAX_REG_ARGS - MAX_BPF_FUNC_REG_ARGS)
+#define PRIV_STACK_GUARD_SZ	16
 
 /* verify runtime detection extension status */
 #define rv_ext_enabled(ext) \
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 788fcc21fd5b..9a83770f0460 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -2122,12 +2122,34 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 	return 0;
 }
 
+static void emit_priv_stack_frame(struct rv_jit_context *ctx)
+{
+	u64 fp = (__force u64)ctx->prog->aux->priv_stack_ptr + PRIV_STACK_GUARD_SZ;
+
+	fp += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
+	emit_imm(RV_REG_S5, fp, ctx);
+#ifdef CONFIG_SMP
+	emit_lw(RV_REG_T1, offsetof(struct thread_info, cpu), RV_REG_TP, ctx);
+	emit_imm(RV_REG_T2, (u64)__per_cpu_offset, ctx);
+	emit_sh3add(RV_REG_T1, RV_REG_T1, RV_REG_T2, ctx);
+	emit_ld(RV_REG_T1, 0, RV_REG_T1, ctx);
+	emit_add(RV_REG_S5, RV_REG_S5, RV_REG_T1, ctx);
+#endif
+}
+
 void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 {
+	bool priv_stack = ctx->prog->aux->priv_stack_ptr;
 	int i, stack_adjust = 0, store_offset, bpf_stack_adjust = ctx->stack_arg_sz;
 
-	bpf_stack_adjust += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
-	if (bpf_stack_adjust)
+	/*
+	 * With a private stack only the outgoing stack argument area is left in
+	 * this frame. It has to stay: the caller writes it relative to SP while
+	 * the callee reads it relative to RV_REG_FP, the caller's SP set below.
+	 */
+	if (!priv_stack)
+		bpf_stack_adjust += round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
+	if (bpf_stack_adjust || priv_stack)
 		mark_fp(ctx);
 
 	if (seen_reg(RV_REG_RA, ctx))
@@ -2205,7 +2227,16 @@ void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 
 	emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust, ctx);
 
-	if (bpf_stack_adjust)
+	/*
+	 * Point BPF_REG_FP at the top of this CPU's private stack. A program
+	 * with one can still be entered as a tail call target, at
+	 * RV_TAILCALL_OFFSET, so this has to come after that entry point; the
+	 * offset is a constant added to the target's bpf_func, so everything in
+	 * front of it must also be the same size in every program.
+	 */
+	if (priv_stack)
+		emit_priv_stack_frame(ctx);
+	else if (bpf_stack_adjust)
 		emit_addi(RV_REG_S5, RV_REG_SP, bpf_stack_adjust, ctx);
 
 	ctx->stack_size = stack_adjust;
@@ -2303,3 +2334,8 @@ bool bpf_jit_supports_callx(void)
 {
 	return true;
 }
+
+bool bpf_jit_supports_private_stack(void)
+{
+	return true;
+}
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index 5265dd5bd39e..06f74d6943a9 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -9,6 +9,7 @@
 #include <linux/bpf.h>
 #include <linux/filter.h>
 #include <linux/memory.h>
+#include <linux/percpu.h>
 #include <asm/text-patching.h>
 #include <asm/cfi.h>
 #include "bpf_jit.h"
@@ -44,6 +45,62 @@ bool bpf_jit_needs_zext(void)
 	return true;
 }
 
+#define PRIV_STACK_GUARD_VAL	0xEB9F12345678eb9fULL
+
+static u32 priv_stack_size(struct bpf_prog *prog)
+{
+	return round_up(prog->aux->stack_depth, STACK_ALIGN) +
+	       2 * PRIV_STACK_GUARD_SZ;
+}
+
+static int alloc_priv_stack(struct bpf_prog *prog)
+{
+	u32 size = priv_stack_size(prog);
+	u32 end = (size - PRIV_STACK_GUARD_SZ) / sizeof(u64);
+	void __percpu *ptr;
+	int cpu, i;
+
+	ptr = __alloc_percpu_gfp(size, STACK_ALIGN, GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	for_each_possible_cpu(cpu) {
+		u64 *stack = per_cpu_ptr(ptr, cpu);
+
+		for (i = 0; i < PRIV_STACK_GUARD_SZ / sizeof(u64); i++) {
+			stack[i] = PRIV_STACK_GUARD_VAL;
+			stack[end + i] = PRIV_STACK_GUARD_VAL;
+		}
+	}
+	prog->aux->priv_stack_ptr = ptr;
+	return 0;
+}
+
+static void priv_stack_check_guard(struct bpf_prog *prog)
+{
+	u32 end = (priv_stack_size(prog) - PRIV_STACK_GUARD_SZ) / sizeof(u64);
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		u64 *stack = per_cpu_ptr(prog->aux->priv_stack_ptr, cpu);
+
+		if (stack[0] != PRIV_STACK_GUARD_VAL ||
+		    stack[1] != PRIV_STACK_GUARD_VAL ||
+		    stack[end] != PRIV_STACK_GUARD_VAL ||
+		    stack[end + 1] != PRIV_STACK_GUARD_VAL) {
+			pr_err("bpf-jit: private stack overflow/underflow guard of prog %s corrupted on CPU %d\n",
+			       bpf_jit_get_prog_name(prog), cpu);
+			return;
+		}
+	}
+}
+
+static void free_priv_stack(struct bpf_prog *prog)
+{
+	free_percpu(prog->aux->priv_stack_ptr);
+	prog->aux->priv_stack_ptr = NULL;
+}
+
 struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog)
 {
 	unsigned int prog_size = 0, extable_size = 0;
@@ -82,12 +139,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 			    round_up((stack_arg_cnt - RV_EXTRA_STK_ARGS) * 8, STACK_ALIGN) : 0;
 
 	ctx->prog = prog;
+	if (prog->aux->jits_use_priv_stack && alloc_priv_stack(prog))
+		goto out_free_hdr;
+
 	ctx->offset = kvzalloc_objs(int, prog->len);
 	if (!ctx->offset)
-		goto out_offset;
+		goto out_free_hdr;
 
 	if (build_body(ctx, extra_pass, NULL))
-		goto out_offset;
+		goto out_free_hdr;
 
 	for (i = 0; i < prog->len; i++) {
 		prev_ninsns += 32;
@@ -102,7 +162,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 		ctx->prologue_len = ctx->ninsns;
 
 		if (build_body(ctx, extra_pass, ctx->offset))
-			goto out_offset;
+			goto out_free_hdr;
 
 		ctx->epilogue_offset = ctx->ninsns;
 		bpf_jit_build_epilogue(ctx);
@@ -122,7 +182,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 							  bpf_fill_ill_insns,
 							  bpf_prog_was_classic(prog));
 			if (!jit_data->ro_header)
-				goto out_offset;
+				goto out_free_hdr;
 
 			/*
 			 * Use the image(RW) for writing the JITed instructions. But also save
@@ -188,10 +248,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 			ctx->offset[i] = ctx->offset[i - 1];
 		ctx->offset[0] = ninsns_rvoff(ctx->prologue_len);
 		bpf_prog_update_insn_ptrs(prog, ctx->offset, jit_data->ro_image);
-out_offset:
-		kvfree(ctx->offset);
-		kfree(jit_data);
-		prog->aux->jit_data = NULL;
+		goto out_free_ctx;
 	}
 
 	return prog;
@@ -207,7 +264,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 				   sizeof(jit_data->header->size));
 		bpf_jit_binary_pack_free(jit_data->ro_header, jit_data->header);
 	}
-	goto out_offset;
+	free_priv_stack(prog);
+out_free_ctx:
+	kvfree(ctx->offset);
+	kfree(jit_data);
+	prog->aux->jit_data = NULL;
+	return prog;
 }
 
 u64 bpf_jit_alloc_exec_limit(void)
@@ -259,6 +321,10 @@ void bpf_jit_free(struct bpf_prog *prog)
 		prog->bpf_func = (void *)prog->bpf_func - cfi_get_offset();
 		hdr = bpf_jit_binary_pack_hdr(prog);
 		bpf_jit_binary_pack_free(hdr, NULL);
+		if (prog->aux->priv_stack_ptr) {
+			priv_stack_check_guard(prog);
+			free_priv_stack(prog);
+		}
 		WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(prog));
 	}
 
-- 
2.50.1

Re: [PATCH bpf-next 1/3] riscv: bpf: Add BPF private stack support

[bot+bpf-ci]

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

> riscv: bpf: Add BPF private stack support

TL;DR: Commit message only: it says the guard layout follows x86-64, but
x86-64 uses an 8 byte single-copy guard, and it may overstate why
__per_cpu_offset needs absolute moves.

> riscv: bpf: Add BPF private stack support

[ ... ]

> The base is materialized with absolute moves, as on arm64, x86-64 and
> powerpc64: the dynamic per-CPU area is vmalloc'd, beyond a PC-relative
> auipc pair from the JIT region, and riscv64 has no segment or register
> holding the per-CPU offset, so __per_cpu_offset needs materializing the
> same way. Both constants are fixed before the offset convergence loop,
> so the prologue length is stable across passes.
>
> The guard layout and value follow arm64, x86-64 and powerpc64: a 16
> byte region at each end holding two copies of the pattern. A jited
> program has them checked when it is freed, reporting the first
> corrupted CPU only; failure paths release the area unchecked since the
> image never ran, which also clears priv_stack_ptr for the later
> bpf_jit_free().

Is the statement that the guard layout follows x86-64 accurate?

arm64 and powerpc64 use a 16 byte guard with two copies of the pattern at
each end, but x86-64 differs:

arch/x86/net/bpf_jit_comp.c:
    #define PRIV_STACK_GUARD_SZ    8
    ...
    stack_ptr[0] = PRIV_STACK_GUARD_VAL;
    stack_ptr[underflow_idx] = PRIV_STACK_GUARD_VAL;

It uses an 8 byte guard with a single copy at each end, and it rounds the
stack depth to 8 rather than 16.  Only the guard value (0xEB9F12345678eb9f)
is shared by all three.  Could the changelog say that the 16 byte, two-copy
layout follows arm64 and powerpc64, and that only the guard value follows
x86-64?

Separately, the changelog says __per_cpu_offset "needs materializing the
same way", meaning with absolute moves.  The existing BPF_MOV64_PERCPU_REG
handler in bpf_jit_emit_insn() already loads the same symbol PC-relatively:

    /* Load address of __per_cpu_offset array in T2 */
    emit_addr(RV_REG_T2, (u64)&__per_cpu_offset, extra_pass, ctx);

Is emit_imm() for __per_cpu_offset in emit_priv_stack_frame() required, or a
choice made so that the prologue length stays fixed across passes?  The
changelog could say which.


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

[PATCH bpf-next 2/3] selftests/bpf: Enable private stack tests for riscv64

[Chen Pei]

The RV64 JIT now supports the private stack. Allow the existing
verifier and struct_ops private-stack tests to run on riscv64 and add
the expected RV64 JITed instruction patterns.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 .../bpf/prog_tests/struct_ops_private_stack.c |  3 +-
 .../bpf/progs/verifier_private_stack.c        | 41 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
index 2b3ec2b79091..103179ff8733 100644
--- a/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
+++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_private_stack.c
@@ -6,7 +6,8 @@
 #include "struct_ops_private_stack_recur.skel.h"
 #include "struct_ops_private_stack_large.skel.h"
 
-#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__)
+#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__) || \
+	(defined(__riscv) && __riscv_xlen == 64)
 static void test_private_stack(void)
 {
 	struct struct_ops_private_stack *skel;
diff --git a/tools/testing/selftests/bpf/progs/verifier_private_stack.c b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
index 3d9994a52464..d9cebbc7355d 100644
--- a/tools/testing/selftests/bpf/progs/verifier_private_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
@@ -8,7 +8,8 @@
 /* From include/linux/filter.h */
 #define MAX_BPF_STACK    512
 
-#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64)
+#if defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64) || \
+	(defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64)
 
 struct elem {
 	struct bpf_timer t;
@@ -42,6 +43,14 @@ __jited("	mov	x0, #0x2a")
 __jited("	str	x0, [x27]")
 __jited("...")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x10(sp)")
+__jited("...")
+__jited("	sd	a0, -0x100(s5)")
+__jited("...")
+__jited("	ld	s5, 0x10(sp)")
 __naked void private_stack_single_prog(void)
 {
 	asm volatile ("			\
@@ -60,6 +69,10 @@ __jited("	subq	$0x8, %rsp")
 __arch_arm64
 __jited("	mov	x25, sp")
 __jited("	sub	sp, sp, #0x10")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x30")
+__jited("...")
+__jited("	addi	s5, sp, 0x10")
 __naked void no_private_stack_nested(void)
 {
 	asm volatile ("			\
@@ -113,6 +126,14 @@ __jited("	str	x0, [x27]")
 __jited("	bl	{{.*}}")
 __jited("...")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
+__jited("...")
+__jited("	sd	a0, -0x200(s5)")
+__jited("...")
+__jited("	ld	s5, 0x8(sp)")
 __naked void private_stack_nested_1(void)
 {
 	asm volatile ("				\
@@ -183,6 +204,16 @@ __jited("	bl	0x{{.*}}")
 __jited("	mov	x8, x0")
 __jited("	mov	x8, #0x0")
 __jited("	ldp	x25, x27, [sp], {{.*}}")
+__arch_riscv64
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
+__jited("...")
+__jited("	sd	a0, -0x200(s5)")
+__jited("...")
+__jited("	ld	s5, 0x8(sp)")
 __naked void private_stack_callback(void)
 {
 	asm volatile ("			\
@@ -313,6 +344,10 @@ __arch_arm64
 __jited("	mrs	x10, TPIDR_EL{{[0-1]}}")
 __jited("	add	x27, x27, x10")
 __jited("	add	x25, x27, {{.*}}")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x20")
+__jited("...")
+__jited("	sd	s5, 0x8(sp)")
 int private_stack_async_callback_1(void)
 {
 	struct bpf_timer *arr_timer;
@@ -342,6 +377,10 @@ __arch_x86_64
 __jited("	subq	$0x100, %rsp")
 __arch_arm64
 __jited("	sub	sp, sp, #0x100")
+__arch_riscv64
+__jited("	addi	sp, sp, -0x120")
+__jited("...")
+__jited("	addi	s5, sp, 0x100")
 int private_stack_async_callback_2(void)
 {
 	struct bpf_timer *arr_timer;
-- 
2.50.1

Re: [PATCH bpf-next 2/3] selftests/bpf: Enable private stack tests for riscv64

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: Missing enablement for private stack test in verifier_large_stack.c for riscv64
--

commit 6110b152bad2869b21761f9abd7de948a19afdbf
Author: Chen Pei <cp0613@linux.alibaba.com>

selftests/bpf: Enable private stack tests for riscv64

This commit enables the verifier and struct_ops private-stack tests to
run on riscv64 by adding the expected RV64 JITed instruction patterns.

> selftests/bpf: Enable private stack tests for riscv64
>
> The RV64 JIT now supports the private stack. Allow the existing
> verifier and struct_ops private-stack tests to run on riscv64 and add
> the expected RV64 JITed instruction patterns.

[Severity: Medium]
Was the private_stack_two_frames test in verifier_large_stack.c missed
in this patch?

The commit enables private-stack tests in verifier_private_stack.c and
struct_ops_private_stack.c, but verifier_large_stack.c contains a test
that explicitly validates the verifier log messages for a 2048-byte large
stack (private stack) setup, which is currently restricted to x86_64 and
arm64:

tools/testing/selftests/bpf/progs/verifier_large_stack.c:
__arch_x86_64
__arch_arm64
__success __log_level(4)
__msg("stack depth max 2048")
__msg("subprog 0 (private_stack_two_frames) main {{.*}} stack 2048")
__msg("subprog 1 (priv_stack_frame_2048) static {{.*}} stack 2048")
__naked void private_stack_two_frames(void)

Since the RV64 JIT now supports the private stack, should __arch_riscv64
be added to this test as well so that the riscv64 architecture receives
this large stack test coverage?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015021238.5657-1-cp0613@linux.alibaba.com?part=2

Re: [PATCH bpf-next 2/3] selftests/bpf: Enable private stack tests for riscv64

[Chen Pei]

This one I am not taking: what gates the test is large stack support, a
JIT capability separate from the private stack this series adds.

> [Severity: Medium]
> Was the private_stack_two_frames test in verifier_large_stack.c missed
> in this patch?

> Since the RV64 JIT now supports the private stack, should __arch_riscv64
> be added to this test as well so that the riscv64 architecture receives
> this large stack test coverage?

The test builds a 2048 byte frame. Above MAX_BPF_STACK that is only
accepted when bpf_prog_stack_limit() returns MAX_BPF_STACK_JIT, which
requires bpf_jit_supports_large_stack() in the JIT as well as
bpf_jit_supports_subprog_tailcalls(); the former exists only for x86-64
and arm64. riscv64 rejects the program on the depth with or without a
private stack, which is why it skips behind
is_large_stack_supported(). Adding __arch_riscv64 would turn that skip
into a failure.

The other comments on this set are valid and are taken in v2.

Best regards,
Chen

[PATCH bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[Chen Pei]

The stack-argument tests all run from SEC("tc"), which
bpf_enable_priv_stack() leaves at NO_PRIV_STACK, and the private-stack
tests only call subprograms of up to three arguments, which stay in
registers. Nothing exercises a private stack together with outgoing
stack arguments, where the caller writes the argument area relative to
SP and the callee reads it relative to the frame pointer.

Add a runtime test for that combination. A static ten-argument callee
writes through a pointer into the caller frame and keeps a 128-byte
volatile local array, so its own depth crosses the 64-byte threshold at
which the verifier grants a subprog the adaptive private stack. The same
workload runs from a raw tracepoint, which gets a private stack, and
from tc, which does not; the test pins itself to each allowed CPU in
turn and checks the CPU the program reported.

A runtime result cannot show where the three stack areas landed, so the
riscv64 __jited patterns pin the layout: the caller's kernel frame is 48
bytes against the control group's 176 and the callee's 64 against 208,
the 128 and 144 byte differences being exactly the BPF stack that moved
to the per-CPU area. Both groups write the two outgoing arguments at
0x0(sp) and 0x8(sp) and the callee reads them back at 0x0(s0) and
0x8(s0), so the outgoing area, the saved registers and the BPF stack
occupy disjoint ranges. None of the patterns assert the per-CPU lookup,
which is only emitted on CONFIG_SMP, so one set matches SMP and UP
alike.

The zero-depth struct_ops program is a control case for the kernel
stack path, not a private-stack allocation: the verifier grants the
adaptive private stack only once a subprog's stack depth, rounded up to
the JIT's 16-byte granularity, reaches 64 bytes.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---

Notes:
Reachability of a private stack together with outgoing stack arguments,
per architecture, for review context only:

- x86-64 cannot reach it. The verifier denies the private stack to
  subprograms with stack arguments there because R9 doubles as the
  private stack frame pointer and argument 6, so both halves of this
  test would be kernel stack runs.
- powerpc64 has no stack arguments at all, as it does not report
  bpf_jit_supports_stack_args(), so it cannot reach it either.
- arm64 reports both bpf_jit_supports_private_stack() and
  bpf_jit_supports_stack_args(), so the combination is reachable there
  today and has no test for it. Widening the gate to arm64 needs arm64
  __jited patterns and an arm64 machine to validate them, so I would
  post that separately once the maintainers agree this runtime test is
  worth keeping.

 .../selftests/bpf/private_stack_runtime.h     |  12 ++
 .../bpf/prog_tests/private_stack_runtime.c    |  83 +++++++++++
 .../bpf/progs/private_stack_runtime.c         | 140 ++++++++++++++++++
 3 files changed, 235 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/private_stack_runtime.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
 create mode 100644 tools/testing/selftests/bpf/progs/private_stack_runtime.c

diff --git a/tools/testing/selftests/bpf/private_stack_runtime.h b/tools/testing/selftests/bpf/private_stack_runtime.h
new file mode 100644
index 000000000000..8ed49ff66a6e
--- /dev/null
+++ b/tools/testing/selftests/bpf/private_stack_runtime.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PRIVATE_STACK_RUNTIME_H
+#define __PRIVATE_STACK_RUNTIME_H
+
+struct private_stack_result {
+	long value;
+	long first;
+	long last;
+	__u32 cpu;
+};
+
+#endif
diff --git a/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
new file mode 100644
index 000000000000..3af24ca8009f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include "private_stack_runtime.h"
+#include "private_stack_runtime.skel.h"
+
+void test_private_stack_runtime(void)
+{
+#if defined(__riscv) && __riscv_xlen == 64
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = &pkt_v4,
+		    .data_size_in = sizeof(pkt_v4),
+	);
+	struct private_stack_runtime *skel;
+	struct bpf_link *trace = NULL, *ops = NULL;
+	cpu_set_t saved, single;
+	int cpu, err, tested = 0;
+
+	/*
+	 * Two independent halves. RUN_TESTS() validates the __jited patterns
+	 * through the test loader, which builds its own object from the ELF
+	 * bytes. The half below loads a separate skeleton to check the runtime
+	 * values and the CPU the program reported, which disassembly patterns
+	 * cannot express.
+	 */
+	RUN_TESTS(private_stack_runtime);
+
+	if (!ASSERT_OK(sched_getaffinity(0, sizeof(saved), &saved), "getaffinity"))
+		return;
+
+	skel = private_stack_runtime__open();
+	if (!ASSERT_OK_PTR(skel, "open"))
+		return;
+	if (!skel->rodata->has_stack_args) {
+		test__skip();
+		goto out;
+	}
+	skel->rodata->target_tgid = getpid();
+	if (!ASSERT_OK(private_stack_runtime__load(skel), "load"))
+		goto out;
+
+	trace = bpf_program__attach_raw_tracepoint(skel->progs.private_args, "sys_enter");
+	if (!ASSERT_OK_PTR(trace, "attach_tracepoint"))
+		goto out;
+
+	for (cpu = 0; cpu < CPU_SETSIZE; cpu++) {
+		if (!CPU_ISSET(cpu, &saved))
+			continue;
+		CPU_ZERO(&single);
+		CPU_SET(cpu, &single);
+		if (!ASSERT_OK(sched_setaffinity(0, sizeof(single), &single), "setaffinity"))
+			break;
+
+		err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.kernel_args), &opts);
+		if (!ASSERT_OK(err, "kernel_args") || !ASSERT_EQ(opts.retval, 0, "retval"))
+			break;
+		syscall(__NR_getpgid, 0);
+		ASSERT_EQ(skel->bss->private_result.value, 324 + getpid(), "private_value");
+		ASSERT_EQ(skel->bss->private_result.first, 29, "private_first");
+		ASSERT_EQ(skel->bss->private_result.last, 324, "private_last");
+		ASSERT_EQ(skel->bss->private_result.cpu, cpu, "private_cpu");
+		ASSERT_EQ(skel->bss->kernel_result.value, 324 + getpid(), "kernel_value");
+		ASSERT_EQ(skel->bss->kernel_result.first, 29, "kernel_first");
+		ASSERT_EQ(skel->bss->kernel_result.last, 324, "kernel_last");
+		ASSERT_EQ(skel->bss->kernel_result.cpu, cpu, "kernel_cpu");
+		tested++;
+	}
+	ASSERT_GT(tested, 0, "tested_cpus");
+
+	ops = bpf_map__attach_struct_ops(skel->maps.zero_stack_ops);
+	if (!ASSERT_OK_PTR(ops, "attach_zero_stack"))
+		goto out;
+	ASSERT_OK(trigger_module_test_read(256), "trigger_zero_stack");
+	ASSERT_EQ(skel->bss->zero_calls, 1, "zero_stack_calls");
+out:
+	bpf_link__destroy(ops);
+	bpf_link__destroy(trace);
+	private_stack_runtime__destroy(skel);
+	ASSERT_OK(sched_setaffinity(0, sizeof(saved), &saved), "restore_affinity");
+#else
+	test__skip();
+#endif
+}
diff --git a/tools/testing/selftests/bpf/progs/private_stack_runtime.c b/tools/testing/selftests/bpf/progs/private_stack_runtime.c
new file mode 100644
index 000000000000..61c144fd5cbe
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/private_stack_runtime.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../test_kmods/bpf_testmod.h"
+#include "../private_stack_runtime.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+const volatile __u32 target_tgid;
+const volatile long seed = 1;
+
+struct private_stack_result private_result, kernel_result;
+int zero_calls;
+
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+const volatile bool has_stack_args = true;
+
+struct frame {
+	long slots[16];
+};
+
+static __noinline long check_args(long a, long b, long c, long d, long e,
+				  long f, long g, long h, long i,
+				  struct frame *caller)
+{
+	volatile long local[16] = {};
+	long sum;
+
+	local[0] = a;
+	local[15] = i;
+	sum = a + 2 * b + 3 * c + 4 * d + 5 * e + 6 * f + 7 * g + 8 * h + 9 * i;
+	caller->slots[15] = caller->slots[0] + sum + local[0] + local[15];
+	return caller->slots[15] + (bpf_get_current_pid_tgid() >> 32);
+}
+
+static __always_inline void run_args(struct private_stack_result *result)
+{
+	struct frame caller = {};
+	long n = seed;
+
+	caller.slots[0] = 29;
+	result->value = check_args(n, n + 1, n + 2, n + 3, n + 4,
+				   n + 5, n + 6, n + 7, n + 8, &caller);
+	result->first = caller.slots[0];
+	result->last = caller.slots[15];
+	result->cpu = bpf_get_smp_processor_id();
+}
+#else
+const volatile bool has_stack_args = false;
+
+static __always_inline void run_args(struct private_stack_result *result)
+{
+}
+#endif
+
+SEC("raw_tp/sys_enter")
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+__arch_riscv64
+/*
+ * func #0, the caller. With a private stack the kernel frame holds only the
+ * saved registers and the outgoing stack argument area, so it is 48 bytes =
+ * 32 (ra, fp, s5, tcc) + 16 (two 8 byte outgoing slots). The 128 byte BPF
+ * stack is not in this frame; it lives on the per-CPU private stack reached
+ * through s5. The caller writes the two outgoing arguments relative to sp.
+ */
+__jited("	addi	sp, sp, -0x30")
+__jited("...")
+__jited("	sd	{{.*}}, 0x0(sp)")
+__jited("...")
+__jited("	sd	{{.*}}, 0x8(sp)")
+__jited("...")
+/*
+ * func #1, the ten argument callee, gets a private stack of its own, so its
+ * kernel frame is the 64 bytes of saved registers and nothing else. It reads
+ * the two incoming stack arguments relative to fp (s0), which its prologue
+ * set to the caller's sp, i.e. exactly the area the caller wrote above.
+ */
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0x40")
+__jited("...")
+__jited("	ld	{{.*}}, 0x0(s0)")
+__jited("...")
+__jited("	ld	{{.*}}, 0x8(s0)")
+#endif
+
+int private_args(void *ctx)
+{
+	if ((bpf_get_current_pid_tgid() >> 32) == target_tgid)
+		run_args(&private_result);
+	return 0;
+}
+
+SEC("tc")
+#ifdef __BPF_FEATURE_STACK_ARGUMENT
+__arch_riscv64
+/*
+ * func #0, the caller, control group. BPF_PROG_TYPE_SCHED_CLS is
+ * NO_PRIV_STACK, so the 128 byte BPF stack stays in the kernel frame and s5
+ * is sp relative: 176 bytes = 32 saved + 128 BPF stack + 16 outgoing, i.e.
+ * 128 bytes larger than the private stack caller above. The outgoing stack
+ * argument area is written relative to sp in both cases.
+ */
+__jited("	addi	sp, sp, -0xb0")
+__jited("...")
+__jited("	addi	s5, sp, 0x90")
+__jited("...")
+__jited("	sd	{{.*}}, 0x0(sp)")
+__jited("...")
+__jited("	sd	{{.*}}, 0x8(sp)")
+__jited("...")
+/* func #1, the callee: 208 bytes = 64 saved + 144 BPF stack. */
+__jited("func #1")
+__jited("...")
+__jited("	addi	sp, sp, -0xd0")
+__jited("...")
+__jited("	ld	{{.*}}, 0x0(s0)")
+__jited("...")
+__jited("	ld	{{.*}}, 0x8(s0)")
+#endif
+
+int kernel_args(struct __sk_buff *skb)
+{
+	run_args(&kernel_result);
+	return 0;
+}
+
+SEC("struct_ops")
+int BPF_PROG(zero_stack)
+{
+	zero_calls++;
+	return 0;
+}
+
+SEC(".struct_ops")
+struct bpf_testmod_ops3 zero_stack_ops = {
+	.test_1 = (void *)zero_stack,
+};
-- 
2.50.1

Re: [PATCH bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: test flakiness due to parallel struct_ops attachment
--

commit 30217e3723f8427977b857ce4b668a913d8e6da3
Author: Chen Pei <cp0613@linux.alibaba.com>
selftests/bpf: Add a runtime test for private stack and stack arguments

This commit adds a runtime test for combining private stacks with outgoing
stack arguments. It validates the structural layout via disassembly patterns
and checks runtime execution context across CPUs. It also includes a
struct_ops test to act as a kernel stack control case.

> diff --git a/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
> new file mode 100644
> index 0000000000000..3af24ca8009f4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +#include "private_stack_runtime.h"
> +#include "private_stack_runtime.skel.h"
> +
> +void test_private_stack_runtime(void)
> +{

[Severity: Medium]
Will this cause test flakiness in CI due to parallel execution?

The test function lacks the serial_ prefix, so it will run concurrently in
the test_progs worker threads rather than in isolation.

[ ... ]

> +	ASSERT_GT(tested, 0, "tested_cpus");
> +
> +	ops = bpf_map__attach_struct_ops(skel->maps.zero_stack_ops);
> +	if (!ASSERT_OK_PTR(ops, "attach_zero_stack"))
> +		goto out;

[Severity: Medium]
Since zero_stack_ops targets the singleton bpf_testmod_ops3 in bpf_testmod,
executing this concurrently with other non-serial tests that attach to the
same struct_ops (such as test_struct_ops_private_stack) can cause random
-EEXIST failures.

st_ops3_reg() in bpf_testmod.c enforces a single active attachment:

    if (st_ops3)
        ... err = -EEXIST

Could this function be renamed to serial_test_private_stack_runtime to avoid
resource conflicts with other tests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015021238.5657-1-cp0613@linux.alibaba.com?part=3

Re: [PATCH bpf-next 3/3] selftests/bpf: Add a runtime test for private stack and stack arguments

[bot+bpf-ci]

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

> selftests/bpf: Add a runtime test for private stack and stack arguments

TL;DR: Test issue only: the new private stack selftest calls test__skip()
without printing a reason, so a SKIP doesn't show why. The review says this
may be acceptable, since other tests do the same.

> diff --git a/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
> new file mode 100644
> index 000000000000..3af24ca8009f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/private_stack_runtime.c
> @@ -0,0 +1,83 @@

[ ... ]

> +	if (!skel->rodata->has_stack_args) {
> +		test__skip();
> +		goto out;
> +	}

A subsystem pattern flags this as potentially concerning: a selftest that
skips without printing a reason.  Both skip paths call test__skip() bare,
this one when the compiler lacks __BPF_FEATURE_STACK_ARGUMENT
(has_stack_args is false), and the other in the non-riscv64 branch:

    #else
        test__skip();
    #endif

Without a message, the test shows up as SKIP with no hint whether clang
needs to be upgraded or the test is only built for riscv64.  Would a
printf() before test__skip(), like the "%s:SKIP: ..." message that
test_loader's run_subtest() prints, make the cause visible?

This may be acceptable as is: test_progs does report the test as SKIP, and
prog_tests/stack_arg.c and prog_tests/struct_ops_private_stack.c also call
test__skip() without a message.


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