Re: [PATCH bpf-next v2 1/3] riscv: stacktrace: Implement arch_bpf_stack_walk() for BPF

From: "Björn Töpel" <bjorn@kernel.org>
To: Varun R Mallya <varunrmallya@gmail.com>,
	pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	eddyz87@gmail.com, memxor@gmail.com, pulehui@huawei.com
Cc: alex@ghiti.fr, martin.lau@linux.dev, song@kernel.org,
	yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com,
	puranjay@kernel.org, shuah@kernel.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
	varunrmallya@gmail.com
Subject: Re: [PATCH bpf-next v2 1/3] riscv: stacktrace: Implement arch_bpf_stack_walk() for BPF
Date: Sun, 28 Jun 2026 10:26:28 -0700	[thread overview]
Message-ID: <87ik72y3sr.fsf@all.your.base.are.belong.to.us> (raw)
In-Reply-To: <20260628081710.113333-2-varunrmallya@gmail.com>

Varun!

Thanks for spending time on getting RV eBPF more feature complete! Sorry
for the slow replies.

Varun R Mallya <varunrmallya@gmail.com> writes:

> This will be used by bpf_throw() to unwind till the program marked as
> exception boundary and run the callback with the stack of the main
> program.
> This is required for supporting BPF exceptions on RISC-V.
> This depends on the frame pointer unwinder, so it is only built under
> CONFIG_FRAME_POINTER, else falls back to the weak no-op.
>
> Signed-off-by: Varun R Mallya <varunrmallya@gmail.com>
> Reviewed-by: Pu Lehui <pulehui@huawei.com>
> ---
>  arch/riscv/kernel/stacktrace.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index c7555447149b..64929381bb30 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/export.h>
> +#include <linux/filter.h>
>  #include <linux/kallsyms.h>
>  #include <linux/sched.h>
>  #include <linux/sched/debug.h>
> @@ -102,6 +103,36 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>  	}
>  }
>  
> +void notrace arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp),
> +				 void *cookie)
> +{
> +	unsigned long fp, sp, pc;
> +	int graph_idx = 0;
> +
> +	fp = (unsigned long)__builtin_frame_address(0);
> +	sp = current_stack_pointer;
> +	pc = (unsigned long)arch_bpf_stack_walk;
> +
> +	for (;;) {
> +		struct stackframe *frame;
> +
> +		if (unlikely(!__kernel_text_address(pc)))
> +			break;
> +		/* pc belongs to the function whose frame pointer is fp */
> +		if (!consume_fn(cookie, pc, sp, fp))
> +			break;
> +		if (unlikely(!fp_is_valid(fp, sp)))
> +			break;
> +
> +		frame = (struct stackframe *)fp - 1;
> +		sp = fp;
> +		fp = READ_ONCE_TASK_STACK(current, frame->fp);
> +		pc = READ_ONCE_TASK_STACK(current, frame->ra);
> +		pc = ftrace_graph_ret_addr(current, &graph_idx, pc,
> +					   &frame->ra);
> +	}
> +}

We don't need more stack unwinders. Can you see if you can extend
walk_stackframe() with "cookie" to match BPF's needs? Have a look at
what arm64 does.

Sashiko had a good point about ftrace_graph_ret_addr() -- now what about
kretprobe? Do we need to take that in consideration as well?

Thanks,
Björn

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv