Re: [PATCH bpf-next v2 2/3] riscv, bpf: Fix support for BPF_MOVSX in RV32 JIT

[Kuan-Wei Chiu]

Hi Lehui,

On Wed, Jun 17, 2026 at 03:30:03PM +0800, Pu Lehui wrote:
> 
> 
> On 2026/5/12 6:16, Kuan-Wei Chiu wrote:
> > The current rv32 bpf jit compiler incorrectly treats BPF_MOVSX as a
> > standard zero-extended move operation. The bpf instruction set allows
> > sign-extension moves by reusing the BPF_MOV opcode with the instruction
> > offset set to 8, 16, or 32.
> > 
> > Update the bpf_jit_emit_insn() function to check the offset field for
> > both ALU and ALU64 MOV operations. If the offset is non-zero, emit the
> > correct slli and srai instructions to perform the sign extension.
> > 
> > Before this patch:
> > [   19.549705] test_bpf: #82 ALU_MOVSX | BPF_B jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
> > [   19.551354] test_bpf: #83 ALU_MOVSX | BPF_H jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
> > [   19.552576] test_bpf: #84 ALU64_MOVSX | BPF_B jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
> > [   19.553542] test_bpf: #85 ALU64_MOVSX | BPF_H jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
> > [   19.554807] test_bpf: #86 ALU64_MOVSX | BPF_W jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
> > 
> > After this patch:
> > [   17.931172] test_bpf: #82 ALU_MOVSX | BPF_B jited:1 125 PASS
> > [   17.932198] test_bpf: #83 ALU_MOVSX | BPF_H jited:1 124 PASS
> > [   17.933039] test_bpf: #84 ALU64_MOVSX | BPF_B jited:1 124 PASS
> > [   17.933918] test_bpf: #85 ALU64_MOVSX | BPF_H jited:1 124 PASS
> > [   17.934751] test_bpf: #86 ALU64_MOVSX | BPF_W jited:1 122 PASS
> > 
> > Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")
> 
> not a fix, the fixes tag should remove. And the title should `Add support
> for xxx`

Thanks for the review.

I did consider using 'Add support' instead of 'Fix' for the title. I
went with 'Fix' because when the JIT doesn't support a BPF instruction,
it's supposed to return an error code to trigger a fallback to the
interpreter. Right now, it doesn't return an error code and just
executes it anyway, producing results that violate the spec and
breaking bpf_test.

Regards,
Kuan-Wei

> 
> others lgtm,
> 
> Reviewed-by: Pu Lehui <pulehui@xxxxxxxxxx>
> 
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
> > ---
> >   arch/riscv/net/bpf_jit_comp32.c | 32 ++++++++++++++++++++++++++++++++
> >   1 file changed, 32 insertions(+)
> > 
> > diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
> > index 7396899ea276..f8509950fed4 100644
> > --- a/arch/riscv/net/bpf_jit_comp32.c
> > +++ b/arch/riscv/net/bpf_jit_comp32.c
> > @@ -974,6 +974,24 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> >   	switch (code) {
> >   	case BPF_ALU64 | BPF_MOV | BPF_X:
> > +		if (insn->off != 0) {
> > +			const s8 *rd = bpf_get_reg64(dst, tmp1, ctx);
> > +			const s8 *rs = bpf_get_reg64(src, tmp2, ctx);
> > +
> > +			if (insn->off == 8) {
> > +				emit(rv_slli(lo(rd), lo(rs), 24), ctx);
> > +				emit(rv_srai(lo(rd), lo(rd), 24), ctx);
> > +			} else if (insn->off == 16) {
> > +				emit(rv_slli(lo(rd), lo(rs), 16), ctx);
> > +				emit(rv_srai(lo(rd), lo(rd), 16), ctx);
> > +			} else {
> > +				emit(rv_addi(lo(rd), lo(rs), 0), ctx);
> > +			}
> > +			emit(rv_srai(hi(rd), lo(rd), 31), ctx);
> > +			bpf_put_reg64(dst, rd, ctx);
> > +			break;
> > +		}
> > +		fallthrough;
> >   	case BPF_ALU64 | BPF_ADD | BPF_X:
> >   	case BPF_ALU64 | BPF_ADD | BPF_K:
> > @@ -1024,6 +1042,20 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> >   			emit_zext64(dst, ctx);
> >   			break;
> >   		}
> > +		if (insn->off != 0) {
> > +			const s8 *rd = bpf_get_reg32(dst, tmp1, ctx);
> > +			const s8 *rs = bpf_get_reg32(src, tmp2, ctx);
> > +
> > +			if (insn->off == 8) {
> > +				emit(rv_slli(lo(rd), lo(rs), 24), ctx);
> > +				emit(rv_srai(lo(rd), lo(rd), 24), ctx);
> > +			} else if (insn->off == 16) {
> > +				emit(rv_slli(lo(rd), lo(rs), 16), ctx);
> > +				emit(rv_srai(lo(rd), lo(rd), 16), ctx);
> > +			}
> > +			bpf_put_reg32(dst, rd, ctx);
> > +			break;
> > +		}
> >   		fallthrough;
> >   	case BPF_ALU | BPF_ADD | BPF_X: