2023-09-13 09:10:53

by Puranjay Mohan

[permalink] [raw]
Subject: [PATCH bpf-next 0/6] bpf: verifier: stop emitting zext for LDX

All 64-bit architectures that support the BPF JIT do LDX + zero extension
with a single CPU instruction. Some 64-bit architectures like riscv64,
s390, mips64, etc. have bpf_jit_needs_zext() -> true. This means although
these architectures do LDX + zero extension with a single CPU instruction,
the verifier emits extra zero extension instructions after LDX | B/H/W.

After a discussion about this in [1], it was decided that the verifier
should not emit zext instructions for LDX and all JITs that can't do a LDX
+ zero extension with a single instruction should emit two instructions by
default for LDX.

All 32 bit JITs checked for ctx->prog->aux->verifier_zext and did not do
explicit zero extension after LDX if this is set by the verifier.

This patch series changes all applicable 32-bit JITs to always do a zero
extension after LDX without checking ctx->prog->aux->verifier_zext.

The last patch modifies the verifier to always mark the destination of LDX
as 64 bit which in turn stops the verifier from emitting zext after LDX.

These changes have not been tested because I don't have the hardware to do
so, I would request the JIT maintainers to help me test this. Especially,
the powerpc32 JTI where amount of code change is more.

[1] https://lore.kernel.org/all/CANk7y0j2f-gPgZwd+YfTL71-6wfvky+f=kBC_ccqsS0EHAysyA@mail.gmail.com/

Puranjay Mohan (6):
bpf, riscv32: Always zero extend for LDX with B/W/H
bpf, x86-32: Always zero extend for LDX with B/W/H
bpf, parisc32: Always zero extend for LDX with B/W/H
bpf, powerpc32: Always zero extend for LDX
bpf, arm32: Always zero extend for LDX with B/H/W
bpf, verifier: always mark destination of LDX as 64-bit

arch/arm/net/bpf_jit_32.c | 9 +++------
arch/parisc/net/bpf_jit_comp32.c | 9 +++------
arch/powerpc/net/bpf_jit_comp32.c | 25 ++++++++-----------------
arch/riscv/net/bpf_jit_comp32.c | 9 +++------
arch/x86/net/bpf_jit_comp32.c | 2 --
kernel/bpf/verifier.c | 4 +---
6 files changed, 18 insertions(+), 40 deletions(-)

--
2.39.2


2023-09-13 15:23:13

by Puranjay Mohan

[permalink] [raw]
Subject: [PATCH bpf-next 1/6] bpf, riscv32: Always zero extend for LDX with B/W/H

The JITs should not depend on the verifier for zero extending the upper
32 bits of the destination register when loading a byte, half-word, or
word.

A following patch will make the verifier stop patching zext instructions
after LDX.

Signed-off-by: Puranjay Mohan <[email protected]>
---
arch/riscv/net/bpf_jit_comp32.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
index 529a83b85c1c..8f8255519ba1 100644
--- a/arch/riscv/net/bpf_jit_comp32.c
+++ b/arch/riscv/net/bpf_jit_comp32.c
@@ -847,18 +847,15 @@ static int emit_load_r64(const s8 *dst, const s8 *src, s16 off,
switch (size) {
case BPF_B:
emit(rv_lbu(lo(rd), 0, RV_REG_T0), ctx);
- if (!ctx->prog->aux->verifier_zext)
- emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
+ emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
break;
case BPF_H:
emit(rv_lhu(lo(rd), 0, RV_REG_T0), ctx);
- if (!ctx->prog->aux->verifier_zext)
- emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
+ emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
break;
case BPF_W:
emit(rv_lw(lo(rd), 0, RV_REG_T0), ctx);
- if (!ctx->prog->aux->verifier_zext)
- emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
+ emit(rv_addi(hi(rd), RV_REG_ZERO, 0), ctx);
break;
case BPF_DW:
emit(rv_lw(lo(rd), 0, RV_REG_T0), ctx);
--
2.39.2

2023-09-13 20:40:49

by Puranjay Mohan

[permalink] [raw]
Subject: [PATCH bpf-next 6/6] bpf, verifier: always mark destination of LDX as 64-bit

All 64-bit JITs utilize a single instruction to load + zero-extend a
byte, word, or a half-word. The optimisation of emitting zext for LDX is
not useful for most of the JITs.

All the JITs that relied on the verifier for zero extension of LDX
desitination registers have been modified to always zero extend the
destination.

Now the verifier can safely mark LDX destination as 64-bit and stop
emitting zero-extension instructions for it.

Signed-off-by: Puranjay Mohan <[email protected]>
---
kernel/bpf/verifier.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dbba2b806017..02a1ac1a1327 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3028,9 +3028,7 @@ static bool is_reg64(struct bpf_verifier_env *env, struct bpf_insn *insn,
return false;

if (class == BPF_LDX) {
- if (t != SRC_OP)
- return BPF_SIZE(code) == BPF_DW;
- /* LDX source must be ptr. */
+ /* LDX source must be a ptr. and LDX destination is always zero-extended. */
return true;
}

--
2.39.2