diff mbox

[v5] target-tilegx: Support iret instruction and related special registers

Message ID COL130-W94A2EEB4179B2EC3C42AAB9370@phx.gbl
State New
Headers show

Commit Message

Chen Gang Oct. 6, 2015, 2:55 p.m. UTC 
From fa0950e403bbb98989117f632215ae0e698457d7 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5i5j@gmail.com>
Date: Sun, 4 Oct 2015 17:41:14 +0800
Subject: [PATCH v5] target-tilegx: Support iret instruction and related special registers

Acording to the __longjmp tilegx libc implementation, and reference from
tilegx ISA document, and suggested by tilegx architecture member, we can
treat iret instruction as "jrp lr". The related code is below:

  ENTRY (__longjmp)
         FEEDBACK_ENTER(__longjmp)

  #define RESTORE(r) { LD r, r0 ; ADDI_PTR r0, r0, REGSIZE }
         FOR_EACH_CALLEE_SAVED_REG(RESTORE)

         {
          LD r2, r0       ; retrieve ICS bit from jmp_buf
          movei r3, 1
          CMPEQI r0, r1, 0
         }

         {
          mtspr INTERRUPT_CRITICAL_SECTION, r3
          shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
         }

         {
          mtspr EX_CONTEXT_0_0, lr
          ori r2, r2, RETURN_PL
         }

         {
          or r0, r1, r0
          mtspr EX_CONTEXT_0_1, r2
         }

         iret

         jrp lr

EX_CONTEXT_0_0 is used for jumping address, and EX_CONTEXT_0_1 is for
INTERRUPT_CRITICAL_SECTION, which should only be 0 or 1 in user mode, or
it will cause target SEGV (and the patch doesn't implement system mode).

iret will "jrp EX_CONTEXT_0_0", for __longjmp, it is lr (but in other
cases, it may be not).  And the last "jrp lr" in __longjmp is for
historical reasons, and might get removed in the future.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 target-tilegx/cpu.h       |  2 ++
 target-tilegx/helper.c    | 22 ++++++++++++++++++++++
 target-tilegx/helper.h    |  1 +
 target-tilegx/translate.c | 14 +++++++++++++-
 4 files changed, 38 insertions(+), 1 deletion(-)

-- 
1.9.3

Comments

Chris Metcalf Oct. 6, 2015, 4:13 p.m. UTC | #1 
Comments just on the commit message:

On 10/06/2015 10:55 AM, Chen Gang wrote:
>  From fa0950e403bbb98989117f632215ae0e698457d7 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sun, 4 Oct 2015 17:41:14 +0800
> Subject: [PATCH v5] target-tilegx: Support iret instruction and related special registers
>
> Acording to the __longjmp tilegx libc implementation, and reference from
> tilegx ISA document, and suggested by tilegx architecture member, we can
> treat iret instruction as "jrp lr".

You need to update this comment to reflect the actual code in the
commit.  You aren't treating iret as jrp lr anymore.

>   The related code is below:
>
>    ENTRY (__longjmp)
>           FEEDBACK_ENTER(__longjmp)
>
>    #define RESTORE(r) { LD r, r0 ; ADDI_PTR r0, r0, REGSIZE }
>           FOR_EACH_CALLEE_SAVED_REG(RESTORE)
>
>           {
>            LD r2, r0       ; retrieve ICS bit from jmp_buf
>            movei r3, 1
>            CMPEQI r0, r1, 0
>           }
>
>           {
>            mtspr INTERRUPT_CRITICAL_SECTION, r3
>            shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
>           }
>
>           {
>            mtspr EX_CONTEXT_0_0, lr
>            ori r2, r2, RETURN_PL
>           }
>
>           {
>            or r0, r1, r0
>            mtspr EX_CONTEXT_0_1, r2
>           }
>
>           iret
>
>           jrp lr

I don't think this code snippet is helpful to the commit message.

> EX_CONTEXT_0_0 is used for jumping address, and EX_CONTEXT_0_1 is for
> INTERRUPT_CRITICAL_SECTION, which should only be 0 or 1 in user mode, or
> it will cause target SEGV (and the patch doesn't implement system mode).

You correctly modified the change to raise SIGILL, so you should
also update the commit message the same way.
Chen Gang Oct. 6, 2015, 10:53 p.m. UTC | #2 
Oh, sorry. I will send patch v6 for it.

Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed


----------------------------------------
> Subject: Re: [PATCH v5] target-tilegx: Support iret instruction and related special registers
> To: xili_gchen_5257@hotmail.com; rth@twiddle.net; peter.maydell@linaro.org
> CC: qemu-devel@nongnu.org
> From: cmetcalf@ezchip.com
> Date: Tue, 6 Oct 2015 12:13:34 -0400
>
> Comments just on the commit message:
>
> On 10/06/2015 10:55 AM, Chen Gang wrote:
>> From fa0950e403bbb98989117f632215ae0e698457d7 Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>> Date: Sun, 4 Oct 2015 17:41:14 +0800
>> Subject: [PATCH v5] target-tilegx: Support iret instruction and related special registers
>>
>> Acording to the __longjmp tilegx libc implementation, and reference from
>> tilegx ISA document, and suggested by tilegx architecture member, we can
>> treat iret instruction as "jrp lr".
>
> You need to update this comment to reflect the actual code in the
> commit. You aren't treating iret as jrp lr anymore.
>
>> The related code is below:
>>
>> ENTRY (__longjmp)
>> FEEDBACK_ENTER(__longjmp)
>>
>> #define RESTORE(r) { LD r, r0 ; ADDI_PTR r0, r0, REGSIZE }
>> FOR_EACH_CALLEE_SAVED_REG(RESTORE)
>>
>> {
>> LD r2, r0 ; retrieve ICS bit from jmp_buf
>> movei r3, 1
>> CMPEQI r0, r1, 0
>> }
>>
>> {
>> mtspr INTERRUPT_CRITICAL_SECTION, r3
>> shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
>> }
>>
>> {
>> mtspr EX_CONTEXT_0_0, lr
>> ori r2, r2, RETURN_PL
>> }
>>
>> {
>> or r0, r1, r0
>> mtspr EX_CONTEXT_0_1, r2
>> }
>>
>> iret
>>
>> jrp lr
>
> I don't think this code snippet is helpful to the commit message.
>
>> EX_CONTEXT_0_0 is used for jumping address, and EX_CONTEXT_0_1 is for
>> INTERRUPT_CRITICAL_SECTION, which should only be 0 or 1 in user mode, or
>> it will cause target SEGV (and the patch doesn't implement system mode).
>
> You correctly modified the change to raise SIGILL, so you should
> also update the commit message the same way.
>
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
diff mbox

Patch

diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 6f04fe7..6c0fd53 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -53,6 +53,8 @@  enum {
     TILEGX_SPR_CMPEXCH = 0,
     TILEGX_SPR_CRITICAL_SEC = 1,
     TILEGX_SPR_SIM_CONTROL = 2,
+    TILEGX_SPR_EX_CONTEXT_0_0 = 3,
+    TILEGX_SPR_EX_CONTEXT_0_1 = 4,
     TILEGX_SPR_COUNT
 };
 
diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c
index 36b287f..dda821f 100644
--- a/target-tilegx/helper.c
+++ b/target-tilegx/helper.c
@@ -22,6 +22,7 @@ 
 #include "qemu-common.h"
 #include "exec/helper-proto.h"
 #include <zlib.h> /* For crc32 */
+#include "syscall_defs.h"
 
 void helper_exception(CPUTLGState *env, uint32_t excp)
 {
@@ -31,6 +32,27 @@  void helper_exception(CPUTLGState *env, uint32_t excp)
     cpu_loop_exit(cs);
 }
 
+void helper_ext01_ics(CPUTLGState *env)
+{
+    uint64_t val = env->spregs[TILEGX_SPR_EX_CONTEXT_0_1];
+
+    switch (val) {
+    case 0:
+    case 1:
+        env->spregs[TILEGX_SPR_CRITICAL_SEC] = val;
+        break;
+    default:
+#if defined(CONFIG_USER_ONLY)
+        env->signo = TARGET_SIGILL;
+        env->sigcode = TARGET_ILL_ILLOPC;
+        helper_exception(env, TILEGX_EXCP_SIGNAL);
+#else
+        helper_exception(env, TILEGX_EXCP_OPCODE_UNIMPLEMENTED);
+#endif
+        break;
+    }
+}
+
 uint64_t helper_cntlz(uint64_t arg)
 {
     return clz64(arg);
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index bbcc476..9281d0f 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -1,4 +1,5 @@ 
 DEF_HELPER_2(exception, noreturn, env, i32)
+DEF_HELPER_1(ext01_ics, void, env)
 DEF_HELPER_FLAGS_1(cntlz, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(cnttz, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(pcnt, TCG_CALL_NO_RWG_SE, i64, i64)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index ab3fc81..acb9ec4 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -529,6 +529,15 @@  static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext,
         /* ??? This should yield, especially in system mode.  */
         mnemonic = "nap";
         goto done0;
+    case OE_RR_X1(IRET):
+        gen_helper_ext01_ics(cpu_env);
+        dc->jmp.cond = TCG_COND_ALWAYS;
+        dc->jmp.dest = tcg_temp_new();
+        tcg_gen_ld_tl(dc->jmp.dest, cpu_env,
+                      offsetof(CPUTLGState, spregs[TILEGX_SPR_EX_CONTEXT_0_0]));
+        tcg_gen_andi_tl(dc->jmp.dest, dc->jmp.dest, ~7);
+        mnemonic = "iret";
+        goto done0;
     case OE_RR_X1(SWINT0):
     case OE_RR_X1(SWINT2):
     case OE_RR_X1(SWINT3):
@@ -606,7 +615,6 @@  static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext,
         break;
     case OE_RR_X0(FSINGLE_PACK1):
     case OE_RR_Y0(FSINGLE_PACK1):
-    case OE_RR_X1(IRET):
         return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
     case OE_RR_X1(LD1S):
         memop = MO_SB;
@@ -1947,6 +1955,10 @@  static const TileSPR *find_spr(unsigned spr)
       offsetof(CPUTLGState, spregs[TILEGX_SPR_CRITICAL_SEC]), 0, 0)
     D(SIM_CONTROL,
       offsetof(CPUTLGState, spregs[TILEGX_SPR_SIM_CONTROL]), 0, 0)
+    D(EX_CONTEXT_0_0,
+      offsetof(CPUTLGState, spregs[TILEGX_SPR_EX_CONTEXT_0_0]), 0, 0)
+    D(EX_CONTEXT_0_1,
+      offsetof(CPUTLGState, spregs[TILEGX_SPR_EX_CONTEXT_0_1]), 0, 0)
     }
 
 #undef D