[05/20] target/arm: Fix arm_cpu_data_is_big_endian for aa64 user-only

Message ID 20180809042206.15726-6-richard.henderson@linaro.org
State New
Headers show
Series
  • target/arm: sve system mode patches
Related show

Commit Message

Richard Henderson Aug. 9, 2018, 4:21 a.m.
Unlike aa32, endianness cannot be adjusted by userland in aa64.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/cpu.h | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

-- 
2.17.1

Comments

Peter Maydell Aug. 17, 2018, 4:02 p.m. | #1
On 9 August 2018 at 05:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Unlike aa32, endianness cannot be adjusted by userland in aa64.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/cpu.h | 27 +++++++++++++++++----------

>  1 file changed, 17 insertions(+), 10 deletions(-)

>

> diff --git a/target/arm/cpu.h b/target/arm/cpu.h

> index 9526ed27cb..2d6d7d03aa 100644

> --- a/target/arm/cpu.h

> +++ b/target/arm/cpu.h

> @@ -2709,8 +2709,6 @@ static inline bool arm_sctlr_b(CPUARMState *env)

>  /* Return true if the processor is in big-endian mode. */

>  static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

>  {

> -    int cur_el;

> -

>      /* In 32bit endianness is determined by looking at CPSR's E bit */

>      if (!is_a64(env)) {

>          return

> @@ -2729,15 +2727,24 @@ static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

>              arm_sctlr_b(env) ||

>  #endif

>                  ((env->uncached_cpsr & CPSR_E) ? 1 : 0);

> +    } else {

> +#ifdef CONFIG_USER_ONLY

> +        /* AArch64 does not have a SETEND instruction; endianness

> +         * for usermode is fixed at compile-time.

> +         */

> +# ifdef TARGET_WORDS_BIGENDIAN

> +        return true;

> +# else

> +        return false;

> +# endif

> +#else

> +        int cur_el = arm_current_el(env);

> +        if (cur_el == 0) {

> +            return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;

> +        }

> +        return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;

> +#endif

>      }

> -

> -    cur_el = arm_current_el(env);

> -

> -    if (cur_el == 0) {

> -        return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;

> -    }

> -

> -    return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;

>  }

>


When does this make a difference? For user-mode, we've already
dealt with the "aa32" case, so the code here is aa64-only.
In linux-user/aarch64/cpu_loop.c we set sctlr_el[1]'s E0E bit
if TARGET_WORDS_BIGENDIAN is defined, and cur_el is definitely
zero, so we should already be returning true from this function
if TARGET_WORDS_BIGENDIAN and false otherwise.

thanks
-- PMM
Richard Henderson Aug. 17, 2018, 4:47 p.m. | #2
On 08/17/2018 09:02 AM, Peter Maydell wrote:
> On 9 August 2018 at 05:21, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>> Unlike aa32, endianness cannot be adjusted by userland in aa64.

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>  target/arm/cpu.h | 27 +++++++++++++++++----------

>>  1 file changed, 17 insertions(+), 10 deletions(-)

>>

>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h

>> index 9526ed27cb..2d6d7d03aa 100644

>> --- a/target/arm/cpu.h

>> +++ b/target/arm/cpu.h

>> @@ -2709,8 +2709,6 @@ static inline bool arm_sctlr_b(CPUARMState *env)

>>  /* Return true if the processor is in big-endian mode. */

>>  static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

>>  {

>> -    int cur_el;

>> -

>>      /* In 32bit endianness is determined by looking at CPSR's E bit */

>>      if (!is_a64(env)) {

>>          return

>> @@ -2729,15 +2727,24 @@ static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)

>>              arm_sctlr_b(env) ||

>>  #endif

>>                  ((env->uncached_cpsr & CPSR_E) ? 1 : 0);

>> +    } else {

>> +#ifdef CONFIG_USER_ONLY

>> +        /* AArch64 does not have a SETEND instruction; endianness

>> +         * for usermode is fixed at compile-time.

>> +         */

>> +# ifdef TARGET_WORDS_BIGENDIAN

>> +        return true;

>> +# else

>> +        return false;

>> +# endif

>> +#else

>> +        int cur_el = arm_current_el(env);

>> +        if (cur_el == 0) {

>> +            return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;

>> +        }

>> +        return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;

>> +#endif

>>      }

>> -

>> -    cur_el = arm_current_el(env);

>> -

>> -    if (cur_el == 0) {

>> -        return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;

>> -    }

>> -

>> -    return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;

>>  }

>>

> 

> When does this make a difference? For user-mode, we've already

> dealt with the "aa32" case, so the code here is aa64-only.

> In linux-user/aarch64/cpu_loop.c we set sctlr_el[1]'s E0E bit

> if TARGET_WORDS_BIGENDIAN is defined, and cur_el is definitely

> zero, so we should already be returning true from this function

> if TARGET_WORDS_BIGENDIAN and false otherwise.


I should have re-ordered this after the other following
simplifications to see if it still matters.  But I was
after a code-size reduction.


r~

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9526ed27cb..2d6d7d03aa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2709,8 +2709,6 @@  static inline bool arm_sctlr_b(CPUARMState *env)
 /* Return true if the processor is in big-endian mode. */
 static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)
 {
-    int cur_el;
-
     /* In 32bit endianness is determined by looking at CPSR's E bit */
     if (!is_a64(env)) {
         return
@@ -2729,15 +2727,24 @@  static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)
             arm_sctlr_b(env) ||
 #endif
                 ((env->uncached_cpsr & CPSR_E) ? 1 : 0);
+    } else {
+#ifdef CONFIG_USER_ONLY
+        /* AArch64 does not have a SETEND instruction; endianness
+         * for usermode is fixed at compile-time.
+         */
+# ifdef TARGET_WORDS_BIGENDIAN
+        return true;
+# else
+        return false;
+# endif
+#else
+        int cur_el = arm_current_el(env);
+        if (cur_el == 0) {
+            return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
+        }
+        return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
+#endif
     }
-
-    cur_el = arm_current_el(env);
-
-    if (cur_el == 0) {
-        return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
-    }
-
-    return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
 }
 
 #include "exec/cpu-all.h"