qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] ARMv6: Fix SRS/RFE instruction
@ 2008-07-22  0:44 Hans(Hyeonseung) Jang
  0 siblings, 0 replies; 3+ messages in thread
From: Hans(Hyeonseung) Jang @ 2008-07-22  0:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Carl van Schaik, Matthew Warton

We are in the process of implementing a KZM (i.MX31) ARMv6 machine  
port in order to run the OKL4 kernel. We found the new CPS/SRS/RFE  
instructions were broken.
Vincent Palatin released a patch recently which fixes the CPS problem.  
Attached is a patch to fix the SRS/RFE bugs. Could this patch please  
be applied to the main trunk.
Thanks
- Hyeonsung Jang.


- The encoding of 'IA' condition must be '01' instead of '02'.
- SRS instruction must store banked SPSR instead of CPSR at the
specific address.
- 'return' statements are missing


Index: target-arm/translate.c
===================================================================
--- target-arm/translate.c	(revision 4921)
+++ target-arm/translate.c	(working copy)
@@ -5702,7 +5702,7 @@
              }
          } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
              /* srs */
-            uint32_t offset;
+            int32_t offset;
              if (IS_USER(s))
                  goto illegal_op;
              ARCH(6);
@@ -5716,8 +5716,8 @@
              i = (insn >> 23) & 3;
              switch (i) {
              case 0: offset = -4; break; /* DA */
-            case 1: offset = -8; break; /* DB */
-            case 2: offset = 0; break; /* IA */
+            case 1: offset = 0; break; /* IA */
+            case 2: offset = -8; break; /* DB */
              case 3: offset = 4; break; /* IB */
              default: abort();
              }
@@ -5725,32 +5725,33 @@
                  tcg_gen_addi_i32(addr, addr, offset);
              tmp = load_reg(s, 14);
              gen_st32(tmp, addr, 0);
-            tmp = new_tmp();
-            gen_helper_cpsr_read(tmp);
+            tmp = load_cpu_field(spsr);
              tcg_gen_addi_i32(addr, addr, 4);
              gen_st32(tmp, addr, 0);
              if (insn & (1 << 21)) {
                  /* Base writeback.  */
                  switch (i) {
                  case 0: offset = -8; break;
-                case 1: offset = -4; break;
-                case 2: offset = 4; break;
+                case 1: offset = 4; break;
+                case 2: offset = -4; break;
                  case 3: offset = 0; break;
                  default: abort();
                  }
                  if (offset)
-                    tcg_gen_addi_i32(addr, tmp, offset);
+                    tcg_gen_addi_i32(addr, addr, offset);
                  if (op1 == (env->uncached_cpsr & CPSR_M)) {
-                    gen_movl_reg_T1(s, 13);
+                    store_reg(s, 13, addr);
                  } else {
-                    gen_helper_set_r13_banked(cpu_env,  
tcg_const_i32(op1), cpu_T[1]);
+                    gen_helper_set_r13_banked(cpu_env,  
tcg_const_i32(op1), addr);
+                    dead_tmp(addr);
                  }
              } else {
                  dead_tmp(addr);
              }
+            return;
          } else if ((insn & 0x0e5fffe0) == 0x081d0a00) {
              /* rfe */
-            uint32_t offset;
+            int32_t offset;
              if (IS_USER(s))
                  goto illegal_op;
              ARCH(6);
@@ -5759,8 +5760,8 @@
              i = (insn >> 23) & 3;
              switch (i) {
              case 0: offset = -4; break; /* DA */
-            case 1: offset = -8; break; /* DB */
-            case 2: offset = 0; break; /* IA */
+            case 1: offset = 0; break; /* IA */
+            case 2: offset = -8; break; /* DB */
              case 3: offset = 4; break; /* IB */
              default: abort();
              }
@@ -5774,8 +5775,8 @@
                  /* Base writeback.  */
                  switch (i) {
                  case 0: offset = -8; break;
-                case 1: offset = -4; break;
-                case 2: offset = 4; break;
+                case 1: offset = 4; break;
+                case 2: offset = -4; break;
                  case 3: offset = 0; break;
                  default: abort();
                  }
@@ -5786,6 +5787,7 @@
                  dead_tmp(addr);
              }
              gen_rfe(s, tmp, tmp2);
+            return;
          } else if ((insn & 0x0e000000) == 0x0a000000) {
              /* branch link and change to thumb (blx <offset>) */
              int32_t offset;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Qemu-devel] ARMv6: Fix SRS/RFE instruction
@ 2008-07-22  2:20 Hans Jang
  2009-07-18 16:07 ` Filip Navara
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Jang @ 2008-07-22  2:20 UTC (permalink / raw)
  To: qemu-devel

We are in the process of implementing a KZM (i.MX31) ARMv6 machine  
port in order to run the OKL4 kernel. We found the new CPS/SRS/RFE  
instructions were broken.
Vincent Palatin released a patch recently which fixes the CPS problem.  
Attached is a patch to fix the SRS/RFE bugs. Could this patch please  
be applied to the main trunk.
Thanks
- Hyeonsung Jang.


- The encoding of 'IA' condition must be '01' instead of '02'.
- SRS instruction must store banked SPSR instead of CPSR at the
specific address.
- 'return' statements are missing


Index: target-arm/translate.c
===================================================================
--- target-arm/translate.c	(revision 4921)
+++ target-arm/translate.c	(working copy)
@@ -5702,7 +5702,7 @@
             }
         } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
             /* srs */
-            uint32_t offset;
+            int32_t offset;
             if (IS_USER(s))
                 goto illegal_op;
             ARCH(6);
@@ -5716,8 +5716,8 @@
             i = (insn >> 23) & 3;
             switch (i) {
             case 0: offset = -4; break; /* DA */
-            case 1: offset = -8; break; /* DB */
-            case 2: offset = 0; break; /* IA */
+            case 1: offset = 0; break; /* IA */
+            case 2: offset = -8; break; /* DB */
             case 3: offset = 4; break; /* IB */
             default: abort();
             }
@@ -5725,32 +5725,33 @@
                 tcg_gen_addi_i32(addr, addr, offset);
             tmp = load_reg(s, 14);
             gen_st32(tmp, addr, 0);
-            tmp = new_tmp();
-            gen_helper_cpsr_read(tmp);
+            tmp = load_cpu_field(spsr);
             tcg_gen_addi_i32(addr, addr, 4);
             gen_st32(tmp, addr, 0);
             if (insn & (1 << 21)) {
                 /* Base writeback.  */
                 switch (i) {
                 case 0: offset = -8; break;
-                case 1: offset = -4; break;
-                case 2: offset = 4; break;
+                case 1: offset = 4; break;
+                case 2: offset = -4; break;
                 case 3: offset = 0; break;
                 default: abort();
                 }
                 if (offset)
-                    tcg_gen_addi_i32(addr, tmp, offset);
+                    tcg_gen_addi_i32(addr, addr, offset);
                 if (op1 == (env->uncached_cpsr & CPSR_M)) {
-                    gen_movl_reg_T1(s, 13);
+                    store_reg(s, 13, addr);
                 } else {
-                    gen_helper_set_r13_banked(cpu_env,  
tcg_const_i32(op1), cpu_T[1]);
+                    gen_helper_set_r13_banked(cpu_env,  
tcg_const_i32(op1), addr);
+                    dead_tmp(addr);
                 }
             } else {
                 dead_tmp(addr);
             }
+            return;
         } else if ((insn & 0x0e5fffe0) == 0x081d0a00) {
             /* rfe */
-            uint32_t offset;
+            int32_t offset;
             if (IS_USER(s))
                 goto illegal_op;
             ARCH(6);
@@ -5759,8 +5760,8 @@
             i = (insn >> 23) & 3;
             switch (i) {
             case 0: offset = -4; break; /* DA */
-            case 1: offset = -8; break; /* DB */
-            case 2: offset = 0; break; /* IA */
+            case 1: offset = 0; break; /* IA */
+            case 2: offset = -8; break; /* DB */
             case 3: offset = 4; break; /* IB */
             default: abort();
             }
@@ -5774,8 +5775,8 @@
                 /* Base writeback.  */
                 switch (i) {
                 case 0: offset = -8; break;
-                case 1: offset = -4; break;
-                case 2: offset = 4; break;
+                case 1: offset = 4; break;
+                case 2: offset = -4; break;
                 case 3: offset = 0; break;
                 default: abort();
                 }
@@ -5786,6 +5787,7 @@
                 dead_tmp(addr);
             }
             gen_rfe(s, tmp, tmp2);
+            return;
         } else if ((insn & 0x0e000000) == 0x0a000000) {
             /* branch link and change to thumb (blx <offset>) */
             int32_t offset;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] ARMv6: Fix SRS/RFE instruction
  2008-07-22  2:20 [Qemu-devel] ARMv6: Fix SRS/RFE instruction Hans Jang
@ 2009-07-18 16:07 ` Filip Navara
  0 siblings, 0 replies; 3+ messages in thread
From: Filip Navara @ 2009-07-18 16:07 UTC (permalink / raw)
  To: qemu-devel

On Tue, Jul 22, 2008 at 4:20 AM, Hans Jang<hsjang@ok-labs.com> wrote:
> We are in the process of implementing a KZM (i.MX31) ARMv6 machine port in
> order to run the OKL4 kernel. We found the new CPS/SRS/RFE instructions were
> broken.
> Vincent Palatin released a patch recently which fixes the CPS problem.
> Attached is a patch to fix the SRS/RFE bugs. Could this patch please be
> applied to the main trunk.
> Thanks
> - Hyeonsung Jang.
>
>
> - The encoding of 'IA' condition must be '01' instead of '02'.
> - SRS instruction must store banked SPSR instead of CPSR at the
> specific address.
> - 'return' statements are missing
>
>
> Index: target-arm/translate.c
> ===================================================================
> --- target-arm/translate.c      (revision 4921)
> +++ target-arm/translate.c      (working copy)
> @@ -5702,7 +5702,7 @@
>            }
>        } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
>            /* srs */
> -            uint32_t offset;
> +            int32_t offset;
>            if (IS_USER(s))
>                goto illegal_op;
>            ARCH(6);
> @@ -5716,8 +5716,8 @@
>            i = (insn >> 23) & 3;
>            switch (i) {
>            case 0: offset = -4; break; /* DA */
> -            case 1: offset = -8; break; /* DB */
> -            case 2: offset = 0; break; /* IA */
> +            case 1: offset = 0; break; /* IA */
> +            case 2: offset = -8; break; /* DB */
>            case 3: offset = 4; break; /* IB */
>            default: abort();
>            }
> @@ -5725,32 +5725,33 @@
>                tcg_gen_addi_i32(addr, addr, offset);
>            tmp = load_reg(s, 14);
>            gen_st32(tmp, addr, 0);
> -            tmp = new_tmp();
> -            gen_helper_cpsr_read(tmp);
> +            tmp = load_cpu_field(spsr);
>            tcg_gen_addi_i32(addr, addr, 4);
>            gen_st32(tmp, addr, 0);
>            if (insn & (1 << 21)) {
>                /* Base writeback.  */
>                switch (i) {
>                case 0: offset = -8; break;
> -                case 1: offset = -4; break;
> -                case 2: offset = 4; break;
> +                case 1: offset = 4; break;
> +                case 2: offset = -4; break;
>                case 3: offset = 0; break;
>                default: abort();
>                }
>                if (offset)
> -                    tcg_gen_addi_i32(addr, tmp, offset);
> +                    tcg_gen_addi_i32(addr, addr, offset);
>                if (op1 == (env->uncached_cpsr & CPSR_M)) {
> -                    gen_movl_reg_T1(s, 13);
> +                    store_reg(s, 13, addr);
>                } else {
> -                    gen_helper_set_r13_banked(cpu_env, tcg_const_i32(op1),
> cpu_T[1]);
> +                    gen_helper_set_r13_banked(cpu_env, tcg_const_i32(op1),
> addr);
> +                    dead_tmp(addr);
>                }
>            } else {
>                dead_tmp(addr);
>            }
> +            return;
>        } else if ((insn & 0x0e5fffe0) == 0x081d0a00) {
>            /* rfe */
> -            uint32_t offset;
> +            int32_t offset;
>            if (IS_USER(s))
>                goto illegal_op;
>            ARCH(6);
> @@ -5759,8 +5760,8 @@
>            i = (insn >> 23) & 3;
>            switch (i) {
>            case 0: offset = -4; break; /* DA */
> -            case 1: offset = -8; break; /* DB */
> -            case 2: offset = 0; break; /* IA */
> +            case 1: offset = 0; break; /* IA */
> +            case 2: offset = -8; break; /* DB */
>            case 3: offset = 4; break; /* IB */
>            default: abort();
>            }
> @@ -5774,8 +5775,8 @@
>                /* Base writeback.  */
>                switch (i) {
>                case 0: offset = -8; break;
> -                case 1: offset = -4; break;
> -                case 2: offset = 4; break;
> +                case 1: offset = 4; break;
> +                case 2: offset = -4; break;
>                case 3: offset = 0; break;
>                default: abort();
>                }
> @@ -5786,6 +5787,7 @@
>                dead_tmp(addr);
>            }
>            gen_rfe(s, tmp, tmp2);
> +            return;
>        } else if ((insn & 0x0e000000) == 0x0a000000) {
>            /* branch link and change to thumb (blx <offset>) */
>            int32_t offset;
>
>
>
>

Can this patch be applied please? The code as it is now is wrong to
the point of being unusable, it references registers that are not
filled with any values...

Best regards,
Filip Navara

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-07-18 16:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-22  2:20 [Qemu-devel] ARMv6: Fix SRS/RFE instruction Hans Jang
2009-07-18 16:07 ` Filip Navara
  -- strict thread matches above, loose matches on Subject: below --
2008-07-22  0:44 Hans(Hyeonseung) Jang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).