Message ID | E1ieH2l-0004io-VM@rmk-PC.armlinux.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ARM: net: bpf: improve endian conversion | expand |
Hi Russell, I love your patch! Yet something to improve: [auto build test ERROR on bpf-next/master] [also build test ERROR on bpf/master v5.5-rc1 next-20191209] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Russell-King/ARM-net-bpf-improve-endian-conversion/20191210-011341 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=arm If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): arch/arm/net/bpf_jit_32.c: In function 'emit_a32_endian': >> arch/arm/net/bpf_jit_32.c:1263:11: error: 'imm' undeclared (first use in this function) switch (imm) { ^~~ arch/arm/net/bpf_jit_32.c:1263:11: note: each undeclared identifier is reported only once for each function it appears in arch/arm/net/bpf_jit_32.c:1252:12: warning: unused variable 'tmp2' [-Wunused-variable] const s8 *tmp2 = bpf2a32[TMP_REG_2]; ^~~~ vim +/imm +1263 arch/arm/net/bpf_jit_32.c 1247 1248 static void emit_a32_endian(const s8 dst[], u8 code, s32 bits, 1249 struct jit_ctx *ctx) 1250 { 1251 const s8 *tmp = bpf2a32[TMP_REG_1]; 1252 const s8 *tmp2 = bpf2a32[TMP_REG_2]; 1253 const s8 *rd; 1254 1255 /* Converting from LE and 64-bit value is a no-op. */ 1256 if (code == BPF_FROM_LE && bits == 64) 1257 return; 1258 1259 rd = arm_bpf_get_reg64(dst, tmp, ctx); 1260 1261 if (code != BPF_FROM_LE) { 1262 /* endian swap */ > 1263 switch (imm) { 1264 case 16: 1265 emit_rev16(rd[1], rd[1], ctx); 1266 break; 1267 case 32: 1268 emit_rev32(rd[1], rd[1], ctx); 1269 break; 1270 case 64: 1271 emit_rev32(ARM_LR, rd[1], ctx); 1272 emit_rev32(rd[1], rd[0], ctx); 1273 emit(ARM_MOV_R(rd[0], ARM_LR), ctx); 1274 break; 1275 } 1276 } 1277 1278 /* zero-extend size to 64-bit */ 1279 switch (imm) { 1280 case 16: 1281 #if __LINUX_ARM_ARCH__ < 6 1282 emit_a32_mov_i(tmp2[1], 0xffff, ctx); 1283 emit(ARM_AND_R(rd[1], rd[1], tmp2[1]), ctx); 1284 #else /* ARMv6+ */ 1285 emit(ARM_UXTH(rd[1], rd[1]), ctx); 1286 #endif 1287 /* FALLTHROUGH */ 1288 case 32: 1289 if (!ctx->prog->aux->verifier_zext) 1290 emit(ARM_MOV_I(rd[0], 0), ctx); 1291 break; 1292 } 1293 1294 arm_bpf_put_reg64(dst, rd, ctx); 1295 } 1296 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c index cc29869d12a3..646ab5785ca4 100644 --- a/arch/arm/net/bpf_jit_32.c +++ b/arch/arm/net/bpf_jit_32.c @@ -1245,6 +1245,55 @@ static inline void emit_rev32(const u8 rd, const u8 rn, struct jit_ctx *ctx) #endif } +static void emit_a32_endian(const s8 dst[], u8 code, s32 bits, + struct jit_ctx *ctx) +{ + const s8 *tmp = bpf2a32[TMP_REG_1]; + const s8 *tmp2 = bpf2a32[TMP_REG_2]; + const s8 *rd; + + /* Converting from LE and 64-bit value is a no-op. */ + if (code == BPF_FROM_LE && bits == 64) + return; + + rd = arm_bpf_get_reg64(dst, tmp, ctx); + + if (code != BPF_FROM_LE) { + /* endian swap */ + switch (imm) { + case 16: + emit_rev16(rd[1], rd[1], ctx); + break; + case 32: + emit_rev32(rd[1], rd[1], ctx); + break; + case 64: + emit_rev32(ARM_LR, rd[1], ctx); + emit_rev32(rd[1], rd[0], ctx); + emit(ARM_MOV_R(rd[0], ARM_LR), ctx); + break; + } + } + + /* zero-extend size to 64-bit */ + switch (imm) { + case 16: +#if __LINUX_ARM_ARCH__ < 6 + emit_a32_mov_i(tmp2[1], 0xffff, ctx); + emit(ARM_AND_R(rd[1], rd[1], tmp2[1]), ctx); +#else /* ARMv6+ */ + emit(ARM_UXTH(rd[1], rd[1]), ctx); +#endif + /* FALLTHROUGH */ + case 32: + if (!ctx->prog->aux->verifier_zext) + emit(ARM_MOV_I(rd[0], 0), ctx); + break; + } + + arm_bpf_put_reg64(dst, rd, ctx); +} + // push the scratch stack register on top of the stack static inline void emit_push_r64(const s8 src[], struct jit_ctx *ctx) { @@ -1523,47 +1572,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx) /* dst = htobe(dst) */ case BPF_ALU | BPF_END | BPF_FROM_LE: case BPF_ALU | BPF_END | BPF_FROM_BE: - rd = arm_bpf_get_reg64(dst, tmp, ctx); - if (BPF_SRC(code) == BPF_FROM_LE) - goto emit_bswap_uxt; - switch (imm) { - case 16: - emit_rev16(rd[1], rd[1], ctx); - goto emit_bswap_uxt; - case 32: - emit_rev32(rd[1], rd[1], ctx); - goto emit_bswap_uxt; - case 64: - emit_rev32(ARM_LR, rd[1], ctx); - emit_rev32(rd[1], rd[0], ctx); - emit(ARM_MOV_R(rd[0], ARM_LR), ctx); - break; - } - goto exit; -emit_bswap_uxt: - switch (imm) { - case 16: - /* zero-extend 16 bits into 64 bits */ -#if __LINUX_ARM_ARCH__ < 6 - emit_a32_mov_i(tmp2[1], 0xffff, ctx); - emit(ARM_AND_R(rd[1], rd[1], tmp2[1]), ctx); -#else /* ARMv6+ */ - emit(ARM_UXTH(rd[1], rd[1]), ctx); -#endif - if (!ctx->prog->aux->verifier_zext) - emit(ARM_EOR_R(rd[0], rd[0], rd[0]), ctx); - break; - case 32: - /* zero-extend 32 bits into 64 bits */ - if (!ctx->prog->aux->verifier_zext) - emit(ARM_EOR_R(rd[0], rd[0], rd[0]), ctx); - break; - case 64: - /* nop */ - break; - } -exit: - arm_bpf_put_reg64(dst, rd, ctx); + emit_a32_endian(dst, BPF_SRC(code), imm, ctx); break; /* dst = imm64 */ case BPF_LD | BPF_IMM | BPF_DW:
Make the endian conversion function easier to read by moving it out of the big switch, and avoid doing anything if we're requested to convert from a 64-bit LE value (we're LE anyway here.) Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- arch/arm/net/bpf_jit_32.c | 91 +++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 41 deletions(-)