On Thu, Jun 15, 2023 at 09:37:23AM +0200, Richard Henderson wrote:
> 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
ok.
>
> > @@ -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.
ok.
>
> 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.
I'll look into that. However, this is out of scope for this patch series.
>
> 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.
Yes, it does. A[11] is the link register (not only by convention), so it is hard
coded to save the return address to A[11]. See [1] page 29. Why does it look
like a bug to you?
Thanks,
Bastian
[1]
https://www.infineon.com/dgdl/Infineon-AURIX_TC3xx_Architecture_vol1-UserManual-v01_00-EN.pdf?fileId=5546d46276fb756a01771bc4c2e33bdd
