Re: [PATCH v2 03/10] target/arm: Set disassemble_info::endian value in disas_set_info()

[Philippe Mathieu-Daudé]

On 10/2/25 23:10, Richard Henderson wrote:
> On 2/10/25 13:29, Philippe Mathieu-Daudé wrote:
> > Have the CPUClass::disas_set_info() callback set the
> > disassemble_info::endian field.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   target/arm/cpu.c | 10 +++-------
> >   1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 94f1c55622b..68b3a9d3ab0 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1188,7 +1188,7 @@ static void arm_disas_set_info(CPUState *cpu, 
> > disassemble_info *info)
> >   {
> >       ARMCPU *ac = ARM_CPU(cpu);
> >       CPUARMState *env = &ac->env;
> > -    bool sctlr_b;
> > +    bool sctlr_b = arm_sctlr_b(env);
> >       if (is_a64(env)) {
> >           info->cap_arch = CS_ARCH_ARM64;
> > @@ -1215,13 +1215,9 @@ static void arm_disas_set_info(CPUState *cpu, 
> > disassemble_info *info)
> >           info->cap_mode = cap_mode;
> >       }
> > -    sctlr_b = arm_sctlr_b(env);
> > +    info->endian = BFD_ENDIAN_LITTLE;
> >       if (bswap_code(sctlr_b)) {
> > -#if TARGET_BIG_ENDIAN
> > -        info->endian = BFD_ENDIAN_LITTLE;
> > -#else
> > -        info->endian = BFD_ENDIAN_BIG;
> > -#endif
> > +        info->endian = TARGET_BIG_ENDIAN ? BFD_ENDIAN_LITTLE : 
> > BFD_ENDIAN_BIG;
> >       }
> >       info->flags &= ~INSN_ARM_BE32;
> >   #ifndef CONFIG_USER_ONLY
>
> This is a faithful adjustment to the existing code, so,
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> However:
>
> (1) aarch64 code is always little-endian,
> (2) sctlr_b is always false from armv7 (and thus always false for aarch64)
> (3) I think the BE32 logic is wrong -- CONFIG_USER_ONLY is irrelevant.
>      See linux-user/arm/cpu_loop.c, target_cpu_copy_regs.

What about v7-R [*]? I don't see SCTLR_IE defined as 1<<31 for AArch32,
only:

#define SCTLR_EnIA    (1U << 31) /* v8.3, AArch64 only */
#define SCTLR_DSSBS_32 (1U << 31) /* v8.5, AArch32 only */

---

[*] 
https://developer.arm.com/documentation/ddi0406/cb/System-Level-Architecture/System-Control-Registers-in-a-PMSA-implementation/PMSA-System-control-registers-descriptions--in-register-order/SCTLR--System-Control-Register--PMSA

SCTLR_IE, bit[31]

Instruction Endianness. This bit indicates the endianness of the
instructions issued to the processor. The possible values of this bit are:

0: Little-endian byte ordering in the instructions.
1: Big-endian byte ordering in the instructions.

When set to 1, this bit causes the byte order of instructions to be 
reversed at runtime.

This bit is read-only. It is implementation defined which instruction 
endianness is used by an ARMv7-R implementation, and this bit must 
indicate the implemented endianness.

---

For the Cortex-r5* we have, SCTLR_IE is always 0 in reset_sctlr.

Is it OK to consider v7-R implemented as little-endian in QEMU?