[Qemu-devel] [PATCH 0/6] POWER9 TCG enablements - part15

[Qemu-devel] [PATCH 0/6] POWER9 TCG enablements - part15

[Nikunj A Dadhania]

This series contains 6 new instructions for POWER9 ISA3.0

Nikunj A Dadhania (6):
  target-ppc: generate exception for copy/paste
  target-ppc: add slbieg instruction
  target-ppc: add slbsync implementation
  target-ppc: add wait instruction
  target-ppc: support for 32-bit carry and overflow
  target-ppc: add mcrxrx instruction

 target/ppc/cpu.h        | 26 ++++++++++++++++++
 target/ppc/helper.h     |  1 +
 target/ppc/mmu-hash64.c | 16 +++++++++--
 target/ppc/translate.c  | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 2 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/6] target-ppc: generate exception for copy/paste

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 255735a..80f9f15 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6132,6 +6132,19 @@ static inline void gen_cp_abort(DisasContext *ctx)
     // Do Nothing
 }
 
+#define GEN_CP_PASTE_NOOP(name)                           \
+static inline void gen_##name(DisasContext *ctx)          \
+{                                                         \
+    /* Generate invalid exception until                   \
+     * we have an implementation of the copy              \
+     * paste facility                                     \
+     */                                                   \
+    gen_invalid(ctx);                                     \
+}
+
+GEN_CP_PASTE_NOOP(copy)
+GEN_CP_PASTE_NOOP(paste)
+
 static void gen_tcheck(DisasContext *ctx)
 {
     if (unlikely(!ctx->tm_enabled)) {
@@ -6281,7 +6294,9 @@ GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(cntlzw, 0x1F, 0x1A, 0x00, 0x00000000, PPC_INTEGER),
 GEN_HANDLER_E(cnttzw, 0x1F, 0x1A, 0x10, 0x00000000, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(copy, 0x1F, 0x06, 0x18, 0x03C00001, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(cp_abort, 0x1F, 0x06, 0x1A, 0x03FFF801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(paste, 0x1F, 0x06, 0x1C, 0x03C00000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(or, 0x1F, 0x1C, 0x0D, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(xor, 0x1F, 0x1C, 0x09, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(ori, 0x18, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-- 
2.7.4

[Qemu-devel] [PATCH 2/6] target-ppc: add slbieg instruction

[Nikunj A Dadhania]

slbieg: SLB Invalidate Entry Global

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/helper.h     |  1 +
 target/ppc/mmu-hash64.c | 16 ++++++++++++++--
 target/ppc/translate.c  | 14 ++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index cc81709..007a837 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -669,6 +669,7 @@ DEF_HELPER_2(load_slb_vsid, tl, env, tl)
 DEF_HELPER_2(find_slb_vsid, tl, env, tl)
 DEF_HELPER_FLAGS_1(slbia, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(slbie, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_FLAGS_2(slbieg, TCG_CALL_NO_RWG, void, env, tl)
 #endif
 DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index bb78fb5..2791f29 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -115,7 +115,8 @@ void helper_slbia(CPUPPCState *env)
     }
 }
 
-void helper_slbie(CPUPPCState *env, target_ulong addr)
+static void __helper_slbie(CPUPPCState *env, target_ulong addr,
+                           target_ulong global)
 {
     PowerPCCPU *cpu = ppc_env_get_cpu(env);
     ppc_slb_t *slb;
@@ -132,10 +133,21 @@ void helper_slbie(CPUPPCState *env, target_ulong addr)
          *      and we still don't have a tlb_flush_mask(env, n, mask)
          *      in QEMU, we just invalidate all TLBs
          */
-        env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
+        env->tlb_need_flush |=
+            (global == false ? TLB_NEED_LOCAL_FLUSH : TLB_NEED_GLOBAL_FLUSH);
     }
 }
 
+void helper_slbie(CPUPPCState *env, target_ulong addr)
+{
+    __helper_slbie(env, addr, false);
+}
+
+void helper_slbieg(CPUPPCState *env, target_ulong addr)
+{
+    __helper_slbie(env, addr, true);
+}
+
 int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
                   target_ulong esid, target_ulong vsid)
 {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 80f9f15..b0f3c3b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4484,6 +4484,19 @@ static void gen_slbie(DisasContext *ctx)
     gen_helper_slbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
+
+/* slbieg */
+static void gen_slbieg(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    GEN_PRIV;
+#else
+    CHK_SV;
+
+    gen_helper_slbieg(cpu_env, cpu_gpr[rB(ctx->opcode)]);
+#endif /* defined(CONFIG_USER_ONLY) */
+}
+
 #endif  /* defined(TARGET_PPC64) */
 
 /***                              External control                         ***/
@@ -6439,6 +6452,7 @@ GEN_HANDLER(tlbsync, 0x1F, 0x16, 0x11, 0x03FFF801, PPC_MEM_TLBSYNC),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(slbia, 0x1F, 0x12, 0x0F, 0x031FFC01, PPC_SLBI),
 GEN_HANDLER(slbie, 0x1F, 0x12, 0x0D, 0x03FF0001, PPC_SLBI),
+GEN_HANDLER_E(slbieg, 0x1F, 0x12, 0x0E, 0x001F0001, PPC_NONE, PPC2_ISA300),
 #endif
 GEN_HANDLER(eciwx, 0x1F, 0x16, 0x0D, 0x00000001, PPC_EXTERN),
 GEN_HANDLER(ecowx, 0x1F, 0x16, 0x09, 0x00000001, PPC_EXTERN),
-- 
2.7.4

[Qemu-devel] [PATCH 3/6] target-ppc: add slbsync implementation

[Nikunj A Dadhania]

slbsync: SLB Synchoronize

The instruction provides an ordering function for the effects of all
slbieg instructions executed by the thread executing the slbsync
instruction.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b0f3c3b..b1a6aee 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4497,6 +4497,17 @@ static void gen_slbieg(DisasContext *ctx)
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
+/* slbsync */
+static void gen_slbsync(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    GEN_PRIV;
+#else
+    CHK_SV;
+    gen_check_tlb_flush(ctx, true);
+#endif /* defined(CONFIG_USER_ONLY) */
+}
+
 #endif  /* defined(TARGET_PPC64) */
 
 /***                              External control                         ***/
@@ -6453,6 +6464,7 @@ GEN_HANDLER(tlbsync, 0x1F, 0x16, 0x11, 0x03FFF801, PPC_MEM_TLBSYNC),
 GEN_HANDLER(slbia, 0x1F, 0x12, 0x0F, 0x031FFC01, PPC_SLBI),
 GEN_HANDLER(slbie, 0x1F, 0x12, 0x0D, 0x03FF0001, PPC_SLBI),
 GEN_HANDLER_E(slbieg, 0x1F, 0x12, 0x0E, 0x001F0001, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(slbsync, 0x1F, 0x12, 0x0A, 0x03FFF801, PPC_NONE, PPC2_ISA300),
 #endif
 GEN_HANDLER(eciwx, 0x1F, 0x16, 0x0D, 0x00000001, PPC_EXTERN),
 GEN_HANDLER(ecowx, 0x1F, 0x16, 0x09, 0x00000001, PPC_EXTERN),
-- 
2.7.4

[Qemu-devel] [PATCH 4/6] target-ppc: add wait instruction

[Nikunj A Dadhania]

Use the available wait instruction implementation.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b1a6aee..3ba2616 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6391,6 +6391,7 @@ GEN_HANDLER_E(stqcx_, 0x1F, 0x16, 0x05, 0, PPC_NONE, PPC2_LSQ_ISA207),
 #endif
 GEN_HANDLER(sync, 0x1F, 0x16, 0x12, 0x039FF801, PPC_MEM_SYNC),
 GEN_HANDLER(wait, 0x1F, 0x1E, 0x01, 0x03FFF801, PPC_WAIT),
+GEN_HANDLER_E(wait, 0x1F, 0x1E, 0x00, 0x039FF801, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(b, 0x12, 0xFF, 0xFF, 0x00000000, PPC_FLOW),
 GEN_HANDLER(bc, 0x10, 0xFF, 0xFF, 0x00000000, PPC_FLOW),
 GEN_HANDLER(bcctr, 0x13, 0x10, 0x10, 0x00000000, PPC_FLOW),
-- 
2.7.4

[Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
and corresponding defines. Moreover, CA32 is set when CA is set and
OV32 is set when OV is set, there is no need to have a new
fields in the CPUPPCState structure.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/cpu.h       | 26 ++++++++++++++++++++++++++
 target/ppc/translate.c |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index bc2a2ce..181919b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1354,11 +1354,15 @@ int ppc_compat_max_threads(PowerPCCPU *cpu);
 #define XER_SO  31
 #define XER_OV  30
 #define XER_CA  29
+#define XER_OV32  19
+#define XER_CA32  18
 #define XER_CMP  8
 #define XER_BC   0
 #define xer_so  (env->so)
 #define xer_ov  (env->ov)
 #define xer_ca  (env->ca)
+#define xer_ov32  (env->ov)
+#define xer_ca32  (env->ca)
 #define xer_cmp ((env->xer >> XER_CMP) & 0xFF)
 #define xer_bc  ((env->xer >> XER_BC)  & 0x7F)
 
@@ -2325,11 +2329,21 @@ enum {
 
 /*****************************************************************************/
 
+#ifndef TARGET_PPC64
 static inline target_ulong cpu_read_xer(CPUPPCState *env)
 {
     return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
 }
+#else
+static inline target_ulong cpu_read_xer(CPUPPCState *env)
+{
+    return env->xer | (env->so << XER_SO) |
+        (env->ov << XER_OV) | (env->ca << XER_CA) |
+        (env->ov << XER_OV32) | (env->ca << XER_CA32);
+}
+#endif
 
+#ifndef TARGET_PPC64
 static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
 {
     env->so = (xer >> XER_SO) & 1;
@@ -2337,6 +2351,18 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
     env->ca = (xer >> XER_CA) & 1;
     env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
 }
+#else
+static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
+{
+    env->so = (xer >> XER_SO) & 1;
+    env->ov = ((xer >> XER_OV) & 1) | ((xer >> XER_OV32) & 1);
+    env->ca = ((xer >> XER_CA) & 1) | ((xer >> XER_CA32) & 1);
+    env->xer = xer & ~((1ul << XER_SO) |
+                       (1ul << XER_OV) | (1ul << XER_CA) |
+                       (1ul << XER_OV32) | (1ul << XER_CA32));
+}
+#endif
+
 
 static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
                                         target_ulong *cs_base, uint32_t *flags)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 3ba2616..724ad17 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3715,6 +3715,12 @@ static void gen_read_xer(TCGv dst)
     tcg_gen_or_tl(t0, t0, t1);
     tcg_gen_or_tl(dst, dst, t2);
     tcg_gen_or_tl(dst, dst, t0);
+#ifdef TARGET_PPC64
+    tcg_gen_shli_tl(t0, cpu_ov, XER_OV32);
+    tcg_gen_or_tl(dst, dst, t0);
+    tcg_gen_shli_tl(t0, cpu_ca, XER_CA32);
+    tcg_gen_or_tl(dst, dst, t0);
+#endif
     tcg_temp_free(t0);
     tcg_temp_free(t1);
     tcg_temp_free(t2);
-- 
2.7.4

[Qemu-devel] [PATCH 6/6] target-ppc: add mcrxrx instruction

[Nikunj A Dadhania]

mcrxrx: Move to CR from XER Extended

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 724ad17..72c8a46 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3761,6 +3761,29 @@ static void gen_mcrxr(DisasContext *ctx)
     tcg_gen_movi_tl(cpu_ca, 0);
 }
 
+#ifdef TARGET_PPC64
+/* mcrxrx */
+static void gen_mcrxrx(DisasContext *ctx)
+{
+    TCGv_i32 t0 = tcg_temp_new_i32();
+    TCGv_i32 t1 = tcg_temp_new_i32();
+    TCGv_i32 dst = cpu_crf[crfD(ctx->opcode)];
+
+    /* copy OV and OV32 */
+    tcg_gen_trunc_tl_i32(t0, cpu_so);
+    tcg_gen_shli_i32(dst, t0, 1);
+    tcg_gen_or_i32(dst, dst, t0);
+    tcg_gen_shli_i32(dst, dst, 2);
+    /* copy CA and CA32 */
+    tcg_gen_trunc_tl_i32(t0, cpu_ca);
+    tcg_gen_shli_i32(t1, t0, 1);
+    tcg_gen_or_i32(t1, t1, t0);
+    tcg_gen_or_i32(dst, dst, t1);
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i32(t1);
+}
+#endif
+
 /* mfcr mfocrf */
 static void gen_mfcr(DisasContext *ctx)
 {
@@ -6430,6 +6453,7 @@ GEN_HANDLER(mtcrf, 0x1F, 0x10, 0x04, 0x00000801, PPC_MISC),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(mtmsrd, 0x1F, 0x12, 0x05, 0x001EF801, PPC_64B),
 GEN_HANDLER_E(setb, 0x1F, 0x00, 0x04, 0x0003F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(mcrxrx, 0x1F, 0x00, 0x12, 0x007FF801, PPC_NONE, PPC2_ISA300),
 #endif
 GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
 GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),
-- 
2.7.4

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]

On Thu, Feb 09, 2017 at 04:04:04PM +0530, Nikunj A Dadhania wrote:
> POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
> and corresponding defines. Moreover, CA32 is set when CA is set and
> OV32 is set when OV is set, there is no need to have a new
> fields in the CPUPPCState structure.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

Um.. I don't quite understand this.  If CA always has the same value
as CA32, what's the point?

> ---
>  target/ppc/cpu.h       | 26 ++++++++++++++++++++++++++
>  target/ppc/translate.c |  6 ++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index bc2a2ce..181919b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1354,11 +1354,15 @@ int ppc_compat_max_threads(PowerPCCPU *cpu);
>  #define XER_SO  31
>  #define XER_OV  30
>  #define XER_CA  29
> +#define XER_OV32  19
> +#define XER_CA32  18
>  #define XER_CMP  8
>  #define XER_BC   0
>  #define xer_so  (env->so)
>  #define xer_ov  (env->ov)
>  #define xer_ca  (env->ca)
> +#define xer_ov32  (env->ov)
> +#define xer_ca32  (env->ca)
>  #define xer_cmp ((env->xer >> XER_CMP) & 0xFF)
>  #define xer_bc  ((env->xer >> XER_BC)  & 0x7F)
>  
> @@ -2325,11 +2329,21 @@ enum {
>  
>  /*****************************************************************************/
>  
> +#ifndef TARGET_PPC64
>  static inline target_ulong cpu_read_xer(CPUPPCState *env)
>  {
>      return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
>  }
> +#else
> +static inline target_ulong cpu_read_xer(CPUPPCState *env)
> +{
> +    return env->xer | (env->so << XER_SO) |
> +        (env->ov << XER_OV) | (env->ca << XER_CA) |
> +        (env->ov << XER_OV32) | (env->ca << XER_CA32);
> +}
> +#endif
>  
> +#ifndef TARGET_PPC64
>  static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>  {
>      env->so = (xer >> XER_SO) & 1;
> @@ -2337,6 +2351,18 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>      env->ca = (xer >> XER_CA) & 1;
>      env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
>  }
> +#else
> +static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> +{
> +    env->so = (xer >> XER_SO) & 1;
> +    env->ov = ((xer >> XER_OV) & 1) | ((xer >> XER_OV32) & 1);
> +    env->ca = ((xer >> XER_CA) & 1) | ((xer >> XER_CA32) & 1);
> +    env->xer = xer & ~((1ul << XER_SO) |
> +                       (1ul << XER_OV) | (1ul << XER_CA) |
> +                       (1ul << XER_OV32) | (1ul << XER_CA32));
> +}
> +#endif
> +
>  
>  static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
>                                          target_ulong *cs_base, uint32_t *flags)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 3ba2616..724ad17 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3715,6 +3715,12 @@ static void gen_read_xer(TCGv dst)
>      tcg_gen_or_tl(t0, t0, t1);
>      tcg_gen_or_tl(dst, dst, t2);
>      tcg_gen_or_tl(dst, dst, t0);
> +#ifdef TARGET_PPC64
> +    tcg_gen_shli_tl(t0, cpu_ov, XER_OV32);
> +    tcg_gen_or_tl(dst, dst, t0);
> +    tcg_gen_shli_tl(t0, cpu_ca, XER_CA32);
> +    tcg_gen_or_tl(dst, dst, t0);
> +#endif
>      tcg_temp_free(t0);
>      tcg_temp_free(t1);
>      tcg_temp_free(t2);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 0/6] POWER9 TCG enablements - part15

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

On Thu, Feb 09, 2017 at 04:03:59PM +0530, Nikunj A Dadhania wrote:
> This series contains 6 new instructions for POWER9 ISA3.0

I've merged 1-4, 5 has comments.

> 
> Nikunj A Dadhania (6):
>   target-ppc: generate exception for copy/paste
>   target-ppc: add slbieg instruction
>   target-ppc: add slbsync implementation
>   target-ppc: add wait instruction
>   target-ppc: support for 32-bit carry and overflow
>   target-ppc: add mcrxrx instruction
> 
>  target/ppc/cpu.h        | 26 ++++++++++++++++++
>  target/ppc/helper.h     |  1 +
>  target/ppc/mmu-hash64.c | 16 +++++++++--
>  target/ppc/translate.c  | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 113 insertions(+), 2 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Thu, Feb 09, 2017 at 04:04:04PM +0530, Nikunj A Dadhania wrote:
>> POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
>> and corresponding defines. Moreover, CA32 is set when CA is set and
>> OV32 is set when OV is set, there is no need to have a new
>> fields in the CPUPPCState structure.
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>
> Um.. I don't quite understand this.  If CA always has the same value
> as CA32, what's the point?

I am not clear either. I think that as CA was set for both 32/64-bit
mode, that couldn't be changed for backward compatibility. CA32 should
have affected only the instructions working one word variants.

Re-scanning the ISA 3.0, found this in 3.3.9 Fixed-Point Arithmetic
Instructions:

=================================================================
addic, addic., subfic, addc, subfc, adde, subfe,
addme, subfme, addze, and subfze always set CA, to
reflect the carry out of bit 0 in 64-bit mode and out of bit
32 in 32-bit mode. These instructions also always set
CA32 to reflect the carry out of bit 32.
=================================================================

Which is conflicting to what is said in 3.2.2 Fixed-Point Exception
Register:
=================================================================
Carry32 (CA32)
CA32 is set whenever CA is set, and is set to
the same value that CA is defined to be set to
in 32-bit mode.
=================================================================

Regards
Nikunj

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]

On Fri, Feb 10, 2017 at 09:49:17AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Thu, Feb 09, 2017 at 04:04:04PM +0530, Nikunj A Dadhania wrote:
> >> POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
> >> and corresponding defines. Moreover, CA32 is set when CA is set and
> >> OV32 is set when OV is set, there is no need to have a new
> >> fields in the CPUPPCState structure.
> >> 
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >
> > Um.. I don't quite understand this.  If CA always has the same value
> > as CA32, what's the point?
> 
> I am not clear either. I think that as CA was set for both 32/64-bit
> mode, that couldn't be changed for backward compatibility. CA32 should
> have affected only the instructions working one word variants.
> 
> Re-scanning the ISA 3.0, found this in 3.3.9 Fixed-Point Arithmetic
> Instructions:
> 
> =================================================================
> addic, addic., subfic, addc, subfc, adde, subfe,
> addme, subfme, addze, and subfze always set CA, to
> reflect the carry out of bit 0 in 64-bit mode and out of bit
> 32 in 32-bit mode. These instructions also always set
> CA32 to reflect the carry out of bit 32.
> =================================================================
> 
> Which is conflicting to what is said in 3.2.2 Fixed-Point Exception
> Register:
> =================================================================
> Carry32 (CA32)
> CA32 is set whenever CA is set, and is set to
> the same value that CA is defined to be set to
> in 32-bit mode.
> =================================================================

Well, that's certainly confusing.

Can you try and find out what's going on here within IBM, and repost
these patches once there's a straight story about how this bit works.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]

On Fri, Feb 10, 2017 at 09:49:17AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Thu, Feb 09, 2017 at 04:04:04PM +0530, Nikunj A Dadhania wrote:
> >> POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
> >> and corresponding defines. Moreover, CA32 is set when CA is set and
> >> OV32 is set when OV is set, there is no need to have a new
> >> fields in the CPUPPCState structure.
> >> 
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >
> > Um.. I don't quite understand this.  If CA always has the same value
> > as CA32, what's the point?
> 
> I am not clear either. I think that as CA was set for both 32/64-bit
> mode, that couldn't be changed for backward compatibility. CA32 should
> have affected only the instructions working one word variants.
> 
> Re-scanning the ISA 3.0, found this in 3.3.9 Fixed-Point Arithmetic
> Instructions:
> 
> =================================================================
> addic, addic., subfic, addc, subfc, adde, subfe,
> addme, subfme, addze, and subfze always set CA, to
> reflect the carry out of bit 0 in 64-bit mode and out of bit
> 32 in 32-bit mode. These instructions also always set
> CA32 to reflect the carry out of bit 32.
> =================================================================
> 
> Which is conflicting to what is said in 3.2.2 Fixed-Point Exception
> Register:
> =================================================================
> Carry32 (CA32)
> CA32 is set whenever CA is set, and is set to
> the same value that CA is defined to be set to
> in 32-bit mode.
> =================================================================

Ok, I've had a look at the ISA and discussed this with Michael
Ellerman.  We think what's going on here is that it's using some
unfortunately unclear wording.  When it says "OV32 is set when OV is
set" we think that means "OV32 is updated when OV is updated", not
that "OV32 is set to the same value as OV".

So although they're updated at the same time, the 32-bit variants can
have different values and will need real representation in the CPU
model.  Well, at least in 64-bit mode.  When the CPU is in 32-bit
mode, I believe they really will have the same values.

That would make your implementation suggested here incorrect.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Fri, Feb 10, 2017 at 09:49:17AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>> > [ Unknown signature status ]
>> > On Thu, Feb 09, 2017 at 04:04:04PM +0530, Nikunj A Dadhania wrote:
>> >> POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
>> >> and corresponding defines. Moreover, CA32 is set when CA is set and
>> >> OV32 is set when OV is set, there is no need to have a new
>> >> fields in the CPUPPCState structure.
>> >> 
>> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> >
>> > Um.. I don't quite understand this.  If CA always has the same value
>> > as CA32, what's the point?
>> 
>> I am not clear either. I think that as CA was set for both 32/64-bit
>> mode, that couldn't be changed for backward compatibility. CA32 should
>> have affected only the instructions working one word variants.
>> 
>> Re-scanning the ISA 3.0, found this in 3.3.9 Fixed-Point Arithmetic
>> Instructions:
>> 
>> =================================================================
>> addic, addic., subfic, addc, subfc, adde, subfe,
>> addme, subfme, addze, and subfze always set CA, to
>> reflect the carry out of bit 0 in 64-bit mode and out of bit
>> 32 in 32-bit mode. These instructions also always set
>> CA32 to reflect the carry out of bit 32.
>> =================================================================
>> 
>> Which is conflicting to what is said in 3.2.2 Fixed-Point Exception
>> Register:
>> =================================================================
>> Carry32 (CA32)
>> CA32 is set whenever CA is set, and is set to
>> the same value that CA is defined to be set to
>> in 32-bit mode.
>> =================================================================
>
> Ok, I've had a look at the ISA and discussed this with Michael
> Ellerman.  We think what's going on here is that it's using some
> unfortunately unclear wording.  When it says "OV32 is set when OV is
> set" we think that means "OV32 is updated when OV is updated", not
> that "OV32 is set to the same value as OV".
>
> So although they're updated at the same time, the 32-bit variants can
> have different values and will need real representation in the CPU
> model.  Well, at least in 64-bit mode.  When the CPU is in 32-bit
> mode, I believe they really will have the same values.
>
> That would make your implementation suggested here incorrect.

Yes, you are right. I had a discussion with Paul Mackerras yesterday, he
explained to me in detail about the bits. I am working on the revised
implementation. Will detail it in the commit message.

Regards
Nikunj

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Richard Henderson]

On 02/14/2017 02:05 PM, Nikunj A Dadhania wrote:
> Yes, you are right. I had a discussion with Paul Mackerras yesterday, he
> explained to me in detail about the bits. I am working on the revised
> implementation. Will detail it in the commit message.

As you're working on this, consider changing the definition of cpu_ov such that 
the MSB is OV and bit 31 is OV32.

E.g.


  static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
                                             TCGv arg1, TCGv arg2, int sub)
  {
      TCGv t0 = tcg_temp_new();

      tcg_gen_xor_tl(cpu_ov, arg0, arg2);
      tcg_gen_xor_tl(t0, arg1, arg2);
      if (sub) {
          tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
      } else {
          tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
      }
      tcg_temp_free(t0);
      if (NARROW_MODE(ctx)) {
          tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
      }
-    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
      tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
  }


is all that is required for arithmetic to compute OV and OV32 into those two bits.

Obviously more is required for multiplication and division, and you'd also have 
to change cpu_so to examine the MSB as well.


r~

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/14/2017 02:05 PM, Nikunj A Dadhania wrote:
>> Yes, you are right. I had a discussion with Paul Mackerras yesterday, he
>> explained to me in detail about the bits. I am working on the revised
>> implementation. Will detail it in the commit message.
>
> As you're working on this, consider changing the definition of cpu_ov such that 
> the MSB is OV and bit 31 is OV32.
>
> E.g.
>
>
>   static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>                                              TCGv arg1, TCGv arg2, int sub)
>   {
>       TCGv t0 = tcg_temp_new();
>
>       tcg_gen_xor_tl(cpu_ov, arg0, arg2);
>       tcg_gen_xor_tl(t0, arg1, arg2);
>       if (sub) {
>           tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
>       } else {
>           tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>       }
>       tcg_temp_free(t0);
>       if (NARROW_MODE(ctx)) {
>           tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>       }
> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>       tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>   }
>
>
> is all that is required for arithmetic to compute OV and OV32 into those two bits.

How about the below?

@@ -809,10 +809,11 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
         tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
     }
     tcg_temp_free(t0);
+    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
+    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
     if (NARROW_MODE(ctx)) {
-        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
+        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
     }
-    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
     tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
 }

Regards
Nikunj

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Richard Henderson]

On 02/16/2017 04:08 PM, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 02/14/2017 02:05 PM, Nikunj A Dadhania wrote:
>>> Yes, you are right. I had a discussion with Paul Mackerras yesterday, he
>>> explained to me in detail about the bits. I am working on the revised
>>> implementation. Will detail it in the commit message.
>>
>> As you're working on this, consider changing the definition of cpu_ov such that
>> the MSB is OV and bit 31 is OV32.
>>
>> E.g.
>>
>>
>>   static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>>                                              TCGv arg1, TCGv arg2, int sub)
>>   {
>>       TCGv t0 = tcg_temp_new();
>>
>>       tcg_gen_xor_tl(cpu_ov, arg0, arg2);
>>       tcg_gen_xor_tl(t0, arg1, arg2);
>>       if (sub) {
>>           tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
>>       } else {
>>           tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>>       }
>>       tcg_temp_free(t0);
>>       if (NARROW_MODE(ctx)) {
>>           tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>>       }
>> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>>       tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>>   }
>>
>>
>> is all that is required for arithmetic to compute OV and OV32 into those two bits.
>
> How about the below?
>
> @@ -809,10 +809,11 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>          tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>      }
>      tcg_temp_free(t0);
> +    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
> +    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
>      if (NARROW_MODE(ctx)) {
> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
> +        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
>      }
> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>      tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>  }

Why do you want to extract these bits?


r~

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/16/2017 04:08 PM, Nikunj A Dadhania wrote:
>> Richard Henderson <rth@twiddle.net> writes:
>>
>>> On 02/14/2017 02:05 PM, Nikunj A Dadhania wrote:
>>>> Yes, you are right. I had a discussion with Paul Mackerras yesterday, he
>>>> explained to me in detail about the bits. I am working on the revised
>>>> implementation. Will detail it in the commit message.
>>>
>>> As you're working on this, consider changing the definition of cpu_ov such that
>>> the MSB is OV and bit 31 is OV32.
>>>
>>> E.g.
>>>
>>>
>>>   static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>>>                                              TCGv arg1, TCGv arg2, int sub)
>>>   {
>>>       TCGv t0 = tcg_temp_new();
>>>
>>>       tcg_gen_xor_tl(cpu_ov, arg0, arg2);
>>>       tcg_gen_xor_tl(t0, arg1, arg2);
>>>       if (sub) {
>>>           tcg_gen_and_tl(cpu_ov, cpu_ov, t0);
>>>       } else {
>>>           tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>>>       }
>>>       tcg_temp_free(t0);
>>>       if (NARROW_MODE(ctx)) {
>>>           tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>>>       }
>>> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>>>       tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>>>   }
>>>
>>>
>>> is all that is required for arithmetic to compute OV and OV32 into those two bits.
>>
>> How about the below?
>>
>> @@ -809,10 +809,11 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>>          tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
>>      }
>>      tcg_temp_free(t0);
>> +    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
>> +    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
>>      if (NARROW_MODE(ctx)) {
>> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>> +        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
>>      }
>> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>>      tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>>  }
>
> Why do you want to extract these bits?

Convinient to copy that to XER later.

Regards
Nikunj

Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

[Richard Henderson]

On 02/17/2017 03:47 PM, Nikunj A Dadhania wrote:
>> Why do you want to extract these bits?
>
> Convinient to copy that to XER later.

Ideally you make the most common operation cheapest, and the more rare 
operation more expensive.  That said, I suppose even using ADDO is rare in the 
first place.  So it probably doesn't much matter.


r~