linux-mips
[Top] [All Lists]

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

To: Lluis Batlle i Rossell <viric@viric.name>
Subject: Re: [PATCH] MIPS: Add emulation for fpureg-mem unaligned access
From: Jonas Gorski <jonas.gorski@gmail.com>
Date: Sat, 16 Jun 2012 13:21:27 +0200
Cc: linux-mips@linux-mips.org, loongson-dev@googlegroups.com
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; bh=z9Th8LycqySzyXJcghl9EVbbIIqzWYvI/fGroaL4d4Y=; b=JnrYvWg4WcBqn49zqskNav3wg/adqW9N2dsztGKYgZs67pPAJADpReQDRbvyYc4Efw psdBZPX0g+3zGuzDev5Gk0L0Ag73AgtbzCQFNuehjCkzwrbU1/I9O/1fFU+GJt3nc3vF PESQaOSx/zZrbLA6Dlab0II7P4ILiebuGRJjeI93yrO7GI3H8+y0cQNdG1amC8z8hMJ+ CGKBROdSbfoll8GwE3lSRz0mFIxQ68+6ZNu4pXj8TWxuwqSovSOvOFM7myeFce8c7bei i+o4Elmb5MTw37mASRIEWnv/bALa6uLoPxsLcGlmqq9YW97F1phbbnKs2slUAI/QOtIS xWPg==
In-reply-to: <20120615234641.6938B58FE7C@mail.viric.name>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
References: <20120615234641.6938B58FE7C@mail.viric.name>
Sender: linux-mips-bounce@linux-mips.org
Hi,

some comments ...

On 16 June 2012 00:22, Lluis Batlle i Rossell <viric@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.

> 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 <viric@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

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
        case ldc1_op:
...

>                /*
>                 * 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

Same here.

>                /*
>                 * A 32-bit kernel might be running on a 64-bit processor.  But


Regards,
Jonas
<Prev in Thread] Current Thread [Next in Thread>