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

[Pu Lehui <pulehui@huawei.com>]

On 2026/6/29 1:26, Björn Töpel wrote:
> 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?

IIUC, arch_bpf_stack_walk only unwind for bpf prog stack, maybe we could 
not need to handle kretprobe.

> 
> 
> Thanks,
> Björn