Skip to content

Commit db7a382

Browse files
peilin-yeAlexei Starovoitov
authored andcommitted
bpf, riscv64: Skip redundant zext instruction after load-acquire
Currently, the verifier inserts a zext instruction right after every 8-, 16- or 32-bit load-acquire, which is already zero-extending. Skip such redundant zext instructions. While we are here, update that already-obsolete comment about "skip the next instruction" in build_body(). Also change emit_atomic_rmw()'s parameters to keep it consistent with emit_atomic_ld_st(). Note that checking 'insn[1]' relies on 'insn' not being the last instruction, which should have been guaranteed by the verifier; we already use 'insn[1]' elsewhere in the file for similar purposes. Additionally, we don't check if 'insn[1]' is actually a zext for our load-acquire's dst_reg, or some other registers - in other words, here we are relying on the verifier to always insert a redundant zext right after a 8/16/32-bit load-acquire, for its dst_reg. Acked-by: Björn Töpel <[email protected]> Reviewed-by: Pu Lehui <[email protected]> Tested-by: Björn Töpel <[email protected]> # QEMU/RVA23 Signed-off-by: Peilin Ye <[email protected]> Link: https://lore.kernel.org/r/10e90e0eab042f924d35ad0d1c1f7ca29f673152.1746588351.git.yepeilin@google.com Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 8afd317 commit db7a382

File tree

2 files changed

+19
-7
lines changed

2 files changed

+19
-7
lines changed

arch/riscv/net/bpf_jit_comp64.c

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,13 @@ static void emit_store_64(u8 rd, s32 off, u8 rs, struct rv_jit_context *ctx)
607607
emit_sd(RV_REG_T1, 0, rs, ctx);
608608
}
609609

610-
static int emit_atomic_ld_st(u8 rd, u8 rs, s16 off, s32 imm, u8 code, struct rv_jit_context *ctx)
610+
static int emit_atomic_ld_st(u8 rd, u8 rs, const struct bpf_insn *insn,
611+
struct rv_jit_context *ctx)
611612
{
613+
u8 code = insn->code;
614+
s32 imm = insn->imm;
615+
s16 off = insn->off;
616+
612617
switch (imm) {
613618
/* dst_reg = load_acquire(src_reg + off16) */
614619
case BPF_LOAD_ACQ:
@@ -627,6 +632,12 @@ static int emit_atomic_ld_st(u8 rd, u8 rs, s16 off, s32 imm, u8 code, struct rv_
627632
break;
628633
}
629634
emit_fence_r_rw(ctx);
635+
636+
/* If our next insn is a redundant zext, return 1 to tell
637+
* build_body() to skip it.
638+
*/
639+
if (BPF_SIZE(code) != BPF_DW && insn_is_zext(&insn[1]))
640+
return 1;
630641
break;
631642
/* store_release(dst_reg + off16, src_reg) */
632643
case BPF_STORE_REL:
@@ -654,10 +665,12 @@ static int emit_atomic_ld_st(u8 rd, u8 rs, s16 off, s32 imm, u8 code, struct rv_
654665
return 0;
655666
}
656667

657-
static int emit_atomic_rmw(u8 rd, u8 rs, s16 off, s32 imm, u8 code,
668+
static int emit_atomic_rmw(u8 rd, u8 rs, const struct bpf_insn *insn,
658669
struct rv_jit_context *ctx)
659670
{
660-
u8 r0;
671+
u8 r0, code = insn->code;
672+
s16 off = insn->off;
673+
s32 imm = insn->imm;
661674
int jmp_offset;
662675
bool is64;
663676

@@ -2026,9 +2039,9 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
20262039
case BPF_STX | BPF_ATOMIC | BPF_W:
20272040
case BPF_STX | BPF_ATOMIC | BPF_DW:
20282041
if (bpf_atomic_is_load_store(insn))
2029-
ret = emit_atomic_ld_st(rd, rs, off, imm, code, ctx);
2042+
ret = emit_atomic_ld_st(rd, rs, insn, ctx);
20302043
else
2031-
ret = emit_atomic_rmw(rd, rs, off, imm, code, ctx);
2044+
ret = emit_atomic_rmw(rd, rs, insn, ctx);
20322045
if (ret)
20332046
return ret;
20342047
break;

arch/riscv/net/bpf_jit_core.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ static int build_body(struct rv_jit_context *ctx, bool extra_pass, int *offset)
2626
int ret;
2727

2828
ret = bpf_jit_emit_insn(insn, ctx, extra_pass);
29-
/* BPF_LD | BPF_IMM | BPF_DW: skip the next instruction. */
3029
if (ret > 0)
31-
i++;
30+
i++; /* skip the next instruction */
3231
if (offset)
3332
offset[i] = ctx->ninsns;
3433
if (ret < 0)

0 commit comments

Comments
 (0)