Re: [PATCH v2 4/6] powerpc64/bpf: Add arch_bpf_stack_walk() for BPF JIT

[Hari Bathini]

On 14/01/26 6:50 pm, adubey wrote:
> On 2026-01-14 18:07, Christophe Leroy (CS GROUP) wrote:
> > Le 14/01/2026 à 12:44, adubey@xxxxxxxxxxxxx a écrit :
> > > From: Abhishek Dubey <adubey@xxxxxxxxxxxxx>
> > >
> > > This function is used by bpf_throw() to unwind the stack
> > > until frame of exception-boundary during BPF exception
> > > handling.
> > >
> > > This function is necessary to support BPF exceptions on
> > > PowerPC.
> > >
> > > Signed-off-by: Abhishek Dubey <adubey@xxxxxxxxxxxxx>
> > > ---
> > >   arch/powerpc/net/bpf_jit_comp64.c | 28 ++++++++++++++++++++++++++++
> > >   1 file changed, 28 insertions(+)
> > >
> > > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/ 
> > > bpf_jit_comp64.c
> > > index cebf81fbd59f..ec58395f74f7 100644
> > > --- a/arch/powerpc/net/bpf_jit_comp64.c
> > > +++ b/arch/powerpc/net/bpf_jit_comp64.c
> > > @@ -247,6 +247,34 @@ void bpf_jit_build_epilogue(u32 *image, struct 
> > > codegen_context *ctx)
> > >       bpf_jit_build_fentry_stubs(image, ctx);
> > >   }
> > >   +void arch_bpf_stack_walk(bool (*consume_fn)(void *, u64, u64, 
> > > u64), void *cookie)
> > > +{
> > > +    // callback processing always in current context
> > > +    unsigned long fp = current_stack_frame();
> > > +
> > > +    for (;;) {
> > > +        unsigned long *frame = (unsigned long *) fp;
> > > +        unsigned long ip;
> > > +
> > > +        if (!validate_sp(fp, current))
> > > +            return;
> > > +
> > > +        ip = frame[STACK_FRAME_LR_SAVE];
> > > +        if (!ip)
> > > +            break;
> > > +
> > > +        /*
> > > +         * consume_fn common code expects stack pointer(sp) in third
> > > +         * argument. There is no sp in ppc64, rather pass frame
> > > +         * pointer.
> > > +         */
> > > +        if (ip && !consume_fn(cookie, ip, fp, fp))
> > > +            break;
> > > +
> > > +        fp = frame[0];
> > > +    }
> > > +}
> > > +
> >
> > This fonction looks very close to arch_stack_walk(). Would it be
> > possible to refactor and have a common part used by both functions,
> > like ARM64 for instance ?
> Yes, its inspired from arch_stack_walk(). consume_entry() have different 
> parameter count in both cases.
> If merged, it need additional handling to identify which call_back to 
> invoke.
> Also, we need to define arch-specific weak function 
> arch_bpf_stack_walk(), so renaming of arch_stack_walk is needed on merge.
> Stack walker logic with "bpf" name might be confusing when used at other 
> places. Thoughts?

Not sure what you mean by renaming of arch_stack_walk is needed on
merge but refactoring does not have to change API signature or any
common code for that matter..

- Hari