Re: [PATCH 03/20] target/arm: Implement CPUClass::datapath_is_big_endian

[Richard Henderson <richard.henderson@linaro.org>]

On 12/4/24 14:25, Philippe Mathieu-Daudé wrote:
> arm_cpu_virtio_is_big_endian() already returns whether
> the data path is in big endian. Re-use that.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/cpu.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

No, this is wrong.

(1) arm_cpu_virtio_is_big_endian really *is* specific to virtio,
     and legacy virtio at that.  It changes based on the state of the cpu.

(2) The board-level stuff for which you appear to be intending to
     replace qemu_binary_is_bigendian(), for arm, is *always* little-endian,
     and has no relation to the state of the cpu.

(3) I thought you were on a better track plumbing endianness through
     properties on machine state.


r~

> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 6938161b954..3061b2ac18c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1132,7 +1132,7 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
>   #endif
>   }
>   
> -static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
> +static bool arm_cpu_datapath_is_big_endian(CPUState *cs)
>   {
>       ARMCPU *cpu = ARM_CPU(cs);
>       CPUARMState *env = &cpu->env;
> @@ -2664,7 +2664,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_datapath_is_big_endian,
>       .legacy_vmsd = &vmstate_arm_cpu,
>   };
>   #endif
> @@ -2709,6 +2709,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>                                          &acc->parent_phases);
>   
>       cc->class_by_name = arm_cpu_class_by_name;
> +    cc->datapath_is_big_endian = arm_cpu_datapath_is_big_endian;
>       cc->has_work = arm_cpu_has_work;
>       cc->mmu_index = arm_cpu_mmu_index;
>       cc->dump_state = arm_cpu_dump_state;