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

[Philippe Mathieu-Daudé]

On 12/2/25 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.hender...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Message-Id: <20250206131052.30207-5-phi...@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);

It looks to me like another "default value problem", where using
a tri-state with default=unset would have catched.

"disas/dis-asm.h" defines:

enum bfd_endian { BFD_ENDIAN_BIG, BFD_ENDIAN_LITTLE, BFD_ENDIAN_UNKNOWN };

Maybe endianness is common enough than we can add a QAPI enum,
in machine-common.json or even common.json?

Then I'd use qdev_prop_set_enum() here.

      qdev_prop_set_uint32(dev, "kind-of-intr", 0);
---

Why is my CI green?