Re: [PULL 04/32] hw/timer/xilinx_timer: Make device endianness configurable

[Thomas Huth <thuth@redhat.com>]

On 12/02/2025 10.19, Philippe Mathieu-Daudé wrote:
> On 12/2/25 09:27, Thomas Huth wrote:
>> On 10/02/2025 21.41, Philippe Mathieu-Daudé wrote:
>>> Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
>>> of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
>>> Add the "little-endian" property to select the device
>>> endianness, defaulting to little endian.
>>> Set the proper endianness for each machine using the device.
>>>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Message-Id: <20250206131052.30207-5-philmd@linaro.org>
>>> ---
>>>   hw/microblaze/petalogix_ml605_mmu.c      |  1 +
>>>   hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
>>>   hw/ppc/virtex_ml507.c                    |  1 +
>>>   hw/timer/xilinx_timer.c                  | 35 +++++++++++++++---------
>>>   4 files changed, 25 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/ 
>>> petalogix_ml605_mmu.c
>>> index cf3b9574db3..bbda70aa93b 100644
>>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>>> @@ -127,6 +127,7 @@ petalogix_ml605_init(MachineState *machine)
>>>       /* 2 timers at irq 2 @ 100 Mhz.  */
>>>       dev = qdev_new("xlnx.xps-timer");
>>> +    qdev_prop_set_bit(dev, "little-endian", true);
>>>       qdev_prop_set_uint32(dev, "one-timer-only", 0);
>>>       qdev_prop_set_uint32(dev, "clock-frequency", 100 * 1000000);
>>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/ 
>>> petalogix_s3adsp1800_mmu.c
>>> index fbf52ba8f2f..9d4316b4036 100644
>>> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
>>> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
>>> @@ -114,6 +114,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>>       /* 2 timers at irq 2 @ 62 Mhz.  */
>>>       dev = qdev_new("xlnx.xps-timer");
>>> +    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
>>>       qdev_prop_set_uint32(dev, "one-timer-only", 0);
>>>       qdev_prop_set_uint32(dev, "clock-frequency", 62 * 1000000);
>>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
>>> index 23238119273..f87c221d076 100644
>>> --- a/hw/ppc/virtex_ml507.c
>>> +++ b/hw/ppc/virtex_ml507.c
>>> @@ -230,6 +230,7 @@ static void virtex_init(MachineState *machine)
>>>       /* 2 timers at irq 2 @ 62 Mhz.  */
>>>       dev = qdev_new("xlnx.xps-timer");
>>> +    qdev_prop_set_bit(dev, "little-endian", false);
>>>       qdev_prop_set_uint32(dev, "one-timer-only", 0);
>>>       qdev_prop_set_uint32(dev, "clock-frequency", 62 * 1000000);
>>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>
>>   Hi,
>>
>> with this patch applied, the ppc_virtex_ml507 functional test is now 
>> failing for me ... could you please double-check whether "make check- 
>> functional-ppc" still works for you?
> 
> Thanks, not this patch problem, but patch #2 misses:
> 
> -- >8 --
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 23238119273..723f62c904b 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -219,2 +219,3 @@ static void virtex_init(MachineState *machine)
>       dev = qdev_new("xlnx.xps-intc");
> +    qdev_prop_set_bit(dev, "little-endian", false);
>       qdev_prop_set_uint32(dev, "kind-of-intr", 0);
> ---
> 
> Why is my CI green?

Looking at https://gitlab.com/philmd/qemu/-/pipelines/1664238124 it seems 
like you did not start the functional test jobs?

  Thomas