| 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(-)