From f75298f5c3fee750ad170bb98556e366a45a4093 Mon Sep 17 00:00:00 2001 From: Michael Holzheu Date: Wed, 29 Jul 2015 21:15:14 +0200 Subject: [PATCH 1/5] s390/bpf: clear correct BPF accumulator register Currently we assumed the following BPF to eBPF register mapping: - BPF_REG_A -> BPF_REG_7 - BPF_REG_X -> BPF_REG_8 Unfortunately this mapping is wrong. The correct mapping is: - BPF_REG_A -> BPF_REG_0 - BPF_REG_X -> BPF_REG_7 So clear the correct registers and use the BPF_REG_A and BPF_REG_X macros instead of BPF_REG_0/7. Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend") Cc: stable@vger.kernel.org # 4.0+ Signed-off-by: Michael Holzheu Signed-off-by: David S. Miller --- arch/s390/net/bpf_jit_comp.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c index 79c731e8d178..01ad16608f66 100644 --- a/arch/s390/net/bpf_jit_comp.c +++ b/arch/s390/net/bpf_jit_comp.c @@ -448,13 +448,13 @@ static void bpf_jit_prologue(struct bpf_jit *jit) EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0, BPF_REG_1, offsetof(struct sk_buff, data)); } - /* BPF compatibility: clear A (%b7) and X (%b8) registers */ - if (REG_SEEN(BPF_REG_7)) - /* lghi %b7,0 */ - EMIT4_IMM(0xa7090000, BPF_REG_7, 0); - if (REG_SEEN(BPF_REG_8)) - /* lghi %b8,0 */ - EMIT4_IMM(0xa7090000, BPF_REG_8, 0); + /* BPF compatibility: clear A (%b0) and X (%b7) registers */ + if (REG_SEEN(BPF_REG_A)) + /* lghi %ba,0 */ + EMIT4_IMM(0xa7090000, BPF_REG_A, 0); + if (REG_SEEN(BPF_REG_X)) + /* lghi %bx,0 */ + EMIT4_IMM(0xa7090000, BPF_REG_X, 0); } /* From 1df03ffdded54fbb6507ef494ece071f3501410e Mon Sep 17 00:00:00 2001 From: Michael Holzheu Date: Wed, 29 Jul 2015 21:15:15 +0200 Subject: [PATCH 2/5] s390/bpf: Fix multiple macro expansions The EMIT6_DISP_LH macro passes the "disp" parameter to the _EMIT6_DISP_LH macro. The _EMIT6_DISP_LH macro uses the "disp" parameter twice: unsigned int __disp_h = ((u32)disp) & 0xff000; unsigned int __disp_l = ((u32)disp) & 0x00fff; The EMIT6_DISP_LH is used several times with EMIT_CONST_U64() as "disp" parameter. Therefore always two constants are created per usage of EMIT6_DISP_LH. Fix this and add variable "_disp" to avoid multiple expansions. * v2: Move "_disp" to _EMIT6_DISP_LH as suggested by Joe Perches Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend") Signed-off-by: Michael Holzheu Signed-off-by: David S. Miller --- arch/s390/net/bpf_jit_comp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c index 01ad16608f66..66926ab244c1 100644 --- a/arch/s390/net/bpf_jit_comp.c +++ b/arch/s390/net/bpf_jit_comp.c @@ -214,8 +214,9 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 b1) #define _EMIT6_DISP_LH(op1, op2, disp) \ ({ \ - unsigned int __disp_h = ((u32)disp) & 0xff000; \ - unsigned int __disp_l = ((u32)disp) & 0x00fff; \ + u32 _disp = (u32) disp; \ + unsigned int __disp_h = _disp & 0xff000; \ + unsigned int __disp_l = _disp & 0x00fff; \ _EMIT6(op1 | __disp_l, op2 | __disp_h >> 4); \ }) From ce2b6ad9c1856fac5d5e8d3351b3dd5392e09b7a Mon Sep 17 00:00:00 2001 From: Michael Holzheu Date: Wed, 29 Jul 2015 21:15:16 +0200 Subject: [PATCH 3/5] s390/bpf: increase BPF_SIZE_MAX Currently we have the restriction that jitted BPF programs can have a maximum size of one page. The reason is that we use short displacements for the literal pool. The 20 bit displacements are available since z990 and BPF requires z196 as minimum. Therefore we can remove this restriction and use everywhere 20 bit signed long displacements. Acked-by: Martin Schwidefsky Signed-off-by: Michael Holzheu Signed-off-by: David S. Miller --- arch/s390/net/bpf_jit_comp.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c index 66926ab244c1..04af36728a18 100644 --- a/arch/s390/net/bpf_jit_comp.c +++ b/arch/s390/net/bpf_jit_comp.c @@ -45,7 +45,7 @@ struct bpf_jit { int labels[1]; /* Labels for local jumps */ }; -#define BPF_SIZE_MAX 4096 /* Max size for program */ +#define BPF_SIZE_MAX 0x7ffff /* Max size for program (20 bit signed displ) */ #define SEEN_SKB 1 /* skb access */ #define SEEN_MEM 2 /* use mem[] for temporary storage */ @@ -203,15 +203,6 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 b1) _EMIT6(op1 | __disp, op2); \ }) -#define EMIT6_DISP(op1, op2, b1, b2, b3, disp) \ -({ \ - _EMIT6_DISP(op1 | reg(b1, b2) << 16 | \ - reg_high(b3) << 8, op2, disp); \ - REG_SET_SEEN(b1); \ - REG_SET_SEEN(b2); \ - REG_SET_SEEN(b3); \ -}) - #define _EMIT6_DISP_LH(op1, op2, disp) \ ({ \ u32 _disp = (u32) disp; \ @@ -981,8 +972,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i REG_SET_SEEN(BPF_REG_5); jit->seen |= SEEN_FUNC; /* lg %w1,(%l) */ - EMIT6_DISP(0xe3000000, 0x0004, REG_W1, REG_0, REG_L, - EMIT_CONST_U64(func)); + EMIT6_DISP_LH(0xe3000000, 0x0004, REG_W1, REG_0, REG_L, + EMIT_CONST_U64(func)); /* basr %r14,%w1 */ EMIT2(0x0d00, REG_14, REG_W1); /* lgr %b0,%r2: load return value into %b0 */ From cde66c2d88da1d73d755109c80ce4c34b917596b Mon Sep 17 00:00:00 2001 From: Michael Holzheu Date: Wed, 29 Jul 2015 21:15:17 +0200 Subject: [PATCH 4/5] s390/bpf: Only clear A and X for converted BPF programs Only classic BPF programs that have been converted to eBPF need to clear the A and X registers. We can check for converted programs with: bpf_prog->type == BPF_PROG_TYPE_UNSPEC So add the check and skip initialization for real eBPF programs. Signed-off-by: Michael Holzheu Signed-off-by: David S. Miller --- arch/s390/net/bpf_jit_comp.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c index 04af36728a18..3dd01637f83a 100644 --- a/arch/s390/net/bpf_jit_comp.c +++ b/arch/s390/net/bpf_jit_comp.c @@ -387,7 +387,7 @@ static void save_restore_regs(struct bpf_jit *jit, int op) * Save registers and create stack frame if necessary. * See stack frame layout desription in "bpf_jit.h"! */ -static void bpf_jit_prologue(struct bpf_jit *jit) +static void bpf_jit_prologue(struct bpf_jit *jit, bool is_classic) { if (jit->seen & SEEN_TAIL_CALL) { /* xc STK_OFF_TCCNT(4,%r15),STK_OFF_TCCNT(%r15) */ @@ -440,13 +440,15 @@ static void bpf_jit_prologue(struct bpf_jit *jit) EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0, BPF_REG_1, offsetof(struct sk_buff, data)); } - /* BPF compatibility: clear A (%b0) and X (%b7) registers */ - if (REG_SEEN(BPF_REG_A)) - /* lghi %ba,0 */ - EMIT4_IMM(0xa7090000, BPF_REG_A, 0); - if (REG_SEEN(BPF_REG_X)) - /* lghi %bx,0 */ - EMIT4_IMM(0xa7090000, BPF_REG_X, 0); + /* Clear A (%b0) and X (%b7) registers for converted BPF programs */ + if (is_classic) { + if (REG_SEEN(BPF_REG_A)) + /* lghi %ba,0 */ + EMIT4_IMM(0xa7090000, BPF_REG_A, 0); + if (REG_SEEN(BPF_REG_X)) + /* lghi %bx,0 */ + EMIT4_IMM(0xa7090000, BPF_REG_X, 0); + } } /* @@ -1232,7 +1234,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp) jit->lit = jit->lit_start; jit->prg = 0; - bpf_jit_prologue(jit); + bpf_jit_prologue(jit, fp->type == BPF_PROG_TYPE_UNSPEC); for (i = 0; i < fp->len; i += insn_count) { insn_count = bpf_jit_insn(jit, fp, i); if (insn_count < 0) From 9db7f2b818809ef2c40fbd64cfcf5ccb0107d7e8 Mon Sep 17 00:00:00 2001 From: Michael Holzheu Date: Wed, 29 Jul 2015 21:15:18 +0200 Subject: [PATCH 5/5] s390/bpf: recache skb->data/hlen for skb_vlan_push/pop Allow eBPF programs attached to TC qdiscs call skb_vlan_push/pop via helper functions. These functions may change skb->data/hlen. This data is cached by s390 JIT to improve performance of ld_abs/ld_ind instructions. Therefore after a change we have to reload the data. In case of usage of skb_vlan_push/pop, in the prologue we store the SKB pointer on the stack and restore it after BPF_JMP_CALL to skb_vlan_push/pop. Signed-off-by: Michael Holzheu Signed-off-by: David S. Miller --- arch/s390/net/bpf_jit.h | 5 +++- arch/s390/net/bpf_jit_comp.c | 55 +++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/arch/s390/net/bpf_jit.h b/arch/s390/net/bpf_jit.h index f6498eec9ee1..f010c93a88b1 100644 --- a/arch/s390/net/bpf_jit.h +++ b/arch/s390/net/bpf_jit.h @@ -36,6 +36,8 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[]; * | BPF stack | | * | | | * +---------------+ | + * | 8 byte skbp | | + * R15+170 -> +---------------+ | * | 8 byte hlen | | * R15+168 -> +---------------+ | * | 4 byte align | | @@ -51,11 +53,12 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[]; * We get 160 bytes stack space from calling function, but only use * 12 * 8 byte for old backchain, r15..r6, and tail_call_cnt. */ -#define STK_SPACE (MAX_BPF_STACK + 8 + 4 + 4 + 160) +#define STK_SPACE (MAX_BPF_STACK + 8 + 8 + 4 + 4 + 160) #define STK_160_UNUSED (160 - 12 * 8) #define STK_OFF (STK_SPACE - STK_160_UNUSED) #define STK_OFF_TMP 160 /* Offset of tmp buffer on stack */ #define STK_OFF_HLEN 168 /* Offset of SKB header length on stack */ +#define STK_OFF_SKBP 170 /* Offset of SKB pointer on stack */ #define STK_OFF_R6 (160 - 11 * 8) /* Offset of r6 on stack */ #define STK_OFF_TCCNT (160 - 12 * 8) /* Offset of tail_call_cnt on stack */ diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c index 3dd01637f83a..bbbac6da37af 100644 --- a/arch/s390/net/bpf_jit_comp.c +++ b/arch/s390/net/bpf_jit_comp.c @@ -53,6 +53,7 @@ struct bpf_jit { #define SEEN_LITERAL 8 /* code uses literals */ #define SEEN_FUNC 16 /* calls C functions */ #define SEEN_TAIL_CALL 32 /* code uses tail calls */ +#define SEEN_SKB_CHANGE 64 /* code changes skb data */ #define SEEN_STACK (SEEN_FUNC | SEEN_MEM | SEEN_SKB) /* @@ -381,6 +382,26 @@ static void save_restore_regs(struct bpf_jit *jit, int op) } while (re <= 15); } +/* + * For SKB access %b1 contains the SKB pointer. For "bpf_jit.S" + * we store the SKB header length on the stack and the SKB data + * pointer in REG_SKB_DATA. + */ +static void emit_load_skb_data_hlen(struct bpf_jit *jit) +{ + /* Header length: llgf %w1,(%b1) */ + EMIT6_DISP_LH(0xe3000000, 0x0016, REG_W1, REG_0, BPF_REG_1, + offsetof(struct sk_buff, len)); + /* s %w1,(%b1) */ + EMIT4_DISP(0x5b000000, REG_W1, BPF_REG_1, + offsetof(struct sk_buff, data_len)); + /* stg %w1,ST_OFF_HLEN(%r0,%r15) */ + EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0, REG_15, STK_OFF_HLEN); + /* lg %skb_data,data_off(%b1) */ + EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0, + BPF_REG_1, offsetof(struct sk_buff, data)); +} + /* * Emit function prologue * @@ -421,25 +442,12 @@ static void bpf_jit_prologue(struct bpf_jit *jit, bool is_classic) EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0, REG_15, 152); } - /* - * For SKB access %b1 contains the SKB pointer. For "bpf_jit.S" - * we store the SKB header length on the stack and the SKB data - * pointer in REG_SKB_DATA. - */ - if (jit->seen & SEEN_SKB) { - /* Header length: llgf %w1,(%b1) */ - EMIT6_DISP_LH(0xe3000000, 0x0016, REG_W1, REG_0, BPF_REG_1, - offsetof(struct sk_buff, len)); - /* s %w1,(%b1) */ - EMIT4_DISP(0x5b000000, REG_W1, BPF_REG_1, - offsetof(struct sk_buff, data_len)); - /* stg %w1,ST_OFF_HLEN(%r0,%r15) */ + if (jit->seen & SEEN_SKB) + emit_load_skb_data_hlen(jit); + if (jit->seen & SEEN_SKB_CHANGE) + /* stg %b1,ST_OFF_SKBP(%r0,%r15) */ EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0, REG_15, - STK_OFF_HLEN); - /* lg %skb_data,data_off(%b1) */ - EMIT6_DISP_LH(0xe3000000, 0x0004, REG_SKB_DATA, REG_0, - BPF_REG_1, offsetof(struct sk_buff, data)); - } + STK_OFF_SKBP); /* Clear A (%b0) and X (%b7) registers for converted BPF programs */ if (is_classic) { if (REG_SEEN(BPF_REG_A)) @@ -967,10 +975,6 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i */ const u64 func = (u64)__bpf_call_base + imm; - if (bpf_helper_changes_skb_data((void *)func)) - /* TODO reload skb->data, hlen */ - return -1; - REG_SET_SEEN(BPF_REG_5); jit->seen |= SEEN_FUNC; /* lg %w1,(%l) */ @@ -980,6 +984,13 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i EMIT2(0x0d00, REG_14, REG_W1); /* lgr %b0,%r2: load return value into %b0 */ EMIT4(0xb9040000, BPF_REG_0, REG_2); + if (bpf_helper_changes_skb_data((void *)func)) { + jit->seen |= SEEN_SKB_CHANGE; + /* lg %b1,ST_OFF_SKBP(%r15) */ + EMIT6_DISP_LH(0xe3000000, 0x0004, BPF_REG_1, REG_0, + REG_15, STK_OFF_SKBP); + emit_load_skb_data_hlen(jit); + } break; } case BPF_JMP | BPF_CALL | BPF_X: