Re: [PATCH bpf-next v3] m68k, bpf: Add initial BPF JIT compiler support

On Mon, May 11, 2026 at 10:12:31PM +0000, [email protected] wrote:
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b2040011a386..261cc1124685 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4699,6 +4699,12 @@ L:	[email protected]
> >  S:	Maintained
> >  F:	arch/loongarch/net/
> >
> > +BPF JIT for M68K
> 
> This isn't a bug, but the surrounding BPF JIT entries all use lowercase
> 'for': 'BPF JIT for ARM', 'BPF JIT for ARM64', 'BPF JIT for LOONGARCH',
> 'BPF JIT for MIPS', etc. For consistency, should this be
> 'BPF JIT for M68K'?

I think this is an AI hallucination, as my patch already uses a
lowercase 'for'.

> 
> Reference: https://lore.kernel.org/bpf/[email protected]/
> 
> > +M:	Kuan-Wei Chiu <[email protected]>
> > +L:	[email protected]
> > +S:	Maintained
> > +F:	arch/m68k/net/
> > +
> >  BPF JIT for MIPS (32-BIT AND 64-BIT)
> >  M:	Johan Almbladh <[email protected]>
> >  M:	Paul Burton <[email protected]>
> 
> [ ... ]
> 
> > diff --git a/arch/m68k/net/bpf_jit_comp.c b/arch/m68k/net/bpf_jit_comp.c
> > new file mode 100644
> > index 000000000000..1531b321410a
> > --- /dev/null
> > +++ b/arch/m68k/net/bpf_jit_comp.c
> 
> [ ... ]
> 
> > +static int emit_call(const struct bpf_insn *insn, struct jit_ctx *ctx)
> > +{
> > +	const s8 arg_regs[] = { BPF_REG_5, BPF_REG_4, BPF_REG_3, BPF_REG_2, BPF_REG_1 };
> > +	const s8 *tmp1 = bpf2m68k[TMP_REG_1];
> > +	bool extra_pass = ctx->target != NULL;
> > +	u64 func_addr;
> > +	bool fixed;
> > +	int i, err;
> > +
> > +	err = bpf_jit_get_func_addr(ctx->prog, insn, extra_pass, &func_addr, &fixed);
> > +	if (err)
> > +		return err;
> > +
> > +	for (i = 0; i < 5; i++) {
> > +		const s8 *reg = bpf2m68k[arg_regs[i]];
> > +		s8 d_lo = bpf_get_reg32(reg[1], tmp1[1], ctx);
> > +		s8 d_hi = bpf_get_reg32(reg[0], tmp1[0], ctx);
> > +
> > +		emit_16(ctx, 0x2f00 | d_lo);			/* move.l d_lo, -(%sp) */
> > +		emit_16(ctx, 0x2f00 | d_hi);			/* move.l d_hi, -(%sp) */
> > +	}
> > +
> > +	emit_16(ctx, 0x207c);					/* movea.l #func_addr, %a0 */
> > +	emit_32(ctx, (u32)func_addr);
> > +
> > +	emit_16(ctx, 0x4e90);					/* jsr (%a0) */
> > +
> > +	emit_16(ctx, 0x4fef);					/* lea 40(%sp), %sp */
> > +	emit_16(ctx, 40);
> > +
> > +	emit_16(ctx, 0xc340);					/* exg %d0, %d1 */
> > +
> > +	return 0;
> > +}
> 
> Does the exg %d0, %d1 swap the halves of the return value incorrectly
> for BPF-to-BPF subprogram calls (BPF_PSEUDO_CALL)?
> 
> The emit_call() function unconditionally swaps %d0 and %d1 after all
> calls, which appears correct for C ABI helper functions (where high 32
> bits are in %d0, low in %d1). However, BPF subprograms place their 64-bit
> return value in BPF_REG_0, which is mapped to %d1 and %d0.
> 
> If a BPF subprogram returns a value via BPF_REG_0:
>     BPF subprogram: BPF_REG_0 -> %d1:%d0 (high:low)
>     After exg:      %d0:%d1 (swapped - incorrect ordering)
> 
> Additionally, the prologue assumes C ABI and reads a context pointer
> from 8(%fp):
> 
> arch/m68k/net/bpf_jit_comp.c:build_prologue() {
>     emit_16(ctx, 0x202e);       /* move.l 8(%fp), %d0 */
>     emit_16(ctx, 8);
> }
> 
> But BPF-to-BPF calls pass arguments via the stack (pushed by emit_call),
> not via the C calling convention. Wouldn't this cause the prologue to
> read an incorrect context pointer for BPF subprogram entry points?
> 

I think currently it works because pseudo calls will fail at
bpf_jit_get_func_addr() and fall back to the interpreter anyway.

I might try to fix this issue, test it, and submit a v4.

Regards,
Kuan-Wei

> Reference: https://lore.kernel.org/bpf/[email protected]/
> 
> 
> ---
> 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/25698934894