Patchwork target-arm: Handle VMOV between two core and VFP single regs

login
register
mail settings
Submitter Peter Maydell
Date March 1, 2011, 5:35 p.m.
Message ID <1299000919-31368-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/84961/
State New
Headers show

Comments

Peter Maydell - March 1, 2011, 5:35 p.m.
Fix two bugs in the translation of the instructions VMOV sa,sb,rx,ry and
VMOV rx,ry,sa,sb (which copy between a pair of ARM core registers and a
pair of VFP single precision registers):

 * An incorrect condition meant these instruction patterns were being
   treated as load/store multiple, which resulted in the generation
   of bad code and a runtime segfault
 * The order of the core register pair was reversed so the values would
   go to the wrong registers

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
Aurelien Jarno - March 6, 2011, 7:29 p.m.
On Tue, Mar 01, 2011 at 05:35:19PM +0000, Peter Maydell wrote:
> Fix two bugs in the translation of the instructions VMOV sa,sb,rx,ry and
> VMOV rx,ry,sa,sb (which copy between a pair of ARM core registers and a
> pair of VFP single precision registers):
> 
>  * An incorrect condition meant these instruction patterns were being
>    treated as load/store multiple, which resulted in the generation
>    of bad code and a runtime segfault
>  * The order of the core register pair was reversed so the values would
>    go to the wrong registers
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index dbd958b..0111a61 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -3232,7 +3232,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>          break;
>      case 0xc:
>      case 0xd:
> -        if (dp && (insn & 0x03e00000) == 0x00400000) {
> +        if ((insn & 0x03e00000) == 0x00400000) {
>              /* two-register transfer */
>              rn = (insn >> 16) & 0xf;
>              rd = (insn >> 12) & 0xf;
> @@ -3254,10 +3254,10 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                  } else {
>                      gen_mov_F0_vreg(0, rm);
>                      tmp = gen_vfp_mrs();
> -                    store_reg(s, rn, tmp);
> +                    store_reg(s, rd, tmp);
>                      gen_mov_F0_vreg(0, rm + 1);
>                      tmp = gen_vfp_mrs();
> -                    store_reg(s, rd, tmp);
> +                    store_reg(s, rn, tmp);
>                  }
>              } else {
>                  /* arm->vfp */
> @@ -3269,10 +3269,10 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                      gen_vfp_msr(tmp);
>                      gen_mov_vreg_F0(0, rm * 2 + 1);
>                  } else {
> -                    tmp = load_reg(s, rn);
> +                    tmp = load_reg(s, rd);
>                      gen_vfp_msr(tmp);
>                      gen_mov_vreg_F0(0, rm);
> -                    tmp = load_reg(s, rd);
> +                    tmp = load_reg(s, rn);
>                      gen_vfp_msr(tmp);
>                      gen_mov_vreg_F0(0, rm + 1);
>                  }
> -- 
> 1.7.1
> 
> 
>

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index dbd958b..0111a61 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3232,7 +3232,7 @@  static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
         break;
     case 0xc:
     case 0xd:
-        if (dp && (insn & 0x03e00000) == 0x00400000) {
+        if ((insn & 0x03e00000) == 0x00400000) {
             /* two-register transfer */
             rn = (insn >> 16) & 0xf;
             rd = (insn >> 12) & 0xf;
@@ -3254,10 +3254,10 @@  static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                 } else {
                     gen_mov_F0_vreg(0, rm);
                     tmp = gen_vfp_mrs();
-                    store_reg(s, rn, tmp);
+                    store_reg(s, rd, tmp);
                     gen_mov_F0_vreg(0, rm + 1);
                     tmp = gen_vfp_mrs();
-                    store_reg(s, rd, tmp);
+                    store_reg(s, rn, tmp);
                 }
             } else {
                 /* arm->vfp */
@@ -3269,10 +3269,10 @@  static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     gen_vfp_msr(tmp);
                     gen_mov_vreg_F0(0, rm * 2 + 1);
                 } else {
-                    tmp = load_reg(s, rn);
+                    tmp = load_reg(s, rd);
                     gen_vfp_msr(tmp);
                     gen_mov_vreg_F0(0, rm);
-                    tmp = load_reg(s, rd);
+                    tmp = load_reg(s, rn);
                     gen_vfp_msr(tmp);
                     gen_mov_vreg_F0(0, rm + 1);
                 }