[PATCH 0/4] TriCore Privilege Levels

[PATCH 0/4] TriCore Privilege Levels

[Bastian Koppelmann]

Hi,

this patch series tries to properly implement privilege levels for the TriCore,
as discussed in
https://lore.kernel.org/qemu-devel/20230118090319.32n4uto7ogy3gfr6@schnipp.zuhause/.

While implementing privilege traps for the SV/UM1 only insns, I saw that
the RESTORE insn uses the wrong ICR.IE bit. So I fixed that as well.

Cheers,
Bastian

Bastian Koppelmann (4):
  target/tricore: Introduce priv tb flag
  target/tricore: Implement privilege level for all insns
  target/tricore: Honour privilege changes on PSW write
  target/tricore: Fix ICR.IE offset in RESTORE insn

 target/tricore/cpu.h       | 17 +++++++----
 target/tricore/op_helper.c | 11 +++++++
 target/tricore/translate.c | 61 ++++++++++++++++++++++++++++----------
 3 files changed, 68 insertions(+), 21 deletions(-)

-- 
2.40.1

[PATCH 1/4] target/tricore: Introduce priv tb flag

[Bastian Koppelmann]

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/tricore/cpu.h       | 17 ++++++++++++-----
 target/tricore/translate.c | 15 +++++++++------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index 041fc0b6e5..257fcf3cee 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -263,10 +263,11 @@ void icr_set_ie(CPUTriCoreState *env, uint32_t val);
 #define MASK_DBGSR_PEVT 0x40
 #define MASK_DBGSR_EVTSRC 0x1f00
 
-#define TRICORE_HFLAG_KUU     0x3
-#define TRICORE_HFLAG_UM0     0x00002 /* user mode-0 flag          */
-#define TRICORE_HFLAG_UM1     0x00001 /* user mode-1 flag          */
-#define TRICORE_HFLAG_SM      0x00000 /* kernel mode flag          */
+enum tricore_priv_levels {
+    TRICORE_PRIV_UM0 = 0x0, /* user mode-0 flag */
+    TRICORE_PRIV_UM1 = 0x1, /* user mode-1 flag */
+    TRICORE_PRIV_SM  = 0x2, /* kernel mode flag */
+};
 
 enum tricore_features {
     TRICORE_FEATURE_13,
@@ -378,15 +379,21 @@ static inline int cpu_mmu_index(CPUTriCoreState *env, bool ifetch)
 
 #include "exec/cpu-all.h"
 
+FIELD(TB_FLAGS, PRIV, 0, 2)
+
 void cpu_state_reset(CPUTriCoreState *s);
 void tricore_tcg_init(void);
 
 static inline void cpu_get_tb_cpu_state(CPUTriCoreState *env, target_ulong *pc,
                                         target_ulong *cs_base, uint32_t *flags)
 {
+    uint32_t new_flags = 0;
     *pc = env->PC;
     *cs_base = 0;
-    *flags = 0;
+
+    new_flags |= FIELD_DP32(new_flags, TB_FLAGS, PRIV,
+            extract32(env->PSW, 10, 2));
+    *flags = new_flags;
 }
 
 #define TRICORE_CPU_TYPE_SUFFIX "-" TYPE_TRICORE_CPU
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 6712d98f6e..a0644dd120 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -32,6 +32,7 @@
 #include "tricore-opcodes.h"
 #include "exec/translator.h"
 #include "exec/log.h"
+#include "hw/registerfields.h"
 
 #define HELPER_H "helper.h"
 #include "exec/helper-info.c.inc"
@@ -73,7 +74,7 @@ typedef struct DisasContext {
     uint32_t opcode;
     /* Routine used to access memory */
     int mem_idx;
-    uint32_t hflags, saved_hflags;
+    int priv;
     uint64_t features;
     uint32_t icr_ie_mask, icr_ie_offset;
 } DisasContext;
@@ -374,7 +375,7 @@ static inline void gen_mfcr(DisasContext *ctx, TCGv ret, int32_t offset)
 static inline void gen_mtcr(DisasContext *ctx, TCGv r1,
                             int32_t offset)
 {
-    if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) {
+    if (ctx->priv == TRICORE_PRIV_SM) {
         /* since we're caching PSW make this a special case */
         if (offset == 0xfe04) {
             gen_helper_psw_write(cpu_env, r1);
@@ -7911,7 +7912,7 @@ static void decode_sys_interrupts(DisasContext *ctx)
         ctx->base.is_jmp = DISAS_NORETURN;
         break;
     case OPC2_32_SYS_RFM:
-        if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) {
+        if (ctx->priv  == TRICORE_PRIV_SM) {
             tmp = tcg_temp_new();
             l1 = gen_new_label();
 
@@ -7934,8 +7935,7 @@ static void decode_sys_interrupts(DisasContext *ctx)
         break;
     case OPC2_32_SYS_RESTORE:
         if (has_feature(ctx, TRICORE_FEATURE_16)) {
-            if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM ||
-                (ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_UM1) {
+            if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
                 tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1], 8, 1);
             } /* else raise privilege trap */
         } else {
@@ -8305,7 +8305,10 @@ static void tricore_tr_init_disas_context(DisasContextBase *dcbase,
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPUTriCoreState *env = cs->env_ptr;
     ctx->mem_idx = cpu_mmu_index(env, false);
-    ctx->hflags = (uint32_t)ctx->base.tb->flags;
+
+    uint32_t tb_flags = (uint32_t)ctx->base.tb->flags;
+    ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV);
+
     ctx->features = env->features;
     if (has_feature(ctx, TRICORE_FEATURE_161)) {
         ctx->icr_ie_mask = R_ICR_IE_161_MASK;
-- 
2.40.1

Re: [PATCH 1/4] target/tricore: Introduce priv tb flag

[Richard Henderson]

On 6/14/23 18:59, Bastian Koppelmann wrote:
> +    uint32_t tb_flags = (uint32_t)ctx->base.tb->flags;

No need for the cast.  Otherwise,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 2/4] target/tricore: Implement privilege level for all insns

[Bastian Koppelmann]

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/tricore/translate.c | 41 +++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index a0644dd120..edbc319fa1 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -385,7 +385,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv r1,
             }
         }
     } else {
-        /* generate privilege trap */
+        generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
     }
 }
 
@@ -3372,7 +3372,11 @@ static void decode_sc_opc(DisasContext *ctx, int op1)
         tcg_gen_andi_tl(cpu_gpr_d[15], cpu_gpr_d[15], const16);
         break;
     case OPC1_16_SC_BISR:
-        gen_helper_1arg(bisr, const16 & 0xff);
+        if (ctx->priv == TRICORE_PRIV_SM) {
+            gen_helper_1arg(bisr, const16 & 0xff);
+        } else {
+            generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+        }
         break;
     case OPC1_16_SC_LD_A:
         gen_offset_ld(ctx, cpu_gpr_a[15], cpu_gpr_a[10], const16 * 4, MO_LESL);
@@ -5234,7 +5238,11 @@ static void decode_rc_serviceroutine(DisasContext *ctx)
 
     switch (op2) {
     case OPC2_32_RC_BISR:
-        gen_helper_1arg(bisr, const9);
+        if (ctx->priv == TRICORE_PRIV_SM) {
+            gen_helper_1arg(bisr, const9);
+        } else {
+            generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+        }
         break;
     case OPC2_32_RC_SYSCALL:
         generate_trap(ctx, TRAPC_SYSCALL, const9 & 0xff);
@@ -7882,19 +7890,32 @@ static void decode_sys_interrupts(DisasContext *ctx)
         /* raise EXCP_DEBUG */
         break;
     case OPC2_32_SYS_DISABLE:
-        tcg_gen_andi_tl(cpu_ICR, cpu_ICR, ~ctx->icr_ie_mask);
+        if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
+            tcg_gen_andi_tl(cpu_ICR, cpu_ICR, ~ctx->icr_ie_mask);
+        } else {
+            generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+        }
         break;
     case OPC2_32_SYS_DISABLE_D:
         if (has_feature(ctx, TRICORE_FEATURE_16)) {
-            tcg_gen_extract_tl(cpu_gpr_d[r1], cpu_ICR, ctx->icr_ie_offset, 1);
-            tcg_gen_andi_tl(cpu_ICR, cpu_ICR, ~ctx->icr_ie_mask);
+            if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
+                tcg_gen_extract_tl(cpu_gpr_d[r1], cpu_ICR,
+                        ctx->icr_ie_offset, 1);
+                tcg_gen_andi_tl(cpu_ICR, cpu_ICR, ~ctx->icr_ie_mask);
+            } else {
+                generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+            }
         } else {
             generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
         }
     case OPC2_32_SYS_DSYNC:
         break;
     case OPC2_32_SYS_ENABLE:
-        tcg_gen_ori_tl(cpu_ICR, cpu_ICR, ctx->icr_ie_mask);
+        if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
+            tcg_gen_ori_tl(cpu_ICR, cpu_ICR, ctx->icr_ie_mask);
+        } else {
+            generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+        }
         break;
     case OPC2_32_SYS_ISYNC:
         break;
@@ -7924,7 +7945,7 @@ static void decode_sys_interrupts(DisasContext *ctx)
             tcg_gen_exit_tb(NULL, 0);
             ctx->base.is_jmp = DISAS_NORETURN;
         } else {
-            /* generate privilege trap */
+            generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
         }
         break;
     case OPC2_32_SYS_RSLCX:
@@ -7937,7 +7958,9 @@ static void decode_sys_interrupts(DisasContext *ctx)
         if (has_feature(ctx, TRICORE_FEATURE_16)) {
             if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
                 tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1], 8, 1);
-            } /* else raise privilege trap */
+            } else {
+                generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
+            }
         } else {
             generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
         }
-- 
2.40.1

Re: [PATCH 2/4] target/tricore: Implement privilege level for all insns

[Richard Henderson]

On 6/14/23 18:59, Bastian Koppelmann wrote:
> Signed-off-by: Bastian Koppelmann<kbastian@mail.uni-paderborn.de>
> ---
>   target/tricore/translate.c | 41 +++++++++++++++++++++++++++++---------
>   1 file changed, 32 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

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

[Bastian Koppelmann]

the CPU can change the privilege level by writing the corresponding bits
in PSW. If this happens all instructions after this 'mtcr' in the TB are
translated with the wrong privilege level. So we have to exit to the
cpu_loop() and start translating again with the new privilege level.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/tricore/op_helper.c | 11 +++++++++++
 target/tricore/translate.c |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c
index 026e15f3e0..17b78c501c 100644
--- a/target/tricore/op_helper.c
+++ b/target/tricore/op_helper.c
@@ -2839,7 +2839,18 @@ void helper_rslcx(CPUTriCoreState *env)
 
 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);
+    }
 }
 
 uint32_t helper_psw_read(CPUTriCoreState *env)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index edbc319fa1..baf13fc205 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -331,6 +331,7 @@ static void gen_swapmsk(DisasContext *ctx, int reg, TCGv ea)
     tcg_gen_mov_tl(cpu_gpr_d[reg], temp);
 }
 
+static inline void gen_save_pc(target_ulong pc);
 
 /* We generate loads and store to core special function register (csfr) through
    the function gen_mfcr and gen_mtcr. To handle access permissions, we use 3
@@ -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);
         } else {
             switch (offset) {
-- 
2.40.1

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

[Richard Henderson]

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~

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

[Bastian Koppelmann]

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

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

[Bastian Koppelmann]

On Thu, Jun 15, 2023 at 05:15:28PM +0200, Bastian Koppelmann wrote:
> 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?

You're right this is a bug. If A[a] = A[11], then we're overwriting the jump
address. We have to save A[a] into a temp and then save A[11].

Thanks for finding this!

Cheers,
Bastian

[PATCH 4/4] target/tricore: Fix ICR.IE offset in RESTORE insn

[Bastian Koppelmann]

from ISA v1.6.1 onwards the bit position of ICR.IE changed.
ctx->icr_ie_offset contains the correct value for the ISA version used
by the vCPU.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/tricore/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index baf13fc205..e4e57130bf 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -7959,7 +7959,8 @@ static void decode_sys_interrupts(DisasContext *ctx)
     case OPC2_32_SYS_RESTORE:
         if (has_feature(ctx, TRICORE_FEATURE_16)) {
             if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
-                tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1], 8, 1);
+                tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1],
+                        ctx->icr_ie_offset, 1);
             } else {
                 generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
             }
-- 
2.40.1

Re: [PATCH 4/4] target/tricore: Fix ICR.IE offset in RESTORE insn

[Richard Henderson]

On 6/14/23 18:59, Bastian Koppelmann wrote:
> from ISA v1.6.1 onwards the bit position of ICR.IE changed.
> ctx->icr_ie_offset contains the correct value for the ISA version used
> by the vCPU.
> 
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
>   target/tricore/translate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index baf13fc205..e4e57130bf 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -7959,7 +7959,8 @@ static void decode_sys_interrupts(DisasContext *ctx)
>       case OPC2_32_SYS_RESTORE:
>           if (has_feature(ctx, TRICORE_FEATURE_16)) {
>               if (ctx->priv == TRICORE_PRIV_SM || ctx->priv == TRICORE_PRIV_UM1) {
> -                tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1], 8, 1);
> +                tcg_gen_deposit_tl(cpu_ICR, cpu_ICR, cpu_gpr_d[r1],
> +                        ctx->icr_ie_offset, 1);
>               } else {
>                   generate_trap(ctx, TRAPC_PROT, TIN1_PRIV);
>               }

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Additionally, you need to exit to the main loop, so that exceptions may be recognized 
after re-enabling interrupts.  This is missing from ENABLE as well.


r~