Re: [PATCH 3/4] target/tricore: Honour privilege changes on PSW write

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

On 6/14/23 18:59, Bastian Koppelmann wrote:
>   void helper_psw_write(CPUTriCoreState *env, uint32_t arg)
>   {
> +    uint32_t old_priv, new_priv;
> +    CPUState *cs;
> +
> +    old_priv = extract32(env->PSW, 10, 2);
>       psw_write(env, arg);
> +    new_priv = extract32(env->PSW, 10, 2);
> +
> +    if (old_priv != new_priv) {
> +        cs = env_cpu(env);
> +        env->PC = env->PC + 4;
> +        cpu_loop_exit(cs);
> +    }
>   }

I think you should unconditionally end the TB after write to PSW. I think that you should 
not manipulate the PC here, nor use cpu_loop_exit.

You should add

#define DISAS_EXIT         DISAS_TARGET_0
#define DISAS_EXIT_UPDATE  DISAS_TARGET_1

> @@ -378,6 +379,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv r1,
>      if (ctx->priv == TRICORE_PRIV_SM) {
>          /* since we're caching PSW make this a special case */
>          if (offset == 0xfe04) {
> +            gen_save_pc(ctx->base.pc_next);
>              gen_helper_psw_write(cpu_env, r1);

Instead set ctx->base.is_jmp = DISAS_EXIT,

and in tricore_tr_tb_stop add

     case DISAS_EXIT_UPDATE:
         gen_save_pc(ctx->base.pc_next);
         /* fall through */
     case DISAS_EXIT:
         tcg_gen_exit_tb(NULL, 0);
         break;

There are a number of places (e.g. rfe), which can then use DISAS_EXIT instead of issuing 
the exit directly.

I'll also say that there are a number of other places using tcg_gen_exit_tb which should 
instead be using tcg_gen_lookup_and_goto_ptr -- all of the indirect branches for instance. 
  I would suggest adding

#define DISAS_JUMP    DISAS_TARGET_2

to handle those, again with the code within tricore_tr_tb_stop.

Finally, does JLI really clobber A[11] before branching to A[a]?
If so, this could use a comment, because it looks like a bug.


r~