[PATCH 0/2] bpf: fix x86 jit issue

[PATCH 0/2] bpf: fix x86 jit issue

[Konstantin Ananyev]

*** BLURB HERE ***

Konstantin Ananyev (2):
  bpf: fix x86 jit for immediate loads
  test/bpf: add test for immediate load

 lib/librte_bpf/bpf_jit_x86.c |  28 ++++++---
 test/test/test_bpf.c         | 108 +++++++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH 1/2] bpf: fix x86 jit for immediate loads

[Konstantin Ananyev]

x86 jit can generate invalid code for (BPF_LD | BPF_IMM | EBPF_DW)
instructions, when immediate value is bigger then INT32_MAX.

Fixes: cc752e43e079 ("bpf: add JIT compilation for x86_64 ISA")

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 lib/librte_bpf/bpf_jit_x86.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/librte_bpf/bpf_jit_x86.c b/lib/librte_bpf/bpf_jit_x86.c
index 68ea389f2..f70cd6be5 100644
--- a/lib/librte_bpf/bpf_jit_x86.c
+++ b/lib/librte_bpf/bpf_jit_x86.c
@@ -208,6 +208,19 @@ emit_sib(struct bpf_jit_state *st, uint32_t scale, uint32_t idx, uint32_t base)
 	emit_bytes(st, &v, sizeof(v));
 }
 
+/*
+ * emit OPCODE+REGIDX byte
+ */
+static void
+emit_opcode(struct bpf_jit_state *st, uint8_t ops, uint32_t reg)
+{
+	uint8_t v;
+
+	v = ops | (reg & 7);
+	emit_bytes(st, &v, sizeof(v));
+}
+
+
 /*
  * emit xchg %<sreg>, %<dreg>
  */
@@ -472,19 +485,18 @@ static void
 emit_ld_imm64(struct bpf_jit_state *st, uint32_t dreg, uint32_t imm0,
 	uint32_t imm1)
 {
+	uint32_t op;
+
 	const uint8_t ops = 0xB8;
 
-	if (imm1 == 0) {
-		emit_mov_imm(st, EBPF_ALU64 | EBPF_MOV | BPF_K, dreg, imm0);
-		return;
-	}
+	op = (imm1 == 0) ? BPF_ALU : EBPF_ALU64;
 
-	emit_rex(st, EBPF_ALU64, 0, dreg);
-	emit_bytes(st, &ops, sizeof(ops));
-	emit_modregrm(st, MOD_DIRECT, 0, dreg);
+	emit_rex(st, op, 0, dreg);
+	emit_opcode(st, ops, dreg);
 
 	emit_imm(st, imm0, sizeof(imm0));
-	emit_imm(st, imm1, sizeof(imm1));
+	if (imm1 != 0)
+		emit_imm(st, imm1, sizeof(imm1));
 }
 
 /*
-- 
2.17.1

[PATCH 2/2] test/bpf: add test for immediate load

[Konstantin Ananyev]

New test-case to cover (BPF_LD | BPF_IMM | EBPF_DW) instruction.

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 test/test/test_bpf.c | 108 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/test/test/test_bpf.c b/test/test/test_bpf.c
index fa17c4f74..1d50401aa 100644
--- a/test/test/test_bpf.c
+++ b/test/test/test_bpf.c
@@ -48,6 +48,12 @@ struct dummy_vect8 {
 #define TEST_JCC_3	5678
 #define TEST_JCC_4	TEST_FILL_1
 
+#define TEST_IMM_1	UINT64_MAX
+#define TEST_IMM_2	((uint64_t)INT64_MIN)
+#define TEST_IMM_3	((uint64_t)INT64_MAX + INT32_MAX)
+#define TEST_IMM_4	((uint64_t)UINT32_MAX)
+#define TEST_IMM_5	((uint64_t)UINT32_MAX + 1)
+
 struct bpf_test {
 	const char *name;
 	size_t arg_sz;
@@ -268,6 +274,94 @@ test_load1_check(uint64_t rc, const void *arg)
 	return cmp_res(__func__, v, rc, dft, dft, sizeof(*dft));
 }
 
+/* load immediate test-cases */
+static const struct ebpf_insn test_ldimm1_prog[] = {
+
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_0,
+		.imm = (uint32_t)TEST_IMM_1,
+	},
+	{
+		.imm = TEST_IMM_1 >> 32,
+	},
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_3,
+		.imm = (uint32_t)TEST_IMM_2,
+	},
+	{
+		.imm = TEST_IMM_2 >> 32,
+	},
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_5,
+		.imm = (uint32_t)TEST_IMM_3,
+	},
+	{
+		.imm = TEST_IMM_3 >> 32,
+	},
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_7,
+		.imm = (uint32_t)TEST_IMM_4,
+	},
+	{
+		.imm = TEST_IMM_4 >> 32,
+	},
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_9,
+		.imm = (uint32_t)TEST_IMM_5,
+	},
+	{
+		.imm = TEST_IMM_5 >> 32,
+	},
+	/* return sum */
+	{
+		.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+		.dst_reg = EBPF_REG_0,
+		.src_reg = EBPF_REG_3,
+	},
+	{
+		.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+		.dst_reg = EBPF_REG_0,
+		.src_reg = EBPF_REG_5,
+	},
+	{
+		.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+		.dst_reg = EBPF_REG_0,
+		.src_reg = EBPF_REG_7,
+	},
+	{
+		.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+		.dst_reg = EBPF_REG_0,
+		.src_reg = EBPF_REG_9,
+	},
+	{
+		.code = (BPF_JMP | EBPF_EXIT),
+	},
+};
+
+static int
+test_ldimm1_check(uint64_t rc, const void *arg)
+{
+	uint64_t v1, v2;
+
+	v1 = TEST_IMM_1;
+	v2 = TEST_IMM_2;
+	v1 += v2;
+	v2 = TEST_IMM_3;
+	v1 += v2;
+	v2 = TEST_IMM_4;
+	v1 += v2;
+	v2 = TEST_IMM_5;
+	v1 += v2;
+
+	return cmp_res(__func__, v1, rc, arg, arg, 0);
+}
+
+
 /* alu mul test-cases */
 static const struct ebpf_insn test_mul1_prog[] = {
 
@@ -1726,6 +1820,20 @@ static const struct bpf_test tests[] = {
 		.prepare = test_load1_prepare,
 		.check_result = test_load1_check,
 	},
+	{
+		.name = "test_ldimm1",
+		.arg_sz = sizeof(struct dummy_offset),
+		.prm = {
+			.ins = test_ldimm1_prog,
+			.nb_ins = RTE_DIM(test_ldimm1_prog),
+			.prog_arg = {
+				.type = RTE_BPF_ARG_PTR,
+				.size = sizeof(struct dummy_offset),
+			},
+		},
+		.prepare = test_store1_prepare,
+		.check_result = test_ldimm1_check,
+	},
 	{
 		.name = "test_mul1",
 		.arg_sz = sizeof(struct dummy_vect8),
-- 
2.17.1

Re: [PATCH 0/2] bpf: fix x86 jit issue

[Thomas Monjalon]

08/11/2018 13:36, Konstantin Ananyev:
> *** BLURB HERE ***
> 
> Konstantin Ananyev (2):
>   bpf: fix x86 jit for immediate loads
>   test/bpf: add test for immediate load

Applied, thanks