Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/tcg/translate.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 7fcf0656630..68f050746d4 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -6279,6 +6279,13 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)
     }
 }
 
+static MemOp arm_memop_endian_swap(bool sctlr_b)
+{
+    MemOp t = target_big_endian() ? MO_BE : MO_LE;
+    bool do_swap = bswap_code(sctlr_b);
+    return t ^ (do_swap * MO_BSWAP);
+}
+
 /* Ditto, for a halfword (Thumb) instruction */
 static inline uint16_t arm_lduw_code(CPUARMState *env, DisasContextBase* s,
                                      target_ulong addr, bool sctlr_b)
@@ -6290,7 +6297,7 @@ static inline uint16_t arm_lduw_code(CPUARMState *env, DisasContextBase* s,
         addr ^= 2;
     }
 #endif
-    return translator_lduw_swap(env, s, addr, bswap_code(sctlr_b));
+    return translator_lduw_end(env, s, addr, arm_memop_endian_swap(sctlr_b));
 }
 
 static bool insn_crosses_page(CPUARMState *env, DisasContext *s)
@@ -6531,7 +6538,7 @@ static void arm_post_translate_insn(DisasContext *dc)
 static inline uint32_t arm_ldl_code(CPUARMState *env, DisasContextBase *s,
                                     target_ulong addr, bool sctlr_b)
 {
-    return translator_ldl_swap(env, s, addr, bswap_code(sctlr_b));
+    return translator_ldl_end(env, s, addr, arm_memop_endian_swap(sctlr_b));
 }
 
 static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
-- 
2.47.3

On 4/7/26 04:26, Pierrick Bouvier wrote:
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   target/arm/tcg/translate.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> index 7fcf0656630..68f050746d4 100644
> --- a/target/arm/tcg/translate.c
> +++ b/target/arm/tcg/translate.c
> @@ -6279,6 +6279,13 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)
>       }
>   }
>   
> +static MemOp arm_memop_endian_swap(bool sctlr_b)
> +{
> +    MemOp t = target_big_endian() ? MO_BE : MO_LE;
> +    bool do_swap = bswap_code(sctlr_b);
> +    return t ^ (do_swap * MO_BSWAP);
> +}

This and bswap_code can be vastly simplified.

The complications date to when code access could only be done matching TARGET_BIG_ENDIAN. 
Now, of course, we can directly specify the endianness, so we should.

This should be as simple as "sctlr_b ? MO_BE : MO_LE".

I think there should be no need for CONFIG_USER_ONLY tests.  I'm not sure what's still 
different between user and system modes there.  Unfortunately, testing of BE32 might be 
impossible at this point due to armv4be dropping off support lists.


r~

On 4/6/26 3:24 PM, Richard Henderson wrote:
> On 4/7/26 04:26, Pierrick Bouvier wrote:
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/tcg/translate.c | 11 +++++++++--
>>    1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
>> index 7fcf0656630..68f050746d4 100644
>> --- a/target/arm/tcg/translate.c
>> +++ b/target/arm/tcg/translate.c
>> @@ -6279,6 +6279,13 @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn)
>>        }
>>    }
>>    
>> +static MemOp arm_memop_endian_swap(bool sctlr_b)
>> +{
>> +    MemOp t = target_big_endian() ? MO_BE : MO_LE;
>> +    bool do_swap = bswap_code(sctlr_b);
>> +    return t ^ (do_swap * MO_BSWAP);
>> +}
> 
> This and bswap_code can be vastly simplified.
> 
> The complications date to when code access could only be done matching TARGET_BIG_ENDIAN.
> Now, of course, we can directly specify the endianness, so we should.
> 
> This should be as simple as "sctlr_b ? MO_BE : MO_LE".
> 
> I think there should be no need for CONFIG_USER_ONLY tests.  I'm not sure what's still
> different between user and system modes there.  Unfortunately, testing of BE32 might be
> impossible at this point due to armv4be dropping off support lists.
>

I noticed you added a similar comment on v0, but I was not able to 
understand what you were expecting.
Could you please send a snippet of which change you would prefer instead 
of current patch?

> 
> r~

On 4/7/26 08:27, Pierrick Bouvier wrote:
> I noticed you added a similar comment on v0, but I was not able to understand what you 
> were expecting.
> Could you please send a snippet of which change you would prefer instead of current patch?

Delete bswap_code.

In arm_disas_set_info:

      info->endian = BFD_ENDIAN_LITTLE;
-    if (bswap_code(sctlr_b)) {
-        info->endian = target_big_endian() ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
-    }
      info->flags &= ~INSN_ARM_BE32;
-#ifndef CONFIG_USER_ONLY
     if (sctlr_b) {
         info->endian = BFD_ENDIAN_BIG;
         info->flags |= INSN_ARM_BE32;
     }
-#endif

In arm_ldl_code:

-    return translator_ldl_swap(env, s, addr, bswap_code(sctlr_b));
+    return translator_ldl_end(env, s, addr, sctlr_b ? MO_BE : MO_LE);

In arm_lduw_code:

  {
+    MemOp end = MO_LE;
+
-#ifndef CONFIG_USER_ONLY
-    /* In big-endian (BE32) mode, adjacent Thumb instructions have been swapped
-       within each word.  Undo that now.  */
      if (sctlr_b) {
+        /* In BE32 mode, adjacent Thumb instructions are swapped. */
          addr ^= 2;
+        end = MO_BE;
      }
-#endif
-    return translator_lduw_swap(env, s, addr, bswap_code(sctlr_b));
+    return translator_lduw_end(env, s, addr, end);
  }


r~

On 4/6/26 3:44 PM, Richard Henderson wrote:
> On 4/7/26 08:27, Pierrick Bouvier wrote:
>> I noticed you added a similar comment on v0, but I was not able to understand what you
>> were expecting.
>> Could you please send a snippet of which change you would prefer instead of current patch?
> 
> Delete bswap_code.
> 
> In arm_disas_set_info:
> 
>        info->endian = BFD_ENDIAN_LITTLE;
> -    if (bswap_code(sctlr_b)) {
> -        info->endian = target_big_endian() ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
> -    }
>        info->flags &= ~INSN_ARM_BE32;
> -#ifndef CONFIG_USER_ONLY
>       if (sctlr_b) {
>           info->endian = BFD_ENDIAN_BIG;
>           info->flags |= INSN_ARM_BE32;
>       }
> -#endif
> 
> In arm_ldl_code:
> 
> -    return translator_ldl_swap(env, s, addr, bswap_code(sctlr_b));
> +    return translator_ldl_end(env, s, addr, sctlr_b ? MO_BE : MO_LE);
> 
> In arm_lduw_code:
> 
>    {
> +    MemOp end = MO_LE;
> +
> -#ifndef CONFIG_USER_ONLY
> -    /* In big-endian (BE32) mode, adjacent Thumb instructions have been swapped
> -       within each word.  Undo that now.  */
>        if (sctlr_b) {
> +        /* In BE32 mode, adjacent Thumb instructions are swapped. */
>            addr ^= 2;
> +        end = MO_BE;
>        }
> -#endif
> -    return translator_lduw_swap(env, s, addr, bswap_code(sctlr_b));
> +    return translator_lduw_end(env, s, addr, end);
>    }
>

There is still one usage for bswap_code in linux-user/arm/cpu_loop.c 
which I'm not sure what to do with.

> 
> r~

On 4/7/26 09:00, Pierrick Bouvier wrote:
> On 4/6/26 3:44 PM, Richard Henderson wrote:
>> Delete bswap_code.
...
> There is still one usage for bswap_code in linux-user/arm/cpu_loop.c which I'm not sure 
> what to do with.

Essentially,

     x = load_little_endian(...);
     if (arm_sctlr_b(env)) {
        x = bswap32(x);
     }

This is slightly more complicated because there's not currently a get_user macro to force 
an endianness, only __get_user_e.  So it's a question of whether to introduce get_user_e 
or open-code all of the goop in get_user_code_u32.

Actually, there's currently a FIXME in cpu_loop concerning the failure of get_user, so I'd 
say most of the get_user goop belongs there.  Then using __get_user_e is trivial.


r~

On 4/6/26 11:51 PM, Richard Henderson wrote:
> On 4/7/26 09:00, Pierrick Bouvier wrote:
>> On 4/6/26 3:44 PM, Richard Henderson wrote:
>>> Delete bswap_code.
> ...
>> There is still one usage for bswap_code in linux-user/arm/cpu_loop.c which I'm not sure
>> what to do with.
> 
> Essentially,
> 
>       x = load_little_endian(...);
>       if (arm_sctlr_b(env)) {
>          x = bswap32(x);
>       }
> 
> This is slightly more complicated because there's not currently a get_user macro to force
> an endianness, only __get_user_e.  So it's a question of whether to introduce get_user_e
> or open-code all of the goop in get_user_code_u32.
> 
> Actually, there's currently a FIXME in cpu_loop concerning the failure of get_user, so I'd
> say most of the get_user goop belongs there.  Then using __get_user_e is trivial.
> 
>

That's not clear for me what is expected here, I'll let you send a patch 
for the kind of changes you want to see.
I can then integrate it in the series with bswap_code removal.

> r~

Regards,
Pierrick