Richard Henderson <rth7...@gmail.com> writes:
> The translator loop does not allow the tb_start hook to set
> dc->base.is_jmp; the only hook allowed to do that is translate_insn.
>
> Split the work between init_disas_context where we validate
> the gUSA parameters, and translate_insn where we emit code.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
Tested-by: Alex Bennée <alex.ben...@linaro.org>
> ---
>
> This fixes the threadtest failure on Alex's check-tcg branch.
>
>
> r~
>
> ---
> target/sh4/translate.c | 81 +++++++++++++++++++++++-------------------
> 1 file changed, 44 insertions(+), 37 deletions(-)
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 58bdfeb4fb..5295d9cb0d 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -1895,35 +1895,18 @@ static void decode_opc(DisasContext * ctx)
> any sequence via cpu_exec_step_atomic, we can recognize the "normal"
> sequences and transform them into atomic operations as seen by the host.
> */
> -static int decode_gusa(DisasContext *ctx, CPUSH4State *env, int *pmax_insns)
> +static void decode_gusa(DisasContext *ctx, CPUSH4State *env)
> {
> uint16_t insns[5];
> int ld_adr, ld_dst, ld_mop;
> int op_dst, op_src, op_opc;
> int mv_src, mt_dst, st_src, st_mop;
> TCGv op_arg;
> -
> uint32_t pc = ctx->base.pc_next;
> uint32_t pc_end = ctx->base.tb->cs_base;
> - int backup = sextract32(ctx->tbflags, GUSA_SHIFT, 8);
> int max_insns = (pc_end - pc) / 2;
> int i;
>
> - if (pc != pc_end + backup || max_insns < 2) {
> - /* This is a malformed gUSA region. Don't do anything special,
> - since the interpreter is likely to get confused. */
> - ctx->envflags &= ~GUSA_MASK;
> - return 0;
> - }
> -
> - if (ctx->tbflags & GUSA_EXCLUSIVE) {
> - /* Regardless of single-stepping or the end of the page,
> - we must complete execution of the gUSA region while
> - holding the exclusive lock. */
> - *pmax_insns = max_insns;
> - return 0;
> - }
> -
> /* The state machine below will consume only a few insns.
> If there are more than that in a region, fail now. */
> if (max_insns > ARRAY_SIZE(insns)) {
> @@ -2140,7 +2123,6 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State
> *env, int *pmax_insns)
> /*
> * Emit the operation.
> */
> - tcg_gen_insn_start(pc, ctx->envflags);
> switch (op_opc) {
> case -1:
> /* No operation found. Look for exchange pattern. */
> @@ -2235,7 +2217,8 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State
> *env, int *pmax_insns)
> /* The entire region has been translated. */
> ctx->envflags &= ~GUSA_MASK;
> ctx->base.pc_next = pc_end;
> - return max_insns;
> + ctx->base.num_insns += max_insns - 1;
> + return;
>
> fail:
> qemu_log_mask(LOG_UNIMP, "Unrecognized gUSA sequence %08x-%08x\n",
> @@ -2243,7 +2226,6 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State
> *env, int *pmax_insns)
>
> /* Restart with the EXCLUSIVE bit set, within a TB run via
> cpu_exec_step_atomic holding the exclusive lock. */
> - tcg_gen_insn_start(pc, ctx->envflags);
> ctx->envflags |= GUSA_EXCLUSIVE;
> gen_save_cpu_state(ctx, false);
> gen_helper_exclusive(cpu_env);
> @@ -2254,7 +2236,7 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State
> *env, int *pmax_insns)
> entire region consumed via ctx->base.pc_next so that it's immediately
> available in the disassembly dump. */
> ctx->base.pc_next = pc_end;
> - return 1;
> + ctx->base.num_insns += max_insns - 1;
> }
> #endif
>
> @@ -2262,19 +2244,39 @@ static void
> sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
> CPUSH4State *env = cs->env_ptr;
> + uint32_t tbflags;
> int bound;
>
> - ctx->tbflags = (uint32_t)ctx->base.tb->flags;
> - ctx->envflags = ctx->base.tb->flags & TB_FLAG_ENVFLAGS_MASK;
> - ctx->memidx = (ctx->tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
> + ctx->tbflags = tbflags = ctx->base.tb->flags;
> + ctx->envflags = tbflags & TB_FLAG_ENVFLAGS_MASK;
> + ctx->memidx = (tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
> /* We don't know if the delayed pc came from a dynamic or static branch,
> so assume it is a dynamic branch. */
> ctx->delayed_pc = -1; /* use delayed pc from env pointer */
> ctx->features = env->features;
> - ctx->has_movcal = (ctx->tbflags & TB_FLAG_PENDING_MOVCA);
> - ctx->gbank = ((ctx->tbflags & (1 << SR_MD)) &&
> - (ctx->tbflags & (1 << SR_RB))) * 0x10;
> - ctx->fbank = ctx->tbflags & FPSCR_FR ? 0x10 : 0;
> + ctx->has_movcal = (tbflags & TB_FLAG_PENDING_MOVCA);
> + ctx->gbank = ((tbflags & (1 << SR_MD)) &&
> + (tbflags & (1 << SR_RB))) * 0x10;
> + ctx->fbank = tbflags & FPSCR_FR ? 0x10 : 0;
> +
> + if (tbflags & GUSA_MASK) {
> + uint32_t pc = ctx->base.pc_next;
> + uint32_t pc_end = ctx->base.tb->cs_base;
> + int backup = sextract32(ctx->tbflags, GUSA_SHIFT, 8);
> + int max_insns = (pc_end - pc) / 2;
> +
> + if (pc != pc_end + backup || max_insns < 2) {
> + /* This is a malformed gUSA region. Don't do anything special,
> + since the interpreter is likely to get confused. */
> + ctx->envflags &= ~GUSA_MASK;
> + } else if (tbflags & GUSA_EXCLUSIVE) {
> + /* Regardless of single-stepping or the end of the page,
> + we must complete execution of the gUSA region while
> + holding the exclusive lock. */
> + ctx->base.max_insns = max_insns;
> + return;
> + }
> + }
>
> /* Since the ISA is fixed-width, we can bound by the number
> of instructions remaining on the page. */
> @@ -2284,14 +2286,6 @@ static void sh4_tr_init_disas_context(DisasContextBase
> *dcbase, CPUState *cs)
>
> static void sh4_tr_tb_start(DisasContextBase *dcbase, CPUState *cs)
> {
> -#ifdef CONFIG_USER_ONLY
> - DisasContext *ctx = container_of(dcbase, DisasContext, base);
> - CPUSH4State *env = cs->env_ptr;
> -
> - if (ctx->tbflags & GUSA_MASK) {
> - ctx->base.num_insns = decode_gusa(ctx, env, &ctx->base.max_insns);
> - }
> -#endif
> }
>
> static void sh4_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
> @@ -2323,6 +2317,19 @@ static void sh4_tr_translate_insn(DisasContextBase
> *dcbase, CPUState *cs)
> CPUSH4State *env = cs->env_ptr;
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> +#ifdef CONFIG_USER_ONLY
> + if (unlikely(ctx->envflags & GUSA_MASK)
> + && !(ctx->envflags & GUSA_EXCLUSIVE)) {
> + /* We're in an gUSA region, and we have not already fallen
> + back on using an exclusive region. Attempt to parse the
> + region into a single supported atomic operation. Failure
> + is handled within the parser by raising an exception to
> + retry using an exclusive region. */
> + decode_gusa(ctx, env);
> + return;
> + }
> +#endif
> +
> ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next);
> decode_opc(ctx);
> ctx->base.pc_next += 2;
--
Alex Bennée
