[v2,07/18] linux-user: arm: handle CPSR.E correctly in strex emulation

Commit Message

Peter Crosthwaite March 2, 2016, 6:56 a.m. UTC

From: Paolo Bonzini <[e-mail]>

Now that CPSR.E is set correctly, prepare for when setend will be able
to change it; bswap data in and out of strex manually by comparing
SCTLR.B, CPSR.E and TARGET_WORDS_BIGENDIAN (we do not have the luxury
of using TCGMemOps).

Signed-off-by: Paolo Bonzini <[e-mail]>
[ PC changes:
  * Moved SCTLR/CPSR logic to arm_cpu_data_is_big_endian
]
Signed-off-by: Peter Crosthwaite <[e-mail]>
---
Changed since v1:
Removed SCTLR logic from bwap_data()

 linux-user/main.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 target-arm/cpu.h  | 11 +++++++++++
 2 files changed, 54 insertions(+), 7 deletions(-)

Comments

Peter Maydell March 3, 2016, 3:09 p.m. UTC | #1

On 2 March 2016 at 06:56, Peter Crosthwaite <[e-mail]> wrote:
> From: Paolo Bonzini <[e-mail]>
> @@ -610,11 +642,11 @@ static int do_strex(CPUARMState *env)
>          segv = get_user_u8(val, addr);
>          break;
>      case 1:
> -        segv = get_user_u16(val, addr);
> +        segv = get_user_data_u16(val, addr, env);
>          break;
>      case 2:
>      case 3:
> -        segv = get_user_u32(val, addr);
> +        segv = get_user_data_u32(val, addr, env);
>          break;
>      default:
>          abort();
> @@ -625,12 +657,16 @@ static int do_strex(CPUARMState *env)
>      }
>      if (size == 3) {
>          uint32_t valhi;
> -        segv = get_user_u32(valhi, addr + 4);
> +        segv = get_user_data_u32(valhi, addr + 4, env);
>          if (segv) {
>              env->exception.vaddress = addr + 4;
>              goto done;
>          }
> -        val = deposit64(val, 32, 32, valhi);
> +        if (arm_cpu_bswap_data(env)) {
> +            val = deposit64((uint64_t)valhi, 32, 32, val);
> +        } else {
> +            val = deposit64(val, 32, 32, valhi);
> +        }
>      }
>      if (val != env->exclusive_val) {
>          goto fail;
> @@ -642,11 +678,11 @@ static int do_strex(CPUARMState *env)
>          segv = put_user_u8(val, addr);
>          break;
>      case 1:
> -        segv = put_user_u16(val, addr);
> +        segv = put_user_data_u16(val, addr, env);
>          break;
>      case 2:
>      case 3:
> -        segv = put_user_u32(val, addr);
> +        segv = put_user_data_u32(val, addr, env);
>          break;
>      }
>      if (segv) {
> @@ -655,7 +691,7 @@ static int do_strex(CPUARMState *env)
>      }
>      if (size == 3) {
>          val = env->regs[(env->exclusive_info >> 12) & 0xf];
> -        segv = put_user_u32(val, addr + 4);
> +        segv = put_user_data_u32(val, addr + 4, env);
>          if (segv) {
>              env->exception.vaddress = addr + 4;
>              goto done;

This confused me for a bit because it doesn't swap the two
halves of a 64-bit word on the 'store new value' codepath,
but it is correct -- for STREXD for AArch32 the first word
in memory is Rt and the second is Rt2, regardless of
endianness.

Reviewed-by: Peter Maydell <[e-mail]>

thanks
-- PMM

Patch

diff --git a/linux-user/main.c b/linux-user/main.c
index 510b3b7..2b1e755 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -451,6 +451,38 @@  void cpu_loop(CPUX86State *env)
         __r;                                            \
     })
 
+#define get_user_data_u32(x, gaddr, env)                \
+    ({ abi_long __r = get_user_u32((x), (gaddr));       \
+        if (!__r && arm_cpu_bswap_data(env)) {          \
+            (x) = bswap32(x);                           \
+        }                                               \
+        __r;                                            \
+    })
+
+#define get_user_data_u16(x, gaddr, env)                \
+    ({ abi_long __r = get_user_u16((x), (gaddr));       \
+        if (!__r && arm_cpu_bswap_data(env)) {          \
+            (x) = bswap16(x);                           \
+        }                                               \
+        __r;                                            \
+    })
+
+#define put_user_data_u32(x, gaddr, env)                \
+    ({ typeof(x) __x = (x);                             \
+        if (arm_cpu_bswap_data(env)) {                  \
+            __x = bswap32(__x);                         \
+        }                                               \
+        put_user_u32(__x, (gaddr));                     \
+    })
+
+#define put_user_data_u16(x, gaddr, env)                \
+    ({ typeof(x) __x = (x);                             \
+        if (arm_cpu_bswap_data(env)) {                  \
+            __x = bswap16(__x);                         \
+        }                                               \
+        put_user_u16(__x, (gaddr));                     \
+    })
+
 #ifdef TARGET_ABI32
 /* Commpage handling -- there is no commpage for AArch64 */
 
@@ -610,11 +642,11 @@  static int do_strex(CPUARMState *env)
         segv = get_user_u8(val, addr);
         break;
     case 1:
-        segv = get_user_u16(val, addr);
+        segv = get_user_data_u16(val, addr, env);
         break;
     case 2:
     case 3:
-        segv = get_user_u32(val, addr);
+        segv = get_user_data_u32(val, addr, env);
         break;
     default:
         abort();
@@ -625,12 +657,16 @@  static int do_strex(CPUARMState *env)
     }
     if (size == 3) {
         uint32_t valhi;
-        segv = get_user_u32(valhi, addr + 4);
+        segv = get_user_data_u32(valhi, addr + 4, env);
         if (segv) {
             env->exception.vaddress = addr + 4;
             goto done;
         }
-        val = deposit64(val, 32, 32, valhi);
+        if (arm_cpu_bswap_data(env)) {
+            val = deposit64((uint64_t)valhi, 32, 32, val);
+        } else {
+            val = deposit64(val, 32, 32, valhi);
+        }
     }
     if (val != env->exclusive_val) {
         goto fail;
@@ -642,11 +678,11 @@  static int do_strex(CPUARMState *env)
         segv = put_user_u8(val, addr);
         break;
     case 1:
-        segv = put_user_u16(val, addr);
+        segv = put_user_data_u16(val, addr, env);
         break;
     case 2:
     case 3:
-        segv = put_user_u32(val, addr);
+        segv = put_user_data_u32(val, addr, env);
         break;
     }
     if (segv) {
@@ -655,7 +691,7 @@  static int do_strex(CPUARMState *env)
     }
     if (size == 3) {
         val = env->regs[(env->exclusive_info >> 12) & 0xf];
-        segv = put_user_u32(val, addr + 4);
+        segv = put_user_data_u32(val, addr + 4, env);
         if (segv) {
             env->exception.vaddress = addr + 4;
             goto done;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ab0ea92..cbf171c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -2102,6 +2102,17 @@  static inline int fp_exception_el(CPUARMState *env)
     return 0;
 }
 
+#ifdef CONFIG_USER_ONLY
+static inline bool arm_cpu_bswap_data(CPUARMState *env)
+{
+    return
+#ifdef TARGET_WORDS_BIGENDIAN
+       1 ^
+#endif
+       arm_cpu_data_is_big_endian(env);
+}
+#endif
+
 static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {