[PATCH v5 00/16] hw/microblaze: Allow running cross-endian vCPUs

[PATCH v5 00/16] hw/microblaze: Allow running cross-endian vCPUs

[Philippe Mathieu-Daudé]

Missing review: 2 & 14 (new)

Since v4 & v3:
- Addressed Thomas review comments

Since v2:
- Addressed Richard's review comments

Since v1:
- Make device endianness configurable (Edgar)
- Convert more Xilinx devices
- Avoid preprocessor #if (Richard)
- Add R-b tags

Make machines endianness-agnostic, allowing to run a big-endian vCPU
on the little-endian 'qemu-system-microblazeel' binary, and a little
endian one on the big-endian 'qemu-system-microblaze' binary.

Tests added, following combinations covered:
- little-endian vCPU using little-endian binary (in-tree)
- little-endian vCPU using big-endian binary (new)
- big-endian vCPU using little-endian binary (new)
- big-endian vCPU using big-endian binary (in-tree)

To make a target endian-agnostic we need to remove the MO_TE uses.
In order to do that, we propagate the MemOp from earlier in the
call stack, or we extract it from the vCPU env (on MicroBlaze the
CPU endianness is exposed by the 'ENDI' bit).

Next step: Look at unifying binaries.

Please review,

Phil.

Philippe Mathieu-Daudé (16):
  hw/intc/xilinx_intc: Make device endianness configurable
  hw/net/xilinx_ethlite: Make device endianness configurable
  hw/timer/xilinx_timer: Make device endianness configurable
  hw/char/xilinx_uartlite: Make device endianness configurable
  hw/ssi/xilinx_spi: Make device endianness configurable
  hw/arm/xlnx-zynqmp: Use &error_abort for programming errors
  target/microblaze: Explode MO_TExx -> MO_TE | MO_xx
  target/microblaze: Set MO_TE once in do_load() / do_store()
  target/microblaze: Introduce mo_endian() helper
  target/microblaze: Consider endianness while translating code
  hw/microblaze: Support various endianness for s3adsp1800 machines
  tests/functional: Explicit endianness of microblaze assets
  tests/functional: Allow microblaze tests to take a machine name
    argument
  tests/functional: Remove sleep() kludges from microblaze tests
  tests/functional: Have microblaze tests inherit common parent class
  tests/functional: Run cross-endian microblaze tests

 target/microblaze/cpu.h                       |  7 +++
 hw/arm/xlnx-zynqmp.c                          | 38 ++++--------
 hw/char/xilinx_uartlite.c                     | 27 +++++----
 hw/intc/xilinx_intc.c                         | 52 ++++++++++++-----
 hw/microblaze/petalogix_ml605_mmu.c           |  3 +
 hw/microblaze/petalogix_s3adsp1800_mmu.c      | 58 ++++++++++++++++---
 hw/net/xilinx_ethlite.c                       | 20 +++++--
 hw/ppc/virtex_ml507.c                         |  1 +
 hw/ssi/xilinx_spi.c                           | 24 +++++---
 hw/timer/xilinx_timer.c                       | 35 ++++++-----
 target/microblaze/translate.c                 | 49 ++++++++++------
 .../functional/test_microblaze_s3adsp1800.py  | 40 +++++++++++--
 .../test_microblazeel_s3adsp1800.py           | 34 +++--------
 13 files changed, 254 insertions(+), 134 deletions(-)

-- 
2.47.1

[PATCH v5 01/16] hw/intc/xilinx_intc: Make device endianness configurable

[Philippe Mathieu-Daudé]

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>
---
 hw/intc/xilinx_intc.c                    | 52 +++++++++++++++++-------
 hw/microblaze/petalogix_ml605_mmu.c      |  1 +
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 6930f83907a..cd79ac4d4ff 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -3,6 +3,9 @@
  *
  * Copyright (c) 2009 Edgar E. Iglesias.
  *
+ * https://docs.amd.com/v/u/en-US/xps_intc
+ * DS572: LogiCORE IP XPS Interrupt Controller (v2.01a)
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -49,6 +52,7 @@ struct XpsIntc
 {
     SysBusDevice parent_obj;
 
+    bool little_endian_model;
     MemoryRegion mmio;
     qemu_irq parent_irq;
 
@@ -140,18 +144,29 @@ static void pic_write(void *opaque, hwaddr addr,
     update_irq(p);
 }
 
-static const MemoryRegionOps pic_ops = {
-    .read = pic_read,
-    .write = pic_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
+static const MemoryRegionOps pic_ops[2] = {
+    [0 ... 1] = {
+        .read = pic_read,
+        .write = pic_write,
+        .endianness = DEVICE_BIG_ENDIAN,
+        .impl = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+        .valid = {
+            /*
+             * All XPS INTC registers are accessed through the PLB interface.
+             * The base address for these registers is provided by the
+             * configuration parameter, C_BASEADDR. Each register is 32 bits
+             * although some bits may be unused and is accessed on a 4-byte
+             * boundary offset from the base address.
+             */
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
     },
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4
-    }
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void irq_handler(void *opaque, int irq, int level)
@@ -174,13 +189,21 @@ static void xilinx_intc_init(Object *obj)
 
     qdev_init_gpio_in(DEVICE(obj), irq_handler, 32);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &p->parent_irq);
-
-    memory_region_init_io(&p->mmio, obj, &pic_ops, p, "xlnx.xps-intc",
-                          R_MAX * 4);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &p->mmio);
 }
 
+static void xilinx_intc_realize(DeviceState *dev, Error **errp)
+{
+    XpsIntc *p = XILINX_INTC(dev);
+
+    memory_region_init_io(&p->mmio, OBJECT(dev),
+                          &pic_ops[p->little_endian_model],
+                          p, "xlnx.xps-intc",
+                          R_MAX * 4);
+}
+
 static const Property xilinx_intc_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XpsIntc, little_endian_model, true),
     DEFINE_PROP_UINT32("kind-of-intr", XpsIntc, c_kind_of_intr, 0),
 };
 
@@ -188,6 +211,7 @@ static void xilinx_intc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->realize = xilinx_intc_realize;
     device_class_set_props(dc, xilinx_intc_properties);
 }
 
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 8b44be75a22..cf3b9574db3 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -111,6 +111,7 @@ petalogix_ml605_init(MachineState *machine)
 
 
     dev = qdev_new("xlnx.xps-intc");
+    qdev_prop_set_bit(dev, "little-endian", true);
     qdev_prop_set_uint32(dev, "kind-of-intr", 1 << TIMER_IRQ);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR);
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 2c0d8c34cd2..0506497ad0a 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -95,6 +95,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
                           64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     dev = qdev_new("xlnx.xps-intc");
+    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
     qdev_prop_set_uint32(dev, "kind-of-intr",
                          1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-- 
2.47.1

[PATCH v5 02/16] hw/net/xilinx_ethlite: Make device endianness configurable

[Philippe Mathieu-Daudé]

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 on the single machine using the
device.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
 hw/net/xilinx_ethlite.c                  | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 0506497ad0a..fbf52ba8f2f 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -121,6 +121,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
 
     dev = qdev_new("xlnx.xps-ethernetlite");
+    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
     qemu_configure_nic_device(dev, true, NULL);
     qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
     qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 14bf2b2e17a..103e53831a7 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -90,6 +90,7 @@ struct XlnxXpsEthLite
     NICState *nic;
     NICConf conf;
 
+    bool little_endian_model;
     uint32_t c_tx_pingpong;
     uint32_t c_rx_pingpong;
     unsigned int port_index; /* dual port RAM index */
@@ -183,10 +184,10 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
     }
 }
 
-static const MemoryRegionOps eth_porttx_ops = {
+static const MemoryRegionOps eth_porttx_ops[2] = {
+    [0 ... 1] = {
         .read = port_tx_read,
         .write = port_tx_write,
-        .endianness = DEVICE_NATIVE_ENDIAN,
         .impl = {
             .min_access_size = 4,
             .max_access_size = 4,
@@ -195,6 +196,9 @@ static const MemoryRegionOps eth_porttx_ops = {
             .min_access_size = 4,
             .max_access_size = 4,
         },
+    },
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
@@ -232,10 +236,10 @@ static void port_rx_write(void *opaque, hwaddr addr, uint64_t value,
     }
 }
 
-static const MemoryRegionOps eth_portrx_ops = {
+static const MemoryRegionOps eth_portrx_ops[2] = {
+    [0 ... 1] = {
         .read = port_rx_read,
         .write = port_rx_write,
-        .endianness = DEVICE_NATIVE_ENDIAN,
         .impl = {
             .min_access_size = 4,
             .max_access_size = 4,
@@ -244,6 +248,9 @@ static const MemoryRegionOps eth_portrx_ops = {
             .min_access_size = 4,
             .max_access_size = 4,
         },
+    },
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static bool eth_can_rx(NetClientState *nc)
@@ -328,7 +335,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
                                BUFSZ_MAX, &error_abort);
         memory_region_add_subregion(&s->container, 0x0800 * i, &s->port[i].txbuf);
         memory_region_init_io(&s->port[i].txio, OBJECT(dev),
-                              &eth_porttx_ops, s,
+                              &eth_porttx_ops[s->little_endian_model], s,
                               i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
                               4 * TX_MAX);
         memory_region_add_subregion(&s->container, i ? A_TX_BASE1 : A_TX_BASE0,
@@ -340,7 +347,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
         memory_region_add_subregion(&s->container, 0x1000 + 0x0800 * i,
                                     &s->port[i].rxbuf);
         memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
-                              &eth_portrx_ops, s,
+                              &eth_portrx_ops[s->little_endian_model], s,
                               i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
                               4 * RX_MAX);
         memory_region_add_subregion(&s->container, i ? A_RX_BASE1 : A_RX_BASE0,
@@ -363,6 +370,7 @@ static void xilinx_ethlite_init(Object *obj)
 }
 
 static const Property xilinx_ethlite_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XlnxXpsEthLite, little_endian_model, true),
     DEFINE_PROP_UINT32("tx-ping-pong", XlnxXpsEthLite, c_tx_pingpong, 1),
     DEFINE_PROP_UINT32("rx-ping-pong", XlnxXpsEthLite, c_rx_pingpong, 1),
     DEFINE_NIC_PROPERTIES(XlnxXpsEthLite, conf),
-- 
2.47.1

[PATCH v5 03/16] hw/timer/xilinx_timer: Make device endianness configurable

[Philippe Mathieu-Daudé]

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>
---
 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);
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 6595cf5f517..d942ac226e6 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -3,6 +3,9 @@
  *
  * Copyright (c) 2009 Edgar E. Iglesias.
  *
+ * DS573: https://docs.amd.com/v/u/en-US/xps_timer
+ * LogiCORE IP XPS Timer/Counter (v1.02a)
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -69,6 +72,7 @@ struct XpsTimerState
 {
     SysBusDevice parent_obj;
 
+    bool little_endian_model;
     MemoryRegion mmio;
     qemu_irq irq;
     uint8_t one_timer_only;
@@ -189,18 +193,21 @@ timer_write(void *opaque, hwaddr addr,
     timer_update_irq(t);
 }
 
-static const MemoryRegionOps timer_ops = {
-    .read = timer_read,
-    .write = timer_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
+static const MemoryRegionOps timer_ops[2] = {
+    [0 ... 1] = {
+        .read = timer_read,
+        .write = timer_write,
+        .impl = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
     },
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4
-    }
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void timer_hit(void *opaque)
@@ -233,8 +240,9 @@ static void xilinx_timer_realize(DeviceState *dev, Error **errp)
         ptimer_transaction_commit(xt->ptimer);
     }
 
-    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "xlnx.xps-timer",
-                          R_MAX * 4 * num_timers(t));
+    memory_region_init_io(&t->mmio, OBJECT(t),
+                          &timer_ops[t->little_endian_model], t,
+                          "xlnx.xps-timer", R_MAX * 4 * num_timers(t));
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
 }
 
@@ -247,6 +255,7 @@ static void xilinx_timer_init(Object *obj)
 }
 
 static const Property xilinx_timer_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XpsTimerState, little_endian_model, true),
     DEFINE_PROP_UINT32("clock-frequency", XpsTimerState, freq_hz, 62 * 1000000),
     DEFINE_PROP_UINT8("one-timer-only", XpsTimerState, one_timer_only, 0),
 };
-- 
2.47.1

[PATCH v5 04/16] hw/char/xilinx_uartlite: Make device endianness configurable

[Philippe Mathieu-Daudé]

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 on the single machine using the
device.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/xilinx_uartlite.c                | 27 ++++++++++++++----------
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index 56955e0d74a..948da4263b9 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -57,6 +57,7 @@
 struct XilinxUARTLite {
     SysBusDevice parent_obj;
 
+    bool little_endian_model;
     MemoryRegion mmio;
     CharBackend chr;
     qemu_irq irq;
@@ -166,17 +167,21 @@ uart_write(void *opaque, hwaddr addr,
     uart_update_irq(s);
 }
 
-static const MemoryRegionOps uart_ops = {
-    .read = uart_read,
-    .write = uart_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 4
-    }
+static const MemoryRegionOps uart_ops[2] = {
+    [0 ... 1] = {
+        .read = uart_read,
+        .write = uart_write,
+        .valid = {
+            .min_access_size = 1,
+            .max_access_size = 4,
+        },
+    },
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static const Property xilinx_uartlite_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XilinxUARTLite, little_endian_model, true),
     DEFINE_PROP_CHR("chardev", XilinxUARTLite, chr),
 };
 
@@ -214,6 +219,9 @@ static void xilinx_uartlite_realize(DeviceState *dev, Error **errp)
 {
     XilinxUARTLite *s = XILINX_UARTLITE(dev);
 
+    memory_region_init_io(&s->mmio, OBJECT(dev),
+                          &uart_ops[s->little_endian_model],
+                          s, "xlnx.xps-uartlite", R_MAX * 4);
     qemu_chr_fe_set_handlers(&s->chr, uart_can_rx, uart_rx,
                              uart_event, NULL, s, NULL, true);
 }
@@ -223,9 +231,6 @@ static void xilinx_uartlite_init(Object *obj)
     XilinxUARTLite *s = XILINX_UARTLITE(obj);
 
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
-    memory_region_init_io(&s->mmio, obj, &uart_ops, s,
-                          "xlnx.xps-uartlite", R_MAX * 4);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
 }
 
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9d4316b4036..96aed4ed1a3 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -107,6 +107,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     }
 
     dev = qdev_new(TYPE_XILINX_UARTLITE);
+    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
     qdev_prop_set_chr(dev, "chardev", serial_hd(0));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
-- 
2.47.1

[PATCH v5 05/16] hw/ssi/xilinx_spi: Make device endianness configurable

[Philippe Mathieu-Daudé]

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 on the single machine using the
device.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_ml605_mmu.c |  1 +
 hw/ssi/xilinx_spi.c                 | 24 +++++++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index bbda70aa93b..a795c6385b4 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -175,6 +175,7 @@ petalogix_ml605_init(MachineState *machine)
         SSIBus *spi;
 
         dev = qdev_new("xlnx.xps-spi");
+        qdev_prop_set_bit(dev, "little-endian", true);
         qdev_prop_set_uint8(dev, "num-ss-bits", NUM_SPI_FLASHES);
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_realize_and_unref(busdev, &error_fatal);
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index fd1ff12eb1d..299004ff36d 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -83,6 +83,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XilinxSPI, XILINX_SPI)
 struct XilinxSPI {
     SysBusDevice parent_obj;
 
+    bool little_endian_model;
     MemoryRegion mmio;
 
     qemu_irq irq;
@@ -313,14 +314,17 @@ done:
     xlx_spi_update_irq(s);
 }
 
-static const MemoryRegionOps spi_ops = {
-    .read = spi_read,
-    .write = spi_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4
-    }
+static const MemoryRegionOps spi_ops[2] = {
+    [0 ... 1] = {
+        .read = spi_read,
+        .write = spi_write,
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+    },
+    [0].endianness = DEVICE_BIG_ENDIAN,
+    [1].endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void xilinx_spi_realize(DeviceState *dev, Error **errp)
@@ -339,7 +343,8 @@ static void xilinx_spi_realize(DeviceState *dev, Error **errp)
         sysbus_init_irq(sbd, &s->cs_lines[i]);
     }
 
-    memory_region_init_io(&s->mmio, OBJECT(s), &spi_ops, s,
+    memory_region_init_io(&s->mmio, OBJECT(s),
+                          &spi_ops[s->little_endian_model], s,
                           "xilinx-spi", R_MAX * 4);
     sysbus_init_mmio(sbd, &s->mmio);
 
@@ -362,6 +367,7 @@ static const VMStateDescription vmstate_xilinx_spi = {
 };
 
 static const Property xilinx_spi_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XilinxSPI, little_endian_model, true),
     DEFINE_PROP_UINT8("num-ss-bits", XilinxSPI, num_cs, 1),
 };
 
-- 
2.47.1

[PATCH v5 06/16] hw/arm/xlnx-zynqmp: Use &error_abort for programming errors

[Philippe Mathieu-Daudé]

When a property value is static (not provided by QMP or CLI),
error shouldn't happen, otherwise it is a programming error.
Therefore simplify and use &error_abort as this can't fail.

Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Anton Johansson <anjo@rev.ng>
---
 hw/arm/xlnx-zynqmp.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index bd5b0dd5e76..d6022ff2d3d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -689,16 +689,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
          * - SDIO Specification Version 3.0
          * - eMMC Specification Version 4.51
          */
-        if (!object_property_set_uint(sdhci, "sd-spec-version", 3, errp)) {
-            return;
-        }
-        if (!object_property_set_uint(sdhci, "capareg", SDHCI_CAPABILITIES,
-                                      errp)) {
-            return;
-        }
-        if (!object_property_set_uint(sdhci, "uhs", UHS_I, errp)) {
-            return;
-        }
+        object_property_set_uint(sdhci, "sd-spec-version", 3, &error_abort);
+        object_property_set_uint(sdhci, "capareg", SDHCI_CAPABILITIES,
+                                 &error_abort);
+        object_property_set_uint(sdhci, "uhs", UHS_I, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(sdhci), errp)) {
             return;
         }
@@ -763,14 +757,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     xlnx_zynqmp_create_unimp_mmio(s);
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA_CH; i++) {
-        if (!object_property_set_uint(OBJECT(&s->gdma[i]), "bus-width", 128,
-                                      errp)) {
-            return;
-        }
-        if (!object_property_set_link(OBJECT(&s->gdma[i]), "dma",
-                                      OBJECT(system_memory), errp)) {
-            return;
-        }
+        object_property_set_uint(OBJECT(&s->gdma[i]), "bus-width", 128,
+                                 &error_abort);
+        object_property_set_link(OBJECT(&s->gdma[i]), "dma",
+                                 OBJECT(system_memory), &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->gdma[i]), errp)) {
             return;
         }
@@ -811,10 +801,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0,
                        qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 0));
 
-    if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
-                                  OBJECT(&s->qspi_dma), errp)) {
-         return;
-    }
+    object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
+                             OBJECT(&s->qspi_dma), &error_abort);
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
         return;
     }
@@ -833,10 +821,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_USB; i++) {
-        if (!object_property_set_link(OBJECT(&s->usb[i].sysbus_xhci), "dma",
-                                      OBJECT(system_memory), errp)) {
-            return;
-        }
+        object_property_set_link(OBJECT(&s->usb[i].sysbus_xhci), "dma",
+                                 OBJECT(system_memory), &error_abort);
 
         qdev_prop_set_uint32(DEVICE(&s->usb[i].sysbus_xhci), "intrs", 4);
         qdev_prop_set_uint32(DEVICE(&s->usb[i].sysbus_xhci), "slots", 2);
-- 
2.47.1

[PATCH v5 07/16] target/microblaze: Explode MO_TExx -> MO_TE | MO_xx

[Philippe Mathieu-Daudé]

Extract the implicit MO_TE definition in order to replace
it by runtime variable in the next commit.

Mechanical change using:

  $ for n in UW UL UQ UO SW SL SQ; do \
      sed -i -e "s/MO_TE$n/MO_TE | MO_$n/" \
           $(git grep -l MO_TE$n target/microblaze); \
    done

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/microblaze/translate.c | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 24005f05b21..86efabb83b5 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -780,13 +780,13 @@ static bool trans_lbui(DisasContext *dc, arg_typeb *arg)
 static bool trans_lhu(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
 }
 
 static bool trans_lhur(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, true);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
 }
 
 static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
@@ -798,26 +798,26 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_lhui(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
 }
 
 static bool trans_lw(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
 }
 
 static bool trans_lwr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, true);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
 }
 
 static bool trans_lwea(DisasContext *dc, arg_typea *arg)
@@ -829,14 +829,14 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TEUL, MMU_NOMMU_IDX, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_lwi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
 }
 
 static bool trans_lwx(DisasContext *dc, arg_typea *arg)
@@ -846,7 +846,7 @@ static bool trans_lwx(DisasContext *dc, arg_typea *arg)
     /* lwx does not throw unaligned access errors, so force alignment */
     tcg_gen_andi_tl(addr, addr, ~3);
 
-    tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TEUL);
+    tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TE | MO_UL);
     tcg_gen_mov_tl(cpu_res_addr, addr);
 
     if (arg->rd) {
@@ -930,13 +930,13 @@ static bool trans_sbi(DisasContext *dc, arg_typeb *arg)
 static bool trans_sh(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
 }
 
 static bool trans_shr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, true);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
 }
 
 static bool trans_shea(DisasContext *dc, arg_typea *arg)
@@ -948,26 +948,26 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_shi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
 }
 
 static bool trans_sw(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
 }
 
 static bool trans_swr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUL, dc->mem_index, true);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
 }
 
 static bool trans_swea(DisasContext *dc, arg_typea *arg)
@@ -979,14 +979,14 @@ static bool trans_swea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TEUL, MMU_NOMMU_IDX, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_swi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_store(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
 }
 
 static bool trans_swx(DisasContext *dc, arg_typea *arg)
@@ -1015,7 +1015,7 @@ static bool trans_swx(DisasContext *dc, arg_typea *arg)
 
     tcg_gen_atomic_cmpxchg_i32(tval, cpu_res_addr, cpu_res_val,
                                reg_for_write(dc, arg->rd),
-                               dc->mem_index, MO_TEUL);
+                               dc->mem_index, MO_TE | MO_UL);
 
     tcg_gen_brcond_i32(TCG_COND_NE, cpu_res_val, tval, swx_fail);
 
-- 
2.47.1

[PATCH v5 08/16] target/microblaze: Set MO_TE once in do_load() / do_store()

[Philippe Mathieu-Daudé]

All callers of do_load() / do_store() set MO_TE flag.
Set it once in the callees.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 36 +++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 86efabb83b5..0d51b2c468c 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -713,6 +713,8 @@ static bool do_load(DisasContext *dc, int rd, TCGv addr, MemOp mop,
 {
     MemOp size = mop & MO_SIZE;
 
+    mop |= MO_TE;
+
     /*
      * When doing reverse accesses we need to do two things.
      *
@@ -780,13 +782,13 @@ static bool trans_lbui(DisasContext *dc, arg_typeb *arg)
 static bool trans_lhu(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
 }
 
 static bool trans_lhur(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
+    return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, true);
 }
 
 static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
@@ -798,26 +800,26 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
+    return do_load(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_lhui(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
 }
 
 static bool trans_lw(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
 }
 
 static bool trans_lwr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
+    return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, true);
 }
 
 static bool trans_lwea(DisasContext *dc, arg_typea *arg)
@@ -829,14 +831,14 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
+    return do_load(dc, arg->rd, addr, MO_UL, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_lwi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+    return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
 }
 
 static bool trans_lwx(DisasContext *dc, arg_typea *arg)
@@ -863,6 +865,8 @@ static bool do_store(DisasContext *dc, int rd, TCGv addr, MemOp mop,
 {
     MemOp size = mop & MO_SIZE;
 
+    mop |= MO_TE;
+
     /*
      * When doing reverse accesses we need to do two things.
      *
@@ -930,13 +934,13 @@ static bool trans_sbi(DisasContext *dc, arg_typeb *arg)
 static bool trans_sh(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
 }
 
 static bool trans_shr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
+    return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, true);
 }
 
 static bool trans_shea(DisasContext *dc, arg_typea *arg)
@@ -948,26 +952,26 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
+    return do_store(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_shi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
 }
 
 static bool trans_sw(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
 }
 
 static bool trans_swr(DisasContext *dc, arg_typea *arg)
 {
     TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
+    return do_store(dc, arg->rd, addr, MO_UL, dc->mem_index, true);
 }
 
 static bool trans_swea(DisasContext *dc, arg_typea *arg)
@@ -979,14 +983,14 @@ static bool trans_swea(DisasContext *dc, arg_typea *arg)
     return true;
 #else
     TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
+    return do_store(dc, arg->rd, addr, MO_UL, MMU_NOMMU_IDX, false);
 #endif
 }
 
 static bool trans_swi(DisasContext *dc, arg_typeb *arg)
 {
     TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-    return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+    return do_store(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
 }
 
 static bool trans_swx(DisasContext *dc, arg_typea *arg)
-- 
2.47.1

[PATCH v5 09/16] target/microblaze: Introduce mo_endian() helper

[Philippe Mathieu-Daudé]

mo_endian() returns the target endianness, currently static.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 0d51b2c468c..b5389d65b2e 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -708,12 +708,17 @@ static void record_unaligned_ess(DisasContext *dc, int rd,
 }
 #endif
 
+static inline MemOp mo_endian(DisasContext *dc)
+{
+    return MO_TE;
+}
+
 static bool do_load(DisasContext *dc, int rd, TCGv addr, MemOp mop,
                     int mem_index, bool rev)
 {
     MemOp size = mop & MO_SIZE;
 
-    mop |= MO_TE;
+    mop |= mo_endian(dc);
 
     /*
      * When doing reverse accesses we need to do two things.
@@ -848,7 +853,8 @@ static bool trans_lwx(DisasContext *dc, arg_typea *arg)
     /* lwx does not throw unaligned access errors, so force alignment */
     tcg_gen_andi_tl(addr, addr, ~3);
 
-    tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TE | MO_UL);
+    tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index,
+                        mo_endian(dc) | MO_UL);
     tcg_gen_mov_tl(cpu_res_addr, addr);
 
     if (arg->rd) {
@@ -865,7 +871,7 @@ static bool do_store(DisasContext *dc, int rd, TCGv addr, MemOp mop,
 {
     MemOp size = mop & MO_SIZE;
 
-    mop |= MO_TE;
+    mop |= mo_endian(dc);
 
     /*
      * When doing reverse accesses we need to do two things.
@@ -1019,7 +1025,7 @@ static bool trans_swx(DisasContext *dc, arg_typea *arg)
 
     tcg_gen_atomic_cmpxchg_i32(tval, cpu_res_addr, cpu_res_val,
                                reg_for_write(dc, arg->rd),
-                               dc->mem_index, MO_TE | MO_UL);
+                               dc->mem_index, mo_endian(dc) | MO_UL);
 
     tcg_gen_brcond_i32(TCG_COND_NE, cpu_res_val, tval, swx_fail);
 
-- 
2.47.1

[PATCH v5 10/16] target/microblaze: Consider endianness while translating code

[Philippe Mathieu-Daudé]

Consider the CPU ENDI bit, swap instructions when the CPU
endianness doesn't match the binary one.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/cpu.h       | 7 +++++++
 target/microblaze/translate.c | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index f6879eee352..e44ddd53078 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -414,6 +414,13 @@ void mb_translate_code(CPUState *cs, TranslationBlock *tb,
 /* Ensure there is no overlap between the two masks. */
 QEMU_BUILD_BUG_ON(MSR_TB_MASK & IFLAGS_TB_MASK);
 
+static inline bool mb_cpu_is_big_endian(CPUState *cs)
+{
+    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+
+    return !cpu->cfg.endi;
+}
+
 static inline void cpu_get_tb_cpu_state(CPUMBState *env, vaddr *pc,
                                         uint64_t *cs_base, uint32_t *flags)
 {
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index b5389d65b2e..b54e5ac4b2f 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -710,7 +710,7 @@ static void record_unaligned_ess(DisasContext *dc, int rd,
 
 static inline MemOp mo_endian(DisasContext *dc)
 {
-    return MO_TE;
+    return dc->cfg->endi ? MO_LE : MO_BE;
 }
 
 static bool do_load(DisasContext *dc, int rd, TCGv addr, MemOp mop,
@@ -1647,7 +1647,8 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
 
     dc->tb_flags_to_set = 0;
 
-    ir = translator_ldl(cpu_env(cs), &dc->base, dc->base.pc_next);
+    ir = translator_ldl_swap(cpu_env(cs), &dc->base, dc->base.pc_next,
+                             mb_cpu_is_big_endian(cs) != TARGET_BIG_ENDIAN);
     if (!decode(dc, ir)) {
         trap_illegal(dc, true);
     }
-- 
2.47.1

[PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

Introduce an abstract machine parent class which defines
the 'little_endian' property. Duplicate the current machine,
which endian is tied to the binary endianness, to one big
endian and a little endian machine; updating the machine
description. Keep the current default machine for each binary.

'petalogix-s3adsp1800' machine is aliased as:
- 'petalogix-s3adsp1800-be' on big-endian binary,
- 'petalogix-s3adsp1800-le' on little-endian one.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 62 +++++++++++++++++++-----
 1 file changed, 51 insertions(+), 11 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 96aed4ed1a3..aea727eb7ee 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -55,8 +55,17 @@
 #define ETHLITE_IRQ         1
 #define UARTLITE_IRQ        3
 
+typedef struct PetalogixS3adsp1800MachineClass {
+    MachineClass parent_obj;
+
+    bool little_endian;
+} PetalogixS3adsp1800MachineClass;
+
 #define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
-            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
+            MACHINE_TYPE_NAME("petalogix-s3adsp1800-common")
+DECLARE_CLASS_CHECKERS(PetalogixS3adsp1800MachineClass,
+                       PETALOGIX_S3ADSP1800_MACHINE,
+                       TYPE_PETALOGIX_S3ADSP1800_MACHINE)
 
 static void
 petalogix_s3adsp1800_init(MachineState *machine)
@@ -71,11 +80,13 @@ petalogix_s3adsp1800_init(MachineState *machine)
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
     qemu_irq irq[32];
     MemoryRegion *sysmem = get_system_memory();
+    PetalogixS3adsp1800MachineClass *pmc;
 
+    pmc = PETALOGIX_S3ADSP1800_MACHINE_GET_CLASS(machine);
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
     object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
     object_property_set_bool(OBJECT(cpu), "little-endian",
-                             !TARGET_BIG_ENDIAN, &error_abort);
+                             pmc->little_endian, &error_abort);
     qdev_realize(DEVICE(cpu), NULL, &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
@@ -95,7 +106,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
                           64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     dev = qdev_new("xlnx.xps-intc");
-    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
     qdev_prop_set_uint32(dev, "kind-of-intr",
                          1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -107,7 +118,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     }
 
     dev = qdev_new(TYPE_XILINX_UARTLITE);
-    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
     qdev_prop_set_chr(dev, "chardev", serial_hd(0));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
@@ -115,7 +126,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_bit(dev, "little-endian", pmc->little_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);
@@ -123,7 +134,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
 
     dev = qdev_new("xlnx.xps-ethernetlite");
-    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
     qemu_configure_nic_device(dev, true, NULL);
     qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
     qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
@@ -133,26 +144,55 @@ petalogix_s3adsp1800_init(MachineState *machine)
 
     create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
 
-    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
+    microblaze_load_kernel(cpu, pmc->little_endian, ddr_base, ram_size,
                            machine->initrd_filename,
                            BINARY_DEVICE_TREE_FILE,
                            NULL);
 }
 
-static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
+static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
+                                                    bool little_endian)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
 
-    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
     mc->init = petalogix_s3adsp1800_init;
-    mc->is_default = true;
+    pmc->little_endian = little_endian;
+    mc->desc = little_endian
+        ? "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)"
+        : "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
+    if (little_endian == !TARGET_BIG_ENDIAN) {
+        mc->is_default = true;
+        mc->alias = "petalogix-s3adsp1800";
+    }
+}
+
+static void petalogix_s3adsp1800_machine_class_init_be(ObjectClass *oc, void *data)
+{
+    petalogix_s3adsp1800_machine_class_init(oc, false);
+}
+
+static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
+{
+    petalogix_s3adsp1800_machine_class_init(oc, true);
 }
 
 static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
     {
         .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
         .parent         = TYPE_MACHINE,
-        .class_init     = petalogix_s3adsp1800_machine_class_init,
+        .abstract       = true,
+        .class_size     = sizeof(PetalogixS3adsp1800MachineClass),
+    },
+    {
+        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-be"),
+        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
+        .class_init     = petalogix_s3adsp1800_machine_class_init_be,
+    },
+    {
+        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-le"),
+        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
+        .class_init     = petalogix_s3adsp1800_machine_class_init_le,
     },
 };
 
-- 
2.47.1

[PATCH v5 12/16] tests/functional: Explicit endianness of microblaze assets

[Philippe Mathieu-Daudé]

The archive used in test_microblaze_s3adsp1800.py (testing a
big-endian target) contains a big-endian kernel. Rename using
the _BE suffix.

Similarly, the archive in test_microblazeel_s3adsp1800 (testing
a little-endian target) contains a little-endian kernel. Rename
using _LE suffix.

These changes will help when adding cross-endian kernel tests.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/test_microblaze_s3adsp1800.py   | 6 +++---
 tests/functional/test_microblazeel_s3adsp1800.py | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index 2c4464bd05a..fac364b1ea9 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -15,14 +15,14 @@ class MicroblazeMachine(QemuSystemTest):
 
     timeout = 90
 
-    ASSET_IMAGE = Asset(
+    ASSET_IMAGE_BE = Asset(
         ('https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/'
          'day17.tar.xz'),
         '3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
 
-    def test_microblaze_s3adsp1800(self):
+    def test_microblaze_s3adsp1800_be(self):
         self.set_machine('petalogix-s3adsp1800')
-        self.archive_extract(self.ASSET_IMAGE)
+        self.archive_extract(self.ASSET_IMAGE_BE)
         self.vm.set_console()
         self.vm.add_args('-kernel',
                          self.scratch_file('day17', 'ballerina.bin'))
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index c382afe6bfa..5d353dba5d2 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -17,14 +17,14 @@ class MicroblazeelMachine(QemuSystemTest):
 
     timeout = 90
 
-    ASSET_IMAGE = Asset(
+    ASSET_IMAGE_LE = Asset(
         ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
         'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
 
-    def test_microblazeel_s3adsp1800(self):
+    def test_microblazeel_s3adsp1800_le(self):
         self.require_netdev('user')
         self.set_machine('petalogix-s3adsp1800')
-        self.archive_extract(self.ASSET_IMAGE)
+        self.archive_extract(self.ASSET_IMAGE_LE)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
         tftproot = self.scratch_file('day13')
-- 
2.47.1

[PATCH v5 13/16] tests/functional: Allow microblaze tests to take a machine name argument

[Philippe Mathieu-Daudé]

Make microblaze tests a bit more generic.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/test_microblaze_s3adsp1800.py   | 7 +++++--
 tests/functional/test_microblazeel_s3adsp1800.py | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index fac364b1ea9..c4226f49cf3 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -20,8 +20,8 @@ class MicroblazeMachine(QemuSystemTest):
          'day17.tar.xz'),
         '3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
 
-    def test_microblaze_s3adsp1800_be(self):
-        self.set_machine('petalogix-s3adsp1800')
+    def do_ballerina_be_test(self, machine):
+        self.set_machine(machine)
         self.archive_extract(self.ASSET_IMAGE_BE)
         self.vm.set_console()
         self.vm.add_args('-kernel',
@@ -34,5 +34,8 @@ def test_microblaze_s3adsp1800_be(self):
         # message, that's why we don't test for a later string here. This
         # needs some investigation by a microblaze wizard one day...
 
+    def test_microblaze_s3adsp1800_legacy_be(self):
+        self.do_ballerina_be_test('petalogix-s3adsp1800')
+
 if __name__ == '__main__':
     QemuSystemTest.main()
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index 5d353dba5d2..715ef3f79ac 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -21,9 +21,9 @@ class MicroblazeelMachine(QemuSystemTest):
         ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
         'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
 
-    def test_microblazeel_s3adsp1800_le(self):
+    def do_xmaton_le_test(self, machine):
         self.require_netdev('user')
-        self.set_machine('petalogix-s3adsp1800')
+        self.set_machine(machine)
         self.archive_extract(self.ASSET_IMAGE_LE)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
@@ -38,5 +38,8 @@ def test_microblazeel_s3adsp1800_le(self):
                 'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
                 '821cd3cab8efd16ad6ee5acc3642a8ea')
 
+    def test_microblaze_s3adsp1800_legacy_le(self):
+        self.do_xmaton_le_test('petalogix-s3adsp1800')
+
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.47.1

[PATCH v5 14/16] tests/functional: Remove sleep() kludges from microblaze tests

[Philippe Mathieu-Daudé]

Commit f0ec14c78c4 ("tests/avocado: Fix console data loss") fixed
QEMUMachine's problem with console, we don't need to use the sleep()
kludges.

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/functional/test_microblazeel_s3adsp1800.py | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index 715ef3f79ac..60aab4a45e8 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -7,8 +7,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later. See the COPYING file in the top-level directory.
 
-import time
-from qemu_test import exec_command, exec_command_and_wait_for_pattern
+from qemu_test import exec_command_and_wait_for_pattern
 from qemu_test import QemuSystemTest, Asset
 from qemu_test import wait_for_console_pattern
 
@@ -31,9 +30,8 @@ def do_xmaton_le_test(self, machine):
         self.vm.add_args('-nic', f'user,tftp={tftproot}')
         self.vm.launch()
         wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
-        time.sleep(0.1)
-        exec_command(self, 'root')
-        time.sleep(0.1)
+        wait_for_console_pattern(self, 'buildroot login:')
+        exec_command_and_wait_for_pattern(self, 'root', '#')
         exec_command_and_wait_for_pattern(self,
                 'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
                 '821cd3cab8efd16ad6ee5acc3642a8ea')
-- 
2.47.1

[PATCH v5 15/16] tests/functional: Have microblaze tests inherit common parent class

[Philippe Mathieu-Daudé]

Have the MicroblazeMachine class being common to both
MicroblazeBigEndianMachine and MicroblazeLittleEndianMachine
classes. Move the xmaton and ballerina tests to the parent class.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 .../functional/test_microblaze_s3adsp1800.py  | 23 +++++++++++++++
 .../test_microblazeel_s3adsp1800.py           | 29 ++-----------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index c4226f49cf3..177c8a685bc 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -7,6 +7,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later. See the COPYING file in the top-level directory.
 
+from qemu_test import exec_command_and_wait_for_pattern
 from qemu_test import QemuSystemTest, Asset
 from qemu_test import wait_for_console_pattern
 
@@ -20,6 +21,10 @@ class MicroblazeMachine(QemuSystemTest):
          'day17.tar.xz'),
         '3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
 
+    ASSET_IMAGE_LE = Asset(
+        ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
+        'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
+
     def do_ballerina_be_test(self, machine):
         self.set_machine(machine)
         self.archive_extract(self.ASSET_IMAGE_BE)
@@ -34,6 +39,24 @@ def do_ballerina_be_test(self, machine):
         # message, that's why we don't test for a later string here. This
         # needs some investigation by a microblaze wizard one day...
 
+    def do_xmaton_le_test(self, machine):
+        self.require_netdev('user')
+        self.set_machine(machine)
+        self.archive_extract(self.ASSET_IMAGE_LE)
+        self.vm.set_console()
+        self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
+        tftproot = self.scratch_file('day13')
+        self.vm.add_args('-nic', f'user,tftp={tftproot}')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
+        wait_for_console_pattern(self, 'buildroot login:')
+        exec_command_and_wait_for_pattern(self, 'root', '#')
+        exec_command_and_wait_for_pattern(self,
+                'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
+                '821cd3cab8efd16ad6ee5acc3642a8ea')
+
+class MicroblazeBigEndianMachine(MicroblazeMachine):
+
     def test_microblaze_s3adsp1800_legacy_be(self):
         self.do_ballerina_be_test('petalogix-s3adsp1800')
 
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index 60aab4a45e8..56645bd0bb2 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -7,34 +7,11 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later. See the COPYING file in the top-level directory.
 
-from qemu_test import exec_command_and_wait_for_pattern
-from qemu_test import QemuSystemTest, Asset
-from qemu_test import wait_for_console_pattern
+from qemu_test import QemuSystemTest
 
+from test_microblaze_s3adsp1800 import MicroblazeMachine
 
-class MicroblazeelMachine(QemuSystemTest):
-
-    timeout = 90
-
-    ASSET_IMAGE_LE = Asset(
-        ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
-        'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
-
-    def do_xmaton_le_test(self, machine):
-        self.require_netdev('user')
-        self.set_machine(machine)
-        self.archive_extract(self.ASSET_IMAGE_LE)
-        self.vm.set_console()
-        self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
-        tftproot = self.scratch_file('day13')
-        self.vm.add_args('-nic', f'user,tftp={tftproot}')
-        self.vm.launch()
-        wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
-        wait_for_console_pattern(self, 'buildroot login:')
-        exec_command_and_wait_for_pattern(self, 'root', '#')
-        exec_command_and_wait_for_pattern(self,
-                'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
-                '821cd3cab8efd16ad6ee5acc3642a8ea')
+class MicroblazeLittleEndianMachine(MicroblazeMachine):
 
     def test_microblaze_s3adsp1800_legacy_le(self):
         self.do_xmaton_le_test('petalogix-s3adsp1800')
-- 
2.47.1

[PATCH v5 16/16] tests/functional: Run cross-endian microblaze tests

[Philippe Mathieu-Daudé]

Ensure microblaze machines can run cross-endianness by
running all tests on all machines.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/functional/test_microblaze_s3adsp1800.py   | 6 ++++++
 tests/functional/test_microblazeel_s3adsp1800.py | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index 177c8a685bc..c18b132ff5f 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -60,5 +60,11 @@ class MicroblazeBigEndianMachine(MicroblazeMachine):
     def test_microblaze_s3adsp1800_legacy_be(self):
         self.do_ballerina_be_test('petalogix-s3adsp1800')
 
+    def test_microblaze_s3adsp1800_be(self):
+        self.do_ballerina_be_test('petalogix-s3adsp1800-be')
+
+    def test_microblaze_s3adsp1800_le(self):
+        self.do_xmaton_le_test('petalogix-s3adsp1800-le')
+
 if __name__ == '__main__':
     QemuSystemTest.main()
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index 56645bd0bb2..b10944bbb0c 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -16,5 +16,11 @@ class MicroblazeLittleEndianMachine(MicroblazeMachine):
     def test_microblaze_s3adsp1800_legacy_le(self):
         self.do_xmaton_le_test('petalogix-s3adsp1800')
 
+    def test_microblaze_s3adsp1800_le(self):
+        self.do_xmaton_le_test('petalogix-s3adsp1800-le')
+
+    def test_microblaze_s3adsp1800_be(self):
+        self.do_ballerina_be_test('petalogix-s3adsp1800-be')
+
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.47.1

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Daniel P. Berrangé]

On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
> Introduce an abstract machine parent class which defines
> the 'little_endian' property. Duplicate the current machine,
> which endian is tied to the binary endianness, to one big
> endian and a little endian machine; updating the machine
> description. Keep the current default machine for each binary.
> 
> 'petalogix-s3adsp1800' machine is aliased as:
> - 'petalogix-s3adsp1800-be' on big-endian binary,
> - 'petalogix-s3adsp1800-le' on little-endian one.

Does it makes sense to expose these as different machine types ?

If all the HW is identical in both cases, it feels like the
endianness could just be a bool property of the machine type,
rather than a new machine type.

> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 62 +++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 96aed4ed1a3..aea727eb7ee 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -55,8 +55,17 @@
>  #define ETHLITE_IRQ         1
>  #define UARTLITE_IRQ        3
>  
> +typedef struct PetalogixS3adsp1800MachineClass {
> +    MachineClass parent_obj;
> +
> +    bool little_endian;
> +} PetalogixS3adsp1800MachineClass;
> +
>  #define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
> -            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
> +            MACHINE_TYPE_NAME("petalogix-s3adsp1800-common")
> +DECLARE_CLASS_CHECKERS(PetalogixS3adsp1800MachineClass,
> +                       PETALOGIX_S3ADSP1800_MACHINE,
> +                       TYPE_PETALOGIX_S3ADSP1800_MACHINE)
>  
>  static void
>  petalogix_s3adsp1800_init(MachineState *machine)
> @@ -71,11 +80,13 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>      qemu_irq irq[32];
>      MemoryRegion *sysmem = get_system_memory();
> +    PetalogixS3adsp1800MachineClass *pmc;
>  
> +    pmc = PETALOGIX_S3ADSP1800_MACHINE_GET_CLASS(machine);
>      cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
>      object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
>      object_property_set_bool(OBJECT(cpu), "little-endian",
> -                             !TARGET_BIG_ENDIAN, &error_abort);
> +                             pmc->little_endian, &error_abort);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>  
>      /* Attach emulated BRAM through the LMB.  */
> @@ -95,7 +106,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>                            64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
>  
>      dev = qdev_new("xlnx.xps-intc");
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qdev_prop_set_uint32(dev, "kind-of-intr",
>                           1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> @@ -107,7 +118,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      }
>  
>      dev = qdev_new(TYPE_XILINX_UARTLITE);
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qdev_prop_set_chr(dev, "chardev", serial_hd(0));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
> @@ -115,7 +126,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_bit(dev, "little-endian", pmc->little_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);
> @@ -123,7 +134,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
>  
>      dev = qdev_new("xlnx.xps-ethernetlite");
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qemu_configure_nic_device(dev, true, NULL);
>      qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
>      qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
> @@ -133,26 +144,55 @@ petalogix_s3adsp1800_init(MachineState *machine)
>  
>      create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>  
> -    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
> +    microblaze_load_kernel(cpu, pmc->little_endian, ddr_base, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
>  }
>  
> -static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
> +static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
> +                                                    bool little_endian)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
>  
> -    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>      mc->init = petalogix_s3adsp1800_init;
> -    mc->is_default = true;
> +    pmc->little_endian = little_endian;
> +    mc->desc = little_endian
> +        ? "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)"
> +        : "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
> +    if (little_endian == !TARGET_BIG_ENDIAN) {
> +        mc->is_default = true;
> +        mc->alias = "petalogix-s3adsp1800";
> +    }
> +}
> +
> +static void petalogix_s3adsp1800_machine_class_init_be(ObjectClass *oc, void *data)
> +{
> +    petalogix_s3adsp1800_machine_class_init(oc, false);
> +}
> +
> +static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
> +{
> +    petalogix_s3adsp1800_machine_class_init(oc, true);
>  }
>  
>  static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
>      {
>          .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
>          .parent         = TYPE_MACHINE,
> -        .class_init     = petalogix_s3adsp1800_machine_class_init,
> +        .abstract       = true,
> +        .class_size     = sizeof(PetalogixS3adsp1800MachineClass),
> +    },
> +    {
> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-be"),
> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init_be,
> +    },
> +    {
> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-le"),
> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init_le,
>      },
>  };
>  
> -- 
> 2.47.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

Hi Daniel,

On 6/2/25 14:20, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>> Introduce an abstract machine parent class which defines
>> the 'little_endian' property. Duplicate the current machine,
>> which endian is tied to the binary endianness, to one big
>> endian and a little endian machine; updating the machine
>> description. Keep the current default machine for each binary.
>>
>> 'petalogix-s3adsp1800' machine is aliased as:
>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>> - 'petalogix-s3adsp1800-le' on little-endian one.
> 
> Does it makes sense to expose these as different machine types ?
> 
> If all the HW is identical in both cases, it feels like the
> endianness could just be a bool property of the machine type,
> rather than a new machine type.

Our test suites expect "qemu-system-foo -M bar" to work out of
the box, we can not have non-default properties.

(This is related to the raspberry pi discussion in
https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).

My plan is to deprecate 'petalogix-s3adsp1800', so once we
remove it we can merge both qemu-system-microblaze and
qemu-system-microblazeel into a single binary.

If you don't want to add more machines, what should be the
endianness of the 'petalogix-s3adsp1800' machine in a binary
with no particular endianness? Either we add for explicit
endianness (fixing test suites) or we add one machine for
each endianness; I fail to see other options not too
confusing for our users.

This approach is the same I took to merge MIPS*, SH4* and
Xtensa* machines in endianness-agnostic binaries.

Also the same I'm using to merge 32/64-bit targets into the
same binaries.
Assuming we have a qemu-system-x86 binary able to run i386 and
x86_64 machines, what do you expect when starting '-M pc'? How
to not confuse users wanting to run FreeDOS in 32-bit mode?

Again, IMO having '-M pc,mode=32' is simpler, but that breaks
the test suites assumptions than machines can start with no
default values (see QOM introspection tests for example).

> 
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/microblaze/petalogix_s3adsp1800_mmu.c | 62 +++++++++++++++++++-----
>>   1 file changed, 51 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> index 96aed4ed1a3..aea727eb7ee 100644
>> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> @@ -55,8 +55,17 @@
>>   #define ETHLITE_IRQ         1
>>   #define UARTLITE_IRQ        3
>>   
>> +typedef struct PetalogixS3adsp1800MachineClass {
>> +    MachineClass parent_obj;
>> +
>> +    bool little_endian;
>> +} PetalogixS3adsp1800MachineClass;
>> +
>>   #define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
>> -            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
>> +            MACHINE_TYPE_NAME("petalogix-s3adsp1800-common")
>> +DECLARE_CLASS_CHECKERS(PetalogixS3adsp1800MachineClass,
>> +                       PETALOGIX_S3ADSP1800_MACHINE,
>> +                       TYPE_PETALOGIX_S3ADSP1800_MACHINE)
>>   
>>   static void
>>   petalogix_s3adsp1800_init(MachineState *machine)
>> @@ -71,11 +80,13 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>       MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>>       qemu_irq irq[32];
>>       MemoryRegion *sysmem = get_system_memory();
>> +    PetalogixS3adsp1800MachineClass *pmc;
>>   
>> +    pmc = PETALOGIX_S3ADSP1800_MACHINE_GET_CLASS(machine);
>>       cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
>>       object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
>>       object_property_set_bool(OBJECT(cpu), "little-endian",
>> -                             !TARGET_BIG_ENDIAN, &error_abort);
>> +                             pmc->little_endian, &error_abort);
>>       qdev_realize(DEVICE(cpu), NULL, &error_abort);
>>   
>>       /* Attach emulated BRAM through the LMB.  */
>> @@ -95,7 +106,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>                             64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
>>   
>>       dev = qdev_new("xlnx.xps-intc");
>> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
>> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>>       qdev_prop_set_uint32(dev, "kind-of-intr",
>>                            1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> @@ -107,7 +118,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>       }
>>   
>>       dev = qdev_new(TYPE_XILINX_UARTLITE);
>> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
>> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>>       qdev_prop_set_chr(dev, "chardev", serial_hd(0));
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
>> @@ -115,7 +126,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_bit(dev, "little-endian", pmc->little_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);
>> @@ -123,7 +134,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>       sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
>>   
>>       dev = qdev_new("xlnx.xps-ethernetlite");
>> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
>> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>>       qemu_configure_nic_device(dev, true, NULL);
>>       qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
>>       qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
>> @@ -133,26 +144,55 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>   
>>       create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>>   
>> -    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
>> +    microblaze_load_kernel(cpu, pmc->little_endian, ddr_base, ram_size,
>>                              machine->initrd_filename,
>>                              BINARY_DEVICE_TREE_FILE,
>>                              NULL);
>>   }
>>   
>> -static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
>> +static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
>> +                                                    bool little_endian)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> +    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
>>   
>> -    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>>       mc->init = petalogix_s3adsp1800_init;
>> -    mc->is_default = true;
>> +    pmc->little_endian = little_endian;
>> +    mc->desc = little_endian
>> +        ? "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)"
>> +        : "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
>> +    if (little_endian == !TARGET_BIG_ENDIAN) {
>> +        mc->is_default = true;
>> +        mc->alias = "petalogix-s3adsp1800";
>> +    }
>> +}
>> +
>> +static void petalogix_s3adsp1800_machine_class_init_be(ObjectClass *oc, void *data)
>> +{
>> +    petalogix_s3adsp1800_machine_class_init(oc, false);
>> +}
>> +
>> +static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
>> +{
>> +    petalogix_s3adsp1800_machine_class_init(oc, true);
>>   }
>>   
>>   static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
>>       {
>>           .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
>>           .parent         = TYPE_MACHINE,
>> -        .class_init     = petalogix_s3adsp1800_machine_class_init,
>> +        .abstract       = true,
>> +        .class_size     = sizeof(PetalogixS3adsp1800MachineClass),
>> +    },
>> +    {
>> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-be"),
>> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
>> +        .class_init     = petalogix_s3adsp1800_machine_class_init_be,
>> +    },
>> +    {
>> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-le"),
>> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
>> +        .class_init     = petalogix_s3adsp1800_machine_class_init_le,
>>       },
>>   };
>>   
>> -- 
>> 2.47.1
>>
>>
> 
> With regards,
> Daniel

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Daniel P. Berrangé]

On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 6/2/25 14:20, Daniel P. Berrangé wrote:
> > On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
> > > Introduce an abstract machine parent class which defines
> > > the 'little_endian' property. Duplicate the current machine,
> > > which endian is tied to the binary endianness, to one big
> > > endian and a little endian machine; updating the machine
> > > description. Keep the current default machine for each binary.
> > > 
> > > 'petalogix-s3adsp1800' machine is aliased as:
> > > - 'petalogix-s3adsp1800-be' on big-endian binary,
> > > - 'petalogix-s3adsp1800-le' on little-endian one.
> > 
> > Does it makes sense to expose these as different machine types ?
> > 
> > If all the HW is identical in both cases, it feels like the
> > endianness could just be a bool property of the machine type,
> > rather than a new machine type.
> 
> Our test suites expect "qemu-system-foo -M bar" to work out of
> the box, we can not have non-default properties.
> 
> (This is related to the raspberry pi discussion in
> https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
> 
> My plan is to deprecate 'petalogix-s3adsp1800', so once we
> remove it we can merge both qemu-system-microblaze and
> qemu-system-microblazeel into a single binary.
> 
> If you don't want to add more machines, what should be the
> endianness of the 'petalogix-s3adsp1800' machine in a binary
> with no particular endianness? Either we add for explicit
> endianness (fixing test suites) or we add one machine for
> each endianness; I fail to see other options not too
> confusing for our users.

We would pick an arbitrary endianness of our choosing
I guess. How does this work in physical machines ? Is
the choice of endianess a firmware setting, or a choice
by the vendor when manufacturing in some way ?

Picking an arbitrary endianess is compatible with our
test suite, it just has the implication that we would
only end up testing the machine in a single endianness
configuration.

If we wanted to test both endianness options, the test
would need amending to know to try both values of the
endian property on the machine.

> This approach is the same I took to merge MIPS*, SH4* and
> Xtensa* machines in endianness-agnostic binaries.

If we have prior art like this, then remaining consistentv is
desirable and thus my comments are too late.

> Also the same I'm using to merge 32/64-bit targets into the
> same binaries.
> Assuming we have a qemu-system-x86 binary able to run i386 and
> x86_64 machines, what do you expect when starting '-M pc'? How
> to not confuse users wanting to run FreeDOS in 32-bit mode?
> 
> Again, IMO having '-M pc,mode=32' is simpler, but that breaks
> the test suites assumptions than machines can start with no
> default values (see QOM introspection tests for example).

With x86 there's no need for mode=32. Whether the machine
supports 64-bit or not is a property of the CPU model
chosen. eg  "qemu -M pc -cpu Nehalem" would be 64-bit and
"qemu -M pc -cpu pentium" would be 32-bit.

The qemu-system-i386 binary is pretty much pointless as a
separate thing. Libvirt will happily use qemu-system-x86_64
to run 32-bit guests, by just specifying a 32-bit CPU
model


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

On 6/2/25 15:31, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>>
>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>>>> Introduce an abstract machine parent class which defines
>>>> the 'little_endian' property. Duplicate the current machine,
>>>> which endian is tied to the binary endianness, to one big
>>>> endian and a little endian machine; updating the machine
>>>> description. Keep the current default machine for each binary.
>>>>
>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>
>>> Does it makes sense to expose these as different machine types ?
>>>
>>> If all the HW is identical in both cases, it feels like the
>>> endianness could just be a bool property of the machine type,
>>> rather than a new machine type.
>>
>> Our test suites expect "qemu-system-foo -M bar" to work out of
>> the box, we can not have non-default properties.
>>
>> (This is related to the raspberry pi discussion in
>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
>>
>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>> remove it we can merge both qemu-system-microblaze and
>> qemu-system-microblazeel into a single binary.
>>
>> If you don't want to add more machines, what should be the
>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>> with no particular endianness? Either we add for explicit
>> endianness (fixing test suites) or we add one machine for
>> each endianness; I fail to see other options not too
>> confusing for our users.
> 
> We would pick an arbitrary endianness of our choosing
> I guess. How does this work in physical machines ? Is
> the choice of endianess a firmware setting, or a choice
> by the vendor when manufacturing in some way ?

Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
(wired to a CPU pin which is sampled once at cold reset).

> Picking an arbitrary endianess is compatible with our
> test suite, it just has the implication that we would
> only end up testing the machine in a single endianness
> configuration.
> 
> If we wanted to test both endianness options, the test
> would need amending to know to try both values of the
> endian property on the machine.
> 
>> This approach is the same I took to merge MIPS*, SH4* and
>> Xtensa* machines in endianness-agnostic binaries.
> 
> If we have prior art like this, then remaining consistentv is
> desirable and thus my comments are too late.

My worry is about how to not break what distros currently ship.

> 
>> Also the same I'm using to merge 32/64-bit targets into the
>> same binaries.
>> Assuming we have a qemu-system-x86 binary able to run i386 and
>> x86_64 machines, what do you expect when starting '-M pc'? How
>> to not confuse users wanting to run FreeDOS in 32-bit mode?
>>
>> Again, IMO having '-M pc,mode=32' is simpler, but that breaks
>> the test suites assumptions than machines can start with no
>> default values (see QOM introspection tests for example).
> 
> With x86 there's no need for mode=32. Whether the machine
> supports 64-bit or not is a property of the CPU model
> chosen. eg  "qemu -M pc -cpu Nehalem" would be 64-bit and
> "qemu -M pc -cpu pentium" would be 32-bit.
> 
> The qemu-system-i386 binary is pretty much pointless as a
> separate thing. Libvirt will happily use qemu-system-x86_64
> to run 32-bit guests, by just specifying a 32-bit CPU
> model

IIRC Thomas tried to convert TARGET_X86_64 to a runtime check
but it wasn't that easy:

https://lore.kernel.org/qemu-devel/20230425133851.489283-1-thuth@redhat.com/

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Thomas Huth]

On 06/02/2025 16.04, Philippe Mathieu-Daudé wrote:
> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
>>> Hi Daniel,
>>>
>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> Introduce an abstract machine parent class which defines
>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>> which endian is tied to the binary endianness, to one big
>>>>> endian and a little endian machine; updating the machine
>>>>> description. Keep the current default machine for each binary.
>>>>>
>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>
>>>> Does it makes sense to expose these as different machine types ?
>>>>
>>>> If all the HW is identical in both cases, it feels like the
>>>> endianness could just be a bool property of the machine type,
>>>> rather than a new machine type.
>>>
>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>> the box, we can not have non-default properties.
>>>
>>> (This is related to the raspberry pi discussion in
>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1- 
>>> philmd@linaro.org/).
>>>
>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>> remove it we can merge both qemu-system-microblaze and
>>> qemu-system-microblazeel into a single binary.
>>>
>>> If you don't want to add more machines, what should be the
>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>> with no particular endianness? Either we add for explicit
>>> endianness (fixing test suites) or we add one machine for
>>> each endianness; I fail to see other options not too
>>> confusing for our users.
>>
>> We would pick an arbitrary endianness of our choosing
>> I guess. How does this work in physical machines ? Is
>> the choice of endianess a firmware setting, or a choice
>> by the vendor when manufacturing in some way ?
> 
> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
> (wired to a CPU pin which is sampled once at cold reset).

If it's a jumper on the board instead of two separate boards, then I think 
this is a good indication that this should only be a machine property, and 
not two separate boards?

 > My worry is about how to not break what distros currently ship.

So once the two binaries got unified, maybe we could still add a hook 
somewhere that checks argv[0] and sets the endianess property according to 
the name of the binary, so users can still use a symlink to force the 
opposite behavior?

Anyway, you likely don't have to solve this problem right in this series 
here already, we can think of this later when one of the binaries gets 
marked as deprecated. So for this series here, I'd suggest to go ahead 
without adding the -le and -be machine types and only use a property first, 
then tackle the remaining questions later...?

  Thomas

Re: [PATCH v5 14/16] tests/functional: Remove sleep() kludges from microblaze tests

[Thomas Huth]

On 06/02/2025 14.10, Philippe Mathieu-Daudé wrote:
> Commit f0ec14c78c4 ("tests/avocado: Fix console data loss") fixed
> QEMUMachine's problem with console, we don't need to use the sleep()
> kludges.
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/functional/test_microblazeel_s3adsp1800.py | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

Thanks for tackling this!

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Daniel P. Berrangé]

On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
> On 6/2/25 15:31, Daniel P. Berrangé wrote:
> > On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
> > > Hi Daniel,
> > > 
> > > On 6/2/25 14:20, Daniel P. Berrangé wrote:
> > > > On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > Introduce an abstract machine parent class which defines
> > > > > the 'little_endian' property. Duplicate the current machine,
> > > > > which endian is tied to the binary endianness, to one big
> > > > > endian and a little endian machine; updating the machine
> > > > > description. Keep the current default machine for each binary.
> > > > > 
> > > > > 'petalogix-s3adsp1800' machine is aliased as:
> > > > > - 'petalogix-s3adsp1800-be' on big-endian binary,
> > > > > - 'petalogix-s3adsp1800-le' on little-endian one.
> > > > 
> > > > Does it makes sense to expose these as different machine types ?
> > > > 
> > > > If all the HW is identical in both cases, it feels like the
> > > > endianness could just be a bool property of the machine type,
> > > > rather than a new machine type.
> > > 
> > > Our test suites expect "qemu-system-foo -M bar" to work out of
> > > the box, we can not have non-default properties.
> > > 
> > > (This is related to the raspberry pi discussion in
> > > https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
> > > 
> > > My plan is to deprecate 'petalogix-s3adsp1800', so once we
> > > remove it we can merge both qemu-system-microblaze and
> > > qemu-system-microblazeel into a single binary.
> > > 
> > > If you don't want to add more machines, what should be the
> > > endianness of the 'petalogix-s3adsp1800' machine in a binary
> > > with no particular endianness? Either we add for explicit
> > > endianness (fixing test suites) or we add one machine for
> > > each endianness; I fail to see other options not too
> > > confusing for our users.
> > 
> > We would pick an arbitrary endianness of our choosing
> > I guess. How does this work in physical machines ? Is
> > the choice of endianess a firmware setting, or a choice
> > by the vendor when manufacturing in some way ?
> 
> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
> (wired to a CPU pin which is sampled once at cold reset).

That makes me thing even more it is just a machine type property.

None the less, since you've already taken this pattern of
dual machine types for BE & LE on  MIPS/SH4/XTensa, I think
we should stick with your precedent. Consistent modelling
of endian handling across all machine types is most important
IMHO


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v5 02/16] hw/net/xilinx_ethlite: Make device endianness configurable

[Thomas Huth]

On 06/02/2025 14.10, 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 on the single machine using the
> device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
>   hw/net/xilinx_ethlite.c                  | 20 ++++++++++++++------
>   2 files changed, 15 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Max Filippov]

On Thu, Feb 6, 2025 at 7:04 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> On 6/2/25 15:31, Daniel P. Berrangé wrote:
> > We would pick an arbitrary endianness of our choosing
> > I guess. How does this work in physical machines ? Is
> > the choice of endianess a firmware setting, or a choice
> > by the vendor when manufacturing in some way ?
>
> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
> (wired to a CPU pin which is sampled once at cold reset).

This is not exactly the case for xtensa. Each xtensa CPU is either
big- or little-endian and it's a static property of the CPU
configuration. On physical machines it's either fixed (e.g. if
the CPU is a part of an ASIC), or it's a part of a bitstream that
gets loaded into an FPGA and there may be a selector for one
of the bitstreams in the onboard FLASH. In either case there's
normally no board-level switch for the CPU endianness.

Also big- and little-endian instruction encodings are different on
otherwise identical xtensa CPUs.

-- 
Thanks.
-- Max

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

On 6/2/25 18:34, Max Filippov wrote:
> On Thu, Feb 6, 2025 at 7:04 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>> We would pick an arbitrary endianness of our choosing
>>> I guess. How does this work in physical machines ? Is
>>> the choice of endianess a firmware setting, or a choice
>>> by the vendor when manufacturing in some way ?
>>
>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>> (wired to a CPU pin which is sampled once at cold reset).
> 
> This is not exactly the case for xtensa. Each xtensa CPU is either
> big- or little-endian and it's a static property of the CPU
> configuration. On physical machines it's either fixed (e.g. if
> the CPU is a part of an ASIC), or it's a part of a bitstream that
> gets loaded into an FPGA and there may be a selector for one
> of the bitstreams in the onboard FLASH. In either case there's
> normally no board-level switch for the CPU endianness.

Yes, this is what we meant in QEMU terms, "static property".

> Also big- and little-endian instruction encodings are different on
> otherwise identical xtensa CPUs.

This is handled within the CPU decode logic, and doesn't have to
be exposed. IOW, we don't need 2 distinct TCG frontends to decode
the Xtensa ISA.

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

On 6/2/25 18:12, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
>>>> Hi Daniel,
>>>>
>>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>> Introduce an abstract machine parent class which defines
>>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>>> which endian is tied to the binary endianness, to one big
>>>>>> endian and a little endian machine; updating the machine
>>>>>> description. Keep the current default machine for each binary.
>>>>>>
>>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>>
>>>>> Does it makes sense to expose these as different machine types ?
>>>>>
>>>>> If all the HW is identical in both cases, it feels like the
>>>>> endianness could just be a bool property of the machine type,
>>>>> rather than a new machine type.
>>>>
>>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>>> the box, we can not have non-default properties.
>>>>
>>>> (This is related to the raspberry pi discussion in
>>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
>>>>
>>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>>> remove it we can merge both qemu-system-microblaze and
>>>> qemu-system-microblazeel into a single binary.
>>>>
>>>> If you don't want to add more machines, what should be the
>>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>>> with no particular endianness? Either we add for explicit
>>>> endianness (fixing test suites) or we add one machine for
>>>> each endianness; I fail to see other options not too
>>>> confusing for our users.
>>>
>>> We would pick an arbitrary endianness of our choosing
>>> I guess. How does this work in physical machines ? Is
>>> the choice of endianess a firmware setting, or a choice
>>> by the vendor when manufacturing in some way ?
>>
>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>> (wired to a CPU pin which is sampled once at cold reset).
> 
> That makes me thing even more it is just a machine type property.

I'm happy with a machine property, this was even my first approach
using OnOffAuto until I ran the test-suite and have qom-introspection
failing.

What should be the default?

Per the SH4 datasheet:

   Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
   specification external pin (MD5) in a power-on reset by the
   RESET pin. The endian mode of all spaces is determined by this
   bit. ENDIAN is a read-only bit.

There is no default per the spec, and I believe using one is
a mistake.

> None the less, since you've already taken this pattern of
> dual machine types for BE & LE on  MIPS/SH4/XTensa, I think
> we should stick with your precedent. Consistent modelling
> of endian handling across all machine types is most important
> IMHO
> 
> 
> With regards,
> Daniel

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Daniel P. Berrangé]

On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
> On 6/2/25 18:12, Daniel P. Berrangé wrote:
> > On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 6/2/25 15:31, Daniel P. Berrangé wrote:
> > > > On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > Hi Daniel,
> > > > > 
> > > > > On 6/2/25 14:20, Daniel P. Berrangé wrote:
> > > > > > On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > > > Introduce an abstract machine parent class which defines
> > > > > > > the 'little_endian' property. Duplicate the current machine,
> > > > > > > which endian is tied to the binary endianness, to one big
> > > > > > > endian and a little endian machine; updating the machine
> > > > > > > description. Keep the current default machine for each binary.
> > > > > > > 
> > > > > > > 'petalogix-s3adsp1800' machine is aliased as:
> > > > > > > - 'petalogix-s3adsp1800-be' on big-endian binary,
> > > > > > > - 'petalogix-s3adsp1800-le' on little-endian one.
> > > > > > 
> > > > > > Does it makes sense to expose these as different machine types ?
> > > > > > 
> > > > > > If all the HW is identical in both cases, it feels like the
> > > > > > endianness could just be a bool property of the machine type,
> > > > > > rather than a new machine type.
> > > > > 
> > > > > Our test suites expect "qemu-system-foo -M bar" to work out of
> > > > > the box, we can not have non-default properties.
> > > > > 
> > > > > (This is related to the raspberry pi discussion in
> > > > > https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
> > > > > 
> > > > > My plan is to deprecate 'petalogix-s3adsp1800', so once we
> > > > > remove it we can merge both qemu-system-microblaze and
> > > > > qemu-system-microblazeel into a single binary.
> > > > > 
> > > > > If you don't want to add more machines, what should be the
> > > > > endianness of the 'petalogix-s3adsp1800' machine in a binary
> > > > > with no particular endianness? Either we add for explicit
> > > > > endianness (fixing test suites) or we add one machine for
> > > > > each endianness; I fail to see other options not too
> > > > > confusing for our users.
> > > > 
> > > > We would pick an arbitrary endianness of our choosing
> > > > I guess. How does this work in physical machines ? Is
> > > > the choice of endianess a firmware setting, or a choice
> > > > by the vendor when manufacturing in some way ?
> > > 
> > > Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
> > > (wired to a CPU pin which is sampled once at cold reset).
> > 
> > That makes me thing even more it is just a machine type property.
> 
> I'm happy with a machine property, this was even my first approach
> using OnOffAuto until I ran the test-suite and have qom-introspection
> failing.
> 
> What should be the default?
> 
> Per the SH4 datasheet:
> 
>   Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
>   specification external pin (MD5) in a power-on reset by the
>   RESET pin. The endian mode of all spaces is determined by this
>   bit. ENDIAN is a read-only bit.
> 
> There is no default per the spec, and I believe using one is
> a mistake.

If it is left as an unspecified choice in the spec, then I would
presume HW vendors are picking an option based on what they
expect "most" common usage to be amongst their customers. IOW,
if we know of typically used guest OS prefer big or little, that
could influence our choice.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

+Michal

On 6/2/25 19:06, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
>> On 6/2/25 18:12, Daniel P. Berrangé wrote:
>>> On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>>>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>>>> Introduce an abstract machine parent class which defines
>>>>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>>>>> which endian is tied to the binary endianness, to one big
>>>>>>>> endian and a little endian machine; updating the machine
>>>>>>>> description. Keep the current default machine for each binary.
>>>>>>>>
>>>>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>>>>
>>>>>>> Does it makes sense to expose these as different machine types ?
>>>>>>>
>>>>>>> If all the HW is identical in both cases, it feels like the
>>>>>>> endianness could just be a bool property of the machine type,
>>>>>>> rather than a new machine type.
>>>>>>
>>>>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>>>>> the box, we can not have non-default properties.
>>>>>>
>>>>>> (This is related to the raspberry pi discussion in
>>>>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
>>>>>>
>>>>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>>>>> remove it we can merge both qemu-system-microblaze and
>>>>>> qemu-system-microblazeel into a single binary.
>>>>>>
>>>>>> If you don't want to add more machines, what should be the
>>>>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>>>>> with no particular endianness? Either we add for explicit
>>>>>> endianness (fixing test suites) or we add one machine for
>>>>>> each endianness; I fail to see other options not too
>>>>>> confusing for our users.
>>>>>
>>>>> We would pick an arbitrary endianness of our choosing
>>>>> I guess. How does this work in physical machines ? Is
>>>>> the choice of endianess a firmware setting, or a choice
>>>>> by the vendor when manufacturing in some way ?
>>>>
>>>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>>>> (wired to a CPU pin which is sampled once at cold reset).
>>>
>>> That makes me thing even more it is just a machine type property.
>>
>> I'm happy with a machine property, this was even my first approach
>> using OnOffAuto until I ran the test-suite and have qom-introspection
>> failing.
>>
>> What should be the default?
>>
>> Per the SH4 datasheet:
>>
>>    Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
>>    specification external pin (MD5) in a power-on reset by the
>>    RESET pin. The endian mode of all spaces is determined by this
>>    bit. ENDIAN is a read-only bit.
>>
>> There is no default per the spec, and I believe using one is
>> a mistake.
> 
> If it is left as an unspecified choice in the spec, then I would
> presume HW vendors are picking an option based on what they
> expect "most" common usage to be amongst their customers. IOW,
> if we know of typically used guest OS prefer big or little, that
> could influence our choice.

Please have a look at this thread:
https://lore.kernel.org/qemu-devel/d3346a55-584b-427b-8c87-32f7411a9290@amd.com/

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Daniel P. Berrangé]

On Thu, Feb 06, 2025 at 07:24:55PM +0100, Philippe Mathieu-Daudé wrote:
> +Michal
> 
> On 6/2/25 19:06, Daniel P. Berrangé wrote:
> > On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 6/2/25 18:12, Daniel P. Berrangé wrote:
> > > > On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > On 6/2/25 15:31, Daniel P. Berrangé wrote:
> > > > > > On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > > > Hi Daniel,
> > > > > > > 
> > > > > > > On 6/2/25 14:20, Daniel P. Berrangé wrote:
> > > > > > > > On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > > > > > Introduce an abstract machine parent class which defines
> > > > > > > > > the 'little_endian' property. Duplicate the current machine,
> > > > > > > > > which endian is tied to the binary endianness, to one big
> > > > > > > > > endian and a little endian machine; updating the machine
> > > > > > > > > description. Keep the current default machine for each binary.
> > > > > > > > > 
> > > > > > > > > 'petalogix-s3adsp1800' machine is aliased as:
> > > > > > > > > - 'petalogix-s3adsp1800-be' on big-endian binary,
> > > > > > > > > - 'petalogix-s3adsp1800-le' on little-endian one.
> > > > > > > > 
> > > > > > > > Does it makes sense to expose these as different machine types ?
> > > > > > > > 
> > > > > > > > If all the HW is identical in both cases, it feels like the
> > > > > > > > endianness could just be a bool property of the machine type,
> > > > > > > > rather than a new machine type.
> > > > > > > 
> > > > > > > Our test suites expect "qemu-system-foo -M bar" to work out of
> > > > > > > the box, we can not have non-default properties.
> > > > > > > 
> > > > > > > (This is related to the raspberry pi discussion in
> > > > > > > https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
> > > > > > > 
> > > > > > > My plan is to deprecate 'petalogix-s3adsp1800', so once we
> > > > > > > remove it we can merge both qemu-system-microblaze and
> > > > > > > qemu-system-microblazeel into a single binary.
> > > > > > > 
> > > > > > > If you don't want to add more machines, what should be the
> > > > > > > endianness of the 'petalogix-s3adsp1800' machine in a binary
> > > > > > > with no particular endianness? Either we add for explicit
> > > > > > > endianness (fixing test suites) or we add one machine for
> > > > > > > each endianness; I fail to see other options not too
> > > > > > > confusing for our users.
> > > > > > 
> > > > > > We would pick an arbitrary endianness of our choosing
> > > > > > I guess. How does this work in physical machines ? Is
> > > > > > the choice of endianess a firmware setting, or a choice
> > > > > > by the vendor when manufacturing in some way ?
> > > > > 
> > > > > Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
> > > > > (wired to a CPU pin which is sampled once at cold reset).
> > > > 
> > > > That makes me thing even more it is just a machine type property.
> > > 
> > > I'm happy with a machine property, this was even my first approach
> > > using OnOffAuto until I ran the test-suite and have qom-introspection
> > > failing.
> > > 
> > > What should be the default?
> > > 
> > > Per the SH4 datasheet:
> > > 
> > >    Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
> > >    specification external pin (MD5) in a power-on reset by the
> > >    RESET pin. The endian mode of all spaces is determined by this
> > >    bit. ENDIAN is a read-only bit.
> > > 
> > > There is no default per the spec, and I believe using one is
> > > a mistake.
> > 
> > If it is left as an unspecified choice in the spec, then I would
> > presume HW vendors are picking an option based on what they
> > expect "most" common usage to be amongst their customers. IOW,
> > if we know of typically used guest OS prefer big or little, that
> > could influence our choice.
> 
> Please have a look at this thread:
> https://lore.kernel.org/qemu-devel/d3346a55-584b-427b-8c87-32f7411a9290@amd.com/

That seems to give a pretty clear choice for qemu defaults

 "I am not aware about anybody configuring MB to big endian"

so in that particular case, defaulting to LE would be most sensible.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

(sorry, posted too quick)

On 6/2/25 19:24, Philippe Mathieu-Daudé wrote:
> +Michal
> 
> On 6/2/25 19:06, Daniel P. Berrangé wrote:
>> On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 6/2/25 18:12, Daniel P. Berrangé wrote:
>>>> On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>>>>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé 
>>>>>> wrote:
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>>>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé 
>>>>>>>> wrote:
>>>>>>>>> Introduce an abstract machine parent class which defines
>>>>>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>>>>>> which endian is tied to the binary endianness, to one big
>>>>>>>>> endian and a little endian machine; updating the machine
>>>>>>>>> description. Keep the current default machine for each binary.
>>>>>>>>>
>>>>>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>>>>>
>>>>>>>> Does it makes sense to expose these as different machine types ?
>>>>>>>>
>>>>>>>> If all the HW is identical in both cases, it feels like the
>>>>>>>> endianness could just be a bool property of the machine type,
>>>>>>>> rather than a new machine type.
>>>>>>>
>>>>>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>>>>>> the box, we can not have non-default properties.
>>>>>>>
>>>>>>> (This is related to the raspberry pi discussion in
>>>>>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1- 
>>>>>>> philmd@linaro.org/).
>>>>>>>
>>>>>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>>>>>> remove it we can merge both qemu-system-microblaze and
>>>>>>> qemu-system-microblazeel into a single binary.
>>>>>>>
>>>>>>> If you don't want to add more machines, what should be the
>>>>>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>>>>>> with no particular endianness? Either we add for explicit
>>>>>>> endianness (fixing test suites) or we add one machine for
>>>>>>> each endianness; I fail to see other options not too
>>>>>>> confusing for our users.
>>>>>>
>>>>>> We would pick an arbitrary endianness of our choosing
>>>>>> I guess. How does this work in physical machines ? Is
>>>>>> the choice of endianess a firmware setting, or a choice
>>>>>> by the vendor when manufacturing in some way ?
>>>>>
>>>>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>>>>> (wired to a CPU pin which is sampled once at cold reset).
>>>>
>>>> That makes me thing even more it is just a machine type property.
>>>
>>> I'm happy with a machine property, this was even my first approach
>>> using OnOffAuto until I ran the test-suite and have qom-introspection
>>> failing.
>>>
>>> What should be the default?
>>>
>>> Per the SH4 datasheet:
>>>
>>>    Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
>>>    specification external pin (MD5) in a power-on reset by the
>>>    RESET pin. The endian mode of all spaces is determined by this
>>>    bit. ENDIAN is a read-only bit.
>>>
>>> There is no default per the spec, and I believe using one is
>>> a mistake.
>>
>> If it is left as an unspecified choice in the spec, then I would
>> presume HW vendors are picking an option based on what they
>> expect "most" common usage to be amongst their customers. IOW,
>> if we know of typically used guest OS prefer big or little, that
>> could influence our choice.
> 
> Please have a look at this thread:
> https://lore.kernel.org/qemu-devel/ 
> d3346a55-584b-427b-8c87-32f7411a9290@amd.com/

Per Michal:

   Truth is that I am not aware about anybody configuring MB to big
   endian and we  are on AXI for quite a long time. There is still
   code in Linux kernel for it but I can't see any reason to keep it
   around. I don't think that make sense to keep big endian
   configurations alive at all.

So with that in mind, using a default of little-endian=true for
MicroBlaze seems safe as of 2025.

For MIPS, big-endian was the default but as the industry shifted,
it stayed big-endian on low-end 32-bit cores but mostly became
little-endian on high-end 64-bit cores.

For Renesas RX, the "Endian" section in the "CPU" chapter of the
hardware manual mentions:

   Both big and little endian are supported for the treatment of data,
   and the endian is switched by changing the setting on a mode pin (MDE)
   at the time of a power-on reset.

SH-4 "is bi-endian, running in either big-endian or little-endian byte
ordering."

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

On 6/2/25 19:29, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 07:24:55PM +0100, Philippe Mathieu-Daudé wrote:
>> +Michal
>>
>> On 6/2/25 19:06, Daniel P. Berrangé wrote:
>>> On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 6/2/25 18:12, Daniel P. Berrangé wrote:
>>>>> On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>>>>>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>>>> Hi Daniel,
>>>>>>>>
>>>>>>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>>>>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>>>>>>>>>> Introduce an abstract machine parent class which defines
>>>>>>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>>>>>>> which endian is tied to the binary endianness, to one big
>>>>>>>>>> endian and a little endian machine; updating the machine
>>>>>>>>>> description. Keep the current default machine for each binary.
>>>>>>>>>>
>>>>>>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>>>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>>>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>>>>>>
>>>>>>>>> Does it makes sense to expose these as different machine types ?
>>>>>>>>>
>>>>>>>>> If all the HW is identical in both cases, it feels like the
>>>>>>>>> endianness could just be a bool property of the machine type,
>>>>>>>>> rather than a new machine type.
>>>>>>>>
>>>>>>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>>>>>>> the box, we can not have non-default properties.
>>>>>>>>
>>>>>>>> (This is related to the raspberry pi discussion in
>>>>>>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1-philmd@linaro.org/).
>>>>>>>>
>>>>>>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>>>>>>> remove it we can merge both qemu-system-microblaze and
>>>>>>>> qemu-system-microblazeel into a single binary.
>>>>>>>>
>>>>>>>> If you don't want to add more machines, what should be the
>>>>>>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>>>>>>> with no particular endianness? Either we add for explicit
>>>>>>>> endianness (fixing test suites) or we add one machine for
>>>>>>>> each endianness; I fail to see other options not too
>>>>>>>> confusing for our users.
>>>>>>>
>>>>>>> We would pick an arbitrary endianness of our choosing
>>>>>>> I guess. How does this work in physical machines ? Is
>>>>>>> the choice of endianess a firmware setting, or a choice
>>>>>>> by the vendor when manufacturing in some way ?
>>>>>>
>>>>>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>>>>>> (wired to a CPU pin which is sampled once at cold reset).
>>>>>
>>>>> That makes me thing even more it is just a machine type property.
>>>>
>>>> I'm happy with a machine property, this was even my first approach
>>>> using OnOffAuto until I ran the test-suite and have qom-introspection
>>>> failing.
>>>>
>>>> What should be the default?
>>>>
>>>> Per the SH4 datasheet:
>>>>
>>>>     Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
>>>>     specification external pin (MD5) in a power-on reset by the
>>>>     RESET pin. The endian mode of all spaces is determined by this
>>>>     bit. ENDIAN is a read-only bit.
>>>>
>>>> There is no default per the spec, and I believe using one is
>>>> a mistake.
>>>
>>> If it is left as an unspecified choice in the spec, then I would
>>> presume HW vendors are picking an option based on what they
>>> expect "most" common usage to be amongst their customers. IOW,
>>> if we know of typically used guest OS prefer big or little, that
>>> could influence our choice.
>>
>> Please have a look at this thread:
>> https://lore.kernel.org/qemu-devel/d3346a55-584b-427b-8c87-32f7411a9290@amd.com/
> 
> That seems to give a pretty clear choice for qemu defaults
> 
>   "I am not aware about anybody configuring MB to big endian"
> 
> so in that particular case, defaulting to LE would be most sensible.

Or maybe I should stop trying to unify the current binaries, and add
the new data-drive machines in a new binary, as you already suggested:
https://lore.kernel.org/qemu-devel/ZEoyNt0UtSYRt9Go@redhat.com/

But then I'm worried about our users, on how to deprecate the current
microblaze{,el} binaries to have them use the new one seamlessly.

And more importantly, one of the goal is to maintain LESS binaries,
no more.

Re: [PATCH v5 02/16] hw/net/xilinx_ethlite: Make device endianness configurable

[Richard Henderson]

On 2/6/25 05:10, 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 on the single machine using the
> device.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
>   hw/net/xilinx_ethlite.c                  | 20 ++++++++++++++------
>   2 files changed, 15 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v5 14/16] tests/functional: Remove sleep() kludges from microblaze tests

[Richard Henderson]

On 2/6/25 05:10, Philippe Mathieu-Daudé wrote:
> Commit f0ec14c78c4 ("tests/avocado: Fix console data loss") fixed
> QEMUMachine's problem with console, we don't need to use the sleep()
> kludges.
> 
> Suggested-by: Thomas Huth<thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   tests/functional/test_microblazeel_s3adsp1800.py | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v5 00/16] hw/microblaze: Allow running cross-endian vCPUs

[Philippe Mathieu-Daudé]

On 6/2/25 14:10, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (16):
>    hw/intc/xilinx_intc: Make device endianness configurable
>    hw/net/xilinx_ethlite: Make device endianness configurable
>    hw/timer/xilinx_timer: Make device endianness configurable
>    hw/char/xilinx_uartlite: Make device endianness configurable
>    hw/ssi/xilinx_spi: Make device endianness configurable
>    hw/arm/xlnx-zynqmp: Use &error_abort for programming errors

>    tests/functional: Explicit endianness of microblaze assets
>    tests/functional: Allow microblaze tests to take a machine name
>      argument
>    tests/functional: Remove sleep() kludges from microblaze tests
>    tests/functional: Have microblaze tests inherit common parent class

Since there is still an open discussion about what machines to
include, I'm queuing patches 1-6 & 12-15 for now.

Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

[Philippe Mathieu-Daudé]

On 6/2/25 14:20, Daniel P. Berrangé wrote:
> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote:
>> Introduce an abstract machine parent class which defines
>> the 'little_endian' property. Duplicate the current machine,
>> which endian is tied to the binary endianness, to one big
>> endian and a little endian machine; updating the machine
>> description. Keep the current default machine for each binary.
>>
>> 'petalogix-s3adsp1800' machine is aliased as:
>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>> - 'petalogix-s3adsp1800-le' on little-endian one.
> 
> Does it makes sense to expose these as different machine types ?
> 
> If all the HW is identical in both cases, it feels like the
> endianness could just be a bool property of the machine type,
> rather than a new machine type.

To clarify what we are trying to achieve here:

1/ Current situation:

$ qemu-system-microblaze -M help
Supported machines are:
none                 empty machine
petalogix-ml605      PetaLogix linux refdesign for xilinx ml605 (big 
endian) (deprecated)
petalogix-s3adsp1800 PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (default)
xlnx-zynqmp-pmu      Xilinx ZynqMP PMU machine (big endian) (deprecated)
$ qemu-system-microblazeel -M help
Supported machines are:
none                 empty machine
petalogix-ml605      PetaLogix linux refdesign for xilinx ml605 (little 
endian)
petalogix-s3adsp1800 PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (default)
xlnx-zynqmp-pmu      Xilinx ZynqMP PMU machine (little endian)

1 architecture declined in 2 binaries, total of 6 machines

2/ With this series:

qemu-system-microblaze -M help
Supported machines are:
none                 empty machine
petalogix-ml605      PetaLogix linux refdesign for xilinx ml605 (big 
endian) (deprecated)
petalogix-s3adsp1800 PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (big endian) (alias of petalogix-s3adsp1800-be)
petalogix-s3adsp1800-be PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (big endian) (default)
petalogix-s3adsp1800-le PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (little endian)
xlnx-zynqmp-pmu      Xilinx ZynqMP PMU machine (big endian) (deprecated)
qemu-system-microblazeel -M help
Supported machines are:
none                 empty machine
petalogix-ml605      PetaLogix linux refdesign for xilinx ml605 (little 
endian)
petalogix-s3adsp1800-be PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (big endian)
petalogix-s3adsp1800 PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (little endian) (alias of petalogix-s3adsp1800-le)
petalogix-s3adsp1800-le PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (little endian) (default)
xlnx-zynqmp-pmu      Xilinx ZynqMP PMU machine (little endian)

1 architecture declined in 2 binaries, total of 10 machines

3/ Once the deprecation period passed and we can merge the 2 binaries:

$ qemu-system-microblaze -M help
Supported machines are:
none                 empty machine
petalogix-ml605      PetaLogix linux refdesign for xilinx ml605 (little 
endian)
petalogix-s3adsp1800-be PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (big endian)
petalogix-s3adsp1800-le PetaLogix linux refdesign for xilinx Spartan 
3ADSP1800 (little endian)
xlnx-zynqmp-pmu      Xilinx ZynqMP PMU machine (little endian)

1 architecture, 1 binary, 4 machines (which could be reduced to 3 if
we want a default endianness for the s3adsp1800).

Due to the deprecation policy, step 2/ is necessary to reach 3/.