Re: [PATCH bpf-next v3 2/2] bpf, mips: Add support for BPF_MOVSX in the JITs

[bot+bpf-ci]

> commit f3f0abd62d699a78db3e8f185879a2fede98c6f7
> Author: Nicholas Dudar <main.kalliope@xxxxxxxxx>
> Date:   Sat Sep 12 00:12:20 2026 -0400
>
>     bpf, mips: Add support for BPF_MOVSX in the JITs
>
>     Both MIPS JITs ignore insn->off when lowering register MOV
>     instructions, so BPF_MOVSX is emitted as an ordinary move. Since
>     build_insn() accepts it, affected programs are silently miscompiled
>     instead of falling back to the interpreter.
>
>     Decode the MOVSX width from insn->off in the register-move helpers.
>     On MIPS32, propagate the sign into the high word for ALU64 while
>     preserving the existing ALU32 zero-extension handling.
>
>     Use seb/seh on MIPS32 R2-or-newer CPUs and shift pairs otherwise.
>     On MIPS64, seb/seh require a sign-extended 32-bit source. Normalizing
>     an arbitrary BPF register first would cost another instruction, so
>     retain the doubleword shift pairs.
>
>     Assisted-by: Codex:gpt-6
>     Signed-off-by: Nicholas Dudar <main.kalliope@xxxxxxxxx>

The changelog describes a silent miscompilation of already-loadable BPF
programs, which indicates this is a bug fix. Should this include a Fixes:
tag pointing to the commit that introduced BPF_MOVSX to the instruction
set?

    Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")

That commit added BPF_MOVSX by reusing BPF_ALU|BPF_MOV|BPF_X with insn->off
in {8,16,32}. Because the MIPS build_insn() cases match on insn->code only,
every MOVSX became a plain move from that point on.

> diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
> index bfe73b023983..11b080a99b9f 100644
> --- a/arch/mips/net/bpf_jit_comp32.c
> +++ b/arch/mips/net/bpf_jit_comp32.c
> @@ -190,20 +190,67 @@ static void emit_zext_ver(struct jit_context *ctx, const u8 dst[])
>  	}
>  }
>
> -/* Register move operation (32-bit) */
> +/* Sign-extend an 8- or 16-bit field into a native register. */
> +static void emit_movsx_r(struct jit_context *ctx, u8 dst, u8 src, s16 off)
> +{
> +	int shift;
> +
> +	if (cpu_has_mips32r2 || cpu_has_mips32r6) {
> +		if (off == 8)
> +			emit(ctx, seb, dst, src);
> +		else
> +			emit(ctx, seh, dst, src);

Does this compile? The MIPS uasm assembler appears to have no seb/seh
emitter. The emit() macro (arch/mips/net/bpf_jit_comp.h:88-96) expands to
uasm_i_##func(), so these lines become uasm_i_seb() and uasm_i_seh().

Looking at arch/mips/include/asm/uasm.h, the instruction list goes:

    Ip_u2s3u1(_sb);
    Ip_u2s3u1(_sc);
    Ip_u2s3u1(_scd);
    Ip_u2s3u1(_sd);
    Ip_u3u1u2(_seleqz);
    Ip_u3u1u2(_selnez);
    Ip_u2s3u1(_sh);
    Ip_u2u1u3(_sll);

There is no Ip_u2u1(_seb) or Ip_u2u1(_seh) declaration (compare
Ip_u2u1(_wsbh), which is the same 2-operand form needed here).

Also, arch/mips/mm/uasm-mips.c, uasm-micromips.c and uasm.c contain no
insn_seb or insn_seh table entries. The patch touches only the two JIT
files, so no uasm support is added.

With CONFIG_32BIT=y + CONFIG_BPF_JIT, the build of bpf_jit_comp32.o would
fail with:

    error: implicit function declaration of 'uasm_i_seb'
    error: implicit function declaration of 'uasm_i_seh'

The else arm below (shift = 32 - off; sll; sra) is correct for both
off==8 and off==16, so it could be used unconditionally. Or seb/seh could
be added to uasm in a preparatory patch (they are part of special3/bshfl
with opcode seb_op/seh_op, alongside the existing wsbh instruction).

> +	} else {
> +		shift = 32 - off;
> +		emit(ctx, sll, dst, src, shift);
> +		emit(ctx, sra, dst, dst, shift);
> +	}
> +}
> +
> +/* Register move operation (32-bit), optionally with sign extension */
>  static void emit_mov_r32(struct jit_context *ctx, const u8 dst[],
> -			 const u8 src[])
> +			 const u8 src[], s16 off)
>  {
> -	emit_mov_r(ctx, lo(dst), lo(src));
> +	switch (off) {
> +	case 8:
> +	case 16:
> +		emit_movsx_r(ctx, lo(dst), lo(src), off);
> +		break;
> +	default:
> +		/* off == 0 is MOV; the verifier rejects other offsets. */
> +		emit_mov_r(ctx, lo(dst), lo(src));
> +		break;
> +	}
> +	clobber_reg(ctx, lo(dst));
>  	emit_zext_ver(ctx, dst);
>  }

[ ... ]


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