Skip to contentSkip to conversation

mips: bpf: Fix broken BPF_MOD

PATCH In review Denis Kirjanov 4 participants 6 messages Dec 1, 2014 – Dec 9, 2014

Subsystems (5)
Networking Network drivers MIPS BPF +1 more subsystems
lore
Share & tools
lore

Denis Kirjanov

author
#0 lore raw

PATCH 1/1 mips: bpf: Fix broken BPF_MOD

Remove optimize_div() from BPF_MOD | BPF_K case
since we don't know the dividend and fix the emit_mod() by reading the mod operation result from HI register

Signed-off-by Denis Kirjanov <[email protected]>
---
 arch/mips/net/bpf_jit.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/net/bpf_jit.c +2 -2
index 9b55143..9fd6834 100644
@@ -426,7 +426,7 @@ static inline void emit_mod(unsigned int dst, unsigned int src,
 		u32 *p = &ctx->target[ctx->idx];
 		uasm_i_divu(&p, dst, src);
 		p = &ctx->target[ctx->idx + 1];
-		uasm_i_mflo(&p, dst);
+		uasm_i_mfhi(&p, dst);
 	}
 	ctx->idx += 2; /* 2 insts */
 }

@@ -971,7 +971,7 @@ load_ind:
 			break;
 		case BPF_ALU | BPF_MOD | BPF_K:
 			/* A %= k */
-			if (k == 1 || optimize_div(&k)) {
+			if (k == 1) {
 				ctx->flags |= SEEN_A;
 				emit_jit_reg_move(r_A, r_zero, ctx);
 			} else {
Signature · 1 line
1.7.10.4
Reviewed-by on v1 1/1
#1 lore raw
Quoted 14 lines · Denis Kirjanov

Remove optimize_div() from BPF_MOD | BPF_K case
since we don't know the dividend and fix the emit_mod() by reading the mod operation result from HI register

Signed-off-by Denis Kirjanov <[email protected]>
---
 arch/mips/net/bpf_jit.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/net/bpf_jit.c +0 -0
index 9b55143..9fd6834 100644

arch/mips/net/bpf_jit.c static inline void emit_mod(unsigned int dst, unsigned int src, · line 429

Quoted 5 lines
arch/mips/net/bpf_jit.c +1 -1
@@ -426,7 +426,7 @@ static inline void emit_mod(unsigned int dst, unsigned int src,
 		u32 *p = &ctx->target[ctx->idx];
 		uasm_i_divu(&p, dst, src);
 		p = &ctx->target[ctx->idx + 1];
-		uasm_i_mflo(&p, dst);
+		uasm_i_mfhi(&p, dst);

That looks correct.

arch/mips/net/bpf_jit.c load_ind: · line 977

Quoted 13 lines
arch/mips/net/bpf_jit.c +1 -1
 	}
 	ctx->idx += 2; /* 2 insts */
 }

@@ -971,7 +971,7 @@ load_ind:
 			break;
 		case BPF_ALU | BPF_MOD | BPF_K:
 			/* A %= k */
-			if (k == 1 || optimize_div(&k)) {
+			if (k == 1) {
 				ctx->flags |= SEEN_A;
 				emit_jit_reg_move(r_A, r_zero, ctx);
 			} else {

That looks correct too. Thanks for fixing these.

Can you also CC stable for inclusion in >=3.16?

Reviewed-by Markos Chandras <[email protected]>
Signature · 1 line
markos
author
#2 lore raw
Quoted 40 lines · Markos Chandras
Quoted 14 lines · Denis Kirjanov

Remove optimize_div() from BPF_MOD | BPF_K case
since we don't know the dividend and fix the emit_mod() by reading the mod operation result from HI register

Signed-off-by Denis Kirjanov <[email protected]>
---
 arch/mips/net/bpf_jit.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/net/bpf_jit.c +0 -0
index 9b55143..9fd6834 100644

arch/mips/net/bpf_jit.c static inline void emit_mod(unsigned int dst, unsigned · line 430

Quoted 6 lines
arch/mips/net/bpf_jit.c +1 -1
@@ -426,7 +426,7 @@ static inline void emit_mod(unsigned int dst, unsigned
 int src,
 		u32 *p = &ctx->target[ctx->idx];
 		uasm_i_divu(&p, dst, src);
 		p = &ctx->target[ctx->idx + 1];
-		uasm_i_mflo(&p, dst);
+		uasm_i_mfhi(&p, dst);

That looks correct.

arch/mips/net/bpf_jit.c load_ind: · line 977

Quoted 13 lines
arch/mips/net/bpf_jit.c +0 -0
 	}
 	ctx->idx += 2; /* 2 insts */

}

arch/mips/net/bpf_jit.c +1 -1
@@ -971,7 +971,7 @@ load_ind:
 			break;
 		case BPF_ALU | BPF_MOD | BPF_K:
 			/* A %= k */
-			if (k == 1 || optimize_div(&k)) {
+			if (k == 1) {
 				ctx->flags |= SEEN_A;
 				emit_jit_reg_move(r_A, r_zero, ctx);
 			} else {

That looks correct too. Thanks for fixing these.

Quoted 3 lines

Can you also CC stable for inclusion in >=3.16?

Reviewed-by Markos Chandras <[email protected]>
Cc [email protected]
Quoted 3 lines

--
markos

#3 lore raw
In reply to Denis Kirjanov

Hello.

On 12/1/2014 12:57 PM, Denis Kirjanov wrote:

You should CC the 'linux-mips' ML.

Quoted 3 lines

Remove optimize_div() from BPF_MOD | BPF_K case
since we don't know the dividend and fix the emit_mod() by reading the mod operation result from HI register

Isn't this 2 unrelated fixes? They should be in 2 patches, not a single
one in that case.

Quoted 4 lines
Signed-off-by Denis Kirjanov <[email protected]>
---
  arch/mips/net/bpf_jit.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

WBR, Sergei

author
#4 lore raw
Quoted 6 lines · Sergei Shtylyov

Hello.

On 12/1/2014 12:57 PM, Denis Kirjanov wrote:

You should CC the 'linux-mips' ML.

Quoted 8 lines
Quoted 3 lines

Remove optimize_div() from BPF_MOD | BPF_K case
since we don't know the dividend and fix the emit_mod() by reading the mod operation result from HI register

Isn't this 2 unrelated fixes? They should be in 2 patches, not a single

one in that case.

They do fix the BPD_MOD _single_ case. I don't think that it's a good reason here to split the fin in 2 patches

Quoted 8 lines
Quoted 4 lines
Signed-off-by Denis Kirjanov <[email protected]>
---
  arch/mips/net/bpf_jit.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

WBR, Sergei