[PATCH] MIPS: Add emulation for fpureg-mem unaligned access

[Lluis Batlle i Rossell]

Reusing most of the code from lw,ld,sw,sd emulation,
I add the emulation for lwc1,ldc1,swc1,sdc1.

This avoids the direct SIGBUS sent to userspace processes that have
misaligned memory accesses.

I've tested the change in Loongson2F, with an own test program, and
WebKit 1.4.0, as both were killed by sigbus without this patch.

Signed-off: Lluis Batlle i Rossell <vi...@viric.name>
---
arch/mips/kernel/unaligned.c | 43 +++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
index 9c58bdf..4531e6c 100644
--- a/arch/mips/kernel/unaligned.c
+++ b/arch/mips/kernel/unaligned.c
@@ -85,6 +85,7 @@
#include <asm/cop2.h>
#include <asm/inst.h>
#include <asm/uaccess.h>
+#include <asm/fpu.h>

#define STR(x) __STR(x)
#define __STR(x) #x
@@ -108,6 +109,7 @@ static void emulate_load_store_insn(struct pt_regs *regs,
union mips_instruction insn;
unsigned long value;
unsigned int res;
+ fpureg_t *fpuregs;

perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, 0);

@@ -183,6 +185,7 @@ static void emulate_load_store_insn(struct pt_regs *regs,
break;

case lw_op:
+ case lwc1_op:
if (!access_ok(VERIFY_READ, addr, 4))
goto sigbus;

@@ -209,7 +212,12 @@ static void emulate_load_store_insn(struct pt_regs *regs,
if (res)
goto fault;
compute_return_epc(regs);
- regs->regs[insn.i_format.rt] = value;
+ if (insn.i_format.opcode == lw_op) {
+ regs->regs[insn.i_format.rt] = value;
+ } else {
+ fpuregs = get_fpu_regs(current);
+ fpuregs[insn.i_format.rt] = value;
+ }
break;

case lhu_op:
@@ -291,6 +299,7 @@ static void emulate_load_store_insn(struct pt_regs *regs,
goto sigill;

case ld_op:
+ case ldc1_op:
#ifdef CONFIG_64BIT
/*
* A 32-bit kernel might be running on a 64-bit processor. But
@@ -325,7 +334,12 @@ static void emulate_load_store_insn(struct pt_regs *regs,
if (res)
goto fault;
compute_return_epc(regs);
- regs->regs[insn.i_format.rt] = value;
+ if (insn.i_format.opcode == ld_op) {
+ regs->regs[insn.i_format.rt] = value;
+ } else {
+ fpuregs = get_fpu_regs(current);
+ fpuregs[insn.i_format.rt] = value;
+ }
break;
#endif /* CONFIG_64BIT */

@@ -370,10 +384,16 @@ static void emulate_load_store_insn(struct pt_regs *regs,
break;

case sw_op:
+ case swc1_op:
if (!access_ok(VERIFY_WRITE, addr, 4))
goto sigbus;

- value = regs->regs[insn.i_format.rt];
+ if (insn.i_format.opcode == sw_op) {
+ value = regs->regs[insn.i_format.rt];
+ } else {
+ fpuregs = get_fpu_regs(current);
+ value = fpuregs[insn.i_format.rt];
+ }
__asm__ __volatile__ (
#ifdef __BIG_ENDIAN
"1:\tswl\t%1,(%2)\n"
@@ -401,6 +421,7 @@ static void emulate_load_store_insn(struct pt_regs *regs,
break;

case sd_op:
+ case sdc1_op:
#ifdef CONFIG_64BIT
/*
* A 32-bit kernel might be running on a 64-bit processor. But
@@ -412,7 +433,12 @@ static void emulate_load_store_insn(struct pt_regs *regs,
if (!access_ok(VERIFY_WRITE, addr, 8))
goto sigbus;

- value = regs->regs[insn.i_format.rt];
+ if (insn.i_format.opcode == sd_op) {
+ value = regs->regs[insn.i_format.rt];
+ } else {
+ fpuregs = get_fpu_regs(current);
+ value = fpuregs[insn.i_format.rt];
+ }
__asm__ __volatile__ (
#ifdef __BIG_ENDIAN
"1:\tsdl\t%1,(%2)\n"
@@ -443,15 +469,6 @@ static void emulate_load_store_insn(struct pt_regs *regs,
/* Cannot handle 64-bit instructions in 32-bit kernel */
goto sigill;

- case lwc1_op:
- case ldc1_op:
- case swc1_op:
- case sdc1_op:
- /*
- * I herewith declare: this does not happen. So send SIGBUS.
- */
- goto sigbus;
-
/*
* COP2 is available to implementor for application specific use.
* It's up to applications to register a notifier chain and do
--
1.7.9.5

[Lluís Batlle i Rossell]

Hello,

thank you for the review.

On Sat, Jun 16, 2012 at 01:21:27PM +0200, Jonas Gorski wrote:

> On 16 June 2012 00:22, Lluis Batlle i Rossell <vi...@viric.name> wrote:
> > Reusing most of the code from lw,ld,sw,sd emulation,
> > I add the emulation for lwc1,ldc1,swc1,sdc1.
>

> What about lwxc1, ldxc1, swxc1 and sdxc1? These also require alignment.

Looking at gcc code, I could not find those instructions emmitted. I could write
some assembly tests cases though.

> >        case ld_op:
> > +       case ldc1_op:
> >  #ifdef CONFIG_64BIT
>

> From what I can tell, ldc1 is a valid MIPS32 instruction, so this
> should probably be something like
>
>        case ld_op:
> #ifndef CONFIG_64BIT
>                return sigill;
> #endif

I agree! I'll repost with these fixes.

Regards,
Lluís

[Lluís Batlle i Rossell]

Hello again,

On Sat, Jun 16, 2012 at 02:15:13PM +0200, Lluís Batlle i Rossell wrote:
> > From what I can tell, ldc1 is a valid MIPS32 instruction, so this
> > should probably be something like
> >
> >        case ld_op:
> > #ifndef CONFIG_64BIT
> >                return sigill;
> > #endif
>
> I agree! I'll repost with these fixes.

Well, I think I take my words back. Handling the ldc1/sdc1 cases in MIPS32 is
tricker than I thought first, because I can't use ldl/ldr or sdl/sdr there.
Given my ability with mips assembly, I leave the patch as is.

In 'patchwork' I had set the patch first to superseeded, but then I set it back
to New.

Regards,
Lluís.

[Lluís Batlle i Rossell]

On Tue, Jul 31, 2012 at 03:40:01PM +0200, Ralf Baechle wrote:

> On Sat, Jun 16, 2012 at 12:22:53AM +0200, Lluis Batlle i Rossell wrote:
>
> > Reusing most of the code from lw,ld,sw,sd emulation,
> > I add the emulation for lwc1,ldc1,swc1,sdc1.
> >
> > This avoids the direct SIGBUS sent to userspace processes that have
> > misaligned memory accesses.
> >
> > I've tested the change in Loongson2F, with an own test program, and
> > WebKit 1.4.0, as both were killed by sigbus without this patch.
>

> A misaligned FPU access is a strong indication for broken, non-portable
> software. which means you're likely trying to fix the wrong issue. It's
> quite intentional that there is no unaligned handling for the FPU in the
> kernel - and afaics there isn't for any other MIPS UNIX.

Ah, I had no idea it was intentional.

Maybe there could be a cleaner declaration of that intention, though. The only
code there was "I herewith declare: this does not happen. So send SIGBUS."

Thank you,
Lluís.