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

[Qemu-devel] [V2 PATCH 0/9] 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.

V2: Addressing review comments from Alex Graf:
    (1) Refactored user-mode and Little Endian checks in the load and store
        quadword instructions.
    (2) Added reserve_val2 element to CPU state in support of quadword atomic
        instructions.

Tom Musta (9):
  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: Add is_user_mode Utility Routine
  target-ppc: Load Quadword
  target-ppc: Store Quadword
  target-ppc: Add Load Quadword and Reserve
  target-ppc: Add Store Quadword Conditional

 linux-user/main.c           |   18 +++++-
 target-ppc/cpu.h            |   10 ++-
 target-ppc/translate.c      |  160 ++++++++++++++++++++++++++++++++++---------
 target-ppc/translate_init.c |   19 ++++-
 4 files changed, 168 insertions(+), 39 deletions(-)

[Qemu-devel] [V2 PATCH 1/9] 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] [V2 PATCH 2/9] 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] [V2 PATCH 3/9] 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 d9abcca..b89b4ba 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3745,6 +3745,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)
 {
@@ -3753,10 +3754,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 {
@@ -3838,6 +3841,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)                                       \
@@ -9569,6 +9577,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

[Qemu-devel] [V2 PATCH 4/9] 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] [V2 PATCH 5/9] target-ppc: Add is_user_mode Utility Routine

[Tom Musta]

This patch adds a boolean function is_user_mode that can be re-used
in translation code that is sensitive to the MSR[PR] (user-mode)
state.

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

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index b89b4ba..6ec4127 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -622,6 +622,20 @@ static opc_handler_t invalid_handler = {
     .handler = gen_invalid,
 };
 
+#if defined(TARGET_PPC64)
+/* NOTE: as this time, the only use of is_user_mode() is in 64 bit code.  And */
+/*       so the function is wrapped in the standard 64-bit ifdef in order to  */
+/*       avoid compiler warnings in 32-bit implementations.                   */
+static bool is_user_mode(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    return true;
+#else
+    return ctx->mem_idx == 0;
+#endif
+}
+#endif
+
 /***                           Integer comparison                          ***/
 
 static inline void gen_op_cmp(TCGv arg0, TCGv arg1, int s, int crf)
-- 
1.7.1

[Qemu-devel] [V2 PATCH 6/9] 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>
---
V2: Refactored the user-mode and LE checks per Alex Graf's review.

 target-ppc/translate.c      |   40 ++++++++++++++++++++++++----------------
 target-ppc/translate_init.c |    2 +-
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 6ec4127..e36686e 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2872,36 +2872,44 @@ static void gen_ld(DisasContext *ctx)
 /* lq */
 static void gen_lq(DisasContext *ctx)
 {
-#if defined(CONFIG_USER_ONLY)
-    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-#else
-    int ra, rd;
-    TCGv EA;
+    /* lq is a legal user mode instruction starting in ISA 2.07 */
+    bool legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
+    bool le_is_supported = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
 
-    /* Restore CPU state */
-    if (unlikely(ctx->mem_idx == 0)) {
+    if (!legal_in_user_mode && is_user_mode(ctx)) {
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
+
+    if (!le_is_supported && ctx->le_mode) {
+        gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
+        return;
+    }
+
+    int ra, rd;
+    TCGv EA;
+
     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] [V2 PATCH 6/9] target-ppc: Load Quadword

[Alexander Graf]

On 31.01.2014, at 20:34, 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>
> ---
> V2: Refactored the user-mode and LE checks per Alex Graf's review.
> 
> target-ppc/translate.c      |   40 ++++++++++++++++++++++++----------------
> target-ppc/translate_init.c |    2 +-
> 2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 6ec4127..e36686e 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -2872,36 +2872,44 @@ static void gen_ld(DisasContext *ctx)
> /* lq */
> static void gen_lq(DisasContext *ctx)
> {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -#else
> -    int ra, rd;
> -    TCGv EA;
> +    /* lq is a legal user mode instruction starting in ISA 2.07 */
> +    bool legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
> +    bool le_is_supported = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
> 
> -    /* Restore CPU state */
> -    if (unlikely(ctx->mem_idx == 0)) {
> +    if (!legal_in_user_mode && is_user_mode(ctx)) {
>         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>         return;
>     }
> +
> +    if (!le_is_supported && ctx->le_mode) {
> +        gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
> +        return;
> +    }
> +
> +    int ra, rd;
> +    TCGv EA;

I would've expected the compiler to emit a warning at this point because you're declaring variables outside of the beginning of a scope?


Alex

> +
>     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] [V2 PATCH 6/9] target-ppc: Load Quadword

[Tom Musta]

On 2/5/2014 3:43 AM, Alexander Graf wrote:
> I would've expected the compiler to emit a warning at this point because you're declaring variables outside of the beginning of a scope?

I did not see a warning on either of my test platforms (PowerPC/RHEL/GCC 4.4.7, Intel/Ubuntu/GCC 4.6.3).  But I will certainly fix.

Re: [Qemu-devel] [Qemu-ppc] [V2 PATCH 6/9] target-ppc: Load Quadword

[Eric Blake]

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

On 02/05/2014 09:12 AM, Tom Musta wrote:
> On 2/5/2014 3:43 AM, Alexander Graf wrote:
>> I would've expected the compiler to emit a warning at this point because you're declaring variables outside of the beginning of a scope?
>

Only if you enforce C89 compilation.  But qemu requires a C99 compiler,
where it is not a warning in gcc unless you enable -Wjump-misses-init
and have a goto statement that bypasses the declaration.

> I did not see a warning on either of my test platforms (PowerPC/RHEL/GCC 4.4.7, Intel/Ubuntu/GCC 4.6.3).  But I will certainly fix.

Whether or not the compiler enforces it, some projects still like to
stick to C89 declarations-before-statements, even when otherwise
requiring C99 compiler features.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [V2 PATCH 7/9] 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>
---
V2: Refactored user-mode and Little Endian checks per Alex Graf's review.

 target-ppc/translate.c |   39 +++++++++++++++++++++++----------------
 1 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index e36686e..c33d77c 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2995,34 +2995,41 @@ static void gen_std(DisasContext *ctx)
     TCGv EA;
 
     rs = rS(ctx->opcode);
-    if ((ctx->opcode & 0x3) == 0x2) {
-#if defined(CONFIG_USER_ONLY)
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-#else
-        /* stq */
-        if (unlikely(ctx->mem_idx == 0)) {
+    if ((ctx->opcode & 0x3) == 0x2) { /* stq */
+
+        bool legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
+        bool le_is_supported = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
+
+        if (!legal_in_user_mode && is_user_mode(ctx)) {
             gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
             return;
         }
-        if (unlikely(rs & 1)) {
-            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+
+        if (!le_is_supported && ctx->le_mode) {
+            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
             return;
         }
-        if (unlikely(ctx->le_mode)) {
-            /* Little-endian mode is not handled */
-            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
+
+        if (unlikely(rs & 1)) {
+            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
             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] [V2 PATCH 8/9] 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>
---
V2: Additional changes for handling reserve_val2.

 target-ppc/cpu.h       |    1 +
 target-ppc/translate.c |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index b66dd44..214afd9 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -926,6 +926,7 @@ struct CPUPPCState {
     target_ulong reserve_addr;
     /* Reservation value */
     target_ulong reserve_val;
+    target_ulong reserve_val2;
     /* Reservation store address */
     target_ulong reserve_ea;
     /* Reserved store source register and size */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c33d77c..8257dea 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3359,6 +3359,40 @@ 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));
+    tcg_gen_st_tl(gpr2, cpu_env, offsetof(CPUPPCState, reserve_val2));
+
+    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) */
@@ -9598,6 +9632,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

[Qemu-devel] [V2 PATCH 9/9] 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>
---
V2: Updated linux-user/main.c to use the newly added reserve_val2.

 linux-user/main.c      |   18 +++++++++++++++++-
 target-ppc/translate.c |   21 +++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index cabc9e1..6330427 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1490,7 +1490,7 @@ static int do_store_exclusive(CPUPPCState *env)
 {
     target_ulong addr;
     target_ulong page_addr;
-    target_ulong val;
+    target_ulong val, val2;
     int flags;
     int segv = 0;
 
@@ -1513,6 +1513,13 @@ static int do_store_exclusive(CPUPPCState *env)
             case 4: segv = get_user_u32(val, addr); break;
 #if defined(TARGET_PPC64)
             case 8: segv = get_user_u64(val, addr); break;
+            case 16: {
+                segv = get_user_u64(val, addr);
+                if (!segv) {
+                    segv = get_user_u64(val, addr + 8);
+                }
+                break;
+            }
 #endif
             default: abort();
             }
@@ -1524,6 +1531,15 @@ static int do_store_exclusive(CPUPPCState *env)
                 case 4: segv = put_user_u32(val, addr); break;
 #if defined(TARGET_PPC64)
                 case 8: segv = put_user_u64(val, addr); break;
+                case 16: {
+                    if (val2 == env->reserve_val2) {
+                        segv = put_user_u64(val, addr);
+                        if (!segv) {
+                            segv = put_user_u64(val2, addr + 8);
+                        }
+                    }
+                    break;
+                }
 #endif
                 default: abort();
                 }
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 8257dea..c883680 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3329,6 +3329,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);
     }
@@ -3341,6 +3355,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);                          \
@@ -3395,6 +3414,7 @@ static void gen_lqarx(DisasContext *ctx)
 
 /* stdcx. */
 STCX(stdcx_, 8);
+STCX(stqcx_, 16);
 #endif /* defined(TARGET_PPC64) */
 
 /* sync */
@@ -9634,6 +9654,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

Re: [Qemu-devel] [Qemu-ppc] [V2 PATCH 9/9] target-ppc: Add Store Quadword Conditional

[Alexander Graf]

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

> 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>
> ---
> V2: Updated linux-user/main.c to use the newly added reserve_val2.
> 
> linux-user/main.c      |   18 +++++++++++++++++-
> target-ppc/translate.c |   21 +++++++++++++++++++++
> 2 files changed, 38 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index cabc9e1..6330427 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -1490,7 +1490,7 @@ static int do_store_exclusive(CPUPPCState *env)
> {
>     target_ulong addr;
>     target_ulong page_addr;
> -    target_ulong val;
> +    target_ulong val, val2;
>     int flags;
>     int segv = 0;
> 
> @@ -1513,6 +1513,13 @@ static int do_store_exclusive(CPUPPCState *env)
>             case 4: segv = get_user_u32(val, addr); break;
> #if defined(TARGET_PPC64)
>             case 8: segv = get_user_u64(val, addr); break;
> +            case 16: {
> +                segv = get_user_u64(val, addr);
> +                if (!segv) {
> +                    segv = get_user_u64(val, addr + 8);
> +                }
> +                break;
> +            }

Does this work? After this val contains reserve_val2, but further down that one gets checked against reserve_val so the comparison is broken, no?


Alex

> #endif
>             default: abort();
>             }
> @@ -1524,6 +1531,15 @@ static int do_store_exclusive(CPUPPCState *env)
>                 case 4: segv = put_user_u32(val, addr); break;
> #if defined(TARGET_PPC64)
>                 case 8: segv = put_user_u64(val, addr); break;
> +                case 16: {
> +                    if (val2 == env->reserve_val2) {
> +                        segv = put_user_u64(val, addr);
> +                        if (!segv) {
> +                            segv = put_user_u64(val2, addr + 8);
> +                        }
> +                    }
> +                    break;
> +                }
> #endif
>                 default: abort();
>                 }
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 8257dea..c883680 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3329,6 +3329,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);
>     }
> @@ -3341,6 +3355,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);                          \
> @@ -3395,6 +3414,7 @@ static void gen_lqarx(DisasContext *ctx)
> 
> /* stdcx. */
> STCX(stdcx_, 8);
> +STCX(stqcx_, 16);
> #endif /* defined(TARGET_PPC64) */
> 
> /* sync */
> @@ -9634,6 +9654,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
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [V2 PATCH 9/9] target-ppc: Add Store Quadword Conditional

[Tom Musta]

On 2/5/2014 3:48 AM, Alexander Graf wrote:
> Does this work? After this val contains reserve_val2, but further down that one gets checked against reserve_val so the comparison is broken, no?

Yes.  I believe you are correct.  Thanks, Alex.