[PATCH 0/4] hw/microblaze: Endianness clean-ups and deprecations

[PATCH 0/4] hw/microblaze: Endianness clean-ups and deprecations

[Thomas Huth]

Thanks to the great reworks from Philippe, the microblaze targets
can now handle both endiannesses. I'd like to suggest now to use
a "endianness" property to allow to switch the endianness of the
petalogix-s3adsp1800 machine. The endiannes of the other two
microblaze machines should be hard-wired to little-endian now
since the big-endian mode likely never worked. With all those
modifications in place, we can then finally deprecate the little-endian
target binary qemu-system-microblazeel since the qemu-system-microblaze
can handle all use cases. This will help us in the future to reduce
our compilation and testing times.

Thomas Huth (4):
  hw/microblaze: Add endianness property to the petalogix_s3adsp1800
    machine
  tests/functional: Test both microblaze s3adsp1800 endianness variants
  hw/microblaze: Remove the big-endian variants of ml605 and
    xlnx-zynqmp-pmu
  docs: Deprecate the qemu-system-microblazeel binary

 docs/about/deprecated.rst                     | 19 ++++++---
 docs/about/removed-features.rst               |  9 ++++
 hw/microblaze/petalogix_ml605_mmu.c           | 15 ++-----
 hw/microblaze/petalogix_s3adsp1800_mmu.c      | 41 ++++++++++++++++---
 hw/microblaze/xlnx-zynqmp-pmu.c               |  7 +---
 .../functional/test_microblaze_s3adsp1800.py  | 17 +++++---
 .../test_microblazeel_s3adsp1800.py           |  5 ++-
 7 files changed, 79 insertions(+), 34 deletions(-)

-- 
2.49.0

[PATCH 1/4] hw/microblaze: Add endianness property to the petalogix_s3adsp1800 machine

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Since the microblaze target can now handle both endianness, big and
little, we should provide a config knob for the user to select the
desired endianness.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 41 +++++++++++++++++++++---
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 032f6f70eac..edc5d0dcfd0 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -58,9 +58,20 @@
 #define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
             MACHINE_TYPE_NAME("petalogix-s3adsp1800")
 
+struct S3Adsp1800MachineState {
+    MachineState parent_class;
+
+    EndianMode endianness;
+};
+
+OBJECT_DECLARE_TYPE(S3Adsp1800MachineState, MachineClass,
+                    PETALOGIX_S3ADSP1800_MACHINE)
+
+
 static void
 petalogix_s3adsp1800_init(MachineState *machine)
 {
+    S3Adsp1800MachineState *psms = PETALOGIX_S3ADSP1800_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     DeviceState *dev;
     MicroBlazeCPU *cpu;
@@ -71,13 +82,12 @@ petalogix_s3adsp1800_init(MachineState *machine)
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
     qemu_irq irq[32];
     MemoryRegion *sysmem = get_system_memory();
-    EndianMode endianness = TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG
-                                              : ENDIAN_MODE_LITTLE;
+    EndianMode endianness = psms->endianness;
 
     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);
+                             endianness == ENDIAN_MODE_LITTLE, &error_abort);
     qdev_realize(DEVICE(cpu), NULL, &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
@@ -135,20 +145,41 @@ petalogix_s3adsp1800_init(MachineState *machine)
 
     create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
 
-    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
-                           machine->initrd_filename,
+    microblaze_load_kernel(cpu, endianness == ENDIAN_MODE_LITTLE, ddr_base,
+                           ram_size, machine->initrd_filename,
                            BINARY_DEVICE_TREE_FILE,
                            NULL);
 }
 
+static int machine_get_endianness(Object *obj, Error **errp G_GNUC_UNUSED)
+{
+    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
+    return ms->endianness;
+}
+
+static void machine_set_endianness(Object *obj, int endianness, Error **errp)
+{
+    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
+    ms->endianness = endianness;
+}
+
 static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
                                                     const void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    ObjectProperty *prop;
 
     mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
     mc->init = petalogix_s3adsp1800_init;
     mc->is_default = true;
+
+    prop = object_class_property_add_enum(oc, "endianness", "EndianMode",
+                                          &EndianMode_lookup,
+                                          machine_get_endianness,
+                                          machine_set_endianness);
+    object_property_set_default_str(prop, TARGET_BIG_ENDIAN ? "big" : "little");
+    object_class_property_set_description(oc, "endianness",
+            "Defines whether the machine runs in big or little endian mode");
 }
 
 static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
-- 
2.49.0

[PATCH 2/4] tests/functional: Test both microblaze s3adsp1800 endianness variants

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Now that the endianness of the petalogix-s3adsp1800 can be configured,
we should test that the cross-endianness also works as expected, thus
test the big endian variant on the little endian target and vice versa.
(based on an original idea from Philippe Mathieu-Daudé)

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/test_microblaze_s3adsp1800.py  | 17 ++++++++++++-----
 .../functional/test_microblazeel_s3adsp1800.py  |  5 ++++-
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index c93fa14232b..c5e60b555c6 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -25,12 +25,14 @@ class MicroblazeMachine(QemuSystemTest):
         ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
         'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
 
-    def do_ballerina_be_test(self, machine):
-        self.set_machine(machine)
+    def do_ballerina_be_test(self, force_endianness=False):
+        self.set_machine('petalogix-s3adsp1800')
         self.archive_extract(self.ASSET_IMAGE_BE)
         self.vm.set_console()
         self.vm.add_args('-kernel',
                          self.scratch_file('day17', 'ballerina.bin'))
+        if force_endianness:
+            self.vm.add_args('-M', 'endianness=big')
         self.vm.launch()
         wait_for_console_pattern(self, 'This architecture does not have '
                                        'kernel memory protection')
@@ -39,12 +41,14 @@ 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):
+    def do_xmaton_le_test(self, force_endianness=False):
         self.require_netdev('user')
-        self.set_machine(machine)
+        self.set_machine('petalogix-s3adsp1800')
         self.archive_extract(self.ASSET_IMAGE_LE)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
+        if force_endianness:
+            self.vm.add_args('-M', 'endianness=little')
         tftproot = self.scratch_file('day13')
         self.vm.add_args('-nic', f'user,tftp={tftproot}')
         self.vm.launch()
@@ -61,7 +65,10 @@ class MicroblazeBigEndianMachine(MicroblazeMachine):
     ASSET_IMAGE_BE = MicroblazeMachine.ASSET_IMAGE_BE
 
     def test_microblaze_s3adsp1800_legacy_be(self):
-        self.do_ballerina_be_test('petalogix-s3adsp1800')
+        self.do_ballerina_be_test()
+
+    def test_microblaze_s3adsp1800_legacy_le(self):
+        self.do_xmaton_le_test(force_endianness=True)
 
 
 if __name__ == '__main__':
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index ab59941d57a..9de27d09b75 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -15,7 +15,10 @@ class MicroblazeLittleEndianMachine(MicroblazeMachine):
     ASSET_IMAGE_LE = MicroblazeMachine.ASSET_IMAGE_LE
 
     def test_microblaze_s3adsp1800_legacy_le(self):
-        self.do_xmaton_le_test('petalogix-s3adsp1800')
+        self.do_xmaton_le_test()
+
+    def test_microblaze_s3adsp1800_legacy_be(self):
+        self.do_ballerina_be_test(force_endianness=True)
 
 
 if __name__ == '__main__':
-- 
2.49.0

[PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Both machines were added with little-endian in mind only (the
"endianness" CPU property was hard-wired to "true", see commits
133d23b3ad1 and a88bbb006a52), so the variants that showed up
on the big endian target likely never worked. We deprecated these
non-working machine variants two releases ago, and so far nobody
complained, so it should be fine now to disable them. Hard-wire
the machines to little endian now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst           |  6 ------
 docs/about/removed-features.rst     |  9 +++++++++
 hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
 hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1a1b423030c..8c5fe6fb274 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -293,12 +293,6 @@ deprecated; use the new name ``dtb-randomness`` instead. The new name
 better reflects the way this property affects all random data within
 the device tree blob, not just the ``kaslr-seed`` node.
 
-Big-Endian variants of MicroBlaze ``petalogix-ml605`` and ``xlnx-zynqmp-pmu`` machines (since 9.2)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Both ``petalogix-ml605`` and ``xlnx-zynqmp-pmu`` were added for little endian
-CPUs. Big endian support is not tested.
-
 Mips ``mipssim`` machine (since 10.0)
 '''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 063284d4f8a..9df5aba0bb6 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1082,6 +1082,15 @@ This machine was removed because PPC 405 CPU have no known users,
 firmware images are not available, OpenWRT dropped support in 2019,
 U-Boot in 2017, and Linux in 2024.
 
+Big-Endian variants of ``petalogix-ml605`` and ``xlnx-zynqmp-pmu`` machines (removed in 10.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Both the MicroBlaze ``petalogix-ml605`` and ``xlnx-zynqmp-pmu`` machines
+were added for little endian CPUs. Big endian support was never tested
+and likely never worked. Starting with QEMU v10.1, the machines are now
+only available as little-endian machines.
+
+
 linux-user mode CPUs
 --------------------
 
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index bea6b689fd1..6e923c49cfc 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -80,8 +80,6 @@ petalogix_ml605_init(MachineState *machine)
     MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
     qemu_irq irq[32];
-    EndianMode endianness = TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG
-                                              : ENDIAN_MODE_LITTLE;
 
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
@@ -113,7 +111,7 @@ petalogix_ml605_init(MachineState *machine)
 
 
     dev = qdev_new("xlnx.xps-intc");
-    qdev_prop_set_enum(dev, "endianness", endianness);
+    qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_LITTLE);
     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);
@@ -129,7 +127,7 @@ petalogix_ml605_init(MachineState *machine)
 
     /* 2 timers at irq 2 @ 100 Mhz.  */
     dev = qdev_new("xlnx.xps-timer");
-    qdev_prop_set_enum(dev, "endianness", endianness);
+    qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_LITTLE);
     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);
@@ -177,7 +175,7 @@ petalogix_ml605_init(MachineState *machine)
         SSIBus *spi;
 
         dev = qdev_new("xlnx.xps-spi");
-        qdev_prop_set_enum(dev, "endianness", endianness);
+        qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_LITTLE);
         qdev_prop_set_uint8(dev, "num-ss-bits", NUM_SPI_FLASHES);
         busdev = SYS_BUS_DEVICE(dev);
         sysbus_realize_and_unref(busdev, &error_fatal);
@@ -218,12 +216,7 @@ petalogix_ml605_init(MachineState *machine)
 
 static void petalogix_ml605_machine_init(MachineClass *mc)
 {
-    if (TARGET_BIG_ENDIAN) {
-        mc->desc = "PetaLogix linux refdesign for xilinx ml605 (big endian)";
-        mc->deprecation_reason = "big endian support is not tested";
-    } else {
-        mc->desc = "PetaLogix linux refdesign for xilinx ml605 (little endian)";
-    }
+    mc->desc = "PetaLogix linux refdesign for xilinx ml605 (little endian)";
     mc->init = petalogix_ml605_init;
 }
 
diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index ed40b5f2e05..e909802bb74 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -181,12 +181,7 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 
 static void xlnx_zynqmp_pmu_machine_init(MachineClass *mc)
 {
-    if (TARGET_BIG_ENDIAN) {
-        mc->desc = "Xilinx ZynqMP PMU machine (big endian)";
-        mc->deprecation_reason = "big endian support is not tested";
-    } else {
-        mc->desc = "Xilinx ZynqMP PMU machine (little endian)";
-    }
+    mc->desc = "Xilinx ZynqMP PMU machine (little endian)";
     mc->init = xlnx_zynqmp_pmu_init;
 }
 
-- 
2.49.0

[PATCH 4/4] docs: Deprecate the qemu-system-microblazeel binary

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The (former big-endian only) binary qemu-system-microblaze can
handle both endiannesses nowadays, so we don't need the separate
qemu-system-microblazeel binary for little endian anymore. Let's
deprecate it to avoid unnecessary compilation and test time in
the future.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 8c5fe6fb274..0dfcad5d029 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -323,6 +323,19 @@ machine must ensure that they're setting the ``spike`` machine in the
 command line (``-M spike``).
 
 
+System emulator binaries
+------------------------
+
+``qemu-system-microblazeel`` (since 10.1)
+'''''''''''''''''''''''''''''''''''''''''
+
+The ``qemu-system-microblaze`` binary can emulate little-endian machines
+now, too, so the separate binary ``qemu-system-microblazeel`` (with the
+``el`` suffix) for little-endian targets is not required anymore. The
+``petalogix-s3adsp1800`` machine can now be switched to little endian by
+setting its ``endianness`` property to ``little``.
+
+
 Backend options
 ---------------
 
-- 
2.49.0

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Philippe Mathieu-Daudé]

On 15/5/25 15:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Both machines were added with little-endian in mind only (the
> "endianness" CPU property was hard-wired to "true", see commits
> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
> on the big endian target likely never worked. We deprecated these
> non-working machine variants two releases ago, and so far nobody
> complained, so it should be fine now to disable them. Hard-wire
> the machines to little endian now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/about/deprecated.rst           |  6 ------
>   docs/about/removed-features.rst     |  9 +++++++++
>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>   4 files changed, 14 insertions(+), 23 deletions(-)

\o/

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I won't be able to look at the rest of this series until in 10 days,
feel free to merge if necessary.

Thanks!

Phil.

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Thomas Huth]

On 16/05/2025 17.00, Philippe Mathieu-Daudé wrote:
> On 15/5/25 15:20, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Both machines were added with little-endian in mind only (the
>> "endianness" CPU property was hard-wired to "true", see commits
>> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
>> on the big endian target likely never worked. We deprecated these
>> non-working machine variants two releases ago, and so far nobody
>> complained, so it should be fine now to disable them. Hard-wire
>> the machines to little endian now.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   docs/about/deprecated.rst           |  6 ------
>>   docs/about/removed-features.rst     |  9 +++++++++
>>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>>   4 files changed, 14 insertions(+), 23 deletions(-)
> 
> \o/
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

> I won't be able to look at the rest of this series until in 10 days,
> feel free to merge if necessary.

Maybe you could at least provide a quick comment on the idea of controlling 
the endianness of the s3adsp1800 machine via a machine property? (since your 
original idea was to do it via different machine names instead ... does the 
property sound acceptable to you?)

  Thomas

Re: [PATCH 1/4] hw/microblaze: Add endianness property to the petalogix_s3adsp1800 machine

[Richard Henderson]

On 5/15/25 14:20, Thomas Huth wrote:
> +static int machine_get_endianness(Object *obj, Error **errp G_GNUC_UNUSED)
> +{
> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
> +    return ms->endianness;
> +}
> +
> +static void machine_set_endianness(Object *obj, int endianness, Error **errp)
> +{
> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
> +    ms->endianness = endianness;
> +}
> +
>   static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
>                                                       const void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> +    ObjectProperty *prop;
>   
>       mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>       mc->init = petalogix_s3adsp1800_init;
>       mc->is_default = true;
> +
> +    prop = object_class_property_add_enum(oc, "endianness", "EndianMode",
> +                                          &EndianMode_lookup,
> +                                          machine_get_endianness,
> +                                          machine_set_endianness);
> +    object_property_set_default_str(prop, TARGET_BIG_ENDIAN ? "big" : "little");
> +    object_class_property_set_description(oc, "endianness",
> +            "Defines whether the machine runs in big or little endian mode");


Better with Property?  You don't have to write get/set...

   static const Property props[] = {
     DEFINE_PROP_ENDIAN("endianness", S3Adsp1800MachineState, endianness,
                        TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG : ENDIAN_MODE_LITTLE),
   };

   device_class_set_props(dc, props);


r~

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Richard Henderson]

On 5/15/25 14:20, Thomas Huth wrote:
> From: Thomas Huth<thuth@redhat.com>
> 
> Both machines were added with little-endian in mind only (the
> "endianness" CPU property was hard-wired to "true", see commits
> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
> on the big endian target likely never worked. We deprecated these
> non-working machine variants two releases ago, and so far nobody
> complained, so it should be fine now to disable them. Hard-wire
> the machines to little endian now.
> 
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   docs/about/deprecated.rst           |  6 ------
>   docs/about/removed-features.rst     |  9 +++++++++
>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>   4 files changed, 14 insertions(+), 23 deletions(-)

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

r~

Re: [PATCH 4/4] docs: Deprecate the qemu-system-microblazeel binary

[Richard Henderson]

On 5/15/25 14:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The (former big-endian only) binary qemu-system-microblaze can
> handle both endiannesses nowadays, so we don't need the separate
> qemu-system-microblazeel binary for little endian anymore. Let's
> deprecate it to avoid unnecessary compilation and test time in
> the future.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>


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

r~

Re: [PATCH 1/4] hw/microblaze: Add endianness property to the petalogix_s3adsp1800 machine

[Philippe Mathieu-Daudé]

+Markus

On 24/5/25 13:55, Richard Henderson wrote:
> On 5/15/25 14:20, Thomas Huth wrote:
>> +static int machine_get_endianness(Object *obj, Error **errp 
>> G_GNUC_UNUSED)
>> +{
>> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
>> +    return ms->endianness;
>> +}
>> +
>> +static void machine_set_endianness(Object *obj, int endianness, Error 
>> **errp)
>> +{
>> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
>> +    ms->endianness = endianness;
>> +}
>> +
>>   static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
>>                                                       const void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> +    ObjectProperty *prop;
>>       mc->desc = "PetaLogix linux refdesign for xilinx Spartan 
>> 3ADSP1800";
>>       mc->init = petalogix_s3adsp1800_init;
>>       mc->is_default = true;
>> +
>> +    prop = object_class_property_add_enum(oc, "endianness", 
>> "EndianMode",
>> +                                          &EndianMode_lookup,
>> +                                          machine_get_endianness,
>> +                                          machine_set_endianness);
>> +    object_property_set_default_str(prop, TARGET_BIG_ENDIAN ? "big" : 
>> "little");
>> +    object_class_property_set_description(oc, "endianness",
>> +            "Defines whether the machine runs in big or little endian 
>> mode");
> 
> 
> Better with Property?  You don't have to write get/set...
> 
>    static const Property props[] = {
>      DEFINE_PROP_ENDIAN("endianness", S3Adsp1800MachineState, endianness,
>                         TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG : 
> ENDIAN_MODE_LITTLE),
>    };
> 
>    device_class_set_props(dc, props);

DEFINE_PROP_FOO() are restricted to QDev (DeviceClass). Here we have
a MachineClass, which only inherits ObjectClass, not DeviceClass.

Markus once explained me the difference between QDev properties
and bare object ones; I asked why we couldn't make qdev properties
generic to objects, but I don't remember the historical rationale.
QDev predates QOM, QDev used static properties, QOM introduced
dynamic ones? We definitively should document that...

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Philippe Mathieu-Daudé]

On 16/5/25 16:06, Thomas Huth wrote:
> On 16/05/2025 17.00, Philippe Mathieu-Daudé wrote:
>> On 15/5/25 15:20, Thomas Huth wrote:
>>> From: Thomas Huth <thuth@redhat.com>
>>>
>>> Both machines were added with little-endian in mind only (the
>>> "endianness" CPU property was hard-wired to "true", see commits
>>> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
>>> on the big endian target likely never worked. We deprecated these
>>> non-working machine variants two releases ago, and so far nobody
>>> complained, so it should be fine now to disable them. Hard-wire
>>> the machines to little endian now.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   docs/about/deprecated.rst           |  6 ------
>>>   docs/about/removed-features.rst     |  9 +++++++++
>>>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>>>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>>>   4 files changed, 14 insertions(+), 23 deletions(-)
>>
>> \o/
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Thanks!
> 
>> I won't be able to look at the rest of this series until in 10 days,
>> feel free to merge if necessary.
> 
> Maybe you could at least provide a quick comment on the idea of 
> controlling the endianness of the s3adsp1800 machine via a machine 
> property? (since your original idea was to do it via different machine 
> names instead ... does the property sound acceptable to you?)

IIRC my goal was to remove TARGET_BIG_ENDIAN from hw/, but I was
aiming for heterogeneous binary, trying to not introduce any changes
for users. Now we are only considering "single binary" and we have
the TargetInfo API, so things are simpler.

Using the TargetInfo API, that would be:

-- >8 --
@@ -26,7 +26,8 @@
  #include "qemu/osdep.h"
  #include "qemu/units.h"
  #include "qapi/error.h"
+#include "qemu/target-info.h"
  #include "hw/sysbus.h"
  #include "net/net.h"
  #include "hw/block/flash.h"
@@ -71,13 +83,16 @@ petalogix_s3adsp1800_init(MachineState *machine)
      MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
      qemu_irq irq[32];
      MemoryRegion *sysmem = get_system_memory();
-    EndianMode endianness = TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG
-                                              : ENDIAN_MODE_LITTLE;
+    EndianMode endianness = psms->endianness;
+
+    if (endianness == ENDIAN_MODE_UNSPECIFIED) {
+        endianness = target_endian_mode();
+    }

---

(NB target_endian_mode is not on the list yet)

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Thomas Huth]

On 25/05/2025 21.19, Philippe Mathieu-Daudé wrote:
> On 16/5/25 16:06, Thomas Huth wrote:
>> On 16/05/2025 17.00, Philippe Mathieu-Daudé wrote:
>>> On 15/5/25 15:20, Thomas Huth wrote:
>>>> From: Thomas Huth <thuth@redhat.com>
>>>>
>>>> Both machines were added with little-endian in mind only (the
>>>> "endianness" CPU property was hard-wired to "true", see commits
>>>> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
>>>> on the big endian target likely never worked. We deprecated these
>>>> non-working machine variants two releases ago, and so far nobody
>>>> complained, so it should be fine now to disable them. Hard-wire
>>>> the machines to little endian now.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   docs/about/deprecated.rst           |  6 ------
>>>>   docs/about/removed-features.rst     |  9 +++++++++
>>>>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>>>>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>>>>   4 files changed, 14 insertions(+), 23 deletions(-)
>>>
>>> \o/
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Thanks!
>>
>>> I won't be able to look at the rest of this series until in 10 days,
>>> feel free to merge if necessary.
>>
>> Maybe you could at least provide a quick comment on the idea of 
>> controlling the endianness of the s3adsp1800 machine via a machine 
>> property? (since your original idea was to do it via different machine 
>> names instead ... does the property sound acceptable to you?)
> 
> IIRC my goal was to remove TARGET_BIG_ENDIAN from hw/, but I was
> aiming for heterogeneous binary, trying to not introduce any changes
> for users. Now we are only considering "single binary" and we have
> the TargetInfo API, so things are simpler.
> 
> Using the TargetInfo API, that would be:
> 
> -- >8 --
> @@ -26,7 +26,8 @@
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
>   #include "qapi/error.h"
> +#include "qemu/target-info.h"
>   #include "hw/sysbus.h"
>   #include "net/net.h"
>   #include "hw/block/flash.h"
> @@ -71,13 +83,16 @@ petalogix_s3adsp1800_init(MachineState *machine)
>       MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>       qemu_irq irq[32];
>       MemoryRegion *sysmem = get_system_memory();
> -    EndianMode endianness = TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG
> -                                              : ENDIAN_MODE_LITTLE;
> +    EndianMode endianness = psms->endianness;
> +
> +    if (endianness == ENDIAN_MODE_UNSPECIFIED) {
> +        endianness = target_endian_mode();
> +    }

But one of the ideas of this series here is to finally deprecate 
qemu-system-microblazeel, so we need a way that the user can use to 
configure the endianness of the machine. Does the property look like an 
acceptable way for this to you?

  Thomas

Re: [PATCH 1/4] hw/microblaze: Add endianness property to the petalogix_s3adsp1800 machine

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> +Markus
>
> On 24/5/25 13:55, Richard Henderson wrote:
>> On 5/15/25 14:20, Thomas Huth wrote:
>>> +static int machine_get_endianness(Object *obj, Error **errp G_GNUC_UNUSED)
>>> +{
>>> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
>>> +    return ms->endianness;
>>> +}
>>> +
>>> +static void machine_set_endianness(Object *obj, int endianness, Error **errp)
>>> +{
>>> +    S3Adsp1800MachineState *ms = PETALOGIX_S3ADSP1800_MACHINE(obj);
>>> +    ms->endianness = endianness;
>>> +}
>>> +
>>>   static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
>>>                                                       const void *data)
>>>   {
>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>> +    ObjectProperty *prop;
>>>       mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>>>       mc->init = petalogix_s3adsp1800_init;
>>>       mc->is_default = true;
>>> +
>>> +    prop = object_class_property_add_enum(oc, "endianness", "EndianMode",
>>> +                                          &EndianMode_lookup,
>>> +                                          machine_get_endianness,
>>> +                                          machine_set_endianness);
>>> +    object_property_set_default_str(prop, TARGET_BIG_ENDIAN ? "big" : "little");
>>> +    object_class_property_set_description(oc, "endianness",
>>> +            "Defines whether the machine runs in big or little endian mode");
>> Better with Property?  You don't have to write get/set...
>>    static const Property props[] = {
>>      DEFINE_PROP_ENDIAN("endianness", S3Adsp1800MachineState, endianness,
>>                         TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG : ENDIAN_MODE_LITTLE),
>>    };
>>    device_class_set_props(dc, props);
>
> DEFINE_PROP_FOO() are restricted to QDev (DeviceClass). Here we have
> a MachineClass, which only inherits ObjectClass, not DeviceClass.
>
> Markus once explained me the difference between QDev properties
> and bare object ones; I asked why we couldn't make qdev properties
> generic to objects, but I don't remember the historical rationale.
> QDev predates QOM, QDev used static properties, QOM introduced
> dynamic ones? We definitively should document that...

Yes.

Qdev properties are defined in data at compile time, and are connected
to the device class.  Specifically, a DeviceClass has an array of
Property, where each element specifies a property of its DeviceState
instances.  The array never changes.

The DEFINE_PROP_FOO() macros expand into Property literals suitable for
the array.  Boilerplate is relatively light: no need to write setters or
getters unless you define a new FOO.

QOM properties are defined in code at runtime, and are connected to the
Object, i.e. the instance, not the class.  They should be created in
.instance_init(), but nothing prevents creation elsewhere.  Most of the
time, we create the exact same properties for all instances of an
ObjectClass, but not always.

Defining properties at runtime is more flexible than defining them in
data at compile time.  However:

* Static data is much easier to reason about than behavior of code.
  Qdev properties are statically known.  device_add FOO,help can
  reliably show them: it dumps the unchanging array.  QOM properties can
  be different each time you create an object.  device_add FOO,help
  needs to create a temporary instance, and dumps whatever properties
  this instance got.  The next instance may get different ones.

* The property descriptions are duplicated in every instance, which is a
  waste of space.  See "QOM class properties" below.

* The functions to create properties take getter and setter functions as
  arguments.  Functions you get to write basically for each property.
  Much more boilerplate than with qdev.

I challenged the need for this much flexibility at the time, without
success.  We actually use the flexibility only rarely.  I believe this
aspect of QOM's design was a mistake.

QOM actually has a second kind of property: QOM class properties are
also defined in code at runtime, but connected to the ObjectClass.  They
should be created in .class_init(), but nothing prevents creation
elsewhere.  Despite their name, they are properties of the instance,
just like ordinary QOM properties.  The difference is only the sharing.

Most properties should be class properties, but aren't.  This is because
class properties were added late, and are underdocumented.

Qdev is actually a leaky layer above QOM.  I became one when we rebased
it on top of QOM.

A qdev's .instance_init() iterates over the property array and adds a
QOM class property for each element[*].

This Property part of qdev isn't actually device-specific.  We could
lift it into Object.  But would that be an improvement?  I'm not sure;
QOM is confusing enough as it is.

>               We definitively should document that...

I know just enough about QOM to be dangerous :)



[*] It also adds a "legacy property", but I forgot what that is, and why
it exists.

Re: [PATCH 4/4] docs: Deprecate the qemu-system-microblazeel binary

[Philippe Mathieu-Daudé]

On 15/5/25 15:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The (former big-endian only) binary qemu-system-microblaze can
> handle both endiannesses nowadays, so we don't need the separate
> qemu-system-microblazeel binary for little endian anymore. Let's
> deprecate it to avoid unnecessary compilation and test time in
> the future.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/about/deprecated.rst | 13 +++++++++++++
>   1 file changed, 13 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/4] hw/microblaze: Remove the big-endian variants of ml605 and xlnx-zynqmp-pmu

[Philippe Mathieu-Daudé]

On 15/5/25 15:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Both machines were added with little-endian in mind only (the
> "endianness" CPU property was hard-wired to "true", see commits
> 133d23b3ad1 and a88bbb006a52), so the variants that showed up
> on the big endian target likely never worked. We deprecated these
> non-working machine variants two releases ago, and so far nobody
> complained, so it should be fine now to disable them. Hard-wire
> the machines to little endian now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/about/deprecated.rst           |  6 ------
>   docs/about/removed-features.rst     |  9 +++++++++
>   hw/microblaze/petalogix_ml605_mmu.c | 15 ++++-----------
>   hw/microblaze/xlnx-zynqmp-pmu.c     |  7 +------
>   4 files changed, 14 insertions(+), 23 deletions(-)

\o/ :~)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 2/4] tests/functional: Test both microblaze s3adsp1800 endianness variants

[Philippe Mathieu-Daudé]

On 15/5/25 15:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Now that the endianness of the petalogix-s3adsp1800 can be configured,
> we should test that the cross-endianness also works as expected, thus
> test the big endian variant on the little endian target and vice versa.
> (based on an original idea from Philippe Mathieu-Daudé)
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/test_microblaze_s3adsp1800.py  | 17 ++++++++++++-----
>   .../functional/test_microblazeel_s3adsp1800.py  |  5 ++++-
>   2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
> index c93fa14232b..c5e60b555c6 100755
> --- a/tests/functional/test_microblaze_s3adsp1800.py
> +++ b/tests/functional/test_microblaze_s3adsp1800.py
> @@ -25,12 +25,14 @@ class MicroblazeMachine(QemuSystemTest):
>           ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
>           'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
>   
> -    def do_ballerina_be_test(self, machine):
> -        self.set_machine(machine)
> +    def do_ballerina_be_test(self, force_endianness=False):
> +        self.set_machine('petalogix-s3adsp1800')
>           self.archive_extract(self.ASSET_IMAGE_BE)
>           self.vm.set_console()
>           self.vm.add_args('-kernel',
>                            self.scratch_file('day17', 'ballerina.bin'))
> +        if force_endianness:
> +            self.vm.add_args('-M', 'endianness=big')

This 'force_endianness' is a bit confusing, I'd simply always set
the endianness, anyhow:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/4] hw/microblaze: Add endianness property to the petalogix_s3adsp1800 machine

[Philippe Mathieu-Daudé]

On 15/5/25 15:20, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Since the microblaze target can now handle both endianness, big and
> little, we should provide a config knob for the user to select the
> desired endianness.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c | 41 +++++++++++++++++++++---
>   1 file changed, 36 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>