On Tue, Apr 10, 2018 at 13:56:25 +1000, Richard Henderson wrote:
> Ok, well, there are existing bugs within the MIPS translation here, and we
> might as well fix them within this patch set.
>
> (1) The description for BS_STOP says we want to stop, but (what will become)
> mips_tr_tb_stop calls goto_tb.
>
> That's not correct, since we use that after e.g. helper_mtc0_hwrena,
> MIPS_HFLAG_HWRENA_ULR is included in tb->flags, and therefore the next TB is
> not fixed but depends on the actual value stored into hwrena.
>
> We should instead use lookup_and_goto_ptr, which does a full lookup of the
> processor state every time through.
I thought I understood what you meant here but the corresponding change
doesn't seem to work; without the change I can boot a guest in ~1m20s;
with it, boot-up gets stuck -- or at the very least, it's so slow that
I'm giving up after 3+ minutes. The change I made:
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20339,7 +20339,7 @@ void gen_intermediate_code(CPUState *cs, struct
TranslationBlock *tb)
} else {
switch (ctx.is_jmp) {
case DISAS_STOP:
- gen_goto_tb(&ctx, 0, ctx.pc);
+ tcg_gen_lookup_and_goto_ptr();
break;
So I'm afraid I don't understand what the change should be, then.
> (2) The BS_EXCP in generate_exception_err should map to DISAS_NORETURN,
> because
> we do not return after raising an exception.
>
> (3) Otherwise, the use of BS_EXCP has nothing to do with an exception; e.g.
>
> > case 0:
> > save_cpu_state(ctx, 1);
> > gen_helper_mtc0_status(cpu_env, arg);
> > /* BS_STOP isn't good enough here, hflags may have changed. */
> > gen_save_pc(ctx->pc + 4);
> > ctx->bstate = BS_EXCP;
> > rn = "Status";
> > break;
>
> where we are in fact relying on (what will become) mips_tr_tb_stop to emit
> exit_tb. It would be better to name these uses DISAS_EXIT, which would match
> e.g. target/arm.
Thanks for this -- delta with (2) and (3) below.
Emilio
---
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 99ecfa1..4183ec2 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1463,6 +1463,7 @@ typedef struct DisasContext {
#define DISAS_STOP DISAS_TARGET_0
#define DISAS_EXCP DISAS_TARGET_1
+#define DISAS_EXIT DISAS_TARGET_2
static const char * const regnames[] = {
"r0", "at", "v0", "v1", "a0", "a1", "a2", "a3",
@@ -1635,7 +1636,7 @@ static inline void generate_exception_err(DisasContext
*ctx, int excp, int err)
gen_helper_raise_exception_err(cpu_env, texcp, terr);
tcg_temp_free_i32(terr);
tcg_temp_free_i32(texcp);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_NORETURN;
}
static inline void generate_exception(DisasContext *ctx, int excp)
@@ -5333,7 +5334,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
after reading count. DISAS_STOP isn't sufficient, we need to
ensure we break completely out of translated code. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Count";
break;
/* 6,7 are implementation dependent */
@@ -6026,7 +6027,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
gen_helper_mtc0_status(cpu_env, arg);
/* DISAS_STOP isn't good enough here, hflags may have changed. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Status";
break;
case 1:
@@ -6063,7 +6064,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
* DISAS_STOP isn't sufficient, we need to ensure we break out of
* translated code to check for pending interrupts. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Cause";
break;
default:
@@ -6219,7 +6220,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
gen_helper_mtc0_debug(cpu_env, arg); /* EJTAG support */
/* DISAS_STOP isn't good enough here, hflags may have changed. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Debug";
break;
case 1:
@@ -6401,7 +6402,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
/* DISAS_STOP isn't sufficient, we need to ensure we break out of
* translated code to check for pending interrupts. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
}
return;
@@ -6685,7 +6686,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
after reading count. DISAS_STOP isn't sufficient, we need to
ensure we break completely out of translated code. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Count";
break;
/* 6,7 are implementation dependent */
@@ -7365,7 +7366,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
gen_helper_mtc0_status(cpu_env, arg);
/* DISAS_STOP isn't good enough here, hflags may have changed. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Status";
break;
case 1:
@@ -7402,7 +7403,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
* DISAS_STOP isn't sufficient, we need to ensure we break out of
* translated code to check for pending interrupts. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Cause";
break;
default:
@@ -7547,7 +7548,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
gen_helper_mtc0_debug(cpu_env, arg); /* EJTAG support */
/* DISAS_STOP isn't good enough here, hflags may have changed. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
rn = "Debug";
break;
case 1:
@@ -7727,7 +7728,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int
reg, int sel)
/* DISAS_STOP isn't sufficient, we need to ensure we break out of
* translated code to check for pending interrupts. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
}
return;
@@ -10763,7 +10764,7 @@ static void gen_rdhwr(DisasContext *ctx, int rt, int
rd, int sel)
after reading count. DISAS_STOP isn't sufficient, we need to ensure
we break completely out of translated code. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
break;
case 3:
gen_helper_rdhwr_ccres(t0, cpu_env);
@@ -13585,7 +13586,7 @@ static void gen_pool32axf (CPUMIPSState *env,
DisasContext *ctx, int rt, int rs)
/* DISAS_STOP isn't sufficient, we need to ensure we break out
of translated code to check for pending interrupts. */
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
tcg_temp_free(t0);
}
break;
@@ -19710,7 +19711,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
/* DISAS_STOP isn't sufficient, we need to ensure we break
out of translated code to check for pending interrupts
*/
gen_save_pc(ctx->pc + 4);
- ctx->is_jmp = DISAS_EXCP;
+ ctx->is_jmp = DISAS_EXIT;
break;
default: /* Invalid */
MIPS_INVAL("mfmc0");
@@ -20346,6 +20347,7 @@ void gen_intermediate_code(CPUState *cs, struct
TranslationBlock *tb)
gen_goto_tb(&ctx, 0, ctx.pc);
break;
case DISAS_EXCP:
+ case DISAS_EXIT:
tcg_gen_exit_tb(0);
break;
case DISAS_NORETURN:
