[Qemu-devel] [PATCH v2 0/3] Adding new user mode target ppc64le-linux-user

[Qemu-devel] [PATCH v2 0/3] Adding new user mode target ppc64le-linux-user

[Doug Kwan]

Hi

I have made changes based on comments to the previous set of pathces.

1. Target name is now ppc64le-linux-user.
2. I used a memop mask to avoid generating redundant byteswap pairs that
   cancel out each other.  This is suggested by Peter Maydell.
3. There is code to handle machine name correctly in uname.

This is not mean to be final though.  Tom Musta of IBM kindly offered to help
me with this.  So I am handling over this set of patches.  Tom will continue
from this version.

Thanks for all the comments and suggestions so far.

-Doug

Doug Kwan (3):
  linux-user: Support little-endian PPC64 in user mode.
  PPC: Allow little-endian user mode.
  Add a new user mode target for little-endian PPC64.

 configure                              |   6 ++
 default-configs/ppc64le-linux-user.mak |   1 +
 include/elf.h                          |   5 ++
 linux-user/elfload.c                   |  17 +++-
 linux-user/uname.c                     |   6 ++
 target-ppc/mem_helper.c                |  25 +++++-
 target-ppc/translate.c                 | 150 +++++++++++++--------------------
 target-ppc/translate_init.c            |   9 ++
 8 files changed, 122 insertions(+), 97 deletions(-)
 create mode 100644 default-configs/ppc64le-linux-user.mak

-- 
1.9.1.423.g4596e3a

[Qemu-devel] [PATCH v2 1/3] linux-user: Support little-endian PPC64 in user mode.

[Doug Kwan]

Look at ELF header to determin ABI version on PPC64.  This is required
for executing the first instruction correctly.  Also print correct machine
name in uname() system call.

Signed-off-by: Doug Kwan <dougkwan@google.com>
---
 include/elf.h        |  5 +++++
 linux-user/elfload.c | 17 +++++++++++++++--
 linux-user/uname.c   |  6 ++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/include/elf.h b/include/elf.h
index 1599ab2..f9f1675 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -561,6 +561,11 @@ typedef struct {
 #define SHF_ALPHA_GPREL		0x10000000
 
 
+/* PowerPC specific definitions.  */
+
+/* Processor specific flags for the ELF header e_flags field.  */
+#define EF_PPC64_ABI		0x3
+
 /* PowerPC relocations defined by the ABIs */
 #define R_PPC_NONE		0
 #define R_PPC_ADDR32		1	/* 32bit absolute address */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 995f999..b96d64a 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -777,12 +777,18 @@ static uint32_t get_elf_hwcap(void)
         NEW_AUX_ENT(AT_IGNOREPPC, AT_IGNOREPPC);        \
     } while (0)
 
+static inline uint32_t get_ppc64_abi(struct image_info *infop);
+
 static inline void init_thread(struct target_pt_regs *_regs, struct image_info *infop)
 {
     _regs->gpr[1] = infop->start_stack;
 #if defined(TARGET_PPC64) && !defined(TARGET_ABI32)
-    _regs->gpr[2] = ldq_raw(infop->entry + 8) + infop->load_bias;
-    infop->entry = ldq_raw(infop->entry) + infop->load_bias;
+    if (get_ppc64_abi(infop) < 2) {
+      _regs->gpr[2] = ldq_raw(infop->entry + 8) + infop->load_bias;
+      infop->entry = ldq_raw(infop->entry) + infop->load_bias;
+    } else {
+      _regs->gpr[12] = infop->entry;  /* r12 set to global entry address */
+    }
 #endif
     _regs->nip = infop->entry;
 }
@@ -1152,6 +1158,13 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #include "elf.h"
 
+#ifdef TARGET_PPC
+static inline uint32_t get_ppc64_abi(struct image_info *infop)
+{
+  return infop->elf_flags & EF_PPC64_ABI;
+}
+#endif
+
 struct exec
 {
     unsigned int a_info;   /* Use macros N_MAGIC, etc for access */
diff --git a/linux-user/uname.c b/linux-user/uname.c
index f5d4c66..cb1f9a3 100644
--- a/linux-user/uname.c
+++ b/linux-user/uname.c
@@ -65,6 +65,12 @@ const char *cpu_to_uname_machine(void *cpu_env)
         return "i586";
     }
     return "i686";
+#elif defined(TARGET_PPC64)
+#ifdef TARGET_WORDS_BIGENDIAN
+    return UNAME_MACHINE;
+#else
+    return UNAME_MACHINE "le";
+#endif
 #else
     /* default is #define-d in each arch/ subdir */
     return UNAME_MACHINE;
-- 
1.9.1.423.g4596e3a

Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Support little-endian PPC64 in user mode.

[Peter Maydell]

On 10 May 2014 10:16, Doug Kwan <dougkwan@google.com> wrote:
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index f5d4c66..cb1f9a3 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -65,6 +65,12 @@ const char *cpu_to_uname_machine(void *cpu_env)
>          return "i586";
>      }
>      return "i686";
> +#elif defined(TARGET_PPC64)
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    return UNAME_MACHINE;
> +#else
> +    return UNAME_MACHINE "le";
> +#endif
>  #else
>      /* default is #define-d in each arch/ subdir */
>      return UNAME_MACHINE;

I think it would be nicer just to define UNAME_MACHINE correctly
in linux-user/ppc/syscall.h. We should restrict the per-CPU ifdefs
in uname.c to just those CPUs which need to pick the uname based
on runtime CPU features.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Support little-endian PPC64 in user mode.

[Peter Maydell]

On 10 May 2014 10:16, Doug Kwan <dougkwan@google.com> wrote:
> Look at ELF header to determin ABI version on PPC64.  This is required

typo: "determine".

> for executing the first instruction correctly.  Also print correct machine
> name in uname() system call.

thanks
-- PMM

[Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.

[Doug Kwan]

This allow running PPC64 little-endian in user mode if target is configured
that way.  In PPC64 LE user mode we set MSR.LE during initialization.
Overhaul handling of byteswapping in code generation and mem helpers.

Signed-off-by: Doug Kwan <dougkwan@google.com>
---
 target-ppc/mem_helper.c     |  25 ++++++--
 target-ppc/translate.c      | 150 +++++++++++++++++---------------------------
 target-ppc/translate_init.c |   9 +++
 3 files changed, 89 insertions(+), 95 deletions(-)

diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index f35ed03..0112821 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -28,6 +28,14 @@
 
 //#define DEBUG_OP
 
+static inline bool needs_byteswap(const CPUPPCState *env) {
+#if defined(TARGET_WORDS_BIGENDIAN)
+  return msr_le;
+#else
+  return !msr_le;
+#endif
+}
+
 /*****************************************************************************/
 /* Memory load and stores */
 
@@ -47,7 +55,7 @@ static inline target_ulong addr_add(CPUPPCState *env, target_ulong addr,
 void helper_lmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
     for (; reg < 32; reg++) {
-        if (msr_le) {
+        if (needs_byteswap(env)) {
             env->gpr[reg] = bswap32(cpu_ldl_data(env, addr));
         } else {
             env->gpr[reg] = cpu_ldl_data(env, addr);
@@ -59,7 +67,7 @@ void helper_lmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 void helper_stmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
     for (; reg < 32; reg++) {
-        if (msr_le) {
+        if (needs_byteswap(env)) {
             cpu_stl_data(env, addr, bswap32((uint32_t)env->gpr[reg]));
         } else {
             cpu_stl_data(env, addr, (uint32_t)env->gpr[reg]);
@@ -202,6 +210,11 @@ target_ulong helper_lscbx(CPUPPCState *env, target_ulong addr, uint32_t reg,
 #define LO_IDX 0
 #endif
 
+/* We use msr_le to determine index ordering in a vector.  However,
+   byteswapping is not simply controlled by msr_le.  We also need to take
+   into account endianness of the target.  This is done for the little-endian
+   PPC64 user-mode target. */ 
+
 #define LVE(name, access, swap, element)                        \
     void helper_##name(CPUPPCState *env, ppc_avr_t *r,          \
                        target_ulong addr)                       \
@@ -210,9 +223,11 @@ target_ulong helper_lscbx(CPUPPCState *env, target_ulong addr, uint32_t reg,
         int adjust = HI_IDX*(n_elems - 1);                      \
         int sh = sizeof(r->element[0]) >> 1;                    \
         int index = (addr & 0xf) >> sh;                         \
-                                                                \
         if (msr_le) {                                           \
             index = n_elems - index - 1;                        \
+        }                                                       \
+                                                                \
+        if (needs_byteswap(env)) {                              \
             r->element[LO_IDX ? index : (adjust - index)] =     \
                 swap(access(env, addr));                        \
         } else {                                                \
@@ -235,9 +250,11 @@ LVE(lvewx, cpu_ldl_data, bswap32, u32)
         int adjust = HI_IDX * (n_elems - 1);                            \
         int sh = sizeof(r->element[0]) >> 1;                            \
         int index = (addr & 0xf) >> sh;                                 \
-                                                                        \
         if (msr_le) {                                                   \
             index = n_elems - index - 1;                                \
+        }                                                               \
+                                                                        \
+        if (needs_byteswap(env)) {                                      \
             access(env, addr, swap(r->element[LO_IDX ? index :          \
                                               (adjust - index)]));      \
         } else {                                                        \
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index e3fcb03..d28ecc2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -196,6 +196,7 @@ typedef struct DisasContext {
     int access_type;
     /* Translation flags */
     int le_mode;
+    TCGMemOp default_tcg_memop_mask;
 #if defined(TARGET_PPC64)
     int sf_mode;
     int has_cfar;
@@ -210,6 +211,15 @@ typedef struct DisasContext {
     uint64_t insns_flags2;
 } DisasContext;
 
+/* Return true iff byteswap is needed in a scalar memop */
+static inline bool need_byteswap(const DisasContext *ctx) {
+#if defined(TARGET_WORDS_BIGENDIAN)
+     return ctx->le_mode;
+#else
+     return !ctx->le_mode;
+#endif
+}
+
 /* True when active word size < size of target_long.  */
 #ifdef TARGET_PPC64
 # define NARROW_MODE(C)  (!(C)->sf_mode)
@@ -2652,29 +2662,20 @@ static inline void gen_qemu_ld8s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 
 static inline void gen_qemu_ld16u(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
-    if (unlikely(ctx->le_mode)) {
-        tcg_gen_bswap16_tl(arg1, arg1);
-    }
+    TCGMemOp op = MO_UW | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static inline void gen_qemu_ld16s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
-        tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
-        tcg_gen_bswap16_tl(arg1, arg1);
-        tcg_gen_ext16s_tl(arg1, arg1);
-    } else {
-        tcg_gen_qemu_ld16s(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_SW | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static inline void gen_qemu_ld32u(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
-    if (unlikely(ctx->le_mode)) {
-        tcg_gen_bswap32_tl(arg1, arg1);
-    }
+    TCGMemOp op = MO_UL | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static void gen_qemu_ld32u_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
@@ -2687,12 +2688,8 @@ static void gen_qemu_ld32u_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
 
 static inline void gen_qemu_ld32s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
-        tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
-        tcg_gen_bswap32_tl(arg1, arg1);
-        tcg_gen_ext32s_tl(arg1, arg1);
-    } else
-        tcg_gen_qemu_ld32s(arg1, arg2, ctx->mem_idx);
+    TCGMemOp op = MO_SL | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static void gen_qemu_ld32s_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
@@ -2705,10 +2702,8 @@ static void gen_qemu_ld32s_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
 
 static inline void gen_qemu_ld64(DisasContext *ctx, TCGv_i64 arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld64(arg1, arg2, ctx->mem_idx);
-    if (unlikely(ctx->le_mode)) {
-        tcg_gen_bswap64_i64(arg1, arg1);
-    }
+    TCGMemOp op = MO_Q | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_ld_i64(arg1, arg2, ctx->mem_idx, op);
 }
 
 static inline void gen_qemu_st8(DisasContext *ctx, TCGv arg1, TCGv arg2)
@@ -2718,28 +2713,14 @@ static inline void gen_qemu_st8(DisasContext *ctx, TCGv arg1, TCGv arg2)
 
 static inline void gen_qemu_st16(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ext16u_tl(t0, arg1);
-        tcg_gen_bswap16_tl(t0, t0);
-        tcg_gen_qemu_st16(t0, arg2, ctx->mem_idx);
-        tcg_temp_free(t0);
-    } else {
-        tcg_gen_qemu_st16(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_UW | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static inline void gen_qemu_st32(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ext32u_tl(t0, arg1);
-        tcg_gen_bswap32_tl(t0, t0);
-        tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
-        tcg_temp_free(t0);
-    } else {
-        tcg_gen_qemu_st32(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_UL | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
 }
 
 static void gen_qemu_st32_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
@@ -2752,13 +2733,8 @@ static void gen_qemu_st32_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
 
 static inline void gen_qemu_st64(DisasContext *ctx, TCGv_i64 arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
-        TCGv_i64 t0 = tcg_temp_new_i64();
-        tcg_gen_bswap64_i64(t0, arg1);
-        tcg_gen_qemu_st64(t0, arg2, ctx->mem_idx);
-        tcg_temp_free_i64(t0);
-    } else
-        tcg_gen_qemu_st64(arg1, arg2, ctx->mem_idx);
+    TCGMemOp op = MO_Q | ctx->default_tcg_memop_mask;
+    tcg_gen_qemu_st_i64(arg1, arg2, ctx->mem_idx, op);
 }
 
 #define GEN_LD(name, ldop, opc, type)                                         \
@@ -2902,6 +2878,8 @@ static void gen_lq(DisasContext *ctx)
     EA = tcg_temp_new();
     gen_addr_imm_index(ctx, EA, 0x0F);
 
+    /* We only need to swap high and low halves. gen_qemu_ld64 does necessary
+       64-bit byteswap already. */
     if (unlikely(ctx->le_mode)) {
         gen_qemu_ld64(ctx, cpu_gpr[rd+1], EA);
         gen_addr_add(ctx, EA, EA, 8);
@@ -3020,6 +2998,8 @@ static void gen_std(DisasContext *ctx)
         EA = tcg_temp_new();
         gen_addr_imm_index(ctx, EA, 0x03);
 
+        /* We only need to swap high and low halves. gen_qemu_st64 does 
+           necessary 64-bit byteswap already. */
         if (unlikely(ctx->le_mode)) {
             gen_qemu_st64(ctx, cpu_gpr[rs+1], EA);
             gen_addr_add(ctx, EA, EA, 8);
@@ -3049,23 +3029,20 @@ static void gen_std(DisasContext *ctx)
 }
 #endif
 /***                Integer load and store with byte reverse               ***/
+
 /* lhbrx */
 static inline void gen_qemu_ld16ur(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
-    if (likely(!ctx->le_mode)) {
-        tcg_gen_bswap16_tl(arg1, arg1);
-    }
+    TCGMemOp op = MO_UW | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_LDX(lhbr, ld16ur, 0x16, 0x18, PPC_INTEGER);
 
 /* lwbrx */
 static inline void gen_qemu_ld32ur(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
-    if (likely(!ctx->le_mode)) {
-        tcg_gen_bswap32_tl(arg1, arg1);
-    }
+    TCGMemOp op = MO_UL | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
 
@@ -3073,10 +3050,8 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
 /* ldbrx */
 static inline void gen_qemu_ld64ur(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    tcg_gen_qemu_ld64(arg1, arg2, ctx->mem_idx);
-    if (likely(!ctx->le_mode)) {
-        tcg_gen_bswap64_tl(arg1, arg1);
-    }
+    TCGMemOp op = MO_Q | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_ld_i64(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX);
 #endif  /* TARGET_PPC64 */
@@ -3084,30 +3059,16 @@ GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX);
 /* sthbrx */
 static inline void gen_qemu_st16r(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (likely(!ctx->le_mode)) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ext16u_tl(t0, arg1);
-        tcg_gen_bswap16_tl(t0, t0);
-        tcg_gen_qemu_st16(t0, arg2, ctx->mem_idx);
-        tcg_temp_free(t0);
-    } else {
-        tcg_gen_qemu_st16(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_UW | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_STX(sthbr, st16r, 0x16, 0x1C, PPC_INTEGER);
 
 /* stwbrx */
 static inline void gen_qemu_st32r(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (likely(!ctx->le_mode)) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ext32u_tl(t0, arg1);
-        tcg_gen_bswap32_tl(t0, t0);
-        tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
-        tcg_temp_free(t0);
-    } else {
-        tcg_gen_qemu_st32(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_UL | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER);
 
@@ -3115,14 +3076,8 @@ GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER);
 /* stdbrx */
 static inline void gen_qemu_st64r(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (likely(!ctx->le_mode)) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_bswap64_tl(t0, arg1);
-        tcg_gen_qemu_st64(t0, arg2, ctx->mem_idx);
-        tcg_temp_free(t0);
-    } else {
-        tcg_gen_qemu_st64(arg1, arg2, ctx->mem_idx);
-    }
+    TCGMemOp op = MO_Q | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
+    tcg_gen_qemu_st_i64(arg1, arg2, ctx->mem_idx, op);
 }
 GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX);
 #endif  /* TARGET_PPC64 */
@@ -3542,7 +3497,9 @@ static void gen_lfdp(DisasContext *ctx)
     }
     gen_set_access_type(ctx, ACCESS_FLOAT);
     EA = tcg_temp_new();
-    gen_addr_imm_index(ctx, EA, 0);                                           \
+    gen_addr_imm_index(ctx, EA, 0);
+    /* We only need to swap high and low halves. gen_qemu_ld64 does necessary
+       64-bit byteswap already. */
     if (unlikely(ctx->le_mode)) {
         gen_qemu_ld64(ctx, cpu_fpr[rD(ctx->opcode) + 1], EA);
         tcg_gen_addi_tl(EA, EA, 8);
@@ -3566,6 +3523,8 @@ static void gen_lfdpx(DisasContext *ctx)
     gen_set_access_type(ctx, ACCESS_FLOAT);
     EA = tcg_temp_new();
     gen_addr_reg_index(ctx, EA);
+    /* We only need to swap high and low halves. gen_qemu_ld64 does necessary
+       64-bit byteswap already. */
     if (unlikely(ctx->le_mode)) {
         gen_qemu_ld64(ctx, cpu_fpr[rD(ctx->opcode) + 1], EA);
         tcg_gen_addi_tl(EA, EA, 8);
@@ -3714,7 +3673,9 @@ static void gen_stfdp(DisasContext *ctx)
     }
     gen_set_access_type(ctx, ACCESS_FLOAT);
     EA = tcg_temp_new();
-    gen_addr_imm_index(ctx, EA, 0);                                           \
+    gen_addr_imm_index(ctx, EA, 0);
+    /* We only need to swap high and low halves. gen_qemu_st64 does necessary
+       64-bit byteswap already. */
     if (unlikely(ctx->le_mode)) {
         gen_qemu_st64(ctx, cpu_fpr[rD(ctx->opcode) + 1], EA);
         tcg_gen_addi_tl(EA, EA, 8);
@@ -3738,6 +3699,8 @@ static void gen_stfdpx(DisasContext *ctx)
     gen_set_access_type(ctx, ACCESS_FLOAT);
     EA = tcg_temp_new();
     gen_addr_reg_index(ctx, EA);
+    /* We only need to swap high and low halves. gen_qemu_st64 does necessary
+       64-bit byteswap already. */
     if (unlikely(ctx->le_mode)) {
         gen_qemu_st64(ctx, cpu_fpr[rD(ctx->opcode) + 1], EA);
         tcg_gen_addi_tl(EA, EA, 8);
@@ -6691,6 +6654,8 @@ static void glue(gen_, name)(DisasContext *ctx)
     EA = tcg_temp_new();                                                      \
     gen_addr_reg_index(ctx, EA);                                              \
     tcg_gen_andi_tl(EA, EA, ~0xf);                                            \
+    /* We only need to swap high and low halves. gen_qemu_ld64 does necessary \
+       64-bit byteswap already. */                                            \
     if (ctx->le_mode) {                                                       \
         gen_qemu_ld64(ctx, cpu_avrl[rD(ctx->opcode)], EA);                    \
         tcg_gen_addi_tl(EA, EA, 8);                                           \
@@ -6715,6 +6680,8 @@ static void gen_st##name(DisasContext *ctx)                                   \
     EA = tcg_temp_new();                                                      \
     gen_addr_reg_index(ctx, EA);                                              \
     tcg_gen_andi_tl(EA, EA, ~0xf);                                            \
+    /* We only need to swap high and low halves. gen_qemu_st64 does necessary \
+       64-bit byteswap already. */                                            \
     if (ctx->le_mode) {                                                       \
         gen_qemu_st64(ctx, cpu_avrl[rD(ctx->opcode)], EA);                    \
         tcg_gen_addi_tl(EA, EA, 8);                                           \
@@ -11337,6 +11304,7 @@ static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
     ctx.insns_flags2 = env->insns_flags2;
     ctx.access_type = -1;
     ctx.le_mode = env->hflags & (1 << MSR_LE) ? 1 : 0;
+    ctx.default_tcg_memop_mask = ctx.le_mode ? MO_LE : MO_BE;
 #if defined(TARGET_PPC64)
     ctx.sf_mode = msr_is_64bit(env, env->msr);
     ctx.has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
@@ -11401,7 +11369,7 @@ static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
                   ctx.nip, ctx.mem_idx, (int)msr_ir);
         if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO))
             gen_io_start();
-        if (unlikely(ctx.le_mode)) {
+        if (unlikely(need_byteswap(&ctx))) {
             ctx.opcode = bswap32(cpu_ldl_code(env, ctx.nip));
         } else {
             ctx.opcode = cpu_ldl_code(env, ctx.nip);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4d94015..84381ae 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8418,6 +8418,9 @@ static void ppc_cpu_reset(CPUState *s)
     msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
     msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
     msr |= (target_ulong)1 << MSR_PR;
+#if !defined(TARGET_WORDS_BIGENDIAN)
+    msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
+#endif
 #endif
 
 #if defined(TARGET_PPC64)
@@ -8461,6 +8464,12 @@ static void ppc_cpu_reset(CPUState *s)
 
     /* Flush all TLBs */
     tlb_flush(s, 1);
+
+#if defined(CONFIG_USER_ONLY) && !defined(TARGET_WORDS_BIGENDIAN)
+    if (!msr_le) {
+        cpu_abort(CPU(cpu), "Cannot set QEMU to little-endian user mode\n");
+    }
+#endif
 }
 
 static void ppc_cpu_initfn(Object *obj)
-- 
1.9.1.423.g4596e3a

Re: [Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.

[Peter Maydell]

On 10 May 2014 10:16, Doug Kwan <dougkwan@google.com> wrote:
> This allow running PPC64 little-endian in user mode if target is configured
> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
> Overhaul handling of byteswapping in code generation and mem helpers.

This looks pretty good to me (I'll let a ppc person do the detailed
review). One thing I did spot:

Outside target-ppc/, the other place we do memory accesses is
linux-user/main.c:do_store_exclusive(). get_user_u32() and friends
will honour TARGET_WORDS_BIGENDIAN, but the 128 bit store
exclusive is done with two accesses. The system-emulation mode
code handles the LE case by flipping around the register values, so
I think the linux-user code will need to do a similar thing.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.

[Alexander Graf]

On 10.05.14 11:16, Doug Kwan wrote:
> This allow running PPC64 little-endian in user mode if target is configured
> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
> Overhaul handling of byteswapping in code generation and mem helpers.
>
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>   target-ppc/mem_helper.c     |  25 ++++++--
>   target-ppc/translate.c      | 150 +++++++++++++++++---------------------------
>   target-ppc/translate_init.c |   9 +++
>   3 files changed, 89 insertions(+), 95 deletions(-)
>

[...]

> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 4d94015..84381ae 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8418,6 +8418,9 @@ static void ppc_cpu_reset(CPUState *s)
>       msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
>       msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
>       msr |= (target_ulong)1 << MSR_PR;
> +#if !defined(TARGET_WORDS_BIGENDIAN)
> +    msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
> +#endif
>   #endif
>   
>   #if defined(TARGET_PPC64)
> @@ -8461,6 +8464,12 @@ static void ppc_cpu_reset(CPUState *s)
>   
>       /* Flush all TLBs */
>       tlb_flush(s, 1);
> +
> +#if defined(CONFIG_USER_ONLY) && !defined(TARGET_WORDS_BIGENDIAN)
> +    if (!msr_le) {
> +        cpu_abort(CPU(cpu), "Cannot set QEMU to little-endian user mode\n");

We don't have this check the other way around, so why do we need it 
here? How do you ever get to this?


Alex

Re: [Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.

[Doug Kwan (關振德)]

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

On Tue, May 13, 2014 at 12:05 AM, Alexander Graf <agraf@suse.de> wrote:

>
> On 10.05.14 11:16, Doug Kwan wrote:
>
>> This allow running PPC64 little-endian in user mode if target is
>> configured
>> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
>> Overhaul handling of byteswapping in code generation and mem helpers.
>>
>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>> ---
>>   target-ppc/mem_helper.c     |  25 ++++++--
>>   target-ppc/translate.c      | 150 +++++++++++++++++-------------
>> --------------
>>   target-ppc/translate_init.c |   9 +++
>>   3 files changed, 89 insertions(+), 95 deletions(-)
>>
>>
> [...]
>
>
>  diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 4d94015..84381ae 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -8418,6 +8418,9 @@ static void ppc_cpu_reset(CPUState *s)
>>       msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
>>       msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
>>       msr |= (target_ulong)1 << MSR_PR;
>> +#if !defined(TARGET_WORDS_BIGENDIAN)
>> +    msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
>> +#endif
>>   #endif
>>     #if defined(TARGET_PPC64)
>> @@ -8461,6 +8464,12 @@ static void ppc_cpu_reset(CPUState *s)
>>         /* Flush all TLBs */
>>       tlb_flush(s, 1);
>> +
>> +#if defined(CONFIG_USER_ONLY) && !defined(TARGET_WORDS_BIGENDIAN)
>> +    if (!msr_le) {
>> +        cpu_abort(CPU(cpu), "Cannot set QEMU to little-endian user
>> mode\n");
>>
>
> We don't have this check the other way around, so why do we need it here?
> How do you ever get to this?
>
>
> Alex
>
> I am just being paranoid as I am new to this code base.  The reason why
this is asymmetric because the PPC targets are big-endian by default and I
don't know if all CPUs support setting msr.le.  If someone specifies a CPU
that does not support little-endian mode, I want to stop qemu early with an
error.  The check can be removed if it is redundant.

[-- Attachment #2: Type: text/html, Size: 2864 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.

[Alexander Graf]

On 13.05.14 09:30, Doug Kwan (關振德) wrote:
>
>
>
> On Tue, May 13, 2014 at 12:05 AM, Alexander Graf <agraf@suse.de 
> <mailto:agraf@suse.de>> wrote:
>
>
>     On 10.05.14 11:16, Doug Kwan wrote:
>
>         This allow running PPC64 little-endian in user mode if target
>         is configured
>         that way.  In PPC64 LE user mode we set MSR.LE during
>         initialization.
>         Overhaul handling of byteswapping in code generation and mem
>         helpers.
>
>         Signed-off-by: Doug Kwan <dougkwan@google.com
>         <mailto:dougkwan@google.com>>
>         ---
>           target-ppc/mem_helper.c     |  25 ++++++--
>           target-ppc/translate.c      | 150
>         +++++++++++++++++---------------------------
>           target-ppc/translate_init.c |   9 +++
>           3 files changed, 89 insertions(+), 95 deletions(-)
>
>
>     [...]
>
>
>         diff --git a/target-ppc/translate_init.c
>         b/target-ppc/translate_init.c
>         index 4d94015..84381ae 100644
>         --- a/target-ppc/translate_init.c
>         +++ b/target-ppc/translate_init.c
>         @@ -8418,6 +8418,9 @@ static void ppc_cpu_reset(CPUState *s)
>               msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
>               msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
>               msr |= (target_ulong)1 << MSR_PR;
>         +#if !defined(TARGET_WORDS_BIGENDIAN)
>         +    msr |= (target_ulong)1 << MSR_LE; /* Little-endian user
>         mode */
>         +#endif
>           #endif
>             #if defined(TARGET_PPC64)
>         @@ -8461,6 +8464,12 @@ static void ppc_cpu_reset(CPUState *s)
>                 /* Flush all TLBs */
>               tlb_flush(s, 1);
>         +
>         +#if defined(CONFIG_USER_ONLY) && !defined(TARGET_WORDS_BIGENDIAN)
>         +    if (!msr_le) {
>         +        cpu_abort(CPU(cpu), "Cannot set QEMU to little-endian
>         user mode\n");
>
>
>     We don't have this check the other way around, so why do we need
>     it here? How do you ever get to this?
>
>
>     Alex
>
> I am just being paranoid as I am new to this code base.  The reason 
> why this is asymmetric because the PPC targets are big-endian by 
> default and I don't know if all CPUs support setting msr.le.  If 
> someone specifies a CPU that does not support little-endian mode, I 
> want to stop qemu early with an error.  The check can be removed if it 
> is redundant.

Well, all CPUs that Linux supports with LE today also support the MSR_LE 
bit. If anyone would ever want to do an e500 LE port things would become 
messy, as LE is a TLB property there, so we don't have a generic bit to 
look at.

I think we just drop this check. If the user passes in an incompatible 
CPU it's his own fault :).


Alex

[Qemu-devel] [PATCH v2 3/3] Add a new user mode target for little-endian PPC64.

[Doug Kwan]

Signed-off-by: Doug Kwan <dougkwan@google.com>
---
 configure                              | 6 ++++++
 default-configs/ppc64le-linux-user.mak | 1 +
 2 files changed, 7 insertions(+)
 create mode 100644 default-configs/ppc64le-linux-user.mak

diff --git a/configure b/configure
index 46bc0c9..5f0926b 100755
--- a/configure
+++ b/configure
@@ -4912,6 +4912,12 @@ case "$target_name" in
     TARGET_ABI_DIR=ppc
     gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
   ;;
+  ppc64le)
+    TARGET_ARCH=ppc64
+    TARGET_BASE_ARCH=ppc
+    TARGET_ABI_DIR=ppc
+    gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
+  ;;
   ppc64abi32)
     TARGET_ARCH=ppc64
     TARGET_BASE_ARCH=ppc
diff --git a/default-configs/ppc64le-linux-user.mak b/default-configs/ppc64le-linux-user.mak
new file mode 100644
index 0000000..6948225
--- /dev/null
+++ b/default-configs/ppc64le-linux-user.mak
@@ -0,0 +1 @@
+# Default configuration for ppc64el-linux-user
-- 
1.9.1.423.g4596e3a

Re: [Qemu-devel] [PATCH v2 3/3] Add a new user mode target for little-endian PPC64.

[Peter Maydell]

On 10 May 2014 10:16, Doug Kwan <dougkwan@google.com> wrote:
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>  configure                              | 6 ++++++
>  default-configs/ppc64le-linux-user.mak | 1 +
>  2 files changed, 7 insertions(+)
>  create mode 100644 default-configs/ppc64le-linux-user.mak
>
> diff --git a/configure b/configure
> index 46bc0c9..5f0926b 100755
> --- a/configure
> +++ b/configure
> @@ -4912,6 +4912,12 @@ case "$target_name" in
>      TARGET_ABI_DIR=ppc
>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>    ;;
> +  ppc64le)
> +    TARGET_ARCH=ppc64
> +    TARGET_BASE_ARCH=ppc
> +    TARGET_ABI_DIR=ppc
> +    gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> +  ;;
>    ppc64abi32)
>      TARGET_ARCH=ppc64
>      TARGET_BASE_ARCH=ppc
> diff --git a/default-configs/ppc64le-linux-user.mak b/default-configs/ppc64le-linux-user.mak
> new file mode 100644
> index 0000000..6948225
> --- /dev/null
> +++ b/default-configs/ppc64le-linux-user.mak
> @@ -0,0 +1 @@
> +# Default configuration for ppc64el-linux-user

You forgot to change this "el" to "le". Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/3] Add a new user mode target for little-endian PPC64.

[Tom Musta]

On 5/10/2014 4:16 AM, Doug Kwan wrote:
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>  configure                              | 6 ++++++
>  default-configs/ppc64le-linux-user.mak | 1 +
>  2 files changed, 7 insertions(+)
>  create mode 100644 default-configs/ppc64le-linux-user.mak
> 
> diff --git a/configure b/configure
> index 46bc0c9..5f0926b 100755
> --- a/configure
> +++ b/configure
> @@ -4912,6 +4912,12 @@ case "$target_name" in
>      TARGET_ABI_DIR=ppc
>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>    ;;
> +  ppc64le)
> +    TARGET_ARCH=ppc64
> +    TARGET_BASE_ARCH=ppc
> +    TARGET_ABI_DIR=ppc
> +    gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> +  ;;
>    ppc64abi32)
>      TARGET_ARCH=ppc64
>      TARGET_BASE_ARCH=ppc
> diff --git a/default-configs/ppc64le-linux-user.mak b/default-configs/ppc64le-linux-user.mak
> new file mode 100644
> index 0000000..6948225
> --- /dev/null
> +++ b/default-configs/ppc64le-linux-user.mak
> @@ -0,0 +1 @@
> +# Default configuration for ppc64el-linux-user
> 

Now that Alex has taken the decimal floating point into his ppc-next tree, we will also want to
do this, which will trigger building of the decimal floating point library code:

diff --git a/default-configs/ppc64le-linux-user.mak b/default-configs/ppc64le-linux-user.mak
index 6948225..dacbcab 100644
--- a/default-configs/ppc64le-linux-user.mak
+++ b/default-configs/ppc64le-linux-user.mak
@@ -1 +1,2 @@
 # Default configuration for ppc64el-linux-user
+CONFIG_LIBDECNUMBER=y

I will change this (along with the "el" remnant") in V3.

Re: [Qemu-devel] [PATCH v2 3/3] Add a new user mode target for little-endian PPC64.

[Doug Kwan (關振德)]

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

On Mon, May 12, 2014 at 6:05 AM, Tom Musta <tommusta@gmail.com> wrote:

> On 5/10/2014 4:16 AM, Doug Kwan wrote:
> > Signed-off-by: Doug Kwan <dougkwan@google.com>
> > ---
> >  configure                              | 6 ++++++
> >  default-configs/ppc64le-linux-user.mak | 1 +
> >  2 files changed, 7 insertions(+)
> >  create mode 100644 default-configs/ppc64le-linux-user.mak
> >
> > diff --git a/configure b/configure
> > index 46bc0c9..5f0926b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -4912,6 +4912,12 @@ case "$target_name" in
> >      TARGET_ABI_DIR=ppc
> >      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml
> power-spe.xml"
> >    ;;
> > +  ppc64le)
> > +    TARGET_ARCH=ppc64
> > +    TARGET_BASE_ARCH=ppc
> > +    TARGET_ABI_DIR=ppc
> > +    gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml
> power-spe.xml"
> > +  ;;
> >    ppc64abi32)
> >      TARGET_ARCH=ppc64
> >      TARGET_BASE_ARCH=ppc
> > diff --git a/default-configs/ppc64le-linux-user.mak
> b/default-configs/ppc64le-linux-user.mak
> > new file mode 100644
> > index 0000000..6948225
> > --- /dev/null
> > +++ b/default-configs/ppc64le-linux-user.mak
> > @@ -0,0 +1 @@
> > +# Default configuration for ppc64el-linux-user
> >
>
> Now that Alex has taken the decimal floating point into his ppc-next tree,
> we will also want to
> do this, which will trigger building of the decimal floating point library
> code:
>
> diff --git a/default-configs/ppc64le-linux-user.mak
> b/default-configs/ppc64le-linux-user.mak
> index 6948225..dacbcab 100644
> --- a/default-configs/ppc64le-linux-user.mak
> +++ b/default-configs/ppc64le-linux-user.mak
> @@ -1 +1,2 @@
>  # Default configuration for ppc64el-linux-user
> +CONFIG_LIBDECNUMBER=y
>
> I will change this (along with the "el" remnant") in V3.
>
>
> Thank you for taking care of this.

[-- Attachment #2: Type: text/html, Size: 2579 bytes --]

Re: [Qemu-devel] [PATCH v2 0/3] Adding new user mode target ppc64le-linux-user

[Alexander Graf]

On 10.05.14 11:16, Doug Kwan wrote:
> Hi
>
> I have made changes based on comments to the previous set of pathces.
>
> 1. Target name is now ppc64le-linux-user.
> 2. I used a memop mask to avoid generating redundant byteswap pairs that
>     cancel out each other.  This is suggested by Peter Maydell.
> 3. There is code to handle machine name correctly in uname.
>
> This is not mean to be final though.  Tom Musta of IBM kindly offered to help
> me with this.  So I am handling over this set of patches.  Tom will continue
> from this version.
>
> Thanks for all the comments and suggestions so far.

I like the patch set for the most part. I guess in v3 it will be ready 
for inclusion :).


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Adding new user mode target ppc64le-linux-user

[Tom Musta]

On 5/13/2014 2:06 AM, Alexander Graf wrote:
> 
> On 10.05.14 11:16, Doug Kwan wrote:
>> Hi
>>
>> I have made changes based on comments to the previous set of pathces.
>>
>> 1. Target name is now ppc64le-linux-user.
>> 2. I used a memop mask to avoid generating redundant byteswap pairs that
>>     cancel out each other.  This is suggested by Peter Maydell.
>> 3. There is code to handle machine name correctly in uname.
>>
>> This is not mean to be final though.  Tom Musta of IBM kindly offered to help
>> me with this.  So I am handling over this set of patches.  Tom will continue
>> from this version.
>>
>> Thanks for all the comments and suggestions so far.
> 
> I like the patch set for the most part. I guess in v3 it will be ready for inclusion :).
> 
> 
> Alex
> 
> 

Doug & I have agreed that I will take these the rest of the way.   I am still testing in a few
environments that are different from Doug's and may have a few additions.

Also, Peter suggested that LTP be run ... I am going to look into this but as a follow-on effort to
these patches.