From: Kuan-Wei Chiu <visitorckw@gmail.com>
To: Pu Lehui <pulehui@huawei.com>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
eddyz87@gmail.com, memxor@gmail.com, luke.r.nels@gmail.com,
xi.wang@gmail.com, pjw@kernel.org, palmer@dabbelt.com,
aou@eecs.berkeley.edu, martin.lau@linux.dev, song@kernel.org,
yonghong.song@linux.dev, jolsa@kernel.org, alex@ghiti.fr,
jserv@ccns.ncku.edu.tw, eleanor15x@gmail.com,
marscheng@google.com, bpf@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 2/3] riscv, bpf: Fix support for BPF_MOVSX in RV32 JIT
Date: Tue, 23 Jun 2026 05:28:01 +0800 [thread overview]
Message-ID: <ajmo4VeTykr5ozBw@google.com> (raw)
In-Reply-To: <2ca2a85a-d4a9-4459-a534-6159de6e5890@huawei.com>
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@huawei.com>
>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> > 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:
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2026-06-22 21:28 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-11 22:16 [PATCH bpf-next v2 0/3] riscv, bpf: Fix signed operations and add 32 bit atomics Kuan-Wei Chiu
2026-05-11 22:16 ` [PATCH bpf-next v2 1/3] riscv, bpf: Fix support for BPF_SDIV and BPF_SMOD in RV32 JIT Kuan-Wei Chiu
2026-06-17 6:53 ` Pu Lehui
2026-05-11 22:16 ` [PATCH bpf-next v2 2/3] riscv, bpf: Fix support for BPF_MOVSX " Kuan-Wei Chiu
2026-06-17 7:30 ` Pu Lehui
2026-06-22 21:28 ` Kuan-Wei Chiu [this message]
2026-05-11 22:16 ` [PATCH bpf-next v2 3/3] riscv, bpf: Add 32 bit atomic operations to " Kuan-Wei Chiu
2026-06-17 7:54 ` Pu Lehui
2026-06-15 16:18 ` [PATCH bpf-next v2 0/3] riscv, bpf: Fix signed operations and add 32 bit atomics Kuan-Wei Chiu
2026-06-16 1:41 ` Pu Lehui
2026-06-16 2:21 ` Kuan-Wei Chiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajmo4VeTykr5ozBw@google.com \
--to=visitorckw@gmail.com \
--cc=alex@ghiti.fr \
--cc=andrii@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=eleanor15x@gmail.com \
--cc=jolsa@kernel.org \
--cc=jserv@ccns.ncku.edu.tw \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=luke.r.nels@gmail.com \
--cc=marscheng@google.com \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=pulehui@huawei.com \
--cc=song@kernel.org \
--cc=xi.wang@gmail.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox