From 4341631e4d94e63c85de7215a6228fbf62293ec4 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 09:51:27 -0700 Subject: [PATCH 1/7] target/rx: Put tb_flags into DisasContext Copy tb->flags into ctx->tb_flags; we'll want to modify this value throughout the tb in future. Signed-off-by: Richard Henderson Reviewed-by: Yoshinori Sato 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) From 3626a3fe37e993a86dc7cc4a0d3fb0d6a92c667d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 09:51:28 -0700 Subject: [PATCH 2/7] target/rx: Store PSW.U in tb->flags With this, we don't need movcond to determine which stack pointer is current. Signed-off-by: Richard Henderson Reviewed-by: Yoshinori Sato 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; } From d3562fe2588d3e0a15f3785d86792800d717984c Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 09:51:29 -0700 Subject: [PATCH 3/7] target/rx: Move DISAS_UPDATE check for write to PSW Have one check in move_to_cr instead of one in each function that calls move_to_cr. Signed-off-by: Richard Henderson Reviewed-by: Yoshinori Sato 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; } From 3c69336a8773ec9dde145d40f3e715b9395e0aa0 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 09:51:30 -0700 Subject: [PATCH 4/7] target/rx: Swap stack pointers on clrpsw/setpsw instruction 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 Signed-off-by: Richard Henderson Reviewed-by: Yoshinori Sato 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); From bcc6f33b671d223a1d7b81491d45c58b35ed6e3e Mon Sep 17 00:00:00 2001 From: Yoshinori Sato Date: Mon, 7 Feb 2022 22:27:58 +0900 Subject: [PATCH 5/7] hw/rx: rx-gdbsim DTB load address aligned of 16byte. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220207132758.84403-1-ysato@users.sourceforge.jp> Signed-off-by: Richard Henderson --- 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 */ From 335cd065977bda4e2b6290f9aecad320a9391bfe Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Sun, 17 Apr 2022 13:59:38 +0900 Subject: [PATCH 6/7] target/rx: set PSW.I when executing wait instruction 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 Reviewed-by: Yoshinori Sato Reviewed-by: Richard Henderson Message-Id: <20220417045937.2128699-1-i@yvt.jp> Signed-off-by: Richard Henderson --- 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); } From 724eaecec6d22cf3842f896684bdc5b79492f093 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Sun, 17 Apr 2022 15:02:25 +0900 Subject: [PATCH 7/7] target/rx: update PC correctly in wait instruction `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 Reviewed-by: Yoshinori Sato Reviewed-by: Richard Henderson Message-Id: <20220417060224.2131788-1-i@yvt.jp> Signed-off-by: Richard Henderson --- 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;