[Qemu-devel] [PATCH 0/5] tricore: added small features + fixed wrong masks

[Qemu-devel] [PATCH 0/5] tricore: added small features + fixed wrong masks

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Hello,

this is my first commit of a patch set for QEMU. 

This patch series contains a few updates for the target tricore:
- adding a few additional, previously missing cpu instructions
- adding CORE_ID
- fixing some masks
- fixing a wrong shift (context PCXI PIE)


David Brenken (5):
  tricore: added some missing cpu instructions
  tricore: added CORE_ID
  tricore: fixed wrong masking of IE
  tricore: fixed wrong mask of PCXI_PIE
  tricore: fixed wrong shifting of PCXI PIE

 target/tricore/cpu.h             |  5 +++--
 target/tricore/csfr.def          |  1 +
 target/tricore/op_helper.c       |  3 ++-
 target/tricore/translate.c       | 22 ++++++++++++++++++++++
 target/tricore/tricore-opcodes.h |  4 ++++
 5 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/5] tricore: added some missing cpu instructions

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
---
 target/tricore/translate.c       | 22 ++++++++++++++++++++++
 target/tricore/tricore-opcodes.h |  4 ++++
 2 files changed, 26 insertions(+)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 4e5b083..959697f 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3389,10 +3389,18 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, int r1,
         gen_branch_cond(ctx, TCG_COND_EQ, cpu_gpr_d[r1], cpu_gpr_d[15],
                         offset);
         break;
+    case OPC1_16_SBR_JEQ2:
+        gen_branch_cond(ctx, TCG_COND_EQ, cpu_gpr_d[r1], cpu_gpr_d[15],
+                        offset + 16);
+        break;
     case OPC1_16_SBR_JNE:
         gen_branch_cond(ctx, TCG_COND_NE, cpu_gpr_d[r1], cpu_gpr_d[15],
                         offset);
         break;
+    case OPC1_16_SBR_JNE2:
+        gen_branch_cond(ctx, TCG_COND_NE, cpu_gpr_d[r1], cpu_gpr_d[15],
+                        offset + 16);
+        break;
     case OPC1_16_SBR_JNZ:
         gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[r1], 0, offset);
         break;
@@ -4089,6 +4097,10 @@ static void decode_16Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
         gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[15], const16 * 4, MO_LESL);
         break;
 /* SB-format */
+    case OPC1_16_SB_JNE:
+        address = MASK_OP_SBC_DISP4(ctx->opcode);
+        gen_compute_branch(ctx, op1, 0, 0, 0, address);
+        break;
     case OPC1_16_SB_CALL:
     case OPC1_16_SB_J:
     case OPC1_16_SB_JNZ:
@@ -4122,6 +4134,7 @@ static void decode_16Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
         break;
 /* SBR-format */
     case OPC1_16_SBR_JEQ:
+    case OPC1_16_SBR_JEQ2:
     case OPC1_16_SBR_JGEZ:
     case OPC1_16_SBR_JGTZ:
     case OPC1_16_SBR_JLEZ:
@@ -6256,6 +6269,15 @@ static void decode_rr_accumulator(CPUTriCoreState *env, DisasContext *ctx)
             generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
         }
         break;
+    case OPC2_32_RR_MOVS_64:
+        if (tricore_feature(env, TRICORE_FEATURE_16)) {
+            CHECK_REG_PAIR(r3);
+            tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r2]);
+            tcg_gen_sari_tl(cpu_gpr_d[r3 + 1], cpu_gpr_d[r2], 31);
+        } else {
+            generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
+        }
+        break;
     case OPC2_32_RR_NE:
         tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_d[r1],
                            cpu_gpr_d[r2]);
diff --git a/target/tricore/tricore-opcodes.h b/target/tricore/tricore-opcodes.h
index 08394b8..600785e 100644
--- a/target/tricore/tricore-opcodes.h
+++ b/target/tricore/tricore-opcodes.h
@@ -313,6 +313,7 @@ enum {
     OPC1_16_SBC_JEQ                                  = 0x1e,
     OPC1_16_SBC_JEQ2                                 = 0x9e,
     OPC1_16_SBR_JEQ                                  = 0x3e,
+    OPC1_16_SBR_JEQ2                                 = 0xbe,
     OPC1_16_SBR_JGEZ                                 = 0xce,
     OPC1_16_SBR_JGTZ                                 = 0x4e,
     OPC1_16_SR_JI                                    = 0xdc,
@@ -321,11 +322,13 @@ enum {
     OPC1_16_SBC_JNE                                  = 0x5e,
     OPC1_16_SBC_JNE2                                 = 0xde,
     OPC1_16_SBR_JNE                                  = 0x7e,
+    OPC1_16_SBR_JNE2                                 = 0xfe,
     OPC1_16_SB_JNZ                                   = 0xee,
     OPC1_16_SBR_JNZ                                  = 0xf6,
     OPC1_16_SBR_JNZ_A                                = 0x7c,
     OPC1_16_SBRN_JNZ_T                               = 0xae,
     OPC1_16_SB_JZ                                    = 0x6e,
+    OPC1_16_SB_JNE                                   = 0xfe,
     OPC1_16_SBR_JZ                                   = 0x76,
     OPC1_16_SBR_JZ_A                                 = 0xbc,
     OPC1_16_SBRN_JZ_T                                = 0x2e,
@@ -1064,6 +1067,7 @@ enum {
     OPC2_32_RR_MIN_H                             = 0x78,
     OPC2_32_RR_MIN_HU                            = 0x79,
     OPC2_32_RR_MOV                               = 0x1f,
+    OPC2_32_RR_MOVS_64                           = 0x80,
     OPC2_32_RR_MOV_64                            = 0x81,
     OPC2_32_RR_NE                                = 0x11,
     OPC2_32_RR_OR_EQ                             = 0x27,
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/5] tricore: added some missing cpu instructions

[Bastian Koppelmann]

On 02/19/2018 07:18 AM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> +    case OPC1_16_SBR_JEQ2:
> +        gen_branch_cond(ctx, TCG_COND_EQ, cpu_gpr_d[r1], cpu_gpr_d[15],
> +                        offset + 16);
> +        break;

This is a 1.6+ instruction and you need to add a check, like with
OPC2_32_RR_MOVS_64.

>      case OPC1_16_SBR_JNE:
>          gen_branch_cond(ctx, TCG_COND_NE, cpu_gpr_d[r1], cpu_gpr_d[15],
>                          offset);
>          break;
> +    case OPC1_16_SBR_JNE2:
> +        gen_branch_cond(ctx, TCG_COND_NE, cpu_gpr_d[r1], cpu_gpr_d[15],
> +                        offset + 16);
> +        break;

Likewise.

>      case OPC1_16_SBR_JNZ:
>          gen_branch_condi(ctx, TCG_COND_NE, cpu_gpr_d[r1], 0, offset);
>          break;
> @@ -4089,6 +4097,10 @@ static void decode_16Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
>          gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[15], const16 * 4, MO_LESL);
>          break;
>  /* SB-format */
> +    case OPC1_16_SB_JNE:
> +        address = MASK_OP_SBC_DISP4(ctx->opcode);
> +        gen_compute_branch(ctx, op1, 0, 0, 0, address);
> +        break;

Why duplicate the code here? First, OPC1_16_SB_JNE is the same opcode as
OPC1_16_SBR_JNE2. Second, you can just add your case below
OPC1_16_SBR_JNE in decode_16Bit_opc().

> @@ -321,11 +322,13 @@ enum {
>      OPC1_16_SBC_JNE                                  = 0x5e,
>      OPC1_16_SBC_JNE2                                 = 0xde,
>      OPC1_16_SBR_JNE                                  = 0x7e,
> +    OPC1_16_SBR_JNE2                                 = 0xfe,
>      OPC1_16_SB_JNZ                                   = 0xee,
>      OPC1_16_SBR_JNZ                                  = 0xf6,
>      OPC1_16_SBR_JNZ_A                                = 0x7c,
>      OPC1_16_SBRN_JNZ_T                               = 0xae,
>      OPC1_16_SB_JZ                                    = 0x6e,
> +    OPC1_16_SB_JNE                                   = 0xfe,

This is the same opcode as OPC1_16_SBR_JNE2. Don't duplicate it.

[Qemu-devel] [PATCH 2/5] tricore: added CORE_ID

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
---
 target/tricore/cpu.h    | 1 +
 target/tricore/csfr.def | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index f41d2ce..c295763 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -59,6 +59,7 @@ struct CPUTriCoreState {
     uint32_t PC;
     uint32_t SYSCON;
     uint32_t CPU_ID;
+    uint32_t CORE_ID;
     uint32_t BIV;
     uint32_t BTV;
     uint32_t ISP;
diff --git a/target/tricore/csfr.def b/target/tricore/csfr.def
index 05c45dd..ca8209c 100644
--- a/target/tricore/csfr.def
+++ b/target/tricore/csfr.def
@@ -10,6 +10,7 @@ A(0xfe00, PCXI, TRICORE_FEATURE_13)
 A(0xfe08, PC, TRICORE_FEATURE_13)
 A(0xfe14, SYSCON, TRICORE_FEATURE_13)
 R(0xfe18, CPU_ID, TRICORE_FEATURE_13)
+R(0xfe1c, CORE_ID, TRICORE_FEATURE_161)
 E(0xfe20, BIV, TRICORE_FEATURE_13)
 E(0xfe24, BTV, TRICORE_FEATURE_13)
 E(0xfe28, ISP, TRICORE_FEATURE_13)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/5] tricore: added CORE_ID

[Bastian Koppelmann]

On 02/19/2018 07:18 AM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> Signed-off-by: David Brenken <david.brenken@efs-auto.de>
> Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
> Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
> ---
>  target/tricore/cpu.h    | 1 +
>  target/tricore/csfr.def | 1 +
>  2 files changed, 2 insertions(+)
>


Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

[Qemu-devel] [PATCH 3/5] tricore: fixed wrong masking of IE

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
---
 target/tricore/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index c295763..6ed474c 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -257,7 +257,7 @@ void tricore_cpu_dump_state(CPUState *cpu, FILE *f,
 #define MASK_CPUID_REV     0x000000ff
 
 #define MASK_ICR_PIPN 0x00ff0000
-#define MASK_ICR_IE   0x00000100
+#define MASK_ICR_IE   0x00008000
 #define MASK_ICR_CCPN 0x000000ff
 
 #define MASK_FCX_FCXS 0x000f0000
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/5] tricore: fixed wrong masking of IE

[Bastian Koppelmann]

On 02/19/2018 07:18 AM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> Signed-off-by: David Brenken <david.brenken@efs-auto.de>
> Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
> Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
> ---
>  target/tricore/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
> index c295763..6ed474c 100644
> --- a/target/tricore/cpu.h
> +++ b/target/tricore/cpu.h
> @@ -257,7 +257,7 @@ void tricore_cpu_dump_state(CPUState *cpu, FILE *f,
>  #define MASK_CPUID_REV     0x000000ff
>  
>  #define MASK_ICR_PIPN 0x00ff0000
> -#define MASK_ICR_IE   0x00000100
> +#define MASK_ICR_IE   0x00008000
>  #define MASK_ICR_CCPN 0x000000ff
>  
>  #define MASK_FCX_FCXS 0x000f0000
> 

This is architecture specific. You propose the mask for 1.6+ CPUs. The
original is for 1.3 CPUs. You could rename the original mask to
something like MASK_ICR_IE_1_3 and add your own MASK_ICR_IE_1_6 mask.

Cheers,
Bastian

[Qemu-devel] [PATCH 4/5] tricore: fixed wrong mask of PCXI_PIE

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
---
 target/tricore/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index 6ed474c..3f8c029 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -230,7 +230,7 @@ void tricore_cpu_dump_state(CPUState *cpu, FILE *f,
 
 
 #define MASK_PCXI_PCPN 0xff000000
-#define MASK_PCXI_PIE  0x00800000
+#define MASK_PCXI_PIE  0x00200000
 #define MASK_PCXI_UL   0x00400000
 #define MASK_PCXI_PCXS 0x000f0000
 #define MASK_PCXI_PCXO 0x0000ffff
-- 
2.7.4

Re: [Qemu-devel] [PATCH 4/5] tricore: fixed wrong mask of PCXI_PIE

[Bastian Koppelmann]

On 02/19/2018 07:18 AM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> Signed-off-by: David Brenken <david.brenken@efs-auto.de>
> Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
> Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
> ---
>  target/tricore/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
> index 6ed474c..3f8c029 100644
> --- a/target/tricore/cpu.h
> +++ b/target/tricore/cpu.h
> @@ -230,7 +230,7 @@ void tricore_cpu_dump_state(CPUState *cpu, FILE *f,
>  
>  
>  #define MASK_PCXI_PCPN 0xff000000
> -#define MASK_PCXI_PIE  0x00800000
> +#define MASK_PCXI_PIE  0x00200000
>  #define MASK_PCXI_UL   0x00400000
>  #define MASK_PCXI_PCXS 0x000f0000
>  #define MASK_PCXI_PCXO 0x0000ffff
> 

This is the same architecture problem as in patch 03/05.
Also, please squash patch 04 and 05 together.

Cheer,
Bastian

[Qemu-devel] [PATCH 5/5] tricore: fixed wrong shifting of PCXI PIE

[David Brenken]

From: David Brenken <david.brenken@efs-auto.de>

Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Florian Artmeier <florian.artmeier@efs-auto.de>
---
 target/tricore/op_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c
index 7af202c..491f97d 100644
--- a/target/tricore/op_helper.c
+++ b/target/tricore/op_helper.c
@@ -2605,7 +2605,8 @@ void helper_rfe(CPUTriCoreState *env)
     }
     env->PC = env->gpr_a[11] & ~0x1;
     /* ICR.IE = PCXI.PIE; */
-    env->ICR = (env->ICR & ~MASK_ICR_IE) + ((env->PCXI & MASK_PCXI_PIE) >> 15);
+    env->ICR = (env->ICR & ~MASK_ICR_IE)
+            | ((env->PCXI & MASK_PCXI_PIE) >> (21 - 15));
     /* ICR.CCPN = PCXI.PCPN; */
     env->ICR = (env->ICR & ~MASK_ICR_CCPN) +
                ((env->PCXI & MASK_PCXI_PCPN) >> 24);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/5] tricore: added small features + fixed wrong masks

[Bastian Koppelmann]

Hi David,

On 02/19/2018 07:18 AM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> Hello,
> 
> this is my first commit of a patch set for QEMU. 

welcome aboard and thanks for your first patchset. So far it looks good
to me and I will definitively accept it. However, I still have some
comments regarding some of the patches, which you need to address. I in
lined the comments into the mail containing the patch.

Cheers,
Bastian