Re: [PATCH v4 7/9] target/riscv: add support for Zcmt extension

[Richard Henderson <richard.henderson@linaro.org>]

On 11/17/22 23:17, Weiwei Li wrote:
> +target_ulong HELPER(cm_jalt)(CPURISCVState *env, target_ulong index,
> +                             target_ulong next_pc)
> +{
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_JVT);
> +    if (ret != RISCV_EXCP_NONE) {
> +        riscv_raise_exception(env, ret, GETPC());
> +    }
> +#endif
> +
> +    target_ulong target = next_pc;
> +    target_ulong val = 0;
> +    int xlen = riscv_cpu_xlen(env);
> +
> +    val = env->jvt;
> +
> +    uint8_t mode = get_field(val, JVT_MODE);
> +    target_ulong base = get_field(val, JVT_BASE);
> +    target_ulong t0;
> +
> +    if (mode != 0) {
> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +    }
> +
> +    if (xlen == 32) {
> +        t0 = base + (index << 2);
> +        target = cpu_ldl_code(env, t0);
> +    } else {
> +        t0 = base + (index << 3);
> +        target = cpu_ldq_code(env, t0);
> +    }

Much better.  The only problem is here where cpu_ld*_code does not have support for unwind 
from exception.  If this load faults, we won't update env->pc on the way out (we are 
normally loading for code during translation, where pc is perforce up to date).  I should 
have noticed this before.

The way to fix this is to update cpu_pc to the current instruction before calling the 
helper.  At which point none of the other exception exits need to unwind either, so you 
can replace all of the GETPC() with 0.

> +
> +    /* index >= 32 for cm.jalt, otherwise for cm.jt */
> +    if (index >= 32) {
> +        env->gpr[1] = next_pc;
> +    }

This is simple enough to do in the caller, and then you don't need to pass in next_pc.
And since you don't modify xRA in the helper you can do

DEF_HELPER_FLAGS_3(cm_jt, TCG_CALL_NO_WG, tl, env, tl, tl)

static bool trans_cm_jalt(DisasContext *ctx, arg_cm_jalt *a)
{
     REQUIRE_ZCMT(ctx);

     /*
      * Update pc to current for the non-unwinding exception
      * that might come from cpu_ld*_code() in the helper.
      */
     tcg_gen_movi_tl(cpu_pc, s->base.pc_next);
     gen_helper_cm_jt(cpu_pc, cpu_env, tcg_constant_i32(a->index))

     /* c.jt vs c.jalt depends on the index. */
     if (a->index >= 32) {
         gen_set_gpri(ctx, xRA, ctx->pc_succ_insn);
     }
     tcg_gen_lookup_and_goto_ptr();
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
}


r~