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

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

[Doug Kwan]

Hi

This is a series of 3 patches to add a new target for little-endian
PPC64 linux user mode.  This first patch is for loading PPC64 ELFv2 binaries.
The second patch is the biggest and it contains changes to set up the
LE environment and handle byteswapping appropriately.  The third patch
contains the necessary configuration changes for the new target.

This is tested briefly by running big-endian and little-endian Linux
binaries in user mode.  I also booted the QEMU ppc linux test image though
it is 32-bit only.

-Doug

Doug Kwan (3):
  linux-user: Handle ELFv2 PPC64 binaries in user mode.
  PPC: Allow little-endian user mode.
  configure: Add new target ppc64el-linux-user

 configure                              |  6 +++++
 default-configs/ppc64el-linux-user.mak |  1 +
 include/elf.h                          |  5 ++++
 linux-user/elfload.c                   | 17 +++++++++++--
 target-ppc/mem_helper.c                | 16 ++++++++++--
 target-ppc/translate.c                 | 45 ++++++++++++++++++++++------------
 target-ppc/translate_init.c            |  9 +++++++
 7 files changed, 80 insertions(+), 19 deletions(-)
 create mode 100644 default-configs/ppc64el-linux-user.mak

-- 
1.9.1.423.g4596e3a

[Qemu-devel] [PATCH 1/3] linux-user: Handle ELFv2 PPC64 binaries in user mode.

[Doug Kwan]

Look at ELF header to determin ABI version on PPC64.  This is required
for executing the first instruction correctly.

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

diff --git a/include/elf.h b/include/elf.h
index 1599ab2..b39f5db 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		3
+
 /* 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 */
-- 
1.9.1.423.g4596e3a

Re: [Qemu-devel] [PATCH 1/3] linux-user: Handle ELFv2 PPC64 binaries in user mode.

[Alexander Graf]

On 05/08/2014 10:26 AM, Doug Kwan wrote:
> Look at ELF header to determin ABI version on PPC64.  This is required
> for executing the first instruction correctly.
>
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>   include/elf.h        |  5 +++++
>   linux-user/elfload.c | 17 +++++++++++++++--
>   2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/include/elf.h b/include/elf.h
> index 1599ab2..b39f5db 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		3

Please write bitmasks in hex.

> +
>   /* 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 */

Uli, is this correct? Also, why don't w need to adjust for the load_bias 
with ELFv2 anymore?

> +    }
>   #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

I'm not sure this is more readable than doing it inline ... :).


Alex

Re: [Qemu-devel] [PATCH 1/3] linux-user: Handle ELFv2 PPC64 binaries in user mode.

[Doug Kwan (關振德)]

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

Hi


On Thu, May 8, 2014 at 1:36 AM, Alexander Graf <agraf@suse.de> wrote:
   3
>
>
> Please write bitmasks in hex.

Will fix.



>  +
>>   /* 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
>> */
>>
>
> Uli, is this correct? Also, why don't w need to adjust for the load_bias
> with ELFv2 anymore?

No.  This is a bug.  It was not caught by testing because load_bias is 0 I
guess.


>
>  +    }
>>   #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
>>
>
> I'm not sure this is more readable than doing it inline ... :).
>

This is done so because the elf header is not yet included before the
callee.  To eliminate this I need to move the include before ppc's
init_thread.  Is that okay?

Thanks for the comments.

-Doug

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

Re: [Qemu-devel] [PATCH 1/3] linux-user: Handle ELFv2 PPC64 binaries in user mode.

[Alexander Graf]

On 05/08/2014 10:43 AM, Doug Kwan (關振德) wrote:
> Hi
>
>
> On Thu, May 8, 2014 at 1:36 AM, Alexander Graf <agraf@suse.de 
> <mailto:agraf@suse.de>> wrote:      3
>
>
>     Please write bitmasks in hex.
>
> Will fix.
>
>         +
>           /* 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 */
>
>
>     Uli, is this correct? Also, why don't w need to adjust for the
>     load_bias with ELFv2 anymore?
>
> No.  This is a bug.  It was not caught by testing because load_bias is 
> 0 I guess.
>
>
>         +    }
>           #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
>
>
>     I'm not sure this is more readable than doing it inline ... :).
>
>
> This is done so because the elf header is not yet included before the 
> callee.  To eliminate this I need to move the include before ppc's 
> init_thread.  Is that okay?

Ah, I see. I don't have strong feelings either way. Riku, what would you 
prefer?


Alex

Re: [Qemu-devel] [PATCH 1/3] linux-user: Handle ELFv2 PPC64 binaries in user mode.

[Ulrich Weigand]

Doug Kwan (關振德) <dougkwan@google.com> wrote on 08.05.2014 10:43:14:

On Thu, May 8, 2014 at 1:36 AM, Alexander Graf <agraf@suse.de> wrote:
>> -    _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 */
>>
>> Uli, is this correct? Also, why don't w need to adjust for the
>> load_bias with ELFv2 anymore?
>
> No.  This is a bug.  It was not caught by testing because load_bias
> is 0 I guess.

Actually, it looks correct to me.  The value of infop->entry itself
was presumably already adjusted for the load bias by common code.

However, on ELFv1, that value points to the descriptor, but the values
we *load* from that descriptor *also* need to be adjusted by the load
bias, since the image has not yet been relocated at this stage.

With ELFv2, the (already adjusted) infop->entry value points directly
to the code, so no further adjustment is required.

Bye,
Ulrich

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

[Doug Kwan]

This all running PPC64 little-endian in user mode if target is configured
that way.  In PPC64 LE user mode we set MSR.LE during initialization.
Byteswapping logic is reversed also when QEMU is running in that mode.

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

diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index f35ed03..7bacdab 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -202,6 +202,14 @@ target_ulong helper_lscbx(CPUPPCState *env, target_ulong addr, uint32_t reg,
 #define LO_IDX 0
 #endif
 
+static inline bool element_needs_byteswap(CPUPPCState *env) {
+#if defined(TARGET_WORDS_BIGENDIAN)
+  return msr_le;
+#else
+  return !msr_le;
+#endif
+}
+
 #define LVE(name, access, swap, element)                        \
     void helper_##name(CPUPPCState *env, ppc_avr_t *r,          \
                        target_ulong addr)                       \
@@ -210,9 +218,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 (element_needs_byteswap(env)) {                      \
             r->element[LO_IDX ? index : (adjust - index)] =     \
                 swap(access(env, addr));                        \
         } else {                                                \
@@ -235,9 +245,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 (element_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..4b11c5a 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -210,6 +210,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)
@@ -2653,14 +2662,14 @@ 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         tcg_gen_bswap16_tl(arg1, arg1);
     }
 }
 
 static inline void gen_qemu_ld16s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
+    if (unlikely(need_byteswap(ctx))) {
         tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
         tcg_gen_bswap16_tl(arg1, arg1);
         tcg_gen_ext16s_tl(arg1, arg1);
@@ -2672,7 +2681,7 @@ static inline void gen_qemu_ld16s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         tcg_gen_bswap32_tl(arg1, arg1);
     }
 }
@@ -2687,7 +2696,7 @@ 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
         tcg_gen_bswap32_tl(arg1, arg1);
         tcg_gen_ext32s_tl(arg1, arg1);
@@ -2706,7 +2715,7 @@ 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         tcg_gen_bswap64_i64(arg1, arg1);
     }
 }
@@ -2718,7 +2727,7 @@ 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext16u_tl(t0, arg1);
         tcg_gen_bswap16_tl(t0, t0);
@@ -2731,7 +2740,7 @@ static inline void gen_qemu_st16(DisasContext *ctx, TCGv arg1, TCGv arg2)
 
 static inline void gen_qemu_st32(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
-    if (unlikely(ctx->le_mode)) {
+    if (unlikely(need_byteswap(ctx))) {
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext32u_tl(t0, arg1);
         tcg_gen_bswap32_tl(t0, t0);
@@ -2752,7 +2761,7 @@ 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)) {
+    if (unlikely(need_byteswap(ctx))) {
         TCGv_i64 t0 = tcg_temp_new_i64();
         tcg_gen_bswap64_i64(t0, arg1);
         tcg_gen_qemu_st64(t0, arg2, ctx->mem_idx);
@@ -3049,11 +3058,17 @@ static void gen_std(DisasContext *ctx)
 }
 #endif
 /***                Integer load and store with byte reverse               ***/
+
+/* Logic for byteswap test is reversed since these instructions require
+ * a byteswap already.  If we need another byteswap due to endianness of
+ * translation context, the two byteswaps cancel out each other.
+ */
+
 /* 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)) {
+    if (likely(!need_byteswap(ctx))) {
         tcg_gen_bswap16_tl(arg1, arg1);
     }
 }
@@ -3063,7 +3078,7 @@ GEN_LDX(lhbr, ld16ur, 0x16, 0x18, PPC_INTEGER);
 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)) {
+    if (likely(!need_byteswap(ctx))) {
         tcg_gen_bswap32_tl(arg1, arg1);
     }
 }
@@ -3074,7 +3089,7 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
 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)) {
+    if (likely(!need_byteswap(ctx))) {
         tcg_gen_bswap64_tl(arg1, arg1);
     }
 }
@@ -3084,7 +3099,7 @@ 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)) {
+    if (likely(!need_byteswap(ctx))) {
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext16u_tl(t0, arg1);
         tcg_gen_bswap16_tl(t0, t0);
@@ -3099,7 +3114,7 @@ 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)) {
+    if (likely(!need_byteswap(ctx))) {
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext32u_tl(t0, arg1);
         tcg_gen_bswap32_tl(t0, t0);
@@ -3115,7 +3130,7 @@ 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)) {
+    if (likely(!need_byteswap(ctx))) {
         TCGv t0 = tcg_temp_new();
         tcg_gen_bswap64_tl(t0, arg1);
         tcg_gen_qemu_st64(t0, arg2, ctx->mem_idx);
@@ -11401,7 +11416,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 2/3] PPC: Allow little-endian user mode.

[Alexander Graf]

On 05/08/2014 10:26 AM, Doug Kwan wrote:
> This all running PPC64 little-endian in user mode if target is configured
> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
> Byteswapping logic is reversed also when QEMU is running in that mode.
>
> Signed-off-by: Doug Kwan <dougkwan@google.com>

I can't say I'm a huge fan of this patch. It allows for really tricky 
subtile mistakes to happen. Can't we leave the target mode configured on 
big endian?


Alex

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

[Doug Kwan (關振德)]

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

If we leave the target mode as big-endian, we need to teach the elfloader
to handle binary of reversed endianness.  I have looked into that
possibility as well but find the current approach easier.


On Thu, May 8, 2014 at 1:39 AM, Alexander Graf <agraf@suse.de> wrote:

> On 05/08/2014 10:26 AM, Doug Kwan wrote:
>
>> This all running PPC64 little-endian in user mode if target is configured
>> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
>> Byteswapping logic is reversed also when QEMU is running in that mode.
>>
>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>>
>
> I can't say I'm a huge fan of this patch. It allows for really tricky
> subtile mistakes to happen. Can't we leave the target mode configured on
> big endian?
>
>
> Alex
>
>

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

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

[Alexander Graf]

On 05/08/2014 10:49 AM, Doug Kwan (關振德) wrote:
> If we leave the target mode as big-endian, we need to teach the 
> elfloader to handle binary of reversed endianness.  I have looked into 
> that possibility as well but find the current approach easier.

Please don't top post.

Ok, so if we really want to change the code to be able to handle 
!TARGET_WORDS_BIGENDIAN the whole notion of s->le_mode becomes flawed. 
Instead, it should get renamed into needs_swab or needs_byteswap or 
something along those lines. Then we can set it accordingly based on 
msr_le and target configuration.

For the mem_helper.c changes, could you please explain why you have to 
treat msr_le and needs_byteswap separately?


Thanks,

Alex

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

[Doug Kwan (關振德)]

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

There are a few places where we do 128-bit loads/store.  We need to swap
the high/low halves and also do byteswap for each half.  Byteswap of each
half is handled differently depending on TARGET_WORDS_BIGENDIAN but the
swapping of high/low halves is always controlled by msr.le.  Would our
intention be clearer if defined another function swap_high_low_64_bits()
instead of msr.le?

It's 2am in California.  I need to go off-line.  I will continue tomorrow.
Thanks

-Doug


On Thu, May 8, 2014 at 1:55 AM, Alexander Graf <agraf@suse.de> wrote:

> On 05/08/2014 10:49 AM, Doug Kwan (關振德) wrote:
>
>> If we leave the target mode as big-endian, we need to teach the elfloader
>> to handle binary of reversed endianness.  I have looked into that
>> possibility as well but find the current approach easier.
>>
>
> Please don't top post.
>
> Ok, so if we really want to change the code to be able to handle
> !TARGET_WORDS_BIGENDIAN the whole notion of s->le_mode becomes flawed.
> Instead, it should get renamed into needs_swab or needs_byteswap or
> something along those lines. Then we can set it accordingly based on msr_le
> and target configuration.
>
> For the mem_helper.c changes, could you please explain why you have to
> treat msr_le and needs_byteswap separately?
>
>
> Thanks,
>
> Alex
>
>

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

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

[Alexander Graf]

On 05/08/2014 11:05 AM, Doug Kwan (關振德) wrote:
> There are a few places where we do 128-bit loads/store.  We need to 
> swap the high/low halves and also do byteswap for each half.  Byteswap 
> of each half is handled differently depending on 
> TARGET_WORDS_BIGENDIAN but the swapping of high/low halves is always 
> controlled by msr.le.  Would our intention be clearer if defined 
> another function swap_high_low_64_bits() instead of msr.le?

This is my last warning about top posting. One more top post and I will 
ignore any future email from you.

The explanation makes a lot of sense. This probably just deserves a 
quick comment in the code so that nobody stumbles over this when he 
looks at it :).


Alex

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

[Doug Kwan (關振德)]

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

On Thu, May 8, 2014 at 2:11 AM, Alexander Graf <agraf@suse.de> wrote:

> On 05/08/2014 11:05 AM, Doug Kwan (關振德) wrote:
>
>> There are a few places where we do 128-bit loads/store.  We need to swap
>> the high/low halves and also do byteswap for each half.  Byteswap of each
>> half is handled differently depending on TARGET_WORDS_BIGENDIAN but the
>> swapping of high/low halves is always controlled by msr.le.  Would our
>> intention be clearer if defined another function swap_high_low_64_bits()
>> instead of msr.le?
>>
>
> This is my last warning about top posting. One more top post and I will
> ignore any future email from you.
>
> The explanation makes a lot of sense. This probably just deserves a quick
> comment in the code so that nobody stumbles over this when he looks at it
> :).
>
>
> Alex
>
>
I'm terribly sorry about the mail format.  I did not realize what
"top-posting" was until I do a quick search just now. It does not help that
Gmail does top-posting by default.  I just noticed that I may have missed
the multi-word load store.  I will write a small test case tomorrow morning
to check that they are okay.

Thanks

-Doug

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

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

[Peter Maydell]

On 8 May 2014 09:39, Alexander Graf <agraf@suse.de> wrote:
> On 05/08/2014 10:26 AM, Doug Kwan wrote:
>>
>> This all running PPC64 little-endian in user mode if target is configured
>> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
>> Byteswapping logic is reversed also when QEMU is running in that mode.
>>
>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>
>
> I can't say I'm a huge fan of this patch. It allows for really tricky
> subtile mistakes to happen. Can't we leave the target mode configured on big
> endian?

Unfortunately not if you care about linux-user mode:
linux-user mode uses the target-endian setting to figure
out if it needs to do swapping of data in all the syscall
interfaces.

If you're going to overhaul how PPC deals with endian
dependent loads/stores, I suspect you'll end up with a
cleaner result if you convert to the new "specify endian
setting as part of the memory operation" TCG ops:
So for instance rather than having:

    tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
    if (unlikely(ctx->le_mode)) {
        tcg_gen_bswap16_tl(arg1, arg1);
    }

it would be better to do
    TCGMemOp op = MO_UW | (ctx->le_mode ? MO_LE : MO_BE);
    tcg_gen_qemu_ld_i32(arg1, arg2, ctx->mem_idx, op);

This will work regardless of the TARGET_WORD_BIGENDIAN
setting, since we directly ask TCG to do an LE or BE
access, rather than doing a target-endian access and
then swapping. (It's also more efficient if you're
in little-endian mode on a little endian host since
it won't swap at all rather than swapping twice.)

thanks
-- PMM

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

[Alexander Graf]

On 05/08/2014 02:39 PM, Peter Maydell wrote:
> On 8 May 2014 09:39, Alexander Graf <agraf@suse.de> wrote:
>> On 05/08/2014 10:26 AM, Doug Kwan wrote:
>>> This all running PPC64 little-endian in user mode if target is configured
>>> that way.  In PPC64 LE user mode we set MSR.LE during initialization.
>>> Byteswapping logic is reversed also when QEMU is running in that mode.
>>>
>>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>>
>> I can't say I'm a huge fan of this patch. It allows for really tricky
>> subtile mistakes to happen. Can't we leave the target mode configured on big
>> endian?
> Unfortunately not if you care about linux-user mode:
> linux-user mode uses the target-endian setting to figure
> out if it needs to do swapping of data in all the syscall
> interfaces.
>
> If you're going to overhaul how PPC deals with endian
> dependent loads/stores, I suspect you'll end up with a
> cleaner result if you convert to the new "specify endian
> setting as part of the memory operation" TCG ops:
> So for instance rather than having:
>
>      tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
>      if (unlikely(ctx->le_mode)) {
>          tcg_gen_bswap16_tl(arg1, arg1);
>      }
>
> it would be better to do
>      TCGMemOp op = MO_UW | (ctx->le_mode ? MO_LE : MO_BE);
>      tcg_gen_qemu_ld_i32(arg1, arg2, ctx->mem_idx, op);
>
> This will work regardless of the TARGET_WORD_BIGENDIAN
> setting, since we directly ask TCG to do an LE or BE
> access, rather than doing a target-endian access and
> then swapping. (It's also more efficient if you're
> in little-endian mode on a little endian host since
> it won't swap at all rather than swapping twice.)

Good point. We could store the "default TCGMemOp mask" in ctx and then 
only add the width per operation.


Alex

[Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan]

Add a new user mode target for little-endian PPC64.

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

diff --git a/configure b/configure
index ac2fa15..ce44044 100755
--- a/configure
+++ b/configure
@@ -4917,6 +4917,12 @@ case "$target_name" in
     echo "TARGET_ABI32=y" >> $config_target_mak
     gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
   ;;
+  ppc64el)
+    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"
+  ;;
   sh4|sh4eb)
     TARGET_ARCH=sh4
     bflt="yes"
diff --git a/default-configs/ppc64el-linux-user.mak b/default-configs/ppc64el-linux-user.mak
new file mode 100644
index 0000000..6948225
--- /dev/null
+++ b/default-configs/ppc64el-linux-user.mak
@@ -0,0 +1 @@
+# Default configuration for ppc64el-linux-user
-- 
1.9.1.423.g4596e3a

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Alexander Graf]

On 05/08/2014 10:26 AM, Doug Kwan wrote:
> Add a new user mode target for little-endian PPC64.
>
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>   configure                              | 6 ++++++
>   default-configs/ppc64el-linux-user.mak | 1 +
>   2 files changed, 7 insertions(+)
>   create mode 100644 default-configs/ppc64el-linux-user.mak
>
> diff --git a/configure b/configure
> index ac2fa15..ce44044 100755
> --- a/configure
> +++ b/configure
> @@ -4917,6 +4917,12 @@ case "$target_name" in
>       echo "TARGET_ABI32=y" >> $config_target_mak
>       gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>     ;;
> +  ppc64el)
> +    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"

I can't see the part where this switches the ABI definition to be little 
endian? I'm sure that magic is hidden somewhere, mind to guide me to it?


Alex

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

All the magic happens in the patch 2/3.


On Thu, May 8, 2014 at 1:41 AM, Alexander Graf <agraf@suse.de> wrote:

> On 05/08/2014 10:26 AM, Doug Kwan wrote:
>
>> Add a new user mode target for little-endian PPC64.
>>
>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>> ---
>>   configure                              | 6 ++++++
>>   default-configs/ppc64el-linux-user.mak | 1 +
>>   2 files changed, 7 insertions(+)
>>   create mode 100644 default-configs/ppc64el-linux-user.mak
>>
>> diff --git a/configure b/configure
>> index ac2fa15..ce44044 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4917,6 +4917,12 @@ case "$target_name" in
>>       echo "TARGET_ABI32=y" >> $config_target_mak
>>       gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml
>> power-spe.xml"
>>     ;;
>> +  ppc64el)
>> +    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"
>>
>
> I can't see the part where this switches the ABI definition to be little
> endian? I'm sure that magic is hidden somewhere, mind to guide me to it?
>
>
> Alex
>
>

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Alexander Graf]

On 05/08/2014 10:46 AM, Doug Kwan (關振德) wrote:
> All the magic happens in the patch 2/3.

Please don't top post.

I still fail to see where you tell the build system that 
ppc64le-linux-user should not have TARGET_WORDS_BIGENDIAN set.


Alex

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

Sorry about missing the question. TARGET_WORDS_BIGENDIAN is not set because
of this (around line 4805 in configure):

case "$target_name" in
 armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)

  target_bigendian=yes


The configure script recognizes a list of big-endian target. ppc64le is not
one of those.


On Thu, May 8, 2014 at 1:56 AM, Alexander Graf <agraf@suse.de> wrote:

> On 05/08/2014 10:46 AM, Doug Kwan (關振德) wrote:
>
>> All the magic happens in the patch 2/3.
>>
>
> Please don't top post.
>
> I still fail to see where you tell the build system that
> ppc64le-linux-user should not have TARGET_WORDS_BIGENDIAN set.
>
>
> Alex
>
>

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Andreas Färber]

Am 08.05.2014 10:26, schrieb Doug Kwan:
> Add a new user mode target for little-endian PPC64.
> 
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>  configure                              | 6 ++++++
>  default-configs/ppc64el-linux-user.mak | 1 +
>  2 files changed, 7 insertions(+)
>  create mode 100644 default-configs/ppc64el-linux-user.mak
> 
> diff --git a/configure b/configure
> index ac2fa15..ce44044 100755
> --- a/configure
> +++ b/configure
> @@ -4917,6 +4917,12 @@ case "$target_name" in
>      echo "TARGET_ABI32=y" >> $config_target_mak
>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>    ;;
> +  ppc64el)

In SUSE's build system and everywhere I read people talking about it,
including your 2/3, it's called ppc64le. Why did you choose ppc64el?

Regards,
Andreas

> +    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"
> +  ;;
>    sh4|sh4eb)
>      TARGET_ARCH=sh4
>      bflt="yes"
> diff --git a/default-configs/ppc64el-linux-user.mak b/default-configs/ppc64el-linux-user.mak
> new file mode 100644
> index 0000000..6948225
> --- /dev/null
> +++ b/default-configs/ppc64el-linux-user.mak
> @@ -0,0 +1 @@
> +# Default configuration for ppc64el-linux-user
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Peter Maydell]

On 8 May 2014 13:18, Andreas Färber <afaerber@suse.de> wrote:
> Am 08.05.2014 10:26, schrieb Doug Kwan:
>> Add a new user mode target for little-endian PPC64.
>>
>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>> ---
>>  configure                              | 6 ++++++
>>  default-configs/ppc64el-linux-user.mak | 1 +
>>  2 files changed, 7 insertions(+)
>>  create mode 100644 default-configs/ppc64el-linux-user.mak
>>
>> diff --git a/configure b/configure
>> index ac2fa15..ce44044 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4917,6 +4917,12 @@ case "$target_name" in
>>      echo "TARGET_ABI32=y" >> $config_target_mak
>>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>>    ;;
>> +  ppc64el)
>
> In SUSE's build system and everywhere I read people talking about it,
> including your 2/3, it's called ppc64le. Why did you choose ppc64el?

In general we should follow what the kernel does, ie
the output of 'uname -m' (which hopefully coincides with
the gcc triplet name). What is that for this case?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Alexander Graf]

On 05/08/2014 02:28 PM, Peter Maydell wrote:
> On 8 May 2014 13:18, Andreas Färber <afaerber@suse.de> wrote:
>> Am 08.05.2014 10:26, schrieb Doug Kwan:
>>> Add a new user mode target for little-endian PPC64.
>>>
>>> Signed-off-by: Doug Kwan <dougkwan@google.com>
>>> ---
>>>   configure                              | 6 ++++++
>>>   default-configs/ppc64el-linux-user.mak | 1 +
>>>   2 files changed, 7 insertions(+)
>>>   create mode 100644 default-configs/ppc64el-linux-user.mak
>>>
>>> diff --git a/configure b/configure
>>> index ac2fa15..ce44044 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -4917,6 +4917,12 @@ case "$target_name" in
>>>       echo "TARGET_ABI32=y" >> $config_target_mak
>>>       gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>>>     ;;
>>> +  ppc64el)
>> In SUSE's build system and everywhere I read people talking about it,
>> including your 2/3, it's called ppc64le. Why did you choose ppc64el?
> In general we should follow what the kernel does, ie
> the output of 'uname -m' (which hopefully coincides with
> the gcc triplet name). What is that for this case?

$ uname -a
Linux shiraz-1 3.15.0-rc4-2-default+ #22 SMP Wed May 7 22:57:22 CEST 
2014 ppc64le ppc64le ppc64le GNU/Linux

Thanks a lot for the catch. Just because Debian has it all backwards we 
don't have to as well ;).


Alex

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

On Thu, May 8, 2014 at 5:28 AM, Peter Maydell <peter.maydell@linaro.org>wrote:

> On 8 May 2014 13:18, Andreas Färber <afaerber@suse.de> wrote:
> > Am 08.05.2014 10:26, schrieb Doug Kwan:
> >> Add a new user mode target for little-endian PPC64.
> >>
> >> Signed-off-by: Doug Kwan <dougkwan@google.com>
> >> ---
> >>  configure                              | 6 ++++++
> >>  default-configs/ppc64el-linux-user.mak | 1 +
> >>  2 files changed, 7 insertions(+)
> >>  create mode 100644 default-configs/ppc64el-linux-user.mak
> >>
> >> diff --git a/configure b/configure
> >> index ac2fa15..ce44044 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -4917,6 +4917,12 @@ case "$target_name" in
> >>      echo "TARGET_ABI32=y" >> $config_target_mak
> >>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml
> power-spe.xml"
> >>    ;;
> >> +  ppc64el)
> >
> > In SUSE's build system and everywhere I read people talking about it,
> > including your 2/3, it's called ppc64le. Why did you choose ppc64el?
>
> In general we should follow what the kernel does, ie
> the output of 'uname -m' (which hopefully coincides with
> the gcc triplet name). What is that for this case?
>
> thanks
> -- PMM
>

I am aware of the 'le' vs 'el' issue.  However, I saw that all bi-endian
targets have 'el' or 'eb' suffixes (e.g. mipsel-linux-user).  So I thought
qemu follows the Debian convention.  I will fix that.

Thanks.

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Tom Musta]

On 5/8/2014 3:26 AM, Doug Kwan wrote:
> Add a new user mode target for little-endian PPC64.
> 
> Signed-off-by: Doug Kwan <dougkwan@google.com>
> ---
>  configure                              | 6 ++++++
>  default-configs/ppc64el-linux-user.mak | 1 +
>  2 files changed, 7 insertions(+)
>  create mode 100644 default-configs/ppc64el-linux-user.mak
> 
> diff --git a/configure b/configure
> index ac2fa15..ce44044 100755
> --- a/configure
> +++ b/configure
> @@ -4917,6 +4917,12 @@ case "$target_name" in
>      echo "TARGET_ABI32=y" >> $config_target_mak
>      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>    ;;
> +  ppc64el)
> +    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"
> +  ;;
>    sh4|sh4eb)
>      TARGET_ARCH=sh4
>      bflt="yes"
> diff --git a/default-configs/ppc64el-linux-user.mak b/default-configs/ppc64el-linux-user.mak
> new file mode 100644
> index 0000000..6948225
> --- /dev/null
> +++ b/default-configs/ppc64el-linux-user.mak
> @@ -0,0 +1 @@
> +# Default configuration for ppc64el-linux-user
> 

We have TCG targets for PowerPC that are bi-endian. It would be much nicer to have to split
user mode by endianness.  If the user loads a BE ELF, then run in BE mode .... a LE ELF,
then run in LE mode.

I know there is much hand waving in that statement ... but maybe we can do better?  ("we" includes
"me" in that statement ... since I offered to Doug to help out with this).

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

On Thu, May 8, 2014 at 7:41 AM, Tom Musta <tommusta@gmail.com> wrote:

> On 5/8/2014 3:26 AM, Doug Kwan wrote:
> > Add a new user mode target for little-endian PPC64.
> >
> > Signed-off-by: Doug Kwan <dougkwan@google.com>
> > ---
> >  configure                              | 6 ++++++
> >  default-configs/ppc64el-linux-user.mak | 1 +
> >  2 files changed, 7 insertions(+)
> >  create mode 100644 default-configs/ppc64el-linux-user.mak
> >
> > diff --git a/configure b/configure
> > index ac2fa15..ce44044 100755
> > --- a/configure
> > +++ b/configure
> > @@ -4917,6 +4917,12 @@ case "$target_name" in
> >      echo "TARGET_ABI32=y" >> $config_target_mak
> >      gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml
> power-spe.xml"
> >    ;;
> > +  ppc64el)
> > +    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"
> > +  ;;
> >    sh4|sh4eb)
> >      TARGET_ARCH=sh4
> >      bflt="yes"
> > diff --git a/default-configs/ppc64el-linux-user.mak
> b/default-configs/ppc64el-linux-user.mak
> > new file mode 100644
> > index 0000000..6948225
> > --- /dev/null
> > +++ b/default-configs/ppc64el-linux-user.mak
> > @@ -0,0 +1 @@
> > +# Default configuration for ppc64el-linux-user
> >
>
> We have TCG targets for PowerPC that are bi-endian. It would be much nicer
> to have to split
> user mode by endianness.  If the user loads a BE ELF, then run in BE mode
> .... a LE ELF,
> then run in LE mode.
>
> I know there is much hand waving in that statement ... but maybe we can do
> better?  ("we" includes
> "me" in that statement ... since I offered to Doug to help out with this).
>
>
>
> To do this, we need to fix the elf load. I believe it currently only loads
binary the endianness of the same endianness of the configure target.  It
will be a more general change that will benefit other bi-endian
architecture.  Would you help me out in that?

-Doug

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Peter Maydell]

On 8 May 2014 16:19, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> On Thu, May 8, 2014 at 7:41 AM, Tom Musta <tommusta@gmail.com> wrote:
>> We have TCG targets for PowerPC that are bi-endian. It would be much nicer
>> to have to split
>> user mode by endianness.  If the user loads a BE ELF, then run in BE mode
>> .... a LE ELF,
>> then run in LE mode.
>>
>> I know there is much hand waving in that statement ... but maybe we can do
>> better?  ("we" includes
>> "me" in that statement ... since I offered to Doug to help out with this).

> To do this, we need to fix the elf load.

That is far from the only thing that would need changing.
I really don't recommend trying to do something different
from the norm here.

Isn't LE PPC a different ABI anyway? (ELFv2).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

On Thu, May 8, 2014 at 8:25 AM, Peter Maydell <peter.maydell@linaro.org>wrote:

> On 8 May 2014 16:19, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> > On Thu, May 8, 2014 at 7:41 AM, Tom Musta <tommusta@gmail.com> wrote:
> >> We have TCG targets for PowerPC that are bi-endian. It would be much
> nicer
> >> to have to split
> >> user mode by endianness.  If the user loads a BE ELF, then run in BE
> mode
> >> .... a LE ELF,
> >> then run in LE mode.
> >>
> >> I know there is much hand waving in that statement ... but maybe we can
> do
> >> better?  ("we" includes
> >> "me" in that statement ... since I offered to Doug to help out with
> this).
>
> > To do this, we need to fix the elf load.
>
> That is far from the only thing that would need changing.
> I really don't recommend trying to do something different
> from the norm here.
>
> Isn't LE PPC a different ABI anyway? (ELFv2).
>
> thanks
> -- PMM
>

What else needs to be fixed? I suspect that the user-mode linux kernel
interface may also need some work but it worked well enough in practice for
us to build glibc and run gcc testsuite.  Yes ELFv2 is a different ABI but
it is not complete different from the old one.  A notable change is the
removal of function descriptor.  That affect how the user mode start the
first instruction and that is addressed in this set of patches.  It would
also affect signal handling but user-mode signals do not work at all on
PPC64 anyway and the parts that would be affected looks unimplemented.

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Peter Maydell]

On 8 May 2014 16:32, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> What else needs to be fixed?

git grep ENDIAN linux-user is probably a good start on a list.

> I suspect that the user-mode linux kernel
> interface may also need some work but it worked well enough
> in practice for us to build glibc and run gcc testsuite.

The gcc testsuite is really not very much of a workout.
I recommend getting a chroot environment working and then
trying the LTP testsuite:
http://wiki.qemu.org/Testing/LTP

> Yes ELFv2 is a different ABI but it is not complete different
> from the old one.  A notable change is the
> removal of function descriptor.  That affect how the user mode start the
> first instruction and that is addressed in this set of patches.  It would
> also affect signal handling but user-mode signals do not work at all on
> PPC64 anyway and the parts that would be affected looks unimplemented.

Yes, but we need to actually implement signal handling at
some point, so we don't want to end up having to totally
redo the base support of ppc64le at that point.

The general approach linux-user takes is "one kernel
ABI to one QEMU binary". Maybe we could change that,
but would there be any benefit to it? I can't really
see one.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Doug Kwan (關振德)]

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

On Thu, May 8, 2014 at 8:41 AM, Peter Maydell <peter.maydell@linaro.org>wrote:

> On 8 May 2014 16:32, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> > What else needs to be fixed?
>
> git grep ENDIAN linux-user is probably a good start on a list.
>
> > I suspect that the user-mode linux kernel
> > interface may also need some work but it worked well enough
> > in practice for us to build glibc and run gcc testsuite.
>
> The gcc testsuite is really not very much of a workout.
> I recommend getting a chroot environment working and then
> trying the LTP testsuite:
> http://wiki.qemu.org/Testing/LTP
>
> > Yes ELFv2 is a different ABI but it is not complete different
> > from the old one.  A notable change is the
> > removal of function descriptor.  That affect how the user mode start the
> > first instruction and that is addressed in this set of patches.  It would
> > also affect signal handling but user-mode signals do not work at all on
> > PPC64 anyway and the parts that would be affected looks unimplemented.
>
> Yes, but we need to actually implement signal handling at
> some point, so we don't want to end up having to totally
> redo the base support of ppc64le at that point.
>
> The general approach linux-user takes is "one kernel
> ABI to one QEMU binary". Maybe we could change that,
> but would there be any benefit to it? I can't really
> see one.
>
> thanks
> -- PMM
>

May I ask what you would suggest to support ppc64le then?  For signal
handling, we can check the ELF header to see if we use a function
descriptor or an entry for a signal handler when this is implemented for
ppc64 in the future.  I don't understand the "redoing the base support" bit.

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

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Peter Maydell]

On 8 May 2014 16:51, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> On Thu, May 8, 2014 at 8:41 AM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>> The general approach linux-user takes is "one kernel
>> ABI to one QEMU binary". Maybe we could change that,
>> but would there be any benefit to it? I can't really
>> see one.

> May I ask what you would suggest to support ppc64le then?  For signal
> handling, we can check the ELF header to see if we use a function descriptor
> or an entry for a signal handler when this is implemented for ppc64 in the
> future.  I don't understand the "redoing the base support" bit.

Do it the same way we support everything else that can
deal with two endianness settings -- handle the other
TARGET_ENDIAN case in the translator. This is pretty
straightforward with the new TCGMemOp TCG ops.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Tom Musta]

On 5/8/2014 10:25 AM, Peter Maydell wrote:
> On 8 May 2014 16:19, Doug Kwan (關振德) <dougkwan@google.com> wrote:
>> On Thu, May 8, 2014 at 7:41 AM, Tom Musta <tommusta@gmail.com> wrote:
>>> We have TCG targets for PowerPC that are bi-endian. It would be much nicer
>>> to have to split
>>> user mode by endianness.  If the user loads a BE ELF, then run in BE mode
>>> .... a LE ELF,
>>> then run in LE mode.
>>>
>>> I know there is much hand waving in that statement ... but maybe we can do
>>> better?  ("we" includes
>>> "me" in that statement ... since I offered to Doug to help out with this).
> 
>> To do this, we need to fix the elf load.
> 
> That is far from the only thing that would need changing.
> I really don't recommend trying to do something different
> from the norm here.
> 
> Isn't LE PPC a different ABI anyway? (ELFv2).
> 
> thanks
> -- PMM
> 

It is true that the new LE distros are basing on ELFv2.  But ELFv2 ABI is not
limited to LE.  I chatted with Uli on this very topic and he confirmed that
endianness is specified in the e_ident field of the header.

I believe we have thoroughly demonstrated that the (softmmu) TCG code works
correctly in both settings of MSR[LE], without making compile time decisions.
It doesn't seem like we are that far off ... maybe I don't see the problem
yet.

Re: [Qemu-devel] [PATCH 3/3] configure: Add new target ppc64el-linux-user

[Peter Maydell]

On 8 May 2014 16:39, Tom Musta <tommusta@gmail.com> wrote:
> I believe we have thoroughly demonstrated that the (softmmu)
> TCG code works correctly in both settings of MSR[LE], without
> making compile time decisions. It doesn't seem like we are that
> far off ... maybe I don't see the problem yet.

Yes, softmmu can and should all work in one TARGET_ENDIAN
setting (which corresponds to "endianness of CPU bus",
more or less). But linux-user isn't softmmu.

thanks
-- PMM