s390/bpf: fix lib/test_bpf.c failures
Subsystems (5)
Ilya Leoshkevich
maintainercover s390/bpf: fix lib/test_bpf.c failures
This patch series fixes three regressions caused by 4e9b4a6883dd ("s390/bpf: Use relative long branches") reported by Seth Forshee (patches 2-4) and adds two minor related improvements (patches 1 and 5).
Ilya Leoshkevich (5):
selftests: bpf: test_kmod.sh: fix running out of srctree s390/bpf: fix sign extension in branch_ku s390/bpf: use brcl for jumping to exit_ip if necessary s390/bpf: tolerate not converging code shrinking s390/bpf: use bpf_skip() in bpf_jit_prologue()
arch/s390/net/bpf_jit_comp.c | 63 +++++++++++++++--------- tools/testing/selftests/bpf/test_kmod.sh | 12 +++-- 2 files changed, 50 insertions(+), 25 deletions(-)
Signature · 1 line
2.25.4
Ilya Leoshkevich
maintainerPATCH 1/5 selftests: bpf: test_kmod.sh: fix running out of srctree
When running out of srctree, relative path to lib/test_bpf.ko is different than when running in srctree. Check $building_out_of_srctree environment variable and use a different relative path if needed.
--- tools/testing/selftests/bpf/test_kmod.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
tools/testing/selftests/bpf/test_kmod.sh +9 -3
@@ -10,7 +10,13 @@ if [ "$(id -u)" != "0" ]; then exit $ksft_skip fi -SRC_TREE=../../../../ +if [ "$building_out_of_srctree" ]; then + # We are in linux-build/kselftest/bpf + OUTPUT=../../ +else + # We are in linux/tools/testing/selftests/bpf + OUTPUT=../../../../ +fi test_run() {
@@ -19,8 +25,8 @@ test_run() echo "[ JIT enabled:$1 hardened:$2 ]" dmesg -C - if [ -f ${SRC_TREE}/lib/test_bpf.ko ]; then - insmod ${SRC_TREE}/lib/test_bpf.ko 2> /dev/null + if [ -f ${OUTPUT}/lib/test_bpf.ko ]; then + insmod ${OUTPUT}/lib/test_bpf.ko 2> /dev/null if [ $? -ne 0 ]; then rc=1 fi
Signature · 1 line
2.25.4
Ilya Leoshkevich
maintainerPATCH 2/5 s390/bpf: fix sign extension in branch_ku
In reply to Ilya LeoshkevichBoth signed and unsigned variants of BPF_JMP | BPF_K require sign-extending the immediate. JIT emits cgfi for the signed case, which is correct, and clgfi for the unsigned case, which is not
correct: clgfi zero-extends the immediate.
s390 does not provide an instruction that does sign-extension and unsigned comparison at the same time. Therefore, fix by first loading the sign-extended immediate into work register REG_1 and proceeding as if it's BPF_X.
--- arch/s390/net/bpf_jit_comp.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)
arch/s390/net/bpf_jit_comp.c +4 -15
@@ -1507,21 +1507,10 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, }break ; branch_ku: - is_jmp32 =BPF_CLASS (insn->code) ==BPF_JMP32 ; -/* clfi or clgfi %dst,imm */ -EMIT6_IMM (is_jmp32 ?0xc20f0000 :0xc20e0000 , - dst_reg, imm); -if (!is_first_pass (jit) && -can_use_rel (jit, addrs[i + off +1 ])) { -/* brc mask,off */ -EMIT4_PCREL_RIC (0xa7040000 , - mask >>12 , addrs[i + off +1 ]); - }else { -/* brcl mask,off */ -EMIT6_PCREL_RILC (0xc0040000 , - mask >>12 , addrs[i + off +1 ]); - } -break ; +/* lgfi %w1,imm (load sign extend imm) */ + src_reg =REG_1 ; +EMIT6_IMM (0xc0010000 , src_reg, imm); + goto branch_xu; branch_xs: is_jmp32 =BPF_CLASS (insn->code) ==BPF_JMP32 ; if (!is_first_pass(jit) &&
Signature · 1 line
2.25.4
Re: [PATCH 2/5] s390/bpf: fix sign extension in branch_ku
Quoted 5 lines · Ilya Leoshkevich
Both signed and unsigned variants of
BPF_JMP|BPF_Krequire sign-extending the immediate. JIT emits cgfi for the signed case, which is correct, and clgfi for the unsigned case, which is not
correct: clgfi zero-extends the immediate.
Quoted 8 lines
s390 does not provide an instruction that does sign-extension and unsigned comparison at the same time. Therefore, fix by first loading the sign-extended immediate into work register
REG_1and proceeding as if it'sBPF_X.Fixes 4e9b4a6883dd ("s390/bpf: Use relative long branches")Reported-by Seth Forshee <[email protected]>Signed-off-by Ilya Leoshkevich <[email protected]>
This fixes the failing tests I was seeing. Thanks!
Ilya Leoshkevich
maintainerPATCH 3/5 s390/bpf: use brcl for jumping to exit_ip if necessary
In reply to Ilya Leoshkevich"BPF_MAXINSNS: Maximum possible literals" test causes panic with bpf_jit_harden = 2. The reason is that BPF_JMP | BPF_EXIT is always emitted as brc, however, after removal of JITed image size limitations, brcl might be required.
Fix by using brcl when necessary.
--- arch/s390/net/bpf_jit_comp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
arch/s390/net/bpf_jit_comp.c +6 -2
@@ -1358,8 +1358,12 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, last = (i == fp->len -1 ) ?1 :0 ;if (last)break ; -/* j <exit> */ -EMIT4_PCREL (0xa7f40000 , jit->exit_ip - jit->prg); +if (!is_first_pass (jit) &&can_use_rel (jit, jit->exit_ip)) +/* brc 0xf, <exit> */ +EMIT4_PCREL_RIC (0xa7040000 ,0xf , jit->exit_ip); +else +/* brcl 0xf, <exit> */ +EMIT6_PCREL_RILC (0xc0040000 ,0xf , jit->exit_ip);break ;/* * Branch relative (number of skipped instructions) to offset on
Signature · 1 line
2.25.4
Ilya Leoshkevich
maintainerPATCH 4/5 s390/bpf: tolerate not converging code shrinking
In reply to Ilya Leoshkevich"BPF_MAXINSNS: Maximum possible literals" unnecessarily falls back to the interpreter because of failing sanity check in bpf_set_addr. The problem is that there are a lot of branches that can be shrunk, and doing so opens up the possibility to shrink even more. This process does not converge after 3 passes, causing code offsets to change during the codegen pass, which must never happen.
Fix by inserting nops during codegen pass in order to preserve code offets.
--- arch/s390/net/bpf_jit_comp.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
arch/s390/net/bpf_jit_comp.c +26 -1
@@ -490,6 +490,24 @@ static void save_restore_regs(struct bpf_jit *jit, int op, u32 stack_depth) }while (re <= last); } +static void bpf_skip (struct bpf_jit *jit,int size) +{ +if (size >=6 && !is_valid_rel (size)) { +/* brcl 0xf,size */ +EMIT6_PCREL_RIL (0xc0f4000000 , size); + size -=6 ; + }else if (size >=4 &&is_valid_rel (size)) { +/* brc 0xf,size */ +EMIT4_PCREL (0xa7f40000 , size); + size -=4 ; + } +while (size >=2 ) { +/* bcr 0,%0 */ +_EMIT2 (0x0700 ); + size -=2 ; + } +} +/* * Emit function prologue *
@@ -1610,7 +1628,14 @@ static bool bpf_is_new_addr_sane(struct bpf_jit *jit, int i)*/ static int bpf_set_addr (struct bpf_jit *jit,int i) { -if (!bpf_is_new_addr_sane (jit, i)) +int delta; + +if (is_codegen_pass (jit)) { + delta = jit->prg - jit->addrs[i]; +if (delta <0 ) +bpf_skip (jit, -delta); + } +if (WARN_ON_ONCE (!bpf_is_new_addr_sane (jit, i)))return -1 ; jit->addrs[i] = jit->prg;return 0 ;
Signature · 1 line
2.25.4
Ilya Leoshkevich
maintainerPATCH 5/5 s390/bpf: use bpf_skip() in bpf_jit_prologue()
In reply to Ilya LeoshkevichNow that we have bpf_skip() for emitting nops, use it in bpf_jit_prologue() in order to reduce code duplication.
--- arch/s390/net/bpf_jit_comp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
arch/s390/net/bpf_jit_comp.c +5 -4
@@ -520,10 +520,11 @@ static void bpf_jit_prologue(struct bpf_jit *jit, u32 stack_depth)/* xc STK_OFF_TCCNT(4,%r15),STK_OFF_TCCNT(%r15) */ _EMIT6 (0xd703f000 |STK_OFF_TCCNT ,0xf000 |STK_OFF_TCCNT ); }else { -/* j tail_call_start: NOP if no tail calls are used */ -EMIT4_PCREL (0xa7f40000 ,6 ); -/* bcr 0,%0 */ -EMIT2 (0x0700 ,0 ,REG_0 ); +/* +* There are no tail calls. Insert nops in order to have +* tail_call_start at a predictable offset. +*/ +bpf_skip (jit,6 ); }/* Tail calls have to skip above initialization */ jit->tail_call_start = jit->prg;
Signature · 1 line
2.25.4
Alexei Starovoitov
AppliedQuoted 5 lines · Ilya Leoshkevich
This patch series fixes three regressions caused by
4e9b4a6883dd("s390/bpf: Use relative long branches") reported by Seth Forshee (patches 2-4) and adds two minor related improvements (patches 1 and 5).
Applied. Thanks