Re: [PATCH v8] m68k, bpf: Add initial BPF JIT compiler support

[bot+bpf-ci]

> m68k, bpf: Add initial BPF JIT compiler support

TL;DR: m68k JIT returns true from bpf_jit_needs_zext() but still zero-extends
after every ALU32 op and load, so verifier-inserted zexts only add code.
Results stay correct; should it honour verifier_zext or drop the override?

> diff --git a/arch/m68k/net/bpf_jit_comp.c b/arch/m68k/net/bpf_jit_comp.c
> new file mode 100644
> index 000000000000..debbf5fbe95b
> --- /dev/null
> +++ b/arch/m68k/net/bpf_jit_comp.c
> @@ -0,0 +1,1810 @@

[ ... ]

> +static void emit_alu32_x(const struct bpf_insn *insn, struct jit_ctx *ctx)
> +{

[ ... ]

> +	}
> +
> +	bpf_put_reg32(dst[1], d_reg, ctx);
> +	emit_16(ctx, 0x7000 | (tmp1[0] << 9));			/* moveq #0, tmp1 */
> +	bpf_put_reg32(dst[0], tmp1[0], ctx);
> +}

emit_alu32_k() ends with the same three lines.  Is it necessary to clear the
upper 32 bits after every ALU32 operation here?

The file also has:

> +bool bpf_jit_needs_zext(void)
> +{
> +	return true;
> +}

Because bpf_jit_needs_zext() returns true, bpf_opt_subreg_zext_lo32_rnd_hi32()
adds a BPF_ZEXT_REG(dst) after every 32-bit def whose upper half is read
later, and it sets prog->aux->verifier_zext.  Nothing in this JIT reads
verifier_zext, so each inserted zext goes through emit_alu32_x() as a plain
mov32 and clears the high word a second time.

For a stack-backed register (R1-R9, AX) the inserted zext costs six extra
instructions, about 20 bytes and four memory accesses:

  move.l d16(%fp),%d2
  move.l d16(%fp),%d4
  move.l %d4,%d2
  move.l %d2,d16(%fp)
  moveq #0,%d3
  move.l %d3,d16(%fp)

The other 32-bit JITs that return true from bpf_jit_needs_zext() skip the
high word clear when verifier_zext is set, for example arm32
(emit_a32_alu_r64 and the special mov32 for zext case), x86-32, riscv32
(bpf_put_reg32) and powerpc32.  emit_ldx() also always zero-extends here.

Results are still correct, but the opt-in only adds code.  Should
emit_alu32_x(), emit_alu32_k() and emit_ldx() honour
ctx->prog->aux->verifier_zext, with the imm == 1 zext mov clearing only the
high word?  Alternatively, since the JIT already zero-extends everywhere,
could the bpf_jit_needs_zext() override be dropped?


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