[Qemu-devel] [PATCH 0/8] target-ppc: Base ISA V2.07 for Power8

[Qemu-devel] [PATCH 0/8] target-ppc: Base ISA V2.07 for Power8

[Tom Musta]

This patch series adds the branch and integer instructions that were 
introduced in Power ISA 2.07.  Specifically,

  - There is a new conditional Branch to Address Register (bctar) instruction.
  - The load/store quadword instructions are now supported in user mode (Book I).
  - Quadword atomic instructions have been added (lqarx, stqcx.).

ISA 2.07 additions for other categories (VSX, Altivec, Decimal Floating Point,
transactional memory) are not included in this patch series; they will be 
contributed via other patches.

Tom Musta (8):
  target-ppc: Add Flag for bctar
  target-ppc: Add Target Address SPR (TAR) to Power8
  target-ppc: Add bctar Instruction
  target-ppc: Add Flag for ISA 2.07 Load/Store Quadword Instructions
  target-ppc: Load Quadword
  target-ppc: Store Quadword
  target-ppc: Add Load Quadword and Reserve
  target-ppc: Add Store Quadword Conditional

 target-ppc/cpu.h            |    9 ++-
 target-ppc/translate.c      |  154 ++++++++++++++++++++++++++++++++++---------
 target-ppc/translate_init.c |   19 +++++-
 3 files changed, 146 insertions(+), 36 deletions(-)

[Qemu-devel] [PATCH 1/8] target-ppc: Add Flag for bctar

[Tom Musta]

This patch adds a flag for the bctar instruction.  This instruction
is being introduced via Power ISA 2.07.

Also, the flag is added to the Power8 machine model since the P8
processor supports this instruction.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/cpu.h            |    6 ++++--
 target-ppc/translate_init.c |    2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2b8c205..b9d6b10 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1887,12 +1887,14 @@ enum {
     PPC2_FP_CVT_ISA206 = 0x0000000000000400ULL,
     /* ISA 2.06B floating point test instructions                            */
     PPC2_FP_TST_ISA206 = 0x0000000000000800ULL,
-
+    /* ISA 2.07 bctar instruction                                            */
+    PPC2_BCTAR_ISA207  = 0x0000000000001000ULL,
 
 #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
                         PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
                         PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
-                        PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206)
+                        PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | \
+                        PPC2_BCTAR_ISA207)
 };
 
 /*****************************************************************************/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a83c964..62bb200 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7327,7 +7327,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
                         PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
                         PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
-                        PPC2_FP_TST_ISA206;
+                        PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207;
     pcc->msr_mask = 0x800000000284FF36ULL;
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
-- 
1.7.1

[Qemu-devel] [PATCH 2/8] target-ppc: Add Target Address SPR (TAR) to Power8

[Tom Musta]

This patch adds support for the Target Address Register (TAR) to the Power8
model.

Because supported SPRs are typically identified in an init_proc_*()
function and because the Power8 model is currently just using the
init_proc_POWER7() function, a new init_proc_POWER8() function
is added and plugged into the P8 model.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/cpu.h            |    1 +
 target-ppc/translate_init.c |   14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index b9d6b10..810cf6a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1508,6 +1508,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
 #define SPR_RCPU_L2U_RA2      (0x32A)
 #define SPR_MPC_MD_DBRAM1     (0x32A)
 #define SPR_RCPU_L2U_RA3      (0x32B)
+#define SPR_TAR               (0x32F)
 #define SPR_440_INV0          (0x370)
 #define SPR_440_INV1          (0x371)
 #define SPR_440_INV2          (0x372)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 62bb200..9dd6684 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7301,6 +7301,18 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
     pcc->l1_icache_size = 0x8000;
 }
 
+static void init_proc_POWER8(CPUPPCState *env)
+{
+    /* inherit P7 */
+    init_proc_POWER7(env);
+
+    /* P8 supports the TAR */
+    spr_register(env, SPR_TAR, "TAR",
+                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+}
+
 POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -7310,7 +7322,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     dc->desc = "POWER8";
     pcc->pvr = CPU_POWERPC_POWER8_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
-    pcc->init_proc = init_proc_POWER7;
+    pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
-- 
1.7.1

[Qemu-devel] [PATCH 3/8] target-ppc: Add bctar Instruction

[Tom Musta]

This patch adds the Branch Conditional to Address Register (bctar)
instruction.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index f245946..90cbb72 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3748,6 +3748,7 @@ static void gen_b(DisasContext *ctx)
 #define BCOND_IM  0
 #define BCOND_LR  1
 #define BCOND_CTR 2
+#define BCOND_TAR 3
 
 static inline void gen_bcond(DisasContext *ctx, int type)
 {
@@ -3756,10 +3757,12 @@ static inline void gen_bcond(DisasContext *ctx, int type)
     TCGv target;
 
     ctx->exception = POWERPC_EXCP_BRANCH;
-    if (type == BCOND_LR || type == BCOND_CTR) {
+    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
         target = tcg_temp_local_new();
         if (type == BCOND_CTR)
             tcg_gen_mov_tl(target, cpu_ctr);
+        else if (type == BCOND_TAR)
+            gen_load_spr(target, SPR_TAR);
         else
             tcg_gen_mov_tl(target, cpu_lr);
     } else {
@@ -3841,6 +3844,11 @@ static void gen_bclr(DisasContext *ctx)
     gen_bcond(ctx, BCOND_LR);
 }
 
+static void gen_bctar(DisasContext *ctx)
+{
+    gen_bcond(ctx, BCOND_TAR);
+}
+
 /***                      Condition register logical                       ***/
 #define GEN_CRLOGIC(name, tcg_op, opc)                                        \
 static void glue(gen_, name)(DisasContext *ctx)                                       \
@@ -9540,6 +9548,7 @@ 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),
 GEN_HANDLER(bclr, 0x13, 0x10, 0x00, 0x00000000, PPC_FLOW),
+GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0, PPC_NONE, PPC2_BCTAR_ISA207),
 GEN_HANDLER(mcrf, 0x13, 0x00, 0xFF, 0x00000001, PPC_INTEGER),
 GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_FLOW),
 #if defined(TARGET_PPC64)
-- 
1.7.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/8] target-ppc: Add bctar Instruction

[Alexander Graf]

On 27.01.2014, at 18:54, Tom Musta <tommusta@gmail.com> wrote:

> This patch adds the Branch Conditional to Address Register (bctar)
> instruction.
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
> target-ppc/translate.c |   11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index f245946..90cbb72 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3748,6 +3748,7 @@ static void gen_b(DisasContext *ctx)
> #define BCOND_IM  0
> #define BCOND_LR  1
> #define BCOND_CTR 2
> +#define BCOND_TAR 3
> 
> static inline void gen_bcond(DisasContext *ctx, int type)
> {
> @@ -3756,10 +3757,12 @@ static inline void gen_bcond(DisasContext *ctx, int type)
>     TCGv target;
> 
>     ctx->exception = POWERPC_EXCP_BRANCH;
> -    if (type == BCOND_LR || type == BCOND_CTR) {
> +    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
>         target = tcg_temp_local_new();
>         if (type == BCOND_CTR)
>             tcg_gen_mov_tl(target, cpu_ctr);
> +        else if (type == BCOND_TAR)
> +            gen_load_spr(target, SPR_TAR);

How frequently is this used in generated code? Would it make sense to make it a global TCG variable?


Alex

>         else
>             tcg_gen_mov_tl(target, cpu_lr);
>     } else {
> @@ -3841,6 +3844,11 @@ static void gen_bclr(DisasContext *ctx)
>     gen_bcond(ctx, BCOND_LR);
> }
> 
> +static void gen_bctar(DisasContext *ctx)
> +{
> +    gen_bcond(ctx, BCOND_TAR);
> +}
> +
> /***                      Condition register logical                       ***/
> #define GEN_CRLOGIC(name, tcg_op, opc)                                        \
> static void glue(gen_, name)(DisasContext *ctx)                                       \
> @@ -9540,6 +9548,7 @@ 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),
> GEN_HANDLER(bclr, 0x13, 0x10, 0x00, 0x00000000, PPC_FLOW),
> +GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0, PPC_NONE, PPC2_BCTAR_ISA207),
> GEN_HANDLER(mcrf, 0x13, 0x00, 0xFF, 0x00000001, PPC_INTEGER),
> GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_FLOW),
> #if defined(TARGET_PPC64)
> -- 
> 1.7.1
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/8] target-ppc: Add bctar Instruction

[Tom Musta]

On 1/27/2014 12:46 PM, Alexander Graf wrote:
>> static inline void gen_bcond(DisasContext *ctx, int type)
>> > {
>> > @@ -3756,10 +3757,12 @@ static inline void gen_bcond(DisasContext *ctx, int type)
>> >     TCGv target;
>> > 
>> >     ctx->exception = POWERPC_EXCP_BRANCH;
>> > -    if (type == BCOND_LR || type == BCOND_CTR) {
>> > +    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
>> >         target = tcg_temp_local_new();
>> >         if (type == BCOND_CTR)
>> >             tcg_gen_mov_tl(target, cpu_ctr);
>> > +        else if (type == BCOND_TAR)
>> > +            gen_load_spr(target, SPR_TAR);
> How frequently is this used in generated code? Would it make sense to make it a global TCG variable?
> 

I have not yet seen a case of this being generated by the newer compilers.  But it is certainly not difficult
or much more code to make it be a global.

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/8] target-ppc: Add bctar Instruction

[Alexander Graf]

On 27.01.2014, at 20:34, Tom Musta <tommusta@gmail.com> wrote:

> On 1/27/2014 12:46 PM, Alexander Graf wrote:
>>> static inline void gen_bcond(DisasContext *ctx, int type)
>>>> {
>>>> @@ -3756,10 +3757,12 @@ static inline void gen_bcond(DisasContext *ctx, int type)
>>>>    TCGv target;
>>>> 
>>>>    ctx->exception = POWERPC_EXCP_BRANCH;
>>>> -    if (type == BCOND_LR || type == BCOND_CTR) {
>>>> +    if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
>>>>        target = tcg_temp_local_new();
>>>>        if (type == BCOND_CTR)
>>>>            tcg_gen_mov_tl(target, cpu_ctr);
>>>> +        else if (type == BCOND_TAR)
>>>> +            gen_load_spr(target, SPR_TAR);
>> How frequently is this used in generated code? Would it make sense to make it a global TCG variable?
>> 
> 
> I have not yet seen a case of this being generated by the newer compilers.  But it is certainly not difficult
> or much more code to make it be a global.

Well, we shouldn't waste a global on a register that doesn't get used frequently, so I'd say we leave it like this for now and change it to a global if / when we see it used often.


Alex

[Qemu-devel] [PATCH 4/8] target-ppc: Add Flag for ISA 2.07 Load/Store Quadword Instructions

[Tom Musta]

This patch adds a flag to identify the load/store quadword instructions
that are introduced with Power ISA 2.07.

The flag is added to the Power8 model since P8 supports these
instructions.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/cpu.h            |    4 +++-
 target-ppc/translate_init.c |    3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 810cf6a..b66dd44 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1890,12 +1890,14 @@ enum {
     PPC2_FP_TST_ISA206 = 0x0000000000000800ULL,
     /* ISA 2.07 bctar instruction                                            */
     PPC2_BCTAR_ISA207  = 0x0000000000001000ULL,
+    /* ISA 2.07 load/store quadword                                          */
+    PPC2_LSQ_ISA207    = 0x0000000000002000ULL,
 
 #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
                         PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
                         PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
                         PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | \
-                        PPC2_BCTAR_ISA207)
+                        PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207)
 };
 
 /*****************************************************************************/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 9dd6684..886238a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7339,7 +7339,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
                         PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
                         PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
-                        PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207;
+                        PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
+                        PPC2_LSQ_ISA207;
     pcc->msr_mask = 0x800000000284FF36ULL;
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
-- 
1.7.1

[Qemu-devel] [PATCH 5/8] target-ppc: Load Quadword

[Tom Musta]

This patch adds the Book I (user space) Load Quadword (lq) instruction.
This instruction was introduced into Book I in Power ISA V2.07.  Previous
versions of the architecture supported this as a privileged instruction.
Previous versions of the architecture also did not support Little Endian
mode.

Note that this patch also adds the PPC_64BX flag to the Power8 model,
which enables the lq instruction.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c      |   45 ++++++++++++++++++++++++++++--------------
 target-ppc/translate_init.c |    2 +-
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 90cbb72..15a4d1b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2861,36 +2861,51 @@ static void gen_ld(DisasContext *ctx)
 /* lq */
 static void gen_lq(DisasContext *ctx)
 {
+    /* lq is a legal user mode instruction starting in ISA 2.07 */
+    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
+
+    if (!legal_in_user_mode) {
 #if defined(CONFIG_USER_ONLY)
-    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+        return;
 #else
+        if (unlikely(ctx->mem_idx == 0)) {
+            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+            return;
+        }
+
+        if (unlikely(ctx->le_mode)) {
+            /* Little-endian mode is not handled */
+            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
+            return;
+        }
+#endif
+    }
+
     int ra, rd;
     TCGv EA;
 
-    /* Restore CPU state */
-    if (unlikely(ctx->mem_idx == 0)) {
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-        return;
-    }
     ra = rA(ctx->opcode);
     rd = rD(ctx->opcode);
     if (unlikely((rd & 1) || rd == ra)) {
         gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
         return;
     }
-    if (unlikely(ctx->le_mode)) {
-        /* Little-endian mode is not handled */
-        gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
-        return;
-    }
+
     gen_set_access_type(ctx, ACCESS_INT);
     EA = tcg_temp_new();
     gen_addr_imm_index(ctx, EA, 0x0F);
-    gen_qemu_ld64(ctx, cpu_gpr[rd], EA);
-    gen_addr_add(ctx, EA, EA, 8);
-    gen_qemu_ld64(ctx, cpu_gpr[rd+1], EA);
+
+    if (unlikely(ctx->le_mode)) {
+        gen_qemu_ld64(ctx, cpu_gpr[rd+1], EA);
+        gen_addr_add(ctx, EA, EA, 8);
+        gen_qemu_ld64(ctx, cpu_gpr[rd], EA);
+    } else {
+        gen_qemu_ld64(ctx, cpu_gpr[rd], EA);
+        gen_addr_add(ctx, EA, EA, 8);
+        gen_qemu_ld64(ctx, cpu_gpr[rd+1], EA);
+    }
     tcg_temp_free(EA);
-#endif
 }
 #endif
 
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 886238a..d7bcbba 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7333,7 +7333,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                        PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
                        PPC_MEM_SYNC | PPC_MEM_EIEIO |
                        PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
-                       PPC_64B | PPC_ALTIVEC |
+                       PPC_64B | PPC_64BX | PPC_ALTIVEC |
                        PPC_SEGMENT_64B | PPC_SLBI |
                        PPC_POPCNTB | PPC_POPCNTWD;
     pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
-- 
1.7.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword

[Alexander Graf]

On 27.01.2014, at 18:54, Tom Musta <tommusta@gmail.com> wrote:

> This patch adds the Book I (user space) Load Quadword (lq) instruction.
> This instruction was introduced into Book I in Power ISA V2.07.  Previous
> versions of the architecture supported this as a privileged instruction.
> Previous versions of the architecture also did not support Little Endian
> mode.
> 
> Note that this patch also adds the PPC_64BX flag to the Power8 model,
> which enables the lq instruction.
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
> target-ppc/translate.c      |   45 ++++++++++++++++++++++++++++--------------
> target-ppc/translate_init.c |    2 +-
> 2 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 90cbb72..15a4d1b 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -2861,36 +2861,51 @@ static void gen_ld(DisasContext *ctx)
> /* lq */
> static void gen_lq(DisasContext *ctx)
> {
> +    /* lq is a legal user mode instruction starting in ISA 2.07 */
> +    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
> +
> +    if (!legal_in_user_mode) {
> #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +        return;
> #else
> +        if (unlikely(ctx->mem_idx == 0)) {
> +            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +            return;
> +        }
> +
> +        if (unlikely(ctx->le_mode)) {

You're mixing two semantically separate things here. legal_in_user_mode doesn't really indicate that le_mode isn't usable. I'm sure if you just make this two if()'s with two separate bools that get assigned the same value gcc will be smart enough to optimize it just as well as this combined branch.

Reading through the above code we probably eventually want something like

static bool is_user_mode(DisasContext *ctx)
{
#if defined(CONFIG_USER_ONLY)
    return true;
#else
    return ctx->mem_idx == 0;
#endif
}

which would enable us to combine code like the above.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword

[Tom Musta]

On 1/27/2014 12:55 PM, Alexander Graf wrote:
> You're mixing two semantically separate things here. legal_in_user_mode doesn't really indicate that le_mode isn't usable. I'm sure if you just make this two if()'s with two separate bools that get assigned the same value gcc will be smart enough to optimize it just as well as this combined branch.
> 

Hmmm ... I'm not sure that I see the problem.  Perhaps the comment should be clearer.
And I guess there is really no need to compute the legal_in_user_mode flag since it
is only used once.

Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq when MSR[PR]=1
resulted in a privileged instruction exception.  Also, when MSR[PR]=0 and MSR[LE]=1, an
alignment exception was generated irrespective of the computed address.

Starting with ISA 2.07, both of these restrictions are lifted.  So the proposed code is
as follows:

static void gen_lq(DisasContext *ctx)
{
    /* lq is a legal user mode instruction starting in ISA 2.07 */
    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;

    if (!legal_in_user_mode) {
#if defined(CONFIG_USER_ONLY)
        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
        return;
#else
        if (unlikely(ctx->mem_idx == 0)) {
            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
            return;
        }

        if (unlikely(ctx->le_mode)) {
            /* Little-endian mode is not handled */
            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
            return;
        }
#endif
    }

    int ra, rd;
    TCGv EA;
    ... // rest of implementation


P.S.  I think there should be an alignment check after the EA is computed.

Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword

[Alexander Graf]

On 27.01.2014, at 20:53, Tom Musta <tommusta@gmail.com> wrote:

> On 1/27/2014 12:55 PM, Alexander Graf wrote:
>> You're mixing two semantically separate things here. legal_in_user_mode doesn't really indicate that le_mode isn't usable. I'm sure if you just make this two if()'s with two separate bools that get assigned the same value gcc will be smart enough to optimize it just as well as this combined branch.
>> 
> 
> Hmmm ... I'm not sure that I see the problem.  Perhaps the comment should be clearer.
> And I guess there is really no need to compute the legal_in_user_mode flag since it
> is only used once.
> 
> Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq when MSR[PR]=1
> resulted in a privileged instruction exception.  Also, when MSR[PR]=0 and MSR[LE]=1, an
> alignment exception was generated irrespective of the computed address.
> 
> Starting with ISA 2.07, both of these restrictions are lifted.  So the proposed code is
> as follows:
> 
> static void gen_lq(DisasContext *ctx)
> {
>    /* lq is a legal user mode instruction starting in ISA 2.07 */
>    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
> 
>    if (!legal_in_user_mode) {
> #if defined(CONFIG_USER_ONLY)
>        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>        return;
> #else
>        if (unlikely(ctx->mem_idx == 0)) {
>            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>            return;
>        }
> 
>        if (unlikely(ctx->le_mode)) {

Right, but the fact that "we're legal in user mode" has nothing to do with "we can handle LE mode". I was thinking of something along the lines of

{
    bool legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207);
    bool can_handle_le = (ctx->insns_flags2 & PPC2_LSQ_ISA207);

    if (!legal_in_user_mode && is_in_user_mode(ctx)) {
        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
        return;
    }

    if (!can_handle_le && ctx->le_mode) {
        gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
        return;
    }

    [...]
}

>            /* Little-endian mode is not handled */
>            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
>            return;
>        }
> #endif
>    }
> 
>    int ra, rd;
>    TCGv EA;
>    ... // rest of implementation
> 
> 
> P.S.  I think there should be an alignment check after the EA is computed.

I'm fairly sure this isn't the only place missing alignment checks :). But then again alignment checks are tricky because your host kernel may fix them up for you in linux only mode and in general they're not particularly useful.


Alex

[Qemu-devel] [PATCH 6/8] target-ppc: Store Quadword

[Tom Musta]

This patch adds support for the Store Quadword instruction in user mode.  Prior
to Power ISA 2.07, stq was legal only in privileged mode.  Support for Little
Endian mode is also new in ISA 2.07.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 15a4d1b..bb1dc82 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2991,34 +2991,47 @@ static void gen_std(DisasContext *ctx)
     TCGv EA;
 
     rs = rS(ctx->opcode);
-    if ((ctx->opcode & 0x3) == 0x2) {
+    if ((ctx->opcode & 0x3) == 0x2) { /* stq */
+        int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
+        if (!legal_in_user_mode) {
 #if defined(CONFIG_USER_ONLY)
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-#else
-        /* stq */
-        if (unlikely(ctx->mem_idx == 0)) {
             gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
             return;
+#else
+            if (unlikely(ctx->mem_idx == 0)) {
+                gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+                return;
+            }
+
+            if (unlikely(ctx->le_mode)) {
+                /* Little-endian mode is not handled */
+                gen_exception_err(ctx, POWERPC_EXCP_ALIGN,
+                                  POWERPC_EXCP_ALIGN_LE);
+                return;
+            }
+#endif
         }
+
         if (unlikely(rs & 1)) {
             gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
             return;
         }
-        if (unlikely(ctx->le_mode)) {
-            /* Little-endian mode is not handled */
-            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
-            return;
-        }
         gen_set_access_type(ctx, ACCESS_INT);
         EA = tcg_temp_new();
         gen_addr_imm_index(ctx, EA, 0x03);
-        gen_qemu_st64(ctx, cpu_gpr[rs], EA);
-        gen_addr_add(ctx, EA, EA, 8);
-        gen_qemu_st64(ctx, cpu_gpr[rs+1], EA);
+
+        if (unlikely(ctx->le_mode)) {
+            gen_qemu_st64(ctx, cpu_gpr[rs+1], EA);
+            gen_addr_add(ctx, EA, EA, 8);
+            gen_qemu_st64(ctx, cpu_gpr[rs], EA);
+        } else {
+            gen_qemu_st64(ctx, cpu_gpr[rs], EA);
+            gen_addr_add(ctx, EA, EA, 8);
+            gen_qemu_st64(ctx, cpu_gpr[rs+1], EA);
+        }
         tcg_temp_free(EA);
-#endif
     } else {
-        /* std / stdu */
+        /* std / stdu*/
         if (Rc(ctx->opcode)) {
             if (unlikely(rA(ctx->opcode) == 0)) {
                 gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
-- 
1.7.1

[Qemu-devel] [PATCH 7/8] target-ppc: Add Load Quadword and Reserve

[Tom Musta]

This patch adds the Load Quadword and Reserve (lqarx) instruction,
which is new in Power ISA 2.07.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index bb1dc82..589cee9 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3361,6 +3361,39 @@ STCX(stwcx_, 4);
 /* ldarx */
 LARX(ldarx, 8, ld64);
 
+/* lqarx */
+static void gen_lqarx(DisasContext *ctx)
+{
+    TCGv EA;
+    int rd = rD(ctx->opcode);
+    TCGv gpr1, gpr2;
+
+    if (unlikely((rd & 1) || (rd == rA(ctx->opcode)) ||
+                 (rd == rB(ctx->opcode)))) {
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+        return;
+    }
+
+    gen_set_access_type(ctx, ACCESS_RES);
+    EA = tcg_temp_local_new();
+    gen_addr_reg_index(ctx, EA);
+    gen_check_align(ctx, EA, 15);
+    if (unlikely(ctx->le_mode)) {
+        gpr1 = cpu_gpr[rd+1];
+        gpr2 = cpu_gpr[rd];
+    } else {
+        gpr1 = cpu_gpr[rd];
+        gpr2 = cpu_gpr[rd+1];
+    }
+    gen_qemu_ld64(ctx, gpr1, EA);
+    tcg_gen_mov_tl(cpu_reserve, EA);
+    tcg_gen_st_tl(gpr1, cpu_env, offsetof(CPUPPCState, reserve_val));
+
+    gen_addr_add(ctx, EA, EA, 8);
+    gen_qemu_ld64(ctx, gpr2, EA);
+    tcg_temp_free(EA);
+}
+
 /* stdcx. */
 STCX(stdcx_, 8);
 #endif /* defined(TARGET_PPC64) */
@@ -9568,6 +9601,7 @@ GEN_HANDLER_E(sthcx_, 0x1F, 0x16, 0x16, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
 GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x00000000, PPC_RES),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000000, PPC_64B),
+GEN_HANDLER_E(lqarx, 0x1F, 0x14, 0x08, 0, PPC_NONE, PPC2_LSQ_ISA207),
 GEN_HANDLER2(stdcx_, "stdcx.", 0x1F, 0x16, 0x06, 0x00000000, PPC_64B),
 #endif
 GEN_HANDLER(sync, 0x1F, 0x16, 0x12, 0x039FF801, PPC_MEM_SYNC),
-- 
1.7.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH 7/8] target-ppc: Add Load Quadword and Reserve

[Alexander Graf]

On 27.01.2014, at 18:54, Tom Musta <tommusta@gmail.com> wrote:

> This patch adds the Load Quadword and Reserve (lqarx) instruction,
> which is new in Power ISA 2.07.
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
> target-ppc/translate.c |   34 ++++++++++++++++++++++++++++++++++
> 1 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index bb1dc82..589cee9 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3361,6 +3361,39 @@ STCX(stwcx_, 4);
> /* ldarx */
> LARX(ldarx, 8, ld64);
> 
> +/* lqarx */
> +static void gen_lqarx(DisasContext *ctx)
> +{
> +    TCGv EA;
> +    int rd = rD(ctx->opcode);
> +    TCGv gpr1, gpr2;
> +
> +    if (unlikely((rd & 1) || (rd == rA(ctx->opcode)) ||
> +                 (rd == rB(ctx->opcode)))) {
> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> +        return;
> +    }
> +
> +    gen_set_access_type(ctx, ACCESS_RES);
> +    EA = tcg_temp_local_new();
> +    gen_addr_reg_index(ctx, EA);
> +    gen_check_align(ctx, EA, 15);
> +    if (unlikely(ctx->le_mode)) {
> +        gpr1 = cpu_gpr[rd+1];
> +        gpr2 = cpu_gpr[rd];
> +    } else {
> +        gpr1 = cpu_gpr[rd];
> +        gpr2 = cpu_gpr[rd+1];
> +    }
> +    gen_qemu_ld64(ctx, gpr1, EA);
> +    tcg_gen_mov_tl(cpu_reserve, EA);
> +    tcg_gen_st_tl(gpr1, cpu_env, offsetof(CPUPPCState, reserve_val));

I suppose it's ok to only store the first 64bits as reserved?


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH 7/8] target-ppc: Add Load Quadword and Reserve

[Tom Musta]

On 1/27/2014 12:59 PM, Alexander Graf wrote:
> 
> On 27.01.2014, at 18:54, Tom Musta <tommusta@gmail.com> wrote:
> 
>> This patch adds the Load Quadword and Reserve (lqarx) instruction,
>> which is new in Power ISA 2.07.
>>
>> Signed-off-by: Tom Musta <tommusta@gmail.com>
>> ---
>> target-ppc/translate.c |   34 ++++++++++++++++++++++++++++++++++
>> 1 files changed, 34 insertions(+), 0 deletions(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index bb1dc82..589cee9 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -3361,6 +3361,39 @@ STCX(stwcx_, 4);
>> /* ldarx */
>> LARX(ldarx, 8, ld64);
>>
>> +/* lqarx */
>> +static void gen_lqarx(DisasContext *ctx)
>> +{
>> +    TCGv EA;
>> +    int rd = rD(ctx->opcode);
>> +    TCGv gpr1, gpr2;
>> +
>> +    if (unlikely((rd & 1) || (rd == rA(ctx->opcode)) ||
>> +                 (rd == rB(ctx->opcode)))) {
>> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>> +        return;
>> +    }
>> +
>> +    gen_set_access_type(ctx, ACCESS_RES);
>> +    EA = tcg_temp_local_new();
>> +    gen_addr_reg_index(ctx, EA);
>> +    gen_check_align(ctx, EA, 15);
>> +    if (unlikely(ctx->le_mode)) {
>> +        gpr1 = cpu_gpr[rd+1];
>> +        gpr2 = cpu_gpr[rd];
>> +    } else {
>> +        gpr1 = cpu_gpr[rd];
>> +        gpr2 = cpu_gpr[rd+1];
>> +    }
>> +    gen_qemu_ld64(ctx, gpr1, EA);
>> +    tcg_gen_mov_tl(cpu_reserve, EA);
>> +    tcg_gen_st_tl(gpr1, cpu_env, offsetof(CPUPPCState, reserve_val));
> 
> I suppose it's ok to only store the first 64bits as reserved?
> 
> 
> Alex
> 

Thank you, Alex.  It looks like there is some interesting code in linux-user/main.c that is impacted by lqarx/stqcx.

[Qemu-devel] [PATCH 8/8] target-ppc: Add Store Quadword Conditional

[Tom Musta]

This patch adds the Store Quadword Conditionl (stqcx.) instruction
which is introduced in Power ISA 2.07.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 589cee9..c3ddb8e 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3331,6 +3331,20 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA,
         gen_qemu_st32(ctx, cpu_gpr[reg], EA);
     } else if (size == 2) {
         gen_qemu_st16(ctx, cpu_gpr[reg], EA);
+#if defined(TARGET_PPC64)
+    } else if (size == 16) {
+        TCGv gpr1, gpr2;
+        if (unlikely(ctx->le_mode)) {
+            gpr1 = cpu_gpr[reg+1];
+            gpr2 = cpu_gpr[reg];
+        } else {
+            gpr1 = cpu_gpr[reg];
+            gpr2 = cpu_gpr[reg+1];
+        }
+        gen_qemu_st64(ctx, gpr1, EA);
+        gen_addr_add(ctx, EA, EA, 8);
+        gen_qemu_st64(ctx, gpr2, EA);
+#endif
     } else {
         gen_qemu_st8(ctx, cpu_gpr[reg], EA);
     }
@@ -3343,6 +3357,11 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA,
 static void gen_##name(DisasContext *ctx)                 \
 {                                                         \
     TCGv t0;                                              \
+    if (unlikely((len == 16) && (rD(ctx->opcode) & 1))) { \
+        gen_inval_exception(ctx,                          \
+                            POWERPC_EXCP_INVAL_INVAL);    \
+        return;                                           \
+    }                                                     \
     gen_set_access_type(ctx, ACCESS_RES);                 \
     t0 = tcg_temp_local_new();                            \
     gen_addr_reg_index(ctx, t0);                          \
@@ -3396,6 +3415,7 @@ static void gen_lqarx(DisasContext *ctx)
 
 /* stdcx. */
 STCX(stdcx_, 8);
+STCX(stqcx_, 16);
 #endif /* defined(TARGET_PPC64) */
 
 /* sync */
@@ -9603,6 +9623,7 @@ GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x00000000, PPC_RES),
 GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000000, PPC_64B),
 GEN_HANDLER_E(lqarx, 0x1F, 0x14, 0x08, 0, PPC_NONE, PPC2_LSQ_ISA207),
 GEN_HANDLER2(stdcx_, "stdcx.", 0x1F, 0x16, 0x06, 0x00000000, PPC_64B),
+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),
-- 
1.7.1