[PATCH 0/3] MIPS: Fix some R2/FP emulation issues

[PATCH 0/3] MIPS: Fix some R2/FP emulation issues

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

Fix handful of MIPS R2/FP emulation problems that are noticed while
developing and testing Android emulator for Mips.

Aleksandar Markovic (1):
  MIPS: r2-on-r6-emu: Clear BLTZALL and BGEZALL debugfs counters

Douglas Leung (1):
  MIPS: math-emu: Fix BC1EQZ and BC1NEZ condition handling

Leonid Yegoshin (1):
  MIPS: r2-on-r6-emu: Fix BLEZL and BGTZL identification

 arch/mips/kernel/mips-r2-to-r6-emul.c | 16 ++++++++++++++--
 arch/mips/math-emu/cp1emu.c           | 10 ++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH 1/3] MIPS: r2-on-r6-emu: Fix BLEZL and BGTZL identification

[Aleksandar Markovic]

From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>

Fix the problem of inaccurate identification of instructions BLEZL and
BGTZL in R2 emulation code by making sure all necessary encoding
specifications are met.

Previously, certain R6 instructions could be identified as BLEZL or
BGTZL. R2 emulation routine didn't take into account that both BLEZL
and BGTZL instructions require their rt field (bits 20 to 16 of
instruction encoding) to be 0, and that, at same time, if the value in
that field is not 0, the encoding may represent a legitimate MIPS R6
instruction.

This means that a problem could occur after emulation optimization,
when emulation routine tried to pipeline emulation, picked up a next
candidate, and subsequently misrecognized an R6 instruction as BLEZL
or BGTZL.

It should be said that for single pass strategy, the problem does not
happen because CPU doesn't trap on branch-compacts which share opcode
space with BLEZL/BGTZL (but have rt field != 0, of course).

Signed-off-by: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtech.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtech.com>
Reported-by: Douglas Leung <douglas.leung@imgtec.com>
Reviewed-by: Paul Burton <paul.burton@imgtec.com>
---
 arch/mips/kernel/mips-r2-to-r6-emul.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c
index ef2ca28..8fb4eac 100644
--- a/arch/mips/kernel/mips-r2-to-r6-emul.c
+++ b/arch/mips/kernel/mips-r2-to-r6-emul.c
@@ -1096,10 +1096,20 @@ int mipsr2_decoder(struct pt_regs *regs, u32 inst, unsigned long *fcr31)
 		}
 		break;
 
-	case beql_op:
-	case bnel_op:
 	case blezl_op:
 	case bgtzl_op:
+		/*
+		 * For BLEZL and BGTZL, rt field must be set to 0. If this
+		 * is not the case, this may be an encoding of a MIPS R6
+		 * instruction, so return to CPU execution if this occurs
+		 */
+		if (MIPSInst_RT(inst)) {
+			err = SIGILL;
+			break;
+		}
+		/* fall through */
+	case beql_op:
+	case bnel_op:
 		if (delay_slot(regs)) {
 			err = SIGILL;
 			break;
-- 
2.7.4

Re: [PATCH 1/3] MIPS: r2-on-r6-emu: Fix BLEZL and BGTZL identification

[Ralf Baechle]

On Mon, Mar 13, 2017 at 04:36:35PM +0100, Aleksandar Markovic wrote:

> From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
> 
> Fix the problem of inaccurate identification of instructions BLEZL and
> BGTZL in R2 emulation code by making sure all necessary encoding
> specifications are met.
> 
> Previously, certain R6 instructions could be identified as BLEZL or
> BGTZL. R2 emulation routine didn't take into account that both BLEZL
> and BGTZL instructions require their rt field (bits 20 to 16 of
> instruction encoding) to be 0, and that, at same time, if the value in
> that field is not 0, the encoding may represent a legitimate MIPS R6
> instruction.
> 
> This means that a problem could occur after emulation optimization,
> when emulation routine tried to pipeline emulation, picked up a next
> candidate, and subsequently misrecognized an R6 instruction as BLEZL
> or BGTZL.
> 
> It should be said that for single pass strategy, the problem does not
> happen because CPU doesn't trap on branch-compacts which share opcode
> space with BLEZL/BGTZL (but have rt field != 0, of course).
> 
> Signed-off-by: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtech.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtech.com>
> Reported-by: Douglas Leung <douglas.leung@imgtec.com>
> Reviewed-by: Paul Burton <paul.burton@imgtec.com>

Thanks for sorting out the review comments on v1.

Applied,

  Ralf

[PATCH 2/3] MIPS: r2-on-r6-emu: Clear BLTZALL and BGEZALL debugfs counters

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

Add missing clearing of BLTZALL and BGEZALL emulation counters in
function mipsr2_stats_clear_show().

Previously, it was not possible to reset BLTZALL and BGEZALL
emulation counters - their value remained the same even after
explicit request via debugfs. As far as other related counters
are concerned, they all seem to be properly cleared.

This change affects debugfs operation only, core R2 emulation
functionality is not affected.

Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
Reviewed-by: Paul Burton <paul.burton@imgtec.com>
---
 arch/mips/kernel/mips-r2-to-r6-emul.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c
index 8fb4eac..9a0fa1e 100644
--- a/arch/mips/kernel/mips-r2-to-r6-emul.c
+++ b/arch/mips/kernel/mips-r2-to-r6-emul.c
@@ -2339,6 +2339,8 @@ static int mipsr2_stats_clear_show(struct seq_file *s, void *unused)
 	__this_cpu_write((mipsr2bremustats).bgezl, 0);
 	__this_cpu_write((mipsr2bremustats).bltzll, 0);
 	__this_cpu_write((mipsr2bremustats).bgezll, 0);
+	__this_cpu_write((mipsr2bremustats).bltzall, 0);
+	__this_cpu_write((mipsr2bremustats).bgezall, 0);
 	__this_cpu_write((mipsr2bremustats).bltzal, 0);
 	__this_cpu_write((mipsr2bremustats).bgezal, 0);
 	__this_cpu_write((mipsr2bremustats).beql, 0);
-- 
2.7.4

Re: [PATCH 2/3] MIPS: r2-on-r6-emu: Clear BLTZALL and BGEZALL debugfs counters

[Ralf Baechle]

Thanks, applied.

  Ralf

[PATCH 3/3] MIPS: math-emu: Fix BC1EQZ and BC1NEZ condition handling

[Aleksandar Markovic]

From: Douglas Leung <douglas.leung@imgtec.com>

Correct the treatment of branching conditions for BC1EQZ and BC1NEZ
instructions in function isBranchInstr().

Previously, corresponding conditions were swapped, which in turn meant
that, for these two instructions, function isBranchInstr() returned
wrong value in its output parameter contpc.

This change is actually an extension of the fix done by the commit
93583e178ebf ("MIPS: math-emu: Fix BC1{EQ,NE}Z emulation"). That commit
dealt with a similar problem in function cop1Emulate(), while this
commit deals with condition handling in function isBranchInstr().
The code styles of changes in these two commits are kept as
consistent as possible.

Signed-off-by: Douglas Leung <douglas.leung@imgtec.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
Reviewed-by: Paul Burton <paul.burton@imgtec.com>
---
 arch/mips/math-emu/cp1emu.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
index a298ac9..f12fde1 100644
--- a/arch/mips/math-emu/cp1emu.c
+++ b/arch/mips/math-emu/cp1emu.c
@@ -439,6 +439,8 @@ int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
 	union mips_instruction insn = (union mips_instruction)dec_insn.insn;
 	unsigned int fcr31;
 	unsigned int bit = 0;
+	unsigned int bit0;
+	union fpureg *fpr;
 
 	switch (insn.i_format.opcode) {
 	case spec_op:
@@ -706,14 +708,14 @@ int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn,
 		    ((insn.i_format.rs == bc1eqz_op) ||
 		     (insn.i_format.rs == bc1nez_op))) {
 			bit = 0;
+			fpr = &current->thread.fpu.fpr[insn.i_format.rt];
+			bit0 = get_fpr32(fpr, 0) & 0x1;
 			switch (insn.i_format.rs) {
 			case bc1eqz_op:
-				if (get_fpr32(&current->thread.fpu.fpr[insn.i_format.rt], 0) & 0x1)
-				    bit = 1;
+				bit = bit0 == 0;
 				break;
 			case bc1nez_op:
-				if (!(get_fpr32(&current->thread.fpu.fpr[insn.i_format.rt], 0) & 0x1))
-				    bit = 1;
+				bit = bit0 != 0;
 				break;
 			}
 			if (bit)
-- 
2.7.4

Re: [PATCH 3/3] MIPS: math-emu: Fix BC1EQZ and BC1NEZ condition handling

[Ralf Baechle]

Applied.  Thanks,

  Ralf