| Message ID | 20250429050010.971128-10-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: compile target/arm twice | expand |
On 29/04/25, Pierrick Bouvier wrote: > Remove TARGET_BIG_ENDIAN dependency. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@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.
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.bouvier@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
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; } info->flags &= ~INSN_ARM_BE32; #ifndef CONFIG_USER_ONLY @@ -2681,7 +2681,7 @@ static const struct SysemuCPUOps arm_sysemu_ops = { .asidx_from_attrs = arm_asidx_from_attrs, .write_elf32_note = arm_cpu_write_elf32_note, .write_elf64_note = arm_cpu_write_elf64_note, - .virtio_is_big_endian = arm_cpu_virtio_is_big_endian, + .virtio_is_big_endian = arm_cpu_is_big_endian, .legacy_vmsd = &vmstate_arm_cpu, }; #endif
Remove TARGET_BIG_ENDIAN dependency. Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- target/arm/cpu.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)