* [PATCH 0/7] MIPS: math-emu: Branch delay slot emulation fixes
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Hi,
This small patch series addresses the following issues with branch delay
slot emulation in our floating-point emulator:
- NOP emulation sometimes causes SIGILL (Aurelien's bug),
- microMIPS emulation always goes astray,
- microMIPS emulation of ADDIUPC always returns the wrong result.
Also included are a bunch of code clean-ups and comment fixes. See
individual patch descriptions for further details.
I attempted to move clean-ups to the end, so that they do not interfere
with backporting, except with 2/7 which, if reordered, would require 3/7
to become ill-formatted. I hope this is OK. Changes 5-7/7 do not require
backporting.
This series has been validated with a MIPS M5150 processor.
Maciej
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] MIPS: math-emu: Correctly handle NOP emulation
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Fix an issue introduced with commit 9ab4471c9f1b ("MIPS: math-emu:
Correct delay-slot exception propagation") where the emulation of a NOP
instruction signals the need to terminate the emulation loop. This in
turn, if the PC has not changed from the entry to the loop, will cause
the kernel to terminate the program with SIGILL.
Consider this program:
static double div(double d)
{
do
d /= 2.0;
while (d > .5);
return d;
}
int main(int argc, char **argv)
{
return div(argc);
}
which gets compiled to the following binary code:
00400490 <main>:
400490: 44840000 mtc1 a0,$f0
400494: 3c020040 lui v0,0x40
400498: d44207f8 ldc1 $f2,2040(v0)
40049c: 46800021 cvt.d.w $f0,$f0
4004a0: 46220002 mul.d $f0,$f0,$f2
4004a4: 4620103c c.lt.d $f2,$f0
4004a8: 4501fffd bc1t 4004a0 <main+0x10>
4004ac: 00000000 nop
4004b0: 4620000d trunc.w.d $f0,$f0
4004b4: 03e00008 jr ra
4004b8: 44020000 mfc1 v0,$f0
4004bc: 00000000 nop
Where the FPU emulator is used, depending on the number of command-line
arguments this code will either run to completion or terminate with
SIGILL.
If no arguments are specified, then BC1T will not be taken, NOP will not
be emulated and code will complete successfully.
If one argument is specified, then BC1T will be taken once and NOP will
be emulated. At this point the entry PC value will be 0x400498 and the
new PC value, set by `mips_dsemul' will be 0x4004a0, the target of BC1T.
The emulation loop will terminate, but SIGILL will not be issued,
because the PC has changed. The FPU emulator will be entered again and
on the second execution BC1T will not be taken, NOP will not be emulated
and code will complete successfully.
If two or more arguments are specified, then the first execution of BC1T
will proceed as above. Upon reentering the FPU emulator the emulation
loop will continue to BC1T, at which point the branch will be taken and
NOP emulated again. At this point however the entry PC value will be
0x4004a0, the same as the target of BC1T. This will make the emulator
conclude that execution has not advanced and therefore an unsupported
FPU instruction has been encountered, and SIGILL will be sent to the
process.
Fix the problem by extending the internal API of `mips_dsemul', making
it return -1 if no delay slot emulation frame has been made, the
instruction has been handled and execution of the emulation loop needs
to continue as if nothing happened. Remove code from `mips_dsemul' to
reproduce steps made by the emulation loop at the conclusion of each
iteration, as those will be reached normally now. Adjust call sites
accordingly. Document the API.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply. This has to be backported to stable branches. NB I have
deliberately left extraneous parentheses in the condition modified intact
in `mips_dsemul', so as not to mix a semantic change with a syntactic one.
They will go away altogether as the condition is refactored with a later
change in this series.
Maciej
linux-mips-dsemul-nop.diff
Index: linux-sfr-test/arch/mips/math-emu/cp1emu.c
===================================================================
--- linux-sfr-test.orig/arch/mips/math-emu/cp1emu.c 2015-09-04 19:19:07.000000000 +0100
+++ linux-sfr-test/arch/mips/math-emu/cp1emu.c 2016-01-20 21:23:44.313505000 +0000
@@ -1266,6 +1266,8 @@ static int cop1Emulate(struct pt_regs *x
*/
sig = mips_dsemul(xcp, ir,
contpc);
+ if (sig < 0)
+ break;
if (sig)
xcp->cp0_epc = bcpc;
/*
@@ -1319,6 +1321,8 @@ static int cop1Emulate(struct pt_regs *x
* instruction in the dslot
*/
sig = mips_dsemul(xcp, ir, contpc);
+ if (sig < 0)
+ break;
if (sig)
xcp->cp0_epc = bcpc;
/* SIGILL forces out of the emulation loop. */
Index: linux-sfr-test/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-test.orig/arch/mips/math-emu/dsemul.c 2015-09-04 19:19:07.000000000 +0100
+++ linux-sfr-test/arch/mips/math-emu/dsemul.c 2016-01-21 00:19:50.241325000 +0000
@@ -31,18 +31,20 @@ struct emuframe {
unsigned long epc;
};
+/*
+ * Set up an emulation frame for instruction IR, from a delay slot of
+ * a branch jumping to CPC. Return 0 if successful, -1 if no emulation
+ * required, otherwise a signal number causing a frame setup failure.
+ */
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
struct emuframe __user *fr;
int err;
+ /* NOP is easy */
if ((get_isa16_mode(regs->cp0_epc) && ((ir >> 16) == MM_NOP16)) ||
- (ir == 0)) {
- /* NOP is easy */
- regs->cp0_epc = cpc;
- clear_delay_slot(regs);
- return 0;
- }
+ (ir == 0))
+ return -1;
pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc);
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/7] MIPS: math-emu: Correctly handle NOP emulation
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Fix an issue introduced with commit 9ab4471c9f1b ("MIPS: math-emu:
Correct delay-slot exception propagation") where the emulation of a NOP
instruction signals the need to terminate the emulation loop. This in
turn, if the PC has not changed from the entry to the loop, will cause
the kernel to terminate the program with SIGILL.
Consider this program:
static double div(double d)
{
do
d /= 2.0;
while (d > .5);
return d;
}
int main(int argc, char **argv)
{
return div(argc);
}
which gets compiled to the following binary code:
00400490 <main>:
400490: 44840000 mtc1 a0,$f0
400494: 3c020040 lui v0,0x40
400498: d44207f8 ldc1 $f2,2040(v0)
40049c: 46800021 cvt.d.w $f0,$f0
4004a0: 46220002 mul.d $f0,$f0,$f2
4004a4: 4620103c c.lt.d $f2,$f0
4004a8: 4501fffd bc1t 4004a0 <main+0x10>
4004ac: 00000000 nop
4004b0: 4620000d trunc.w.d $f0,$f0
4004b4: 03e00008 jr ra
4004b8: 44020000 mfc1 v0,$f0
4004bc: 00000000 nop
Where the FPU emulator is used, depending on the number of command-line
arguments this code will either run to completion or terminate with
SIGILL.
If no arguments are specified, then BC1T will not be taken, NOP will not
be emulated and code will complete successfully.
If one argument is specified, then BC1T will be taken once and NOP will
be emulated. At this point the entry PC value will be 0x400498 and the
new PC value, set by `mips_dsemul' will be 0x4004a0, the target of BC1T.
The emulation loop will terminate, but SIGILL will not be issued,
because the PC has changed. The FPU emulator will be entered again and
on the second execution BC1T will not be taken, NOP will not be emulated
and code will complete successfully.
If two or more arguments are specified, then the first execution of BC1T
will proceed as above. Upon reentering the FPU emulator the emulation
loop will continue to BC1T, at which point the branch will be taken and
NOP emulated again. At this point however the entry PC value will be
0x4004a0, the same as the target of BC1T. This will make the emulator
conclude that execution has not advanced and therefore an unsupported
FPU instruction has been encountered, and SIGILL will be sent to the
process.
Fix the problem by extending the internal API of `mips_dsemul', making
it return -1 if no delay slot emulation frame has been made, the
instruction has been handled and execution of the emulation loop needs
to continue as if nothing happened. Remove code from `mips_dsemul' to
reproduce steps made by the emulation loop at the conclusion of each
iteration, as those will be reached normally now. Adjust call sites
accordingly. Document the API.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply. This has to be backported to stable branches. NB I have
deliberately left extraneous parentheses in the condition modified intact
in `mips_dsemul', so as not to mix a semantic change with a syntactic one.
They will go away altogether as the condition is refactored with a later
change in this series.
Maciej
linux-mips-dsemul-nop.diff
Index: linux-sfr-test/arch/mips/math-emu/cp1emu.c
===================================================================
--- linux-sfr-test.orig/arch/mips/math-emu/cp1emu.c 2015-09-04 19:19:07.000000000 +0100
+++ linux-sfr-test/arch/mips/math-emu/cp1emu.c 2016-01-20 21:23:44.313505000 +0000
@@ -1266,6 +1266,8 @@ static int cop1Emulate(struct pt_regs *x
*/
sig = mips_dsemul(xcp, ir,
contpc);
+ if (sig < 0)
+ break;
if (sig)
xcp->cp0_epc = bcpc;
/*
@@ -1319,6 +1321,8 @@ static int cop1Emulate(struct pt_regs *x
* instruction in the dslot
*/
sig = mips_dsemul(xcp, ir, contpc);
+ if (sig < 0)
+ break;
if (sig)
xcp->cp0_epc = bcpc;
/* SIGILL forces out of the emulation loop. */
Index: linux-sfr-test/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-test.orig/arch/mips/math-emu/dsemul.c 2015-09-04 19:19:07.000000000 +0100
+++ linux-sfr-test/arch/mips/math-emu/dsemul.c 2016-01-21 00:19:50.241325000 +0000
@@ -31,18 +31,20 @@ struct emuframe {
unsigned long epc;
};
+/*
+ * Set up an emulation frame for instruction IR, from a delay slot of
+ * a branch jumping to CPC. Return 0 if successful, -1 if no emulation
+ * required, otherwise a signal number causing a frame setup failure.
+ */
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
struct emuframe __user *fr;
int err;
+ /* NOP is easy */
if ((get_isa16_mode(regs->cp0_epc) && ((ir >> 16) == MM_NOP16)) ||
- (ir == 0)) {
- /* NOP is easy */
- regs->cp0_epc = cpc;
- clear_delay_slot(regs);
- return 0;
- }
+ (ir == 0))
+ return -1;
pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/7] MIPS: math-emu: dsemul: Fix ill formatting of microMIPS part
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Correct formatting breakage introduced with commit 102cedc32a6e ("MIPS:
microMIPS: Floating point support."), so that further changes to this
code can be consistent.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply; as I noted in 0/7 the structure of this code would become
inconsistent with the next change or the change itself would have to break
our formatting rules, to say nothing of legibility. Consequently this
change needs backporting to stable branches so that 3/7 can be backported
as well.
I'm all but happy about the style of this code itself BTW, the chains of
casts of casts are not really needed, but I'll save a further clean-up for
another day.
Maciej
linux-umips-dsemul-format.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:21:57.194943000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:12.640605000 +0000
@@ -75,10 +75,14 @@ int mips_dsemul(struct pt_regs *regs, mi
return SIGBUS;
if (get_isa16_mode(regs->cp0_epc)) {
- err = __put_user(ir >> 16, (u16 __user *)(&fr->emul));
- err |= __put_user(ir & 0xffff, (u16 __user *)((long)(&fr->emul) + 2));
- err |= __put_user(BREAK_MATH >> 16, (u16 __user *)(&fr->badinst));
- err |= __put_user(BREAK_MATH & 0xffff, (u16 __user *)((long)(&fr->badinst) + 2));
+ err = __put_user(ir >> 16,
+ (u16 __user *)(&fr->emul));
+ err |= __put_user(ir & 0xffff,
+ (u16 __user *)((long)(&fr->emul) + 2));
+ err |= __put_user(BREAK_MATH >> 16,
+ (u16 __user *)(&fr->badinst));
+ err |= __put_user(BREAK_MATH & 0xffff,
+ (u16 __user *)((long)(&fr->badinst) + 2));
} else {
err = __put_user(ir, &fr->emul);
err |= __put_user((mips_instruction)BREAK_MATH, &fr->badinst);
@@ -125,8 +129,10 @@ int do_dsemulret(struct pt_regs *xcp)
* - Is the following memory word the BD_COOKIE?
*/
if (get_isa16_mode(xcp->cp0_epc)) {
- err = __get_user(instr[0], (u16 __user *)(&fr->badinst));
- err |= __get_user(instr[1], (u16 __user *)((long)(&fr->badinst) + 2));
+ err = __get_user(instr[0],
+ (u16 __user *)(&fr->badinst));
+ err |= __get_user(instr[1],
+ (u16 __user *)((long)(&fr->badinst) + 2));
insn = (instr[0] << 16) | instr[1];
} else {
err = __get_user(insn, &fr->badinst);
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 2/7] MIPS: math-emu: dsemul: Fix ill formatting of microMIPS part
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Correct formatting breakage introduced with commit 102cedc32a6e ("MIPS:
microMIPS: Floating point support."), so that further changes to this
code can be consistent.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply; as I noted in 0/7 the structure of this code would become
inconsistent with the next change or the change itself would have to break
our formatting rules, to say nothing of legibility. Consequently this
change needs backporting to stable branches so that 3/7 can be backported
as well.
I'm all but happy about the style of this code itself BTW, the chains of
casts of casts are not really needed, but I'll save a further clean-up for
another day.
Maciej
linux-umips-dsemul-format.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:21:57.194943000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:12.640605000 +0000
@@ -75,10 +75,14 @@ int mips_dsemul(struct pt_regs *regs, mi
return SIGBUS;
if (get_isa16_mode(regs->cp0_epc)) {
- err = __put_user(ir >> 16, (u16 __user *)(&fr->emul));
- err |= __put_user(ir & 0xffff, (u16 __user *)((long)(&fr->emul) + 2));
- err |= __put_user(BREAK_MATH >> 16, (u16 __user *)(&fr->badinst));
- err |= __put_user(BREAK_MATH & 0xffff, (u16 __user *)((long)(&fr->badinst) + 2));
+ err = __put_user(ir >> 16,
+ (u16 __user *)(&fr->emul));
+ err |= __put_user(ir & 0xffff,
+ (u16 __user *)((long)(&fr->emul) + 2));
+ err |= __put_user(BREAK_MATH >> 16,
+ (u16 __user *)(&fr->badinst));
+ err |= __put_user(BREAK_MATH & 0xffff,
+ (u16 __user *)((long)(&fr->badinst) + 2));
} else {
err = __put_user(ir, &fr->emul);
err |= __put_user((mips_instruction)BREAK_MATH, &fr->badinst);
@@ -125,8 +129,10 @@ int do_dsemulret(struct pt_regs *xcp)
* - Is the following memory word the BD_COOKIE?
*/
if (get_isa16_mode(xcp->cp0_epc)) {
- err = __get_user(instr[0], (u16 __user *)(&fr->badinst));
- err |= __get_user(instr[1], (u16 __user *)((long)(&fr->badinst) + 2));
+ err = __get_user(instr[0],
+ (u16 __user *)(&fr->badinst));
+ err |= __get_user(instr[1],
+ (u16 __user *)((long)(&fr->badinst) + 2));
insn = (instr[0] << 16) | instr[1];
} else {
err = __get_user(insn, &fr->badinst);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/7] MIPS: math-emu: Make microMIPS branch delay slot emulation work
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Complement commit 102cedc32a6e ("MIPS: microMIPS: Floating point
support.") which introduced microMIPS FPU emulation, but did not adjust
the encoding of the BREAK instruction used to terminate the branch delay
slot emulation frame. Consequently the execution of any such frame is
indeterminate and, depending on CPU configuration, will result in random
code execution or an offending program being terminated with SIGILL.
This is because the regular MIPS BREAK instruction is encoded with the 0
major and the 0xd minor opcode, however in the microMIPS instruction set
this major/minor opcode pair denotes an encoding reserved for the DSP
ASE. Instead the microMIPS BREAK instruction is encoded with the 0
major and the 0x7 minor opcode.
Use the correct BREAK encoding for microMIPS FPU emulation then.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply and backport to stable branches. This is the program I used
to validate this change:
#include <stdio.h>
static int li(double x, double y, int i)
{
asm(
" .set push \n"
" .set noreorder \n"
" c.eq.d %1, %2 \n"
" bc1t 0f \n"
" li %0, %3 \n"
"0: .set pop "
: "=r" (i)
: "f" (x), "f" (y), "i" (i));
return i;
}
int main(int argc, char **argv)
{
printf("ne 16: %d\n", li(1.0, 2.0, 16));
printf("eq 16: %d\n", li(0.0, 0.0, 16));
return 0;
}
With the current state of our tree, where FPU emulation is involved this
program, when compiled to microMIPS code, produces the following output
(on a DSP-enabled processor):
ne 16: 16
Segmentation fault
whereas with the change applied it prints:
ne 16: 16
eq 16: 16
instead, as expected.
Maciej
linux-umips-dsemul-break.diff
Index: linux-sfr-sead/arch/mips/include/asm/fpu_emulator.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/asm/fpu_emulator.h 2016-01-22 01:56:44.281483000 +0000
+++ linux-sfr-sead/arch/mips/include/asm/fpu_emulator.h 2016-01-22 01:57:50.686025000 +0000
@@ -79,7 +79,7 @@ int mm_isBranchInstr(struct pt_regs *reg
/*
* Break instruction with special math emu break code set
*/
-#define BREAK_MATH (0x0000000d | (BRK_MEMU << 16))
+#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16))
#define SIGNALLING_NAN 0x7ff800007ff80000LL
Index: linux-sfr-sead/arch/mips/include/asm/mips-r2-to-r6-emul.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/asm/mips-r2-to-r6-emul.h 2016-01-22 01:56:44.283483000 +0000
+++ linux-sfr-sead/arch/mips/include/asm/mips-r2-to-r6-emul.h 2016-01-22 01:57:50.711027000 +0000
@@ -52,7 +52,7 @@ do { \
__this_cpu_inc(mipsr2emustats.M); \
err = __get_user(nir, (u32 __user *)regs->cp0_epc); \
if (!err) { \
- if (nir == BREAK_MATH) \
+ if (nir == BREAK_MATH(0)) \
__this_cpu_inc(mipsr2bdemustats.M); \
} \
preempt_enable(); \
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:12.640605000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:50.714027000 +0000
@@ -38,6 +38,7 @@ struct emuframe {
*/
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
+ mips_instruction break_math;
struct emuframe __user *fr;
int err;
@@ -65,6 +66,7 @@ int mips_dsemul(struct pt_regs *regs, mi
* branches, but gives us a cleaner interface to the exception
* handler (single entry point).
*/
+ break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
/* Ensure that the two instructions are in the same cache line */
fr = (struct emuframe __user *)
@@ -79,13 +81,13 @@ int mips_dsemul(struct pt_regs *regs, mi
(u16 __user *)(&fr->emul));
err |= __put_user(ir & 0xffff,
(u16 __user *)((long)(&fr->emul) + 2));
- err |= __put_user(BREAK_MATH >> 16,
+ err |= __put_user(break_math >> 16,
(u16 __user *)(&fr->badinst));
- err |= __put_user(BREAK_MATH & 0xffff,
+ err |= __put_user(break_math & 0xffff,
(u16 __user *)((long)(&fr->badinst) + 2));
} else {
err = __put_user(ir, &fr->emul);
- err |= __put_user((mips_instruction)BREAK_MATH, &fr->badinst);
+ err |= __put_user(break_math, &fr->badinst);
}
err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie);
@@ -139,7 +141,8 @@ int do_dsemulret(struct pt_regs *xcp)
}
err |= __get_user(cookie, &fr->cookie);
- if (unlikely(err || (insn != BREAK_MATH) || (cookie != BD_COOKIE))) {
+ if (unlikely(err || insn != BREAK_MATH(get_isa16_mode(xcp->cp0_epc)) ||
+ cookie != BD_COOKIE)) {
MIPS_FPU_EMU_INC_STATS(errors);
return 0;
}
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 3/7] MIPS: math-emu: Make microMIPS branch delay slot emulation work
@ 2016-01-22 5:20 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:20 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Complement commit 102cedc32a6e ("MIPS: microMIPS: Floating point
support.") which introduced microMIPS FPU emulation, but did not adjust
the encoding of the BREAK instruction used to terminate the branch delay
slot emulation frame. Consequently the execution of any such frame is
indeterminate and, depending on CPU configuration, will result in random
code execution or an offending program being terminated with SIGILL.
This is because the regular MIPS BREAK instruction is encoded with the 0
major and the 0xd minor opcode, however in the microMIPS instruction set
this major/minor opcode pair denotes an encoding reserved for the DSP
ASE. Instead the microMIPS BREAK instruction is encoded with the 0
major and the 0x7 minor opcode.
Use the correct BREAK encoding for microMIPS FPU emulation then.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply and backport to stable branches. This is the program I used
to validate this change:
#include <stdio.h>
static int li(double x, double y, int i)
{
asm(
" .set push \n"
" .set noreorder \n"
" c.eq.d %1, %2 \n"
" bc1t 0f \n"
" li %0, %3 \n"
"0: .set pop "
: "=r" (i)
: "f" (x), "f" (y), "i" (i));
return i;
}
int main(int argc, char **argv)
{
printf("ne 16: %d\n", li(1.0, 2.0, 16));
printf("eq 16: %d\n", li(0.0, 0.0, 16));
return 0;
}
With the current state of our tree, where FPU emulation is involved this
program, when compiled to microMIPS code, produces the following output
(on a DSP-enabled processor):
ne 16: 16
Segmentation fault
whereas with the change applied it prints:
ne 16: 16
eq 16: 16
instead, as expected.
Maciej
linux-umips-dsemul-break.diff
Index: linux-sfr-sead/arch/mips/include/asm/fpu_emulator.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/asm/fpu_emulator.h 2016-01-22 01:56:44.281483000 +0000
+++ linux-sfr-sead/arch/mips/include/asm/fpu_emulator.h 2016-01-22 01:57:50.686025000 +0000
@@ -79,7 +79,7 @@ int mm_isBranchInstr(struct pt_regs *reg
/*
* Break instruction with special math emu break code set
*/
-#define BREAK_MATH (0x0000000d | (BRK_MEMU << 16))
+#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16))
#define SIGNALLING_NAN 0x7ff800007ff80000LL
Index: linux-sfr-sead/arch/mips/include/asm/mips-r2-to-r6-emul.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/asm/mips-r2-to-r6-emul.h 2016-01-22 01:56:44.283483000 +0000
+++ linux-sfr-sead/arch/mips/include/asm/mips-r2-to-r6-emul.h 2016-01-22 01:57:50.711027000 +0000
@@ -52,7 +52,7 @@ do { \
__this_cpu_inc(mipsr2emustats.M); \
err = __get_user(nir, (u32 __user *)regs->cp0_epc); \
if (!err) { \
- if (nir == BREAK_MATH) \
+ if (nir == BREAK_MATH(0)) \
__this_cpu_inc(mipsr2bdemustats.M); \
} \
preempt_enable(); \
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:12.640605000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:57:50.714027000 +0000
@@ -38,6 +38,7 @@ struct emuframe {
*/
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
+ mips_instruction break_math;
struct emuframe __user *fr;
int err;
@@ -65,6 +66,7 @@ int mips_dsemul(struct pt_regs *regs, mi
* branches, but gives us a cleaner interface to the exception
* handler (single entry point).
*/
+ break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
/* Ensure that the two instructions are in the same cache line */
fr = (struct emuframe __user *)
@@ -79,13 +81,13 @@ int mips_dsemul(struct pt_regs *regs, mi
(u16 __user *)(&fr->emul));
err |= __put_user(ir & 0xffff,
(u16 __user *)((long)(&fr->emul) + 2));
- err |= __put_user(BREAK_MATH >> 16,
+ err |= __put_user(break_math >> 16,
(u16 __user *)(&fr->badinst));
- err |= __put_user(BREAK_MATH & 0xffff,
+ err |= __put_user(break_math & 0xffff,
(u16 __user *)((long)(&fr->badinst) + 2));
} else {
err = __put_user(ir, &fr->emul);
- err |= __put_user((mips_instruction)BREAK_MATH, &fr->badinst);
+ err |= __put_user(break_math, &fr->badinst);
}
err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie);
@@ -139,7 +141,8 @@ int do_dsemulret(struct pt_regs *xcp)
}
err |= __get_user(cookie, &fr->cookie);
- if (unlikely(err || (insn != BREAK_MATH) || (cookie != BD_COOKIE))) {
+ if (unlikely(err || insn != BREAK_MATH(get_isa16_mode(xcp->cp0_epc)) ||
+ cookie != BD_COOKIE)) {
MIPS_FPU_EMU_INC_STATS(errors);
return 0;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] MIPS: math-emu: Correct the emulation of microMIPS ADDIUPC instruction
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Emulate the microMIPS ADDIUPC instruction directly in `mips_dsemul'. If
executed in the emulation frame, this instruction produces an incorrect
result, because the value of the PC there is not the same as where the
instruction originated.
Reshape code so as to handle all microMIPS cases together.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply and backport to stable branches. This is the program I used
to validate this change:
#include <stdio.h>
struct pc {
void *pc0;
void *pc1;
};
static struct pc addiupc(double x, double y, int i, int unaligned)
{
struct pc pc;
asm(
" .set push \n"
" .set noreorder \n"
" c.eq.d %2, %3 \n"
" .align 2 \n"
" .rept %5 \n"
" nop16 \n"
" .endr \n"
" bc1t 1f \n"
"0: .fill 0 \n"
" addiu %0, $pc, %4 \n"
"1: la %1, 0b \n"
" addu %1, %1, %z4 \n"
" .set pop "
: "=&u" (pc.pc0), "=&r" (pc.pc1)
: "f" (x), "f" (y), "i" (i), "i" (unaligned));
return pc;
}
int main(int argc, char **argv)
{
struct pc pc;
pc = addiupc(1.0, 2.0, 0, 0);
printf("al ne 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 0, 0);
printf("al eq 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 0, 1);
printf("un ne 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 0, 1);
printf("un eq 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16, 0);
printf("al ne 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16, 0);
printf("al eq 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16, 1);
printf("un ne 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16, 1);
printf("un eq 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16, 0);
printf("al ne -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16, 0);
printf("al eq -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16, 1);
printf("un ne -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16, 1);
printf("un eq -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16777212, 0);
printf("al ne 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16777212, 0);
printf("al eq 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16777212, 1);
printf("un ne 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16777212, 1);
printf("un eq 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16777216, 0);
printf("al ne -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16777216, 0);
printf("al eq -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16777216, 1);
printf("un ne -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16777216, 1);
printf("un eq -16777216: %p, %p\n", pc.pc0, pc.pc1);
return 0;
}
With the current state of our tree, where FPU emulation is involved this
program, when compiled to microMIPS code, produces the following output
(on a DSP-enabled processor):
al ne 0: 0x40052c, 0x40052c
Illegal instruction
(because of the issue fixed with 3/7) with 3/7 applied it prints:
al ne 0: 0x40052c, 0x40052c
al eq 0: 0x7fe144f8, 0x400550
un ne 0: 0x400574, 0x400576
un eq 0: 0x7fe144f8, 0x40059a
al ne 16: 0x4005cc, 0x4005cc
al eq 16: 0x7fe14508, 0x4005f0
un ne 16: 0x400614, 0x400616
un eq 16: 0x7fe14508, 0x40063e
al ne -16: 0x400644, 0x400644
al eq -16: 0x7fe144e8, 0x400668
un ne -16: 0x40068c, 0x40068e
un eq -16: 0x7fe144e8, 0x4006b6
al ne 16777212: 0x14006e8, 0x14006e8
al eq 16777212: 0x80e144f4, 0x1400714
un ne 16777212: 0x1400740, 0x1400742
un eq 16777212: 0x80e144f4, 0x1400772
al ne -16777216: 0xff4007a4, 0xff4007a4
al eq -16777216: 0x7ee144f8, 0xff4007cc
un ne -16777216: 0xff4007f4, 0xff4007f6
un eq -16777216: 0x7ee144f8, 0xff400822
(notice the addresses offsetted from the emulation frame in the "eq" case,
printed on the left) whereas with the change applied it prints:
al ne 0: 0x40052c, 0x40052c
al eq 0: 0x400550, 0x400550
un ne 0: 0x400574, 0x400576
un eq 0: 0x400598, 0x40059a
al ne 16: 0x4005cc, 0x4005cc
al eq 16: 0x4005f0, 0x4005f0
un ne 16: 0x400614, 0x400616
un eq 16: 0x40063c, 0x40063e
al ne -16: 0x400644, 0x400644
al eq -16: 0x400668, 0x400668
un ne -16: 0x40068c, 0x40068e
un eq -16: 0x4006b4, 0x4006b6
al ne 16777212: 0x14006e8, 0x14006e8
al eq 16777212: 0x1400714, 0x1400714
un ne 16777212: 0x1400740, 0x1400742
un eq 16777212: 0x1400770, 0x1400772
al ne -16777216: 0xff4007a4, 0xff4007a4
al eq -16777216: 0xff4007cc, 0xff4007cc
un ne -16777216: 0xff4007f4, 0xff4007f6
un eq -16777216: 0xff400820, 0xff400822
instead, as expected (notice the 4-byte alignment of the results, absent
from the reference values calculated with the LA/ADDU sequence, printed on
the right).
Further changes will be required to correctly handle PC-relative MIPSr6
instructions. I do not intend to make such changes anytime soon, people
are encouraged to make them themselves if interested in MIPSr6 execution
environments.
If adding further cases to `mips_dsemul' I suggest factoring out code to
handle direct emulation (-1 return code).
Maciej
linux-umips-dsemul-addiupc.diff
Index: linux-sfr-sead/arch/mips/include/uapi/asm/inst.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/uapi/asm/inst.h 2016-01-22 01:07:02.925179000 +0000
+++ linux-sfr-sead/arch/mips/include/uapi/asm/inst.h 2016-01-22 01:10:10.472810000 +0000
@@ -799,6 +799,13 @@ struct mm_x_format { /* Scaled indexed
;)))))
};
+struct mm_a_format { /* ADDIUPC format (microMIPS) */
+ __BITFIELD_FIELD(unsigned int opcode : 6,
+ __BITFIELD_FIELD(unsigned int rs : 3,
+ __BITFIELD_FIELD(signed int simmediate : 23,
+ ;)))
+};
+
/*
* microMIPS instruction formats (16-bit length)
*/
@@ -940,6 +947,7 @@ union mips_instruction {
struct mm_i_format mm_i_format;
struct mm_m_format mm_m_format;
struct mm_x_format mm_x_format;
+ struct mm_a_format mm_a_format;
struct mm_b0_format mm_b0_format;
struct mm_b1_format mm_b1_format;
struct mm16_m_format mm16_m_format ;
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:09:07.914682000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:10.476810000 +0000
@@ -43,10 +43,30 @@ int mips_dsemul(struct pt_regs *regs, mi
int err;
/* NOP is easy */
- if ((get_isa16_mode(regs->cp0_epc) && ((ir >> 16) == MM_NOP16)) ||
- (ir == 0))
+ if (ir == 0)
return -1;
+ /* microMIPS instructions */
+ if (get_isa16_mode(regs->cp0_epc)) {
+ union mips_instruction insn = { .word = ir };
+
+ /* NOP16 aka MOVE16 $0, $0 */
+ if ((ir >> 16) == MM_NOP16)
+ return -1;
+
+ /* ADDIUPC */
+ if (insn.mm_a_format.opcode == mm_addiupc_op) {
+ unsigned int rs;
+ s32 v;
+
+ rs = (((insn.mm_a_format.rs + 0x1e) & 0xf) + 2);
+ v = regs->cp0_epc & ~3;
+ v += insn.mm_a_format.simmediate << 2;
+ regs->regs[rs] = (long)v;
+ return -1;
+ }
+ }
+
pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc);
/*
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 4/7] MIPS: math-emu: Correct the emulation of microMIPS ADDIUPC instruction
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Emulate the microMIPS ADDIUPC instruction directly in `mips_dsemul'. If
executed in the emulation frame, this instruction produces an incorrect
result, because the value of the PC there is not the same as where the
instruction originated.
Reshape code so as to handle all microMIPS cases together.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
Please apply and backport to stable branches. This is the program I used
to validate this change:
#include <stdio.h>
struct pc {
void *pc0;
void *pc1;
};
static struct pc addiupc(double x, double y, int i, int unaligned)
{
struct pc pc;
asm(
" .set push \n"
" .set noreorder \n"
" c.eq.d %2, %3 \n"
" .align 2 \n"
" .rept %5 \n"
" nop16 \n"
" .endr \n"
" bc1t 1f \n"
"0: .fill 0 \n"
" addiu %0, $pc, %4 \n"
"1: la %1, 0b \n"
" addu %1, %1, %z4 \n"
" .set pop "
: "=&u" (pc.pc0), "=&r" (pc.pc1)
: "f" (x), "f" (y), "i" (i), "i" (unaligned));
return pc;
}
int main(int argc, char **argv)
{
struct pc pc;
pc = addiupc(1.0, 2.0, 0, 0);
printf("al ne 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 0, 0);
printf("al eq 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 0, 1);
printf("un ne 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 0, 1);
printf("un eq 0: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16, 0);
printf("al ne 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16, 0);
printf("al eq 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16, 1);
printf("un ne 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16, 1);
printf("un eq 16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16, 0);
printf("al ne -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16, 0);
printf("al eq -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16, 1);
printf("un ne -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16, 1);
printf("un eq -16: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16777212, 0);
printf("al ne 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16777212, 0);
printf("al eq 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, 16777212, 1);
printf("un ne 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, 16777212, 1);
printf("un eq 16777212: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16777216, 0);
printf("al ne -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16777216, 0);
printf("al eq -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(1.0, 2.0, -16777216, 1);
printf("un ne -16777216: %p, %p\n", pc.pc0, pc.pc1);
pc = addiupc(0.0, 0.0, -16777216, 1);
printf("un eq -16777216: %p, %p\n", pc.pc0, pc.pc1);
return 0;
}
With the current state of our tree, where FPU emulation is involved this
program, when compiled to microMIPS code, produces the following output
(on a DSP-enabled processor):
al ne 0: 0x40052c, 0x40052c
Illegal instruction
(because of the issue fixed with 3/7) with 3/7 applied it prints:
al ne 0: 0x40052c, 0x40052c
al eq 0: 0x7fe144f8, 0x400550
un ne 0: 0x400574, 0x400576
un eq 0: 0x7fe144f8, 0x40059a
al ne 16: 0x4005cc, 0x4005cc
al eq 16: 0x7fe14508, 0x4005f0
un ne 16: 0x400614, 0x400616
un eq 16: 0x7fe14508, 0x40063e
al ne -16: 0x400644, 0x400644
al eq -16: 0x7fe144e8, 0x400668
un ne -16: 0x40068c, 0x40068e
un eq -16: 0x7fe144e8, 0x4006b6
al ne 16777212: 0x14006e8, 0x14006e8
al eq 16777212: 0x80e144f4, 0x1400714
un ne 16777212: 0x1400740, 0x1400742
un eq 16777212: 0x80e144f4, 0x1400772
al ne -16777216: 0xff4007a4, 0xff4007a4
al eq -16777216: 0x7ee144f8, 0xff4007cc
un ne -16777216: 0xff4007f4, 0xff4007f6
un eq -16777216: 0x7ee144f8, 0xff400822
(notice the addresses offsetted from the emulation frame in the "eq" case,
printed on the left) whereas with the change applied it prints:
al ne 0: 0x40052c, 0x40052c
al eq 0: 0x400550, 0x400550
un ne 0: 0x400574, 0x400576
un eq 0: 0x400598, 0x40059a
al ne 16: 0x4005cc, 0x4005cc
al eq 16: 0x4005f0, 0x4005f0
un ne 16: 0x400614, 0x400616
un eq 16: 0x40063c, 0x40063e
al ne -16: 0x400644, 0x400644
al eq -16: 0x400668, 0x400668
un ne -16: 0x40068c, 0x40068e
un eq -16: 0x4006b4, 0x4006b6
al ne 16777212: 0x14006e8, 0x14006e8
al eq 16777212: 0x1400714, 0x1400714
un ne 16777212: 0x1400740, 0x1400742
un eq 16777212: 0x1400770, 0x1400772
al ne -16777216: 0xff4007a4, 0xff4007a4
al eq -16777216: 0xff4007cc, 0xff4007cc
un ne -16777216: 0xff4007f4, 0xff4007f6
un eq -16777216: 0xff400820, 0xff400822
instead, as expected (notice the 4-byte alignment of the results, absent
from the reference values calculated with the LA/ADDU sequence, printed on
the right).
Further changes will be required to correctly handle PC-relative MIPSr6
instructions. I do not intend to make such changes anytime soon, people
are encouraged to make them themselves if interested in MIPSr6 execution
environments.
If adding further cases to `mips_dsemul' I suggest factoring out code to
handle direct emulation (-1 return code).
Maciej
linux-umips-dsemul-addiupc.diff
Index: linux-sfr-sead/arch/mips/include/uapi/asm/inst.h
===================================================================
--- linux-sfr-sead.orig/arch/mips/include/uapi/asm/inst.h 2016-01-22 01:07:02.925179000 +0000
+++ linux-sfr-sead/arch/mips/include/uapi/asm/inst.h 2016-01-22 01:10:10.472810000 +0000
@@ -799,6 +799,13 @@ struct mm_x_format { /* Scaled indexed
;)))))
};
+struct mm_a_format { /* ADDIUPC format (microMIPS) */
+ __BITFIELD_FIELD(unsigned int opcode : 6,
+ __BITFIELD_FIELD(unsigned int rs : 3,
+ __BITFIELD_FIELD(signed int simmediate : 23,
+ ;)))
+};
+
/*
* microMIPS instruction formats (16-bit length)
*/
@@ -940,6 +947,7 @@ union mips_instruction {
struct mm_i_format mm_i_format;
struct mm_m_format mm_m_format;
struct mm_x_format mm_x_format;
+ struct mm_a_format mm_a_format;
struct mm_b0_format mm_b0_format;
struct mm_b1_format mm_b1_format;
struct mm16_m_format mm16_m_format ;
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:09:07.914682000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:10.476810000 +0000
@@ -43,10 +43,30 @@ int mips_dsemul(struct pt_regs *regs, mi
int err;
/* NOP is easy */
- if ((get_isa16_mode(regs->cp0_epc) && ((ir >> 16) == MM_NOP16)) ||
- (ir == 0))
+ if (ir == 0)
return -1;
+ /* microMIPS instructions */
+ if (get_isa16_mode(regs->cp0_epc)) {
+ union mips_instruction insn = { .word = ir };
+
+ /* NOP16 aka MOVE16 $0, $0 */
+ if ((ir >> 16) == MM_NOP16)
+ return -1;
+
+ /* ADDIUPC */
+ if (insn.mm_a_format.opcode == mm_addiupc_op) {
+ unsigned int rs;
+ s32 v;
+
+ rs = (((insn.mm_a_format.rs + 0x1e) & 0xf) + 2);
+ v = regs->cp0_epc & ~3;
+ v += insn.mm_a_format.simmediate << 2;
+ regs->regs[rs] = (long)v;
+ return -1;
+ }
+ }
+
pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc);
/*
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] MIPS: math-emu: dsemul: Correct description of the emulation frame
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Remove irrelevant content from the description of the emulation frame in
`mips_dsemul', referring to bare-metal configurations. Update the text,
reflecting the change made with commit ba3049ed4086 ("MIPS: Switch FPU
emulator trap to BREAK instruction."), where we switched from using an
address error exception on an unaligned access to the use of a BREAK 514
instruction causing a breakpoint exception instead.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
linux-mips-dsemul-comment.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:10.476810000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:44.420413000 +0000
@@ -78,13 +78,8 @@ int mips_dsemul(struct pt_regs *regs, mi
* Algorithmics used a system call instruction, and
* borrowed that vector. MIPS/Linux version is a bit
* more heavyweight in the interests of portability and
- * multiprocessor support. For Linux we generate a
- * an unaligned access and force an address error exception.
- *
- * For embedded systems (stand-alone) we prefer to use a
- * non-existing CP1 instruction. This prevents us from emulating
- * branches, but gives us a cleaner interface to the exception
- * handler (single entry point).
+ * multiprocessor support. For Linux we use a BREAK 514
+ * instruction causing a breakpoint exception.
*/
break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 5/7] MIPS: math-emu: dsemul: Correct description of the emulation frame
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Remove irrelevant content from the description of the emulation frame in
`mips_dsemul', referring to bare-metal configurations. Update the text,
reflecting the change made with commit ba3049ed4086 ("MIPS: Switch FPU
emulator trap to BREAK instruction."), where we switched from using an
address error exception on an unaligned access to the use of a BREAK 514
instruction causing a breakpoint exception instead.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
linux-mips-dsemul-comment.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:10.476810000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:10:44.420413000 +0000
@@ -78,13 +78,8 @@ int mips_dsemul(struct pt_regs *regs, mi
* Algorithmics used a system call instruction, and
* borrowed that vector. MIPS/Linux version is a bit
* more heavyweight in the interests of portability and
- * multiprocessor support. For Linux we generate a
- * an unaligned access and force an address error exception.
- *
- * For embedded systems (stand-alone) we prefer to use a
- * non-existing CP1 instruction. This prevents us from emulating
- * branches, but gives us a cleaner interface to the exception
- * handler (single entry point).
+ * multiprocessor support. For Linux we use a BREAK 514
+ * instruction causing a breakpoint exception.
*/
break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/7] MIPS: inst.h: Fix some instruction descriptions
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Fix the description of the microMIPS NOP16 encoding or MM_NOP16, which
is not equivalent to the MIPS16 NOP instruction. This is 0x0c00 and
represents the microMIPS `MOVE16 $0, $0' operation, whereas MIPS16 NOP
is encoded as 0x6500, representing `MOVE $0, $16'.
Also fix a typo in `mm_fp0_format' description.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
I don't think these changes are worth two separate patches. Please
apply.
Maciej
linux-mips-inst-comment-fix.diff
Index: linux-sfr-test/arch/mips/include/uapi/asm/inst.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/uapi/asm/inst.h 2016-01-20 21:52:36.000000000 +0000
+++ linux-sfr-test/arch/mips/include/uapi/asm/inst.h 2016-01-20 21:54:36.191277000 +0000
@@ -529,7 +529,7 @@ enum MIPS6e_i8_func {
};
/*
- * (microMIPS & MIPS16e) NOP instruction.
+ * (microMIPS) NOP instruction.
*/
#define MM_NOP16 0x0c00
@@ -679,7 +679,7 @@ struct fp0_format { /* FPU multiply and
;))))))
};
-struct mm_fp0_format { /* FPU multipy and add format (microMIPS) */
+struct mm_fp0_format { /* FPU multiply and add format (microMIPS) */
__BITFIELD_FIELD(unsigned int opcode : 6,
__BITFIELD_FIELD(unsigned int ft : 5,
__BITFIELD_FIELD(unsigned int fs : 5,
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 6/7] MIPS: inst.h: Fix some instruction descriptions
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Fix the description of the microMIPS NOP16 encoding or MM_NOP16, which
is not equivalent to the MIPS16 NOP instruction. This is 0x0c00 and
represents the microMIPS `MOVE16 $0, $0' operation, whereas MIPS16 NOP
is encoded as 0x6500, representing `MOVE $0, $16'.
Also fix a typo in `mm_fp0_format' description.
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
Ralf,
I don't think these changes are worth two separate patches. Please
apply.
Maciej
linux-mips-inst-comment-fix.diff
Index: linux-sfr-test/arch/mips/include/uapi/asm/inst.h
===================================================================
--- linux-sfr-test.orig/arch/mips/include/uapi/asm/inst.h 2016-01-20 21:52:36.000000000 +0000
+++ linux-sfr-test/arch/mips/include/uapi/asm/inst.h 2016-01-20 21:54:36.191277000 +0000
@@ -529,7 +529,7 @@ enum MIPS6e_i8_func {
};
/*
- * (microMIPS & MIPS16e) NOP instruction.
+ * (microMIPS) NOP instruction.
*/
#define MM_NOP16 0x0c00
@@ -679,7 +679,7 @@ struct fp0_format { /* FPU multiply and
;))))))
};
-struct mm_fp0_format { /* FPU multipy and add format (microMIPS) */
+struct mm_fp0_format { /* FPU multiply and add format (microMIPS) */
__BITFIELD_FIELD(unsigned int opcode : 6,
__BITFIELD_FIELD(unsigned int ft : 5,
__BITFIELD_FIELD(unsigned int fs : 5,
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] MIPS: math-emu: dsemul: Reduce `get_isa16_mode' clutter
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
linux-mips-dsemul-isa16.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:58:16.000000000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:59:52.315227000 +0000
@@ -38,6 +38,7 @@ struct emuframe {
*/
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
+ int isa16 = get_isa16_mode(regs->cp0_epc);
mips_instruction break_math;
struct emuframe __user *fr;
int err;
@@ -47,7 +48,7 @@ int mips_dsemul(struct pt_regs *regs, mi
return -1;
/* microMIPS instructions */
- if (get_isa16_mode(regs->cp0_epc)) {
+ if (isa16) {
union mips_instruction insn = { .word = ir };
/* NOP16 aka MOVE16 $0, $0 */
@@ -81,7 +82,7 @@ int mips_dsemul(struct pt_regs *regs, mi
* multiprocessor support. For Linux we use a BREAK 514
* instruction causing a breakpoint exception.
*/
- break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
+ break_math = BREAK_MATH(isa16);
/* Ensure that the two instructions are in the same cache line */
fr = (struct emuframe __user *)
@@ -91,7 +92,7 @@ int mips_dsemul(struct pt_regs *regs, mi
if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe))))
return SIGBUS;
- if (get_isa16_mode(regs->cp0_epc)) {
+ if (isa16) {
err = __put_user(ir >> 16,
(u16 __user *)(&fr->emul));
err |= __put_user(ir & 0xffff,
@@ -113,8 +114,7 @@ int mips_dsemul(struct pt_regs *regs, mi
return SIGBUS;
}
- regs->cp0_epc = ((unsigned long) &fr->emul) |
- get_isa16_mode(regs->cp0_epc);
+ regs->cp0_epc = (unsigned long)&fr->emul | isa16;
flush_cache_sigtramp((unsigned long)&fr->emul);
@@ -123,6 +123,7 @@ int mips_dsemul(struct pt_regs *regs, mi
int do_dsemulret(struct pt_regs *xcp)
{
+ int isa16 = get_isa16_mode(xcp->cp0_epc);
struct emuframe __user *fr;
unsigned long epc;
u32 insn, cookie;
@@ -145,7 +146,7 @@ int do_dsemulret(struct pt_regs *xcp)
* - Is the instruction pointed to by the EPC an BREAK_MATH?
* - Is the following memory word the BD_COOKIE?
*/
- if (get_isa16_mode(xcp->cp0_epc)) {
+ if (isa16) {
err = __get_user(instr[0],
(u16 __user *)(&fr->badinst));
err |= __get_user(instr[1],
@@ -156,8 +157,8 @@ int do_dsemulret(struct pt_regs *xcp)
}
err |= __get_user(cookie, &fr->cookie);
- if (unlikely(err || insn != BREAK_MATH(get_isa16_mode(xcp->cp0_epc)) ||
- cookie != BD_COOKIE)) {
+ if (unlikely(err ||
+ insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) {
MIPS_FPU_EMU_INC_STATS(errors);
return 0;
}
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 7/7] MIPS: math-emu: dsemul: Reduce `get_isa16_mode' clutter
@ 2016-01-22 5:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-01-22 5:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Aurelien Jarno, linux-mips
Signed-off-by: Maciej W. Rozycki <macro@imgtec.com>
---
linux-mips-dsemul-isa16.diff
Index: linux-sfr-sead/arch/mips/math-emu/dsemul.c
===================================================================
--- linux-sfr-sead.orig/arch/mips/math-emu/dsemul.c 2016-01-22 01:58:16.000000000 +0000
+++ linux-sfr-sead/arch/mips/math-emu/dsemul.c 2016-01-22 01:59:52.315227000 +0000
@@ -38,6 +38,7 @@ struct emuframe {
*/
int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc)
{
+ int isa16 = get_isa16_mode(regs->cp0_epc);
mips_instruction break_math;
struct emuframe __user *fr;
int err;
@@ -47,7 +48,7 @@ int mips_dsemul(struct pt_regs *regs, mi
return -1;
/* microMIPS instructions */
- if (get_isa16_mode(regs->cp0_epc)) {
+ if (isa16) {
union mips_instruction insn = { .word = ir };
/* NOP16 aka MOVE16 $0, $0 */
@@ -81,7 +82,7 @@ int mips_dsemul(struct pt_regs *regs, mi
* multiprocessor support. For Linux we use a BREAK 514
* instruction causing a breakpoint exception.
*/
- break_math = BREAK_MATH(get_isa16_mode(regs->cp0_epc));
+ break_math = BREAK_MATH(isa16);
/* Ensure that the two instructions are in the same cache line */
fr = (struct emuframe __user *)
@@ -91,7 +92,7 @@ int mips_dsemul(struct pt_regs *regs, mi
if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe))))
return SIGBUS;
- if (get_isa16_mode(regs->cp0_epc)) {
+ if (isa16) {
err = __put_user(ir >> 16,
(u16 __user *)(&fr->emul));
err |= __put_user(ir & 0xffff,
@@ -113,8 +114,7 @@ int mips_dsemul(struct pt_regs *regs, mi
return SIGBUS;
}
- regs->cp0_epc = ((unsigned long) &fr->emul) |
- get_isa16_mode(regs->cp0_epc);
+ regs->cp0_epc = (unsigned long)&fr->emul | isa16;
flush_cache_sigtramp((unsigned long)&fr->emul);
@@ -123,6 +123,7 @@ int mips_dsemul(struct pt_regs *regs, mi
int do_dsemulret(struct pt_regs *xcp)
{
+ int isa16 = get_isa16_mode(xcp->cp0_epc);
struct emuframe __user *fr;
unsigned long epc;
u32 insn, cookie;
@@ -145,7 +146,7 @@ int do_dsemulret(struct pt_regs *xcp)
* - Is the instruction pointed to by the EPC an BREAK_MATH?
* - Is the following memory word the BD_COOKIE?
*/
- if (get_isa16_mode(xcp->cp0_epc)) {
+ if (isa16) {
err = __get_user(instr[0],
(u16 __user *)(&fr->badinst));
err |= __get_user(instr[1],
@@ -156,8 +157,8 @@ int do_dsemulret(struct pt_regs *xcp)
}
err |= __get_user(cookie, &fr->cookie);
- if (unlikely(err || insn != BREAK_MATH(get_isa16_mode(xcp->cp0_epc)) ||
- cookie != BD_COOKIE)) {
+ if (unlikely(err ||
+ insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) {
MIPS_FPU_EMU_INC_STATS(errors);
return 0;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] MIPS: math-emu: Branch delay slot emulation fixes
2016-01-22 5:20 ` Maciej W. Rozycki
` (7 preceding siblings ...)
(?)
@ 2016-01-23 12:39 ` Aurelien Jarno
-1 siblings, 0 replies; 17+ messages in thread
From: Aurelien Jarno @ 2016-01-23 12:39 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-mips
Hi,
On 2016-01-22 05:20, Maciej W. Rozycki wrote:
> Hi,
>
> This small patch series addresses the following issues with branch delay
> slot emulation in our floating-point emulator:
>
> - NOP emulation sometimes causes SIGILL (Aurelien's bug),
>
> - microMIPS emulation always goes astray,
>
> - microMIPS emulation of ADDIUPC always returns the wrong result.
>
> Also included are a bunch of code clean-ups and comment fixes. See
> individual patch descriptions for further details.
>
> I attempted to move clean-ups to the end, so that they do not interfere
> with backporting, except with 2/7 which, if reordered, would require 3/7
> to become ill-formatted. I hope this is OK. Changes 5-7/7 do not require
> backporting.
>
> This series has been validated with a MIPS M5150 processor.
Thanks for working on that. I have tested this series, and I confirm
this fixes the issue and works well, though I haven't tested them on
a system with microMIPS support.
So for all the patches:
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
And for patch 1:
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 17+ messages in thread