Re: [Qemu-devel] [PATCH v2 07/18] linux-user: arm: handle CPSR.E correctly in strex emulation

[Peter Maydell]

On 2 March 2016 at 06:56, Peter Crosthwaite <address@hidden> wrote:
> From: Paolo Bonzini <address@hidden>
> @@ -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 <address@hidden>

thanks
-- PMM