[Qemu-devel] [PATCH 0/4] TriCore bugfixes

[Qemu-devel] [PATCH 0/4] TriCore bugfixes

[Bastian Koppelmann]

Hi,

this patchset only contains bugfixes, like the ones found by coverity, and some
minor corner cases in some instructions.

Cheers,
Bastian

Bastian Koppelmann (4):
  target-tricore: Several translator and cpu model fixes
  target-tricore: calculate av bits before saturation
  target-tricore: Fix bugs found by coverity
  target-tricore: split up suov32 into suov32_pos and suov32_neg

 target-tricore/cpu.c       |  2 +-
 target-tricore/cpu.h       |  1 +
 target-tricore/op_helper.c | 70 ++++++++++++++++++++++++++++------------------
 target-tricore/translate.c |  9 +++---
 4 files changed, 50 insertions(+), 32 deletions(-)

--
2.2.2

[Qemu-devel] [PATCH 1/4] target-tricore: Several translator and cpu model fixes

[Bastian Koppelmann]

Fix tc1796 cpu model using wrong ISA version.
Fix cond_add sometimes writing back wrong result.
Fix RCR_SEL and RCR_SELN using wrong registers for result and cond.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target-tricore/cpu.c       | 2 +-
 target-tricore/op_helper.c | 1 +
 target-tricore/translate.c | 6 +++---
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index abe16fa..2ba0cf4 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -118,7 +118,7 @@ static void tc1796_initfn(Object *obj)
 {
     TriCoreCPU *cpu = TRICORE_CPU(obj);
 
-    set_feature(&cpu->env, TRICORE_FEATURE_13);
+    set_feature(&cpu->env, TRICORE_FEATURE_131);
 }
 
 static void aurix_initfn(Object *obj)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 13e2729..90fcf8d 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -255,6 +255,7 @@ target_ulong helper_mul_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t t1 = extract64(r1, 0, 32);
     int64_t t2 = extract64(r2, 0, 32);
     int64_t result = t1 * t2;
+
     return suov32(env, result);
 }
 
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index def7f4a..61518f3 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -745,7 +745,7 @@ static inline void gen_cond_add(TCGCond cond, TCGv r1, TCGv r2, TCGv r3,
     tcg_gen_and_tl(temp, temp, mask);
     tcg_gen_or_tl(cpu_PSW_SAV, temp, cpu_PSW_SAV);
     /* write back result */
-    tcg_gen_movcond_tl(cond, r3, r4, t0, result, r3);
+    tcg_gen_movcond_tl(cond, r3, r4, t0, result, r1);
 
     tcg_temp_free(t0);
     tcg_temp_free(temp);
@@ -3898,7 +3898,7 @@ static void decode_rcr_cond_select(CPUTriCoreState *env, DisasContext *ctx)
     case OPC2_32_RCR_SEL:
         temp = tcg_const_i32(0);
         temp2 = tcg_const_i32(const9);
-        tcg_gen_movcond_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_d[r4], temp,
+        tcg_gen_movcond_tl(TCG_COND_NE, cpu_gpr_d[r4], cpu_gpr_d[r3], temp,
                            cpu_gpr_d[r1], temp2);
         tcg_temp_free(temp);
         tcg_temp_free(temp2);
@@ -3906,7 +3906,7 @@ static void decode_rcr_cond_select(CPUTriCoreState *env, DisasContext *ctx)
     case OPC2_32_RCR_SELN:
         temp = tcg_const_i32(0);
         temp2 = tcg_const_i32(const9);
-        tcg_gen_movcond_tl(TCG_COND_EQ, cpu_gpr_d[r3], cpu_gpr_d[r4], temp,
+        tcg_gen_movcond_tl(TCG_COND_EQ, cpu_gpr_d[r4], cpu_gpr_d[r3], temp,
                            cpu_gpr_d[r1], temp2);
         tcg_temp_free(temp);
         tcg_temp_free(temp2);
-- 
2.2.2

[Qemu-devel] [PATCH 2/4] target-tricore: calculate av bits before saturation

[Bastian Koppelmann]

64 bit mac instructions calculated the av bits after the saturation, which
resulted in a wrong PSW. This moves the av bit calculation before the
saturation.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target-tricore/op_helper.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 90fcf8d..889d045 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -371,6 +371,10 @@ uint64_t helper_madd64_ssov(CPUTriCoreState *env, target_ulong r1,
     ret = mul + r2;
     ovf = (ret ^ mul) & ~(mul ^ r2);
 
+    t1 = ret >> 32;
+    env->PSW_USB_AV = t1 ^ t1 * 2u;
+    env->PSW_USB_SAV |= env->PSW_USB_AV;
+
     if ((int64_t)ovf < 0) {
         env->PSW_USB_V = (1 << 31);
         env->PSW_USB_SV = (1 << 31);
@@ -384,9 +388,6 @@ uint64_t helper_madd64_ssov(CPUTriCoreState *env, target_ulong r1,
     } else {
         env->PSW_USB_V = 0;
     }
-    t1 = ret >> 32;
-    env->PSW_USB_AV = t1 ^ t1 * 2u;
-    env->PSW_USB_SAV |= env->PSW_USB_AV;
 
     return ret;
 }
@@ -401,6 +402,10 @@ uint64_t helper_madd64_suov(CPUTriCoreState *env, target_ulong r1,
     mul = t1 * t3;
     ret = mul + r2;
 
+    t1 = ret >> 32;
+    env->PSW_USB_AV = t1 ^ t1 * 2u;
+    env->PSW_USB_SAV |= env->PSW_USB_AV;
+
     if (ret < r2) {
         env->PSW_USB_V = (1 << 31);
         env->PSW_USB_SV = (1 << 31);
@@ -409,9 +414,6 @@ uint64_t helper_madd64_suov(CPUTriCoreState *env, target_ulong r1,
     } else {
         env->PSW_USB_V = 0;
     }
-    t1 = ret >> 32;
-    env->PSW_USB_AV = t1 ^ t1 * 2u;
-    env->PSW_USB_SAV |= env->PSW_USB_AV;
     return ret;
 }
 
@@ -451,6 +453,10 @@ uint64_t helper_msub64_ssov(CPUTriCoreState *env, target_ulong r1,
     ret = r2 - mul;
     ovf = (ret ^ r2) & (mul ^ r2);
 
+    t1 = ret >> 32;
+    env->PSW_USB_AV = t1 ^ t1 * 2u;
+    env->PSW_USB_SAV |= env->PSW_USB_AV;
+
     if ((int64_t)ovf < 0) {
         env->PSW_USB_V = (1 << 31);
         env->PSW_USB_SV = (1 << 31);
@@ -464,9 +470,6 @@ uint64_t helper_msub64_ssov(CPUTriCoreState *env, target_ulong r1,
     } else {
         env->PSW_USB_V = 0;
     }
-    t1 = ret >> 32;
-    env->PSW_USB_AV = t1 ^ t1 * 2u;
-    env->PSW_USB_SAV |= env->PSW_USB_AV;
     return ret;
 }
 
@@ -480,6 +483,10 @@ uint64_t helper_msub64_suov(CPUTriCoreState *env, target_ulong r1,
     mul = t1 * t3;
     ret = r2 - mul;
 
+    t1 = ret >> 32;
+    env->PSW_USB_AV = t1 ^ t1 * 2u;
+    env->PSW_USB_SAV |= env->PSW_USB_AV;
+
     if (ret > r2) {
         env->PSW_USB_V = (1 << 31);
         env->PSW_USB_SV = (1 << 31);
@@ -488,9 +495,6 @@ uint64_t helper_msub64_suov(CPUTriCoreState *env, target_ulong r1,
     } else {
         env->PSW_USB_V = 0;
     }
-    t1 = ret >> 32;
-    env->PSW_USB_AV = t1 ^ t1 * 2u;
-    env->PSW_USB_SAV |= env->PSW_USB_AV;
     return ret;
 }
 
-- 
2.2.2

[Qemu-devel] [PATCH 3/4] target-tricore: Fix bugs found by coverity

[Bastian Koppelmann]

This fixes one bug and one false positive found by coverity. The bug is,
that gen_mtcr was missing a mask to check the flag, which resulted in dead code.

The false positive is a intentional missing break for a jump and link address
insn followed by a jump and link insn. This adds a fall through comment to avoid
the false positive in the future.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target-tricore/cpu.h       | 1 +
 target-tricore/translate.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target-tricore/cpu.h b/target-tricore/cpu.h
index 7555b70..e5409e4 100644
--- a/target-tricore/cpu.h
+++ b/target-tricore/cpu.h
@@ -238,6 +238,7 @@ struct CPUTriCoreState {
 #define MASK_LCX_LCXS 0x000f0000
 #define MASK_LCX_LCX0 0x0000ffff
 
+#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          */
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 61518f3..57949fa 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -343,7 +343,7 @@ static inline void gen_mfcr(CPUTriCoreState *env, TCGv ret, int32_t offset)
 static inline void gen_mtcr(CPUTriCoreState *env, DisasContext *ctx, TCGv r1,
                             int32_t offset)
 {
-    if (ctx->hflags & TRICORE_HFLAG_SM) {
+    if ((ctx->hflags & TRICORE_HFLAG_KUU) == TRICORE_HFLAG_SM) {
         /* since we're caching PSW make this a special case */
         if (offset == 0xfe04) {
             gen_helper_psw_write(cpu_env, r1);
@@ -1647,6 +1647,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, int r1,
         break;
     case OPC1_32_B_JLA:
         tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
+        /* fall through */
     case OPC1_32_B_JA:
         gen_goto_tb(ctx, 0, EA_B_ABSOLUT(offset));
         break;
-- 
2.2.2

[Qemu-devel] [PATCH 4/4] target-tricore: split up suov32 into suov32_pos and suov32_neg

[Bastian Koppelmann]

suov checks unsigned for an overflow and an underflow, after some arithmetic
operations and saturates the result to either max_uint32 or 0. So far we
handled this by expanding to the next bigger data type and compare whether
the result is > max_uint32 or < 0.

However this approach can fail for an 32 bit multiplication, if both operands of
the multiplication are 0x80000000. This sets the sign bit of the 64 bit integer
and would result in a false saturation to 0.

Since unsigned operations, e.g add, sub, mul always result in either a positive
or negative overflow, we split the functions for suov32 up into two functions
(suov32_pos, suov32_neg) for each case.

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

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 889d045..71dcb70 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -80,29 +80,40 @@ static uint32_t ssov32(CPUTriCoreState *env, int64_t arg)
     return ret;
 }
 
-static uint32_t suov32(CPUTriCoreState *env, int64_t arg)
+static uint32_t suov32_pos(CPUTriCoreState *env, uint64_t arg)
 {
     uint32_t ret;
-    int64_t max_pos = UINT32_MAX;
+    uint64_t max_pos = UINT32_MAX;
     if (arg > max_pos) {
         env->PSW_USB_V = (1 << 31);
         env->PSW_USB_SV = (1 << 31);
         ret = (target_ulong)max_pos;
     } else {
-        if (arg < 0) {
-            env->PSW_USB_V = (1 << 31);
-            env->PSW_USB_SV = (1 << 31);
-            ret = 0;
-        } else {
-            env->PSW_USB_V = 0;
-            ret = (target_ulong)arg;
-        }
+        env->PSW_USB_V = 0;
+        ret = (target_ulong)arg;
      }
     env->PSW_USB_AV = arg ^ arg * 2u;
     env->PSW_USB_SAV |= env->PSW_USB_AV;
     return ret;
 }
 
+static uint32_t suov32_neg(CPUTriCoreState *env, int64_t arg)
+{
+    uint32_t ret;
+
+    if (arg < 0) {
+        env->PSW_USB_V = (1 << 31);
+        env->PSW_USB_SV = (1 << 31);
+        ret = 0;
+    } else {
+        env->PSW_USB_V = 0;
+        ret = (target_ulong)arg;
+    }
+    env->PSW_USB_AV = arg ^ arg * 2u;
+    env->PSW_USB_SAV |= env->PSW_USB_AV;
+    return ret;
+}
+
 static uint32_t ssov16(CPUTriCoreState *env, int32_t hw0, int32_t hw1)
 {
     int32_t max_pos = INT16_MAX;
@@ -189,7 +200,7 @@ target_ulong helper_add_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t t1 = extract64(r1, 0, 32);
     int64_t t2 = extract64(r2, 0, 32);
     int64_t result = t1 + t2;
-    return suov32(env, result);
+    return suov32_pos(env, result);
 }
 
 target_ulong helper_add_h_suov(CPUTriCoreState *env, target_ulong r1,
@@ -227,7 +238,7 @@ target_ulong helper_sub_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t t1 = extract64(r1, 0, 32);
     int64_t t2 = extract64(r2, 0, 32);
     int64_t result = t1 - t2;
-    return suov32(env, result);
+    return suov32_neg(env, result);
 }
 
 target_ulong helper_sub_h_suov(CPUTriCoreState *env, target_ulong r1,
@@ -256,7 +267,7 @@ target_ulong helper_mul_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t t2 = extract64(r2, 0, 32);
     int64_t result = t1 * t2;
 
-    return suov32(env, result);
+    return suov32_pos(env, result);
 }
 
 target_ulong helper_sha_ssov(CPUTriCoreState *env, target_ulong r1,
@@ -356,7 +367,7 @@ target_ulong helper_madd32_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t result;
 
     result = t2 + (t1 * t3);
-    return suov32(env, result);
+    return suov32_pos(env, result);
 }
 
 uint64_t helper_madd64_ssov(CPUTriCoreState *env, target_ulong r1,
@@ -438,7 +449,7 @@ target_ulong helper_msub32_suov(CPUTriCoreState *env, target_ulong r1,
     int64_t result;
 
     result = t2 - (t1 * t3);
-    return suov32(env, result);
+    return suov32_neg(env, result);
 }
 
 uint64_t helper_msub64_ssov(CPUTriCoreState *env, target_ulong r1,
-- 
2.2.2