Re: [PATCH 09/13] target/arm/cpu: get endianness from cpu state

[Pierrick Bouvier]

On 4/29/25 5:26 AM, Anton Johansson wrote:
On 29/04/25, Pierrick Bouvier wrote:
Remove TARGET_BIG_ENDIAN dependency.
Signed-off-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
---
  target/arm/cpu.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e7a15ade8b4..85e886944f6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -67,6 +67,15 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
      }
  }
  
+static bool arm_cpu_is_big_endian(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    cpu_synchronize_state(cs);
+    return arm_cpu_data_is_big_endian(env);
+}
+
  static vaddr arm_cpu_get_pc(CPUState *cs)
  {
      ARMCPU *cpu = ARM_CPU(cs);
@@ -1130,15 +1139,6 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, 
int level)
  #endif
  }
  
-static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    cpu_synchronize_state(cs);
-    return arm_cpu_data_is_big_endian(env);
-}
-
  #ifdef CONFIG_TCG
  bool arm_cpu_exec_halt(CPUState *cs)
  {
@@ -1203,7 +1203,7 @@ static void arm_disas_set_info(CPUState *cpu, 
disassemble_info *info)
  
      info->endian = BFD_ENDIAN_LITTLE;
      if (bswap_code(sctlr_b)) {
-        info->endian = TARGET_BIG_ENDIAN ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
+        info->endian = arm_cpu_is_big_endian(cpu) ? BFD_ENDIAN_LITTLE : 
BFD_ENDIAN_BIG;
      }

I'm not the most familiar with arm but as far as I can tell these are
not equivalent. My understanding is that arm_cpu_is_big_endian() models
data endianness, and TARGET_BIG_ENDIAN instruction endianness.

Also, for user mode where this branch is relevant, bswap_code() still
depends on TARGET_BIG_ENDIAN anyway and the above branch would reduce to
(on arm32)

   if (TARGET_BIG_ENDIAN ^ sctlr_b) {
       info->endian = sctlr_b ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
   }

giving the opposite result to the original code.


Ooops, that's a good point, I missed it was calling 
arm_cpu_data_is_big_endian under the hoods.

I'll stick to target_big_endian().

Thanks,
Pierrick