[PULL 0/7] target/rx patch queue

[PULL 0/7] target/rx patch queue

[Richard Henderson]

The following changes since commit 401d46789410e88e9e90d76a11f46e8e9f358d55:

  Merge tag 'pull-target-arm-20220421' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-04-21 08:04:43 -0700)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-rx-20220421

for you to fetch changes up to 724eaecec6d22cf3842f896684bdc5b79492f093:

  target/rx: update PC correctly in wait instruction (2022-04-21 10:09:12 -0700)

----------------------------------------------------------------
Fix usp/isp swapping upon clrpsw/setpsw.
Fix psw.i/pc upon wait.
Align dtb in ram.

----------------------------------------------------------------
Richard Henderson (4):
      target/rx: Put tb_flags into DisasContext
      target/rx: Store PSW.U in tb->flags
      target/rx: Move DISAS_UPDATE check for write to PSW
      target/rx: Swap stack pointers on clrpsw/setpsw instruction

Tomoaki Kawada (2):
      target/rx: set PSW.I when executing wait instruction
      target/rx: update PC correctly in wait instruction

Yoshinori Sato (1):
      hw/rx: rx-gdbsim DTB load address aligned of 16byte.

 target/rx/cpu.h       |  1 +
 hw/rx/rx-gdbsim.c     |  2 +-
 target/rx/op_helper.c |  1 +
 target/rx/translate.c | 69 +++++++++++++++++++++++++++------------------------
 4 files changed, 40 insertions(+), 33 deletions(-)

[PULL 1/7] target/rx: Put tb_flags into DisasContext

[Richard Henderson]

Copy tb->flags into ctx->tb_flags; we'll want to modify
this value throughout the tb in future.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Message-Id: <20220417165130.695085-2-richard.henderson@linaro.org>
---
 target/rx/translate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index c8a8991a63..c9db16553b 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -32,6 +32,7 @@ typedef struct DisasContext {
     DisasContextBase base;
     CPURXState *env;
     uint32_t pc;
+    uint32_t tb_flags;
 } DisasContext;
 
 typedef struct DisasCompare {
@@ -231,7 +232,7 @@ static inline TCGv rx_load_source(DisasContext *ctx, TCGv mem,
 /* Processor mode check */
 static int is_privileged(DisasContext *ctx, int is_exception)
 {
-    if (FIELD_EX32(ctx->base.tb->flags, PSW, PM)) {
+    if (FIELD_EX32(ctx->tb_flags, PSW, PM)) {
         if (is_exception) {
             gen_helper_raise_privilege_violation(cpu_env);
         }
@@ -2292,6 +2293,7 @@ static void rx_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     CPURXState *env = cs->env_ptr;
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     ctx->env = env;
+    ctx->tb_flags = ctx->base.tb->flags;
 }
 
 static void rx_tr_tb_start(DisasContextBase *dcbase, CPUState *cs)
-- 
2.34.1

[PULL 2/7] target/rx: Store PSW.U in tb->flags

[Richard Henderson]

With this, we don't need movcond to determine
which stack pointer is current.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Message-Id: <20220417165130.695085-3-richard.henderson@linaro.org>
---
 target/rx/cpu.h       |  1 +
 target/rx/translate.c | 42 +++++++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/target/rx/cpu.h b/target/rx/cpu.h
index 1c267f83bf..5655dffeff 100644
--- a/target/rx/cpu.h
+++ b/target/rx/cpu.h
@@ -149,6 +149,7 @@ static inline void cpu_get_tb_cpu_state(CPURXState *env, target_ulong *pc,
     *pc = env->pc;
     *cs_base = 0;
     *flags = FIELD_DP32(0, PSW, PM, env->psw_pm);
+    *flags = FIELD_DP32(*flags, PSW, U, env->psw_u);
 }
 
 static inline int cpu_mmu_index(CPURXState *env, bool ifetch)
diff --git a/target/rx/translate.c b/target/rx/translate.c
index c9db16553b..df7a8e5153 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -311,9 +311,8 @@ static void psw_cond(DisasCompare *dc, uint32_t cond)
     }
 }
 
-static void move_from_cr(TCGv ret, int cr, uint32_t pc)
+static void move_from_cr(DisasContext *ctx, TCGv ret, int cr, uint32_t pc)
 {
-    TCGv z = tcg_const_i32(0);
     switch (cr) {
     case 0:     /* PSW */
         gen_helper_pack_psw(ret, cpu_env);
@@ -322,8 +321,11 @@ static void move_from_cr(TCGv ret, int cr, uint32_t pc)
         tcg_gen_movi_i32(ret, pc);
         break;
     case 2:     /* USP */
-        tcg_gen_movcond_i32(TCG_COND_NE, ret,
-                            cpu_psw_u, z, cpu_sp, cpu_usp);
+        if (FIELD_EX32(ctx->tb_flags, PSW, U)) {
+            tcg_gen_mov_i32(ret, cpu_sp);
+        } else {
+            tcg_gen_mov_i32(ret, cpu_usp);
+        }
         break;
     case 3:     /* FPSW */
         tcg_gen_mov_i32(ret, cpu_fpsw);
@@ -335,8 +337,11 @@ static void move_from_cr(TCGv ret, int cr, uint32_t pc)
         tcg_gen_mov_i32(ret, cpu_bpc);
         break;
     case 10:    /* ISP */
-        tcg_gen_movcond_i32(TCG_COND_EQ, ret,
-                            cpu_psw_u, z, cpu_sp, cpu_isp);
+        if (FIELD_EX32(ctx->tb_flags, PSW, U)) {
+            tcg_gen_mov_i32(ret, cpu_isp);
+        } else {
+            tcg_gen_mov_i32(ret, cpu_sp);
+        }
         break;
     case 11:    /* FINTV */
         tcg_gen_mov_i32(ret, cpu_fintv);
@@ -350,28 +355,27 @@ static void move_from_cr(TCGv ret, int cr, uint32_t pc)
         tcg_gen_movi_i32(ret, 0);
         break;
     }
-    tcg_temp_free(z);
 }
 
 static void move_to_cr(DisasContext *ctx, TCGv val, int cr)
 {
-    TCGv z;
     if (cr >= 8 && !is_privileged(ctx, 0)) {
         /* Some control registers can only be written in privileged mode. */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "disallow control register write %s", rx_crname(cr));
         return;
     }
-    z = tcg_const_i32(0);
     switch (cr) {
     case 0:     /* PSW */
         gen_helper_set_psw(cpu_env, val);
         break;
     /* case 1: to PC not supported */
     case 2:     /* USP */
-        tcg_gen_mov_i32(cpu_usp, val);
-        tcg_gen_movcond_i32(TCG_COND_NE, cpu_sp,
-                            cpu_psw_u, z,  cpu_usp, cpu_sp);
+        if (FIELD_EX32(ctx->tb_flags, PSW, U)) {
+            tcg_gen_mov_i32(cpu_sp, val);
+        } else {
+            tcg_gen_mov_i32(cpu_usp, val);
+        }
         break;
     case 3:     /* FPSW */
         gen_helper_set_fpsw(cpu_env, val);
@@ -383,10 +387,11 @@ static void move_to_cr(DisasContext *ctx, TCGv val, int cr)
         tcg_gen_mov_i32(cpu_bpc, val);
         break;
     case 10:    /* ISP */
-        tcg_gen_mov_i32(cpu_isp, val);
-        /* if PSW.U is 0, copy isp to r0 */
-        tcg_gen_movcond_i32(TCG_COND_EQ, cpu_sp,
-                            cpu_psw_u, z,  cpu_isp, cpu_sp);
+        if (FIELD_EX32(ctx->tb_flags, PSW, U)) {
+            tcg_gen_mov_i32(cpu_isp, val);
+        } else {
+            tcg_gen_mov_i32(cpu_sp, val);
+        }
         break;
     case 11:    /* FINTV */
         tcg_gen_mov_i32(cpu_fintv, val);
@@ -399,7 +404,6 @@ static void move_to_cr(DisasContext *ctx, TCGv val, int cr)
                       "Unimplement control register %d", cr);
         break;
     }
-    tcg_temp_free(z);
 }
 
 static void push(TCGv val)
@@ -683,7 +687,7 @@ static bool trans_PUSHC(DisasContext *ctx, arg_PUSHC *a)
 {
     TCGv val;
     val = tcg_temp_new();
-    move_from_cr(val, a->cr, ctx->pc);
+    move_from_cr(ctx, val, a->cr, ctx->pc);
     push(val);
     tcg_temp_free(val);
     return true;
@@ -2221,7 +2225,7 @@ static bool trans_MVTC_r(DisasContext *ctx, arg_MVTC_r *a)
 /* mvfc rs, rd */
 static bool trans_MVFC(DisasContext *ctx, arg_MVFC *a)
 {
-    move_from_cr(cpu_regs[a->rd], a->cr, ctx->pc);
+    move_from_cr(ctx, cpu_regs[a->rd], a->cr, ctx->pc);
     return true;
 }
 
-- 
2.34.1

[PULL 3/7] target/rx: Move DISAS_UPDATE check for write to PSW

[Richard Henderson]

Have one check in move_to_cr instead of one in each
function that calls move_to_cr.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Message-Id: <20220417165130.695085-4-richard.henderson@linaro.org>
---
 target/rx/translate.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index df7a8e5153..bd4d110e8b 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -368,6 +368,10 @@ static void move_to_cr(DisasContext *ctx, TCGv val, int cr)
     switch (cr) {
     case 0:     /* PSW */
         gen_helper_set_psw(cpu_env, val);
+        if (is_privileged(ctx, 0)) {
+            /* PSW.{I,U} may be updated here. exit TB. */
+            ctx->base.is_jmp = DISAS_UPDATE;
+        }
         break;
     /* case 1: to PC not supported */
     case 2:     /* USP */
@@ -631,10 +635,6 @@ static bool trans_POPC(DisasContext *ctx, arg_POPC *a)
     val = tcg_temp_new();
     pop(val);
     move_to_cr(ctx, val, a->cr);
-    if (a->cr == 0 && is_privileged(ctx, 0)) {
-        /* PSW.I may be updated here. exit TB. */
-        ctx->base.is_jmp = DISAS_UPDATE;
-    }
     tcg_temp_free(val);
     return true;
 }
@@ -2205,9 +2205,6 @@ static bool trans_MVTC_i(DisasContext *ctx, arg_MVTC_i *a)
 
     imm = tcg_const_i32(a->imm);
     move_to_cr(ctx, imm, a->cr);
-    if (a->cr == 0 && is_privileged(ctx, 0)) {
-        ctx->base.is_jmp = DISAS_UPDATE;
-    }
     tcg_temp_free(imm);
     return true;
 }
@@ -2216,9 +2213,6 @@ static bool trans_MVTC_i(DisasContext *ctx, arg_MVTC_i *a)
 static bool trans_MVTC_r(DisasContext *ctx, arg_MVTC_r *a)
 {
     move_to_cr(ctx, cpu_regs[a->rs], a->cr);
-    if (a->cr == 0 && is_privileged(ctx, 0)) {
-        ctx->base.is_jmp = DISAS_UPDATE;
-    }
     return true;
 }
 
-- 
2.34.1

[PULL 4/7] target/rx: Swap stack pointers on clrpsw/setpsw instruction

[Richard Henderson]

We properly perform this swap in helper_set_psw for MVTC,
but we missed doing so for the CLRPSW/SETPSW of the U bit.

Reported-by: Tomoaki Kawada <i@yvt.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Message-Id: <20220417165130.695085-5-richard.henderson@linaro.org>
---
 target/rx/translate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index bd4d110e8b..63c062993e 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -2165,7 +2165,12 @@ static inline void clrsetpsw(DisasContext *ctx, int cb, int val)
             ctx->base.is_jmp = DISAS_UPDATE;
             break;
         case PSW_U:
-            tcg_gen_movi_i32(cpu_psw_u, val);
+            if (FIELD_EX32(ctx->tb_flags, PSW, U) != val) {
+                ctx->tb_flags = FIELD_DP32(ctx->tb_flags, PSW, U, val);
+                tcg_gen_movi_i32(cpu_psw_u, val);
+                tcg_gen_mov_i32(val ? cpu_isp : cpu_usp, cpu_sp);
+                tcg_gen_mov_i32(cpu_sp, val ? cpu_usp : cpu_isp);
+            }
             break;
         default:
             qemu_log_mask(LOG_GUEST_ERROR, "Invalid distination %d", cb);
-- 
2.34.1

[PULL 5/7] hw/rx: rx-gdbsim DTB load address aligned of 16byte.

[Richard Henderson]

From: Yoshinori Sato <ysato@users.sourceforge.jp>

Linux kernel required alined address of DTB.
But missing align in dtb load function.
Fixed to load to the correct address.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20220207132758.84403-1-ysato@users.sourceforge.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/rx/rx-gdbsim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c
index 64f897e5b1..be147b4bd9 100644
--- a/hw/rx/rx-gdbsim.c
+++ b/hw/rx/rx-gdbsim.c
@@ -141,7 +141,7 @@ static void rx_gdbsim_init(MachineState *machine)
                 exit(1);
             }
             /* DTB is located at the end of SDRAM space. */
-            dtb_offset = machine->ram_size - dtb_size;
+            dtb_offset = ROUND_DOWN(machine->ram_size - dtb_size, 16);
             rom_add_blob_fixed("dtb", dtb, dtb_size,
                                SDRAM_BASE + dtb_offset);
             /* Set dtb address to R1 */
-- 
2.34.1

[PULL 6/7] target/rx: set PSW.I when executing wait instruction

[Richard Henderson]

From: Tomoaki Kawada <i@yvt.jp>

This patch fixes the implementation of the wait instruction to
implicitly update PSW.I as required by the ISA specification.

Signed-off-by: Tomoaki Kawada <i@yvt.jp>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220417045937.2128699-1-i@yvt.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/op_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c
index 11f952d340..81645adde3 100644
--- a/target/rx/op_helper.c
+++ b/target/rx/op_helper.c
@@ -448,6 +448,7 @@ void QEMU_NORETURN helper_wait(CPURXState *env)
 
     cs->halted = 1;
     env->in_sleep = 1;
+    env->psw_i = 1;
     raise_exception(env, EXCP_HLT, 0);
 }
 
-- 
2.34.1

[PULL 7/7] target/rx: update PC correctly in wait instruction

[Richard Henderson]

From: Tomoaki Kawada <i@yvt.jp>

`cpu_pc` at this point does not necessary point to the current
instruction (i.e., the wait instruction being translated), so it's
incorrect to calculate the new value of `cpu_pc` based on this. It must
be updated with `ctx->base.pc_next`, which contains the correct address
of the next instruction.

This change fixes the wait instruction skipping the subsequent branch
when used in an idle loop like this:

    0:  wait
        bra.b 0b
        brk   // should be unreachable

Signed-off-by: Tomoaki Kawada <i@yvt.jp>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220417060224.2131788-1-i@yvt.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index 63c062993e..62aee66937 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -2285,7 +2285,7 @@ static bool trans_INT(DisasContext *ctx, arg_INT *a)
 static bool trans_WAIT(DisasContext *ctx, arg_WAIT *a)
 {
     if (is_privileged(ctx, 1)) {
-        tcg_gen_addi_i32(cpu_pc, cpu_pc, 2);
+        tcg_gen_movi_i32(cpu_pc, ctx->base.pc_next);
         gen_helper_wait(cpu_env);
     }
     return true;
-- 
2.34.1

Re: [PULL 0/7] target/rx patch queue

[Richard Henderson]

On 4/21/22 10:31, Richard Henderson wrote:
> The following changes since commit 401d46789410e88e9e90d76a11f46e8e9f358d55:
> 
>    Merge tag 'pull-target-arm-20220421' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-04-21 08:04:43 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/rth7680/qemu.git tags/pull-rx-20220421
> 
> for you to fetch changes up to 724eaecec6d22cf3842f896684bdc5b79492f093:
> 
>    target/rx: update PC correctly in wait instruction (2022-04-21 10:09:12 -0700)
> 
> ----------------------------------------------------------------
> Fix usp/isp swapping upon clrpsw/setpsw.
> Fix psw.i/pc upon wait.
> Align dtb in ram.

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate.


r~



> 
> ----------------------------------------------------------------
> Richard Henderson (4):
>        target/rx: Put tb_flags into DisasContext
>        target/rx: Store PSW.U in tb->flags
>        target/rx: Move DISAS_UPDATE check for write to PSW
>        target/rx: Swap stack pointers on clrpsw/setpsw instruction
> 
> Tomoaki Kawada (2):
>        target/rx: set PSW.I when executing wait instruction
>        target/rx: update PC correctly in wait instruction
> 
> Yoshinori Sato (1):
>        hw/rx: rx-gdbsim DTB load address aligned of 16byte.
> 
>   target/rx/cpu.h       |  1 +
>   hw/rx/rx-gdbsim.c     |  2 +-
>   target/rx/op_helper.c |  1 +
>   target/rx/translate.c | 69 +++++++++++++++++++++++++++------------------------
>   4 files changed, 40 insertions(+), 33 deletions(-)