qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [Patch] ARMv6: Fix SRS/RFE instruction
@ 2008-07-22  7:12 Hans Jang
  2008-08-07 23:34 ` [Qemu-devel] " Hyeonseung Jang
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Jang @ 2008-07-22  7:12 UTC (permalink / raw)
  To: qemu-devel

(Sorry for sending again because of white space problem in the  
previous mail)

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] Re: [Patch] ARMv6: Fix SRS/RFE instruction
  2008-07-22  7:12 [Qemu-devel] [Patch] ARMv6: Fix SRS/RFE instruction Hans Jang
@ 2008-08-07 23:34 ` Hyeonseung Jang
  2008-08-08 18:18   ` Ashish Bijlani
  0 siblings, 1 reply; 3+ messages in thread
From: Hyeonseung Jang @ 2008-08-07 23:34 UTC (permalink / raw)
  To: qemu-devel

When can I expect this patch to be applied to the mainline ?
This patch is quite clear but there is no feedback at all.
The maintainer may be busy but I just want him or her not to forget  
this.
(I have an experience where the submitted patch(http://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00351.html 
) was totally ignored for 6 months and then applied recently after the  
other person submitted the same thing)


On 22/07/2008, at 5:12 PM, Hans(Hyeonseung) Jang wrote:

> (Sorry for sending again because of white space problem in the  
> previous mail)
>
> 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] Re: [Patch] ARMv6: Fix SRS/RFE instruction
  2008-08-07 23:34 ` [Qemu-devel] " Hyeonseung Jang
@ 2008-08-08 18:18   ` Ashish Bijlani
  0 siblings, 0 replies; 3+ messages in thread
From: Ashish Bijlani @ 2008-08-08 18:18 UTC (permalink / raw)
  To: qemu-devel

Hello,

Emulation of SRS/RFE instruction is indeed incorrect. I faced the same
issues running okl4 on omap2420 (arm1136jfs) based. I could fix the
problem by applying the patch submitted by Hyeonsung. Did anybody else
face this problem?

Thanks,
Ashish

On Thu, Aug 7, 2008 at 6:34 PM, Hyeonseung Jang <hsjang@ok-labs.com> wrote:
> When can I expect this patch to be applied to the mainline ?
> This patch is quite clear but there is no feedback at all.
> The maintainer may be busy but I just want him or her not to forget this.
> (I have an experience where the submitted
> patch(http://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00351.html)
> was totally ignored for 6 months and then applied recently after the other
> person submitted the same thing)
>
>
> On 22/07/2008, at 5:12 PM, Hans(Hyeonseung) Jang wrote:
>
>> (Sorry for sending again because of white space problem in the previous
>> mail)
>>
>> 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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-22  7:12 [Qemu-devel] [Patch] ARMv6: Fix SRS/RFE instruction Hans Jang
2008-08-07 23:34 ` [Qemu-devel] " Hyeonseung Jang
2008-08-08 18:18   ` Ashish Bijlani

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).