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

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

[Philippe Mathieu-Daudé]

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)

Deprecate untested big-endian machines, likely build on the big
endian binary by mistake:
- petalogix-ml605
- xlnx-zynqmp-pmu

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).

Note, since vCPU can run in any endianness, the
MemoryRegionOps::endianness should not be DEVICE_NATIVE_ENDIAN
anymore, because this definition expand to the binary endianness,
swapping data regardless how the vcpu access it.
See adjust_endianness() -> devend_memop(). Something to keep in
mind, possibly requiring further work and optimizations (avoid
double-swap).

Next step: Look at unifying binaries.

Please review,

Phil.

Philippe Mathieu-Daudé (19):
  target/microblaze: Rename CPU endianness property as 'little-endian'
  hw/microblaze: Deprecate big-endian petalogix-ml605 & xlnx-zynqmp-pmu
  hw/microblaze/s3adsp1800: Explicit CPU endianness
  hw/microblaze/s3adsp1800: Rename unimplemented MMIO region as xps_gpio
  hw/microblaze/s3adsp1800: Declare machine type using DEFINE_TYPES
    macro
  hw/microblaze: Fix MemoryRegionOps coding style
  hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
  hw/microblaze: Propagate CPU endianness to microblaze_load_kernel()
  hw/intc/xilinx_intc: Only expect big-endian accesses
  hw/timer/xilinx_timer: Only expect big-endian accesses
  hw/timer/xilinx_timer: Allow down to 8-bit memory access
  hw/net/xilinx_ethlite: Only expect big-endian accesses
  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: Add microblaze cross-endianness tests

 docs/about/deprecated.rst                     |  6 ++
 .../devices/microblaze-softmmu/default.mak    |  2 -
 .../devices/microblazeel-softmmu/default.mak  |  5 +-
 hw/microblaze/boot.h                          |  4 +-
 target/microblaze/cpu.h                       |  7 ++
 hw/char/xilinx_uartlite.c                     |  8 ++-
 hw/intc/xilinx_intc.c                         | 23 +++++--
 hw/microblaze/boot.c                          |  8 +--
 hw/microblaze/petalogix_ml605_mmu.c           | 11 ++-
 hw/microblaze/petalogix_s3adsp1800_mmu.c      | 67 +++++++++++++++++--
 hw/microblaze/xlnx-zynqmp-pmu.c               | 12 ++--
 hw/net/xilinx_ethlite.c                       | 28 ++++++--
 hw/timer/xilinx_timer.c                       | 15 +++--
 target/microblaze/cpu.c                       |  2 +-
 target/microblaze/translate.c                 | 49 ++++++++------
 .../functional/test_microblaze_s3adsp1800.py  | 27 +++++++-
 .../test_microblazeel_s3adsp1800.py           | 25 ++++++-
 17 files changed, 236 insertions(+), 63 deletions(-)

-- 
2.45.2

[PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Philippe Mathieu-Daudé]

Rename the 'endian' property as 'little-endian' because the 'ENDI'
bit is set when the endianness is in little order, and unset in
big order.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_ml605_mmu.c | 2 +-
 hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
 target/microblaze/cpu.c             | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index b4183c5267d..df808ac323e 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -90,7 +90,7 @@ petalogix_ml605_init(MachineState *machine)
     object_property_set_int(OBJECT(cpu), "use-fpu", 1, &error_abort);
     object_property_set_bool(OBJECT(cpu), "dcache-writeback", true,
                              &error_abort);
-    object_property_set_bool(OBJECT(cpu), "endianness", true, &error_abort);
+    object_property_set_bool(OBJECT(cpu), "little-endian", true, &error_abort);
     qdev_realize(DEVICE(cpu), NULL, &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 1bfc9641d29..43608c2dca4 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -90,7 +90,7 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
     object_property_set_bool(OBJECT(&s->cpu), "use-pcmp-instr", true,
                              &error_abort);
     object_property_set_bool(OBJECT(&s->cpu), "use-mmu", false, &error_abort);
-    object_property_set_bool(OBJECT(&s->cpu), "endianness", true,
+    object_property_set_bool(OBJECT(&s->cpu), "little-endian", true,
                              &error_abort);
     object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",
                             &error_abort);
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 135947ee800..e9f98806274 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -368,7 +368,7 @@ static Property mb_properties[] = {
     DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, cfg.use_non_secure, 0),
     DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, cfg.dcache_writeback,
                      false),
-    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
+    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
     /* Enables bus exceptions on failed data accesses (load/stores).  */
     DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
                      cfg.dopb_bus_exception, false),
-- 
2.45.2

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Rename the 'endian' property as 'little-endian' because the 'ENDI'
> bit is set when the endianness is in little order, and unset in
> big order.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_ml605_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
>  target/microblaze/cpu.c             | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:06 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Rename the 'endian' property as 'little-endian' because the 'ENDI'
> bit is set when the endianness is in little order, and unset in
> big order.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/microblaze/petalogix_ml605_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
>  target/microblaze/cpu.c             | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index b4183c5267d..df808ac323e 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -90,7 +90,7 @@ petalogix_ml605_init(MachineState *machine)
>      object_property_set_int(OBJECT(cpu), "use-fpu", 1, &error_abort);
>      object_property_set_bool(OBJECT(cpu), "dcache-writeback", true,
>                               &error_abort);
> -    object_property_set_bool(OBJECT(cpu), "endianness", true, &error_abort);
> +    object_property_set_bool(OBJECT(cpu), "little-endian", true, &error_abort);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>
>      /* Attach emulated BRAM through the LMB.  */
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 1bfc9641d29..43608c2dca4 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -90,7 +90,7 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
>      object_property_set_bool(OBJECT(&s->cpu), "use-pcmp-instr", true,
>                               &error_abort);
>      object_property_set_bool(OBJECT(&s->cpu), "use-mmu", false, &error_abort);
> -    object_property_set_bool(OBJECT(&s->cpu), "endianness", true,
> +    object_property_set_bool(OBJECT(&s->cpu), "little-endian", true,
>                               &error_abort);
>      object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",
>                              &error_abort);
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 135947ee800..e9f98806274 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -368,7 +368,7 @@ static Property mb_properties[] = {
>      DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, cfg.use_non_secure, 0),
>      DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, cfg.dcache_writeback,
>                       false),
> -    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
> +    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
>      /* Enables bus exceptions on failed data accesses (load/stores).  */
>      DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
>                       cfg.dopb_bus_exception, false),
> --
> 2.45.2
>
>

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:13PM +0100, Philippe Mathieu-Daudé wrote:
> Rename the 'endian' property as 'little-endian' because the 'ENDI'
> bit is set when the endianness is in little order, and unset in
> big order.

Hi Phil,

Unfortunately, these properties are not only QEMU internal these got named
from the bindings Xilinx choose way back in time.

This will likely break many of the Xilinx flows with automatic dts to
qemu property conversions so I don't think it's a good idea to rename it.
If you like to clarify things perhaps we could keep an alias for the old
one?

For example:
https://github.com/torvalds/linux/blob/master/arch/microblaze/boot/dts/system.dts#L73

Cheers,
Edgar


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_ml605_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
>  target/microblaze/cpu.c             | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index b4183c5267d..df808ac323e 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -90,7 +90,7 @@ petalogix_ml605_init(MachineState *machine)
>      object_property_set_int(OBJECT(cpu), "use-fpu", 1, &error_abort);
>      object_property_set_bool(OBJECT(cpu), "dcache-writeback", true,
>                               &error_abort);
> -    object_property_set_bool(OBJECT(cpu), "endianness", true, &error_abort);
> +    object_property_set_bool(OBJECT(cpu), "little-endian", true, &error_abort);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>  
>      /* Attach emulated BRAM through the LMB.  */
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 1bfc9641d29..43608c2dca4 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -90,7 +90,7 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
>      object_property_set_bool(OBJECT(&s->cpu), "use-pcmp-instr", true,
>                               &error_abort);
>      object_property_set_bool(OBJECT(&s->cpu), "use-mmu", false, &error_abort);
> -    object_property_set_bool(OBJECT(&s->cpu), "endianness", true,
> +    object_property_set_bool(OBJECT(&s->cpu), "little-endian", true,
>                               &error_abort);
>      object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",
>                              &error_abort);
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 135947ee800..e9f98806274 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -368,7 +368,7 @@ static Property mb_properties[] = {
>      DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, cfg.use_non_secure, 0),
>      DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, cfg.dcache_writeback,
>                       false),
> -    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
> +    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
>      /* Enables bus exceptions on failed data accesses (load/stores).  */
>      DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
>                       cfg.dopb_bus_exception, false),
> -- 
> 2.45.2
> 

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Philippe Mathieu-Daudé]

Hi Edgar,

On 5/11/24 23:54, Edgar E. Iglesias wrote:
> On Tue, Nov 05, 2024 at 02:04:13PM +0100, Philippe Mathieu-Daudé wrote:
>> Rename the 'endian' property as 'little-endian' because the 'ENDI'
>> bit is set when the endianness is in little order, and unset in
>> big order.
> 
> Hi Phil,
> 
> Unfortunately, these properties are not only QEMU internal these got named
> from the bindings Xilinx choose way back in time.
> 
> This will likely break many of the Xilinx flows with automatic dts to
> qemu property conversions so I don't think it's a good idea to rename it.
> If you like to clarify things perhaps we could keep an alias for the old
> one?

Adding an alias is the safest way, I'll respin this patch.

Note however I'm worried about this fragile disconnect between Xilinx
dts conversion which isn't exercised on mainstream (in particular if
you get busy and can't review).

> 
> For example:
> https://github.com/torvalds/linux/blob/master/arch/microblaze/boot/dts/system.dts#L73
> 
> Cheers,
> Edgar
> 
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/microblaze/petalogix_ml605_mmu.c | 2 +-
>>   hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
>>   target/microblaze/cpu.c             | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
>> index b4183c5267d..df808ac323e 100644
>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>> @@ -90,7 +90,7 @@ petalogix_ml605_init(MachineState *machine)
>>       object_property_set_int(OBJECT(cpu), "use-fpu", 1, &error_abort);
>>       object_property_set_bool(OBJECT(cpu), "dcache-writeback", true,
>>                                &error_abort);
>> -    object_property_set_bool(OBJECT(cpu), "endianness", true, &error_abort);
>> +    object_property_set_bool(OBJECT(cpu), "little-endian", true, &error_abort);
>>       qdev_realize(DEVICE(cpu), NULL, &error_abort);
>>   
>>       /* Attach emulated BRAM through the LMB.  */
>> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
>> index 1bfc9641d29..43608c2dca4 100644
>> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
>> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
>> @@ -90,7 +90,7 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
>>       object_property_set_bool(OBJECT(&s->cpu), "use-pcmp-instr", true,
>>                                &error_abort);
>>       object_property_set_bool(OBJECT(&s->cpu), "use-mmu", false, &error_abort);
>> -    object_property_set_bool(OBJECT(&s->cpu), "endianness", true,
>> +    object_property_set_bool(OBJECT(&s->cpu), "little-endian", true,
>>                                &error_abort);
>>       object_property_set_str(OBJECT(&s->cpu), "version", "8.40.b",
>>                               &error_abort);
>> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
>> index 135947ee800..e9f98806274 100644
>> --- a/target/microblaze/cpu.c
>> +++ b/target/microblaze/cpu.c
>> @@ -368,7 +368,7 @@ static Property mb_properties[] = {
>>       DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, cfg.use_non_secure, 0),
>>       DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, cfg.dcache_writeback,
>>                        false),
>> -    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
>> +    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
>>       /* Enables bus exceptions on failed data accesses (load/stores).  */
>>       DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
>>                        cfg.dopb_bus_exception, false),
>> -- 
>> 2.45.2
>>

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Philippe Mathieu-Daudé]

On 5/11/24 23:01, Philippe Mathieu-Daudé wrote:
> Hi Edgar,
> 
> On 5/11/24 23:54, Edgar E. Iglesias wrote:
>> On Tue, Nov 05, 2024 at 02:04:13PM +0100, Philippe Mathieu-Daudé wrote:
>>> Rename the 'endian' property as 'little-endian' because the 'ENDI'
>>> bit is set when the endianness is in little order, and unset in
>>> big order.
>>
>> Hi Phil,
>>
>> Unfortunately, these properties are not only QEMU internal these got 
>> named
>> from the bindings Xilinx choose way back in time.
>>
>> This will likely break many of the Xilinx flows with automatic dts to
>> qemu property conversions so I don't think it's a good idea to rename it.
>> If you like to clarify things perhaps we could keep an alias for the old
>> one?
> 
> Adding an alias is the safest way, I'll respin this patch.
> 
> Note however I'm worried about this fragile disconnect between Xilinx
> dts conversion which isn't exercised on mainstream (in particular if
> you get busy and can't review).
> 
>>
>> For example:
>> https://github.com/torvalds/linux/blob/master/arch/microblaze/boot/dts/system.dts#L73
>>
>> Cheers,
>> Edgar
>>
>>
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/microblaze/petalogix_ml605_mmu.c | 2 +-
>>>   hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
>>>   target/microblaze/cpu.c             | 2 +-
>>>   3 files changed, 3 insertions(+), 3 deletions(-)


>>> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
>>> index 135947ee800..e9f98806274 100644
>>> --- a/target/microblaze/cpu.c
>>> +++ b/target/microblaze/cpu.c
>>> @@ -368,7 +368,7 @@ static Property mb_properties[] = {
>>>       DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, 
>>> cfg.use_non_secure, 0),
>>>       DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, 
>>> cfg.dcache_writeback,
>>>                        false),
>>> -    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
>>> +    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
>>>       /* Enables bus exceptions on failed data accesses 
>>> (load/stores).  */
>>>       DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
>>>                        cfg.dopb_bus_exception, false),
>>> -- 

OK if I squash the following?

-- >8 --
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index e9f98806274..b322f060777 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -328,9 +328,16 @@ static void mb_cpu_initfn(Object *obj)
      qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_dc, 
"ns_axi_dc", 1);
      qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_ic, 
"ns_axi_ic", 1);
  #endif
+
+    /* Restricted 'endianness' property is equivalent of 'little-endian' */
+    object_property_add_alias(obj, "little-endian", obj, "endianness");
  }

  static Property mb_properties[] = {
+    /*
+     * Following properties are used by Xilinx DTS conversion tool
+     * do not rename them.
+     */
      DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, 
cfg.base_vectors, 0),
      DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
                       false),
@@ -368,7 +375,7 @@ static Property mb_properties[] = {
      DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, 
cfg.use_non_secure, 0),
      DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU, 
cfg.dcache_writeback,
                       false),
-    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
+    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
      /* Enables bus exceptions on failed data accesses (load/stores).  */
      DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
                       cfg.dopb_bus_exception, false),
@@ -387,6 +394,9 @@ static Property mb_properties[] = {
      DEFINE_PROP_UINT8("pvr", MicroBlazeCPU, cfg.pvr, C_PVR_FULL),
      DEFINE_PROP_UINT8("pvr-user1", MicroBlazeCPU, cfg.pvr_user1, 0),
      DEFINE_PROP_UINT32("pvr-user2", MicroBlazeCPU, cfg.pvr_user2, 0),
+    /*
+     * End of properties reserved by Xilinx DTS conversion tool.
+     */
      DEFINE_PROP_END_OF_LIST(),
  };

---

Re: [PATCH 01/19] target/microblaze: Rename CPU endianness property as 'little-endian'

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 11:18:31PM +0000, Philippe Mathieu-Daudé wrote:
> On 5/11/24 23:01, Philippe Mathieu-Daudé wrote:
> > Hi Edgar,
> > 
> > On 5/11/24 23:54, Edgar E. Iglesias wrote:
> > > On Tue, Nov 05, 2024 at 02:04:13PM +0100, Philippe Mathieu-Daudé wrote:
> > > > Rename the 'endian' property as 'little-endian' because the 'ENDI'
> > > > bit is set when the endianness is in little order, and unset in
> > > > big order.
> > > 
> > > Hi Phil,
> > > 
> > > Unfortunately, these properties are not only QEMU internal these got
> > > named
> > > from the bindings Xilinx choose way back in time.
> > > 
> > > This will likely break many of the Xilinx flows with automatic dts to
> > > qemu property conversions so I don't think it's a good idea to rename it.
> > > If you like to clarify things perhaps we could keep an alias for the old
> > > one?
> > 
> > Adding an alias is the safest way, I'll respin this patch.
> > 
> > Note however I'm worried about this fragile disconnect between Xilinx
> > dts conversion which isn't exercised on mainstream (in particular if
> > you get busy and can't review).
> > 
> > > 
> > > For example:
> > > https://github.com/torvalds/linux/blob/master/arch/microblaze/boot/dts/system.dts#L73
> > > 
> > > Cheers,
> > > Edgar
> > > 
> > > 
> > > > 
> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > ---
> > > >   hw/microblaze/petalogix_ml605_mmu.c | 2 +-
> > > >   hw/microblaze/xlnx-zynqmp-pmu.c     | 2 +-
> > > >   target/microblaze/cpu.c             | 2 +-
> > > >   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> 
> > > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > > > index 135947ee800..e9f98806274 100644
> > > > --- a/target/microblaze/cpu.c
> > > > +++ b/target/microblaze/cpu.c
> > > > @@ -368,7 +368,7 @@ static Property mb_properties[] = {
> > > >       DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU,
> > > > cfg.use_non_secure, 0),
> > > >       DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU,
> > > > cfg.dcache_writeback,
> > > >                        false),
> > > > -    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
> > > > +    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
> > > >       /* Enables bus exceptions on failed data accesses
> > > > (load/stores).  */
> > > >       DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
> > > >                        cfg.dopb_bus_exception, false),
> > > > -- 
> 
> OK if I squash the following?


Looks good!
Thanks!

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>




> 
> -- >8 --
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index e9f98806274..b322f060777 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -328,9 +328,16 @@ static void mb_cpu_initfn(Object *obj)
>      qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_dc, "ns_axi_dc", 1);
>      qdev_init_gpio_in_named(DEVICE(cpu), mb_cpu_ns_axi_ic, "ns_axi_ic", 1);
>  #endif
> +
> +    /* Restricted 'endianness' property is equivalent of 'little-endian' */
> +    object_property_add_alias(obj, "little-endian", obj, "endianness");
>  }
> 
>  static Property mb_properties[] = {
> +    /*
> +     * Following properties are used by Xilinx DTS conversion tool
> +     * do not rename them.
> +     */
>      DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
>      DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
>                       false),
> @@ -368,7 +375,7 @@ static Property mb_properties[] = {
>      DEFINE_PROP_UINT8("use-non-secure", MicroBlazeCPU, cfg.use_non_secure,
> 0),
>      DEFINE_PROP_BOOL("dcache-writeback", MicroBlazeCPU,
> cfg.dcache_writeback,
>                       false),
> -    DEFINE_PROP_BOOL("little-endian", MicroBlazeCPU, cfg.endi, false),
> +    DEFINE_PROP_BOOL("endianness", MicroBlazeCPU, cfg.endi, false),
>      /* Enables bus exceptions on failed data accesses (load/stores).  */
>      DEFINE_PROP_BOOL("dopb-bus-exception", MicroBlazeCPU,
>                       cfg.dopb_bus_exception, false),
> @@ -387,6 +394,9 @@ static Property mb_properties[] = {
>      DEFINE_PROP_UINT8("pvr", MicroBlazeCPU, cfg.pvr, C_PVR_FULL),
>      DEFINE_PROP_UINT8("pvr-user1", MicroBlazeCPU, cfg.pvr_user1, 0),
>      DEFINE_PROP_UINT32("pvr-user2", MicroBlazeCPU, cfg.pvr_user2, 0),
> +    /*
> +     * End of properties reserved by Xilinx DTS conversion tool.
> +     */
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> ---

[PATCH 02/19] hw/microblaze: Deprecate big-endian petalogix-ml605 & xlnx-zynqmp-pmu

[Philippe Mathieu-Daudé]

The petalogix-ml605 machine was explicitly added as little-endian only
machine in commit 00914b7d970 ("microblaze: Add PetaLogix ml605 MMU
little-endian ref design"). Mark the big-endian version as deprecated.

When the xlnx-zynqmp-pmu machine's CPU was added in commit 133d23b3ad1
("xlnx-zynqmp-pmu: Add the CPU and memory"), its 'endianness' property
was set to %true, thus wired in little endianness.

Both machine are included in the big-endian system binary, while their
CPU is working in little-endian. Unlikely to work as it. Deprecate now
as broken config so we can remove soon.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/about/deprecated.rst                        | 6 ++++++
 configs/devices/microblaze-softmmu/default.mak   | 2 --
 configs/devices/microblazeel-softmmu/default.mak | 5 ++++-
 hw/microblaze/petalogix_ml605_mmu.c              | 7 ++++++-
 hw/microblaze/xlnx-zynqmp-pmu.c                  | 8 ++++++--
 5 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ff404d44f85..e1c8829e1a4 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -279,6 +279,12 @@ BMC and a witherspoon like OpenPOWER system. It was used for bring up
 of the AST2600 SoC in labs.  It can be easily replaced by the
 ``rainier-bmc`` machine which is a real product.
 
+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.
+
 Backend options
 ---------------
 
diff --git a/configs/devices/microblaze-softmmu/default.mak b/configs/devices/microblaze-softmmu/default.mak
index 583e3959bb7..78941064655 100644
--- a/configs/devices/microblaze-softmmu/default.mak
+++ b/configs/devices/microblaze-softmmu/default.mak
@@ -2,5 +2,3 @@
 
 # Boards are selected by default, uncomment to keep out of the build.
 # CONFIG_PETALOGIX_S3ADSP1800=n
-# CONFIG_PETALOGIX_ML605=n
-# CONFIG_XLNX_ZYNQMP_PMU=n
diff --git a/configs/devices/microblazeel-softmmu/default.mak b/configs/devices/microblazeel-softmmu/default.mak
index 29f7f13816c..4c1086435bf 100644
--- a/configs/devices/microblazeel-softmmu/default.mak
+++ b/configs/devices/microblazeel-softmmu/default.mak
@@ -1,3 +1,6 @@
 # Default configuration for microblazeel-softmmu
 
-include ../microblaze-softmmu/default.mak
+# Boards are selected by default, uncomment to keep out of the build.
+# CONFIG_PETALOGIX_S3ADSP1800=n
+# CONFIG_PETALOGIX_ML605=n
+# CONFIG_XLNX_ZYNQMP_PMU=n
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index df808ac323e..61e47d83988 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -213,7 +213,12 @@ petalogix_ml605_init(MachineState *machine)
 
 static void petalogix_ml605_machine_init(MachineClass *mc)
 {
-    mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian";
+#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)";
+#endif
     mc->init = petalogix_ml605_init;
 }
 
diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 43608c2dca4..567aad47bfc 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -181,9 +181,13 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 
 static void xlnx_zynqmp_pmu_machine_init(MachineClass *mc)
 {
-    mc->desc = "Xilinx ZynqMP PMU machine";
+#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)";
+#endif
     mc->init = xlnx_zynqmp_pmu_init;
 }
 
 DEFINE_MACHINE("xlnx-zynqmp-pmu", xlnx_zynqmp_pmu_machine_init)
-
-- 
2.45.2

Re: [PATCH 02/19] hw/microblaze: Deprecate big-endian petalogix-ml605 & xlnx-zynqmp-pmu

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> The petalogix-ml605 machine was explicitly added as little-endian only
> machine in commit 00914b7d970 ("microblaze: Add PetaLogix ml605 MMU
> little-endian ref design"). Mark the big-endian version as deprecated.
> 
> When the xlnx-zynqmp-pmu machine's CPU was added in commit 133d23b3ad1
> ("xlnx-zynqmp-pmu: Add the CPU and memory"), its 'endianness' property
> was set to %true, thus wired in little endianness.
> 
> Both machine are included in the big-endian system binary, while their
> CPU is working in little-endian. Unlikely to work as it. Deprecate now
> as broken config so we can remove soon.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  docs/about/deprecated.rst                        | 6 ++++++
>  configs/devices/microblaze-softmmu/default.mak   | 2 --
>  configs/devices/microblazeel-softmmu/default.mak | 5 ++++-
>  hw/microblaze/petalogix_ml605_mmu.c              | 7 ++++++-
>  hw/microblaze/xlnx-zynqmp-pmu.c                  | 8 ++++++--
>  5 files changed, 22 insertions(+), 6 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 02/19] hw/microblaze: Deprecate big-endian petalogix-ml605 & xlnx-zynqmp-pmu

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:06 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> The petalogix-ml605 machine was explicitly added as little-endian only
> machine in commit 00914b7d970 ("microblaze: Add PetaLogix ml605 MMU
> little-endian ref design"). Mark the big-endian version as deprecated.
>
> When the xlnx-zynqmp-pmu machine's CPU was added in commit 133d23b3ad1
> ("xlnx-zynqmp-pmu: Add the CPU and memory"), its 'endianness' property
> was set to %true, thus wired in little endianness.
>
> Both machine are included in the big-endian system binary, while their
> CPU is working in little-endian. Unlikely to work as it. Deprecate now
> as broken config so we can remove soon.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  docs/about/deprecated.rst                        | 6 ++++++
>  configs/devices/microblaze-softmmu/default.mak   | 2 --
>  configs/devices/microblazeel-softmmu/default.mak | 5 ++++-
>  hw/microblaze/petalogix_ml605_mmu.c              | 7 ++++++-
>  hw/microblaze/xlnx-zynqmp-pmu.c                  | 8 ++++++--
>  5 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ff404d44f85..e1c8829e1a4 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -279,6 +279,12 @@ BMC and a witherspoon like OpenPOWER system. It was used for bring up
>  of the AST2600 SoC in labs.  It can be easily replaced by the
>  ``rainier-bmc`` machine which is a real product.
>
> +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.
> +
>  Backend options
>  ---------------
>
> diff --git a/configs/devices/microblaze-softmmu/default.mak b/configs/devices/microblaze-softmmu/default.mak
> index 583e3959bb7..78941064655 100644
> --- a/configs/devices/microblaze-softmmu/default.mak
> +++ b/configs/devices/microblaze-softmmu/default.mak
> @@ -2,5 +2,3 @@
>
>  # Boards are selected by default, uncomment to keep out of the build.
>  # CONFIG_PETALOGIX_S3ADSP1800=n
> -# CONFIG_PETALOGIX_ML605=n
> -# CONFIG_XLNX_ZYNQMP_PMU=n
> diff --git a/configs/devices/microblazeel-softmmu/default.mak b/configs/devices/microblazeel-softmmu/default.mak
> index 29f7f13816c..4c1086435bf 100644
> --- a/configs/devices/microblazeel-softmmu/default.mak
> +++ b/configs/devices/microblazeel-softmmu/default.mak
> @@ -1,3 +1,6 @@
>  # Default configuration for microblazeel-softmmu
>
> -include ../microblaze-softmmu/default.mak
> +# Boards are selected by default, uncomment to keep out of the build.
> +# CONFIG_PETALOGIX_S3ADSP1800=n
> +# CONFIG_PETALOGIX_ML605=n
> +# CONFIG_XLNX_ZYNQMP_PMU=n
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index df808ac323e..61e47d83988 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -213,7 +213,12 @@ petalogix_ml605_init(MachineState *machine)
>
>  static void petalogix_ml605_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian";
> +#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)";
> +#endif
>      mc->init = petalogix_ml605_init;
>  }
>
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 43608c2dca4..567aad47bfc 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -181,9 +181,13 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
>
>  static void xlnx_zynqmp_pmu_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "Xilinx ZynqMP PMU machine";
> +#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)";
> +#endif
>      mc->init = xlnx_zynqmp_pmu_init;
>  }
>
>  DEFINE_MACHINE("xlnx-zynqmp-pmu", xlnx_zynqmp_pmu_machine_init)
> -
> --
> 2.45.2
>
>

Re: [PATCH 02/19] hw/microblaze: Deprecate big-endian petalogix-ml605 & xlnx-zynqmp-pmu

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:14PM +0100, Philippe Mathieu-Daudé wrote:
> The petalogix-ml605 machine was explicitly added as little-endian only
> machine in commit 00914b7d970 ("microblaze: Add PetaLogix ml605 MMU
> little-endian ref design"). Mark the big-endian version as deprecated.
> 
> When the xlnx-zynqmp-pmu machine's CPU was added in commit 133d23b3ad1
> ("xlnx-zynqmp-pmu: Add the CPU and memory"), its 'endianness' property
> was set to %true, thus wired in little endianness.
> 
> Both machine are included in the big-endian system binary, while their
> CPU is working in little-endian. Unlikely to work as it. Deprecate now
> as broken config so we can remove soon.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  docs/about/deprecated.rst                        | 6 ++++++
>  configs/devices/microblaze-softmmu/default.mak   | 2 --
>  configs/devices/microblazeel-softmmu/default.mak | 5 ++++-
>  hw/microblaze/petalogix_ml605_mmu.c              | 7 ++++++-
>  hw/microblaze/xlnx-zynqmp-pmu.c                  | 8 ++++++--
>  5 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ff404d44f85..e1c8829e1a4 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -279,6 +279,12 @@ BMC and a witherspoon like OpenPOWER system. It was used for bring up
>  of the AST2600 SoC in labs.  It can be easily replaced by the
>  ``rainier-bmc`` machine which is a real product.
>  
> +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.
> +
>  Backend options
>  ---------------
>  
> diff --git a/configs/devices/microblaze-softmmu/default.mak b/configs/devices/microblaze-softmmu/default.mak
> index 583e3959bb7..78941064655 100644
> --- a/configs/devices/microblaze-softmmu/default.mak
> +++ b/configs/devices/microblaze-softmmu/default.mak
> @@ -2,5 +2,3 @@
>  
>  # Boards are selected by default, uncomment to keep out of the build.
>  # CONFIG_PETALOGIX_S3ADSP1800=n
> -# CONFIG_PETALOGIX_ML605=n
> -# CONFIG_XLNX_ZYNQMP_PMU=n
> diff --git a/configs/devices/microblazeel-softmmu/default.mak b/configs/devices/microblazeel-softmmu/default.mak
> index 29f7f13816c..4c1086435bf 100644
> --- a/configs/devices/microblazeel-softmmu/default.mak
> +++ b/configs/devices/microblazeel-softmmu/default.mak
> @@ -1,3 +1,6 @@
>  # Default configuration for microblazeel-softmmu
>  
> -include ../microblaze-softmmu/default.mak
> +# Boards are selected by default, uncomment to keep out of the build.
> +# CONFIG_PETALOGIX_S3ADSP1800=n
> +# CONFIG_PETALOGIX_ML605=n
> +# CONFIG_XLNX_ZYNQMP_PMU=n
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index df808ac323e..61e47d83988 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -213,7 +213,12 @@ petalogix_ml605_init(MachineState *machine)
>  
>  static void petalogix_ml605_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "PetaLogix linux refdesign for xilinx ml605 little endian";
> +#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)";
> +#endif
>      mc->init = petalogix_ml605_init;
>  }
>  
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 43608c2dca4..567aad47bfc 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -181,9 +181,13 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
>  
>  static void xlnx_zynqmp_pmu_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "Xilinx ZynqMP PMU machine";
> +#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)";
> +#endif
>      mc->init = xlnx_zynqmp_pmu_init;
>  }
>  
>  DEFINE_MACHINE("xlnx-zynqmp-pmu", xlnx_zynqmp_pmu_machine_init)
> -
> -- 
> 2.45.2
> 

[PATCH 03/19] hw/microblaze/s3adsp1800: Explicit CPU endianness

[Philippe Mathieu-Daudé]

By default the machine's CPU endianness is 'big' order
('little-endian' property set to %false).

This corresponds to the default when this machine was added;
see commits 6a8b1ae2020 "microblaze: Add petalogix s3a1800dsp
MMU linux ref-design." and 72b675caacf "microblaze: Hook into
the build-system." which added:

  [ "$target_cpu" = "microblaze" ] && target_bigendian=yes

Later commit 877fdc12b1a ("microblaze: Allow targeting
little-endian mb") added little-endian support, forgetting
to set the CPU endianness to little-endian. Not an issue
since this property was never used, but we will use it soon,
so explicit the endianness to get the expected behavior.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index dad46bd7f98..37e9a05a62a 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -71,6 +71,8 @@ petalogix_s3adsp1800_init(MachineState *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);
     qdev_realize(DEVICE(cpu), NULL, &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
-- 
2.45.2

Re: [PATCH 03/19] hw/microblaze/s3adsp1800: Explicit CPU endianness

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> By default the machine's CPU endianness is 'big' order
> ('little-endian' property set to %false).
> 
> This corresponds to the default when this machine was added;
> see commits 6a8b1ae2020 "microblaze: Add petalogix s3a1800dsp
> MMU linux ref-design." and 72b675caacf "microblaze: Hook into
> the build-system." which added:
> 
>    [ "$target_cpu" = "microblaze" ] && target_bigendian=yes
> 
> Later commit 877fdc12b1a ("microblaze: Allow targeting
> little-endian mb") added little-endian support, forgetting
> to set the CPU endianness to little-endian. Not an issue
> since this property was never used, but we will use it soon,
> so explicit the endianness to get the expected behavior.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 ++
>   1 file changed, 2 insertions(+)

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

r~

Re: [PATCH 03/19] hw/microblaze/s3adsp1800: Explicit CPU endianness

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:07 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> By default the machine's CPU endianness is 'big' order
> ('little-endian' property set to %false).
>
> This corresponds to the default when this machine was added;
> see commits 6a8b1ae2020 "microblaze: Add petalogix s3a1800dsp
> MMU linux ref-design." and 72b675caacf "microblaze: Hook into
> the build-system." which added:
>
>   [ "$target_cpu" = "microblaze" ] && target_bigendian=yes
>
> Later commit 877fdc12b1a ("microblaze: Allow targeting
> little-endian mb") added little-endian support, forgetting
> to set the CPU endianness to little-endian. Not an issue
> since this property was never used, but we will use it soon,
> so explicit the endianness to get the expected behavior.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index dad46bd7f98..37e9a05a62a 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -71,6 +71,8 @@ petalogix_s3adsp1800_init(MachineState *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);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>
>      /* Attach emulated BRAM through the LMB.  */
> --
> 2.45.2
>
>

Re: [PATCH 03/19] hw/microblaze/s3adsp1800: Explicit CPU endianness

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:15PM +0100, Philippe Mathieu-Daudé wrote:
> By default the machine's CPU endianness is 'big' order
> ('little-endian' property set to %false).
> 
> This corresponds to the default when this machine was added;
> see commits 6a8b1ae2020 "microblaze: Add petalogix s3a1800dsp
> MMU linux ref-design." and 72b675caacf "microblaze: Hook into
> the build-system." which added:
> 
>   [ "$target_cpu" = "microblaze" ] && target_bigendian=yes
> 
> Later commit 877fdc12b1a ("microblaze: Allow targeting
> little-endian mb") added little-endian support, forgetting
> to set the CPU endianness to little-endian. Not an issue
> since this property was never used, but we will use it soon,
> so explicit the endianness to get the expected behavior.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index dad46bd7f98..37e9a05a62a 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -71,6 +71,8 @@ petalogix_s3adsp1800_init(MachineState *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);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>  
>      /* Attach emulated BRAM through the LMB.  */
> -- 
> 2.45.2
> 

[PATCH 04/19] hw/microblaze/s3adsp1800: Rename unimplemented MMIO region as xps_gpio

[Philippe Mathieu-Daudé]

The machine datasheet mentions the GPIO device as 'xps_gpio'.
Rename it accordingly to easily find its documentation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 37e9a05a62a..581b0411e29 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -124,7 +124,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
 
-    create_unimplemented_device("gpio", GPIO_BASEADDR, 0x10000);
+    create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
 
     microblaze_load_kernel(cpu, ddr_base, ram_size,
                            machine->initrd_filename,
-- 
2.45.2

Re: [PATCH 04/19] hw/microblaze/s3adsp1800: Rename unimplemented MMIO region as xps_gpio

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> The machine datasheet mentions the GPIO device as 'xps_gpio'.
> Rename it accordingly to easily find its documentation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 04/19] hw/microblaze/s3adsp1800: Rename unimplemented MMIO region as xps_gpio

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:05 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> The machine datasheet mentions the GPIO device as 'xps_gpio'.
> Rename it accordingly to easily find its documentation.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 37e9a05a62a..581b0411e29 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -124,7 +124,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
>
> -    create_unimplemented_device("gpio", GPIO_BASEADDR, 0x10000);
> +    create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>
>      microblaze_load_kernel(cpu, ddr_base, ram_size,
>                             machine->initrd_filename,
> --
> 2.45.2
>
>

Re: [PATCH 04/19] hw/microblaze/s3adsp1800: Rename unimplemented MMIO region as xps_gpio

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:16PM +0100, Philippe Mathieu-Daudé wrote:
> The machine datasheet mentions the GPIO device as 'xps_gpio'.
> Rename it accordingly to easily find its documentation.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 37e9a05a62a..581b0411e29 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -124,7 +124,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
>  
> -    create_unimplemented_device("gpio", GPIO_BASEADDR, 0x10000);
> +    create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>  
>      microblaze_load_kernel(cpu, ddr_base, ram_size,
>                             machine->initrd_filename,
> -- 
> 2.45.2
> 

[PATCH 05/19] hw/microblaze/s3adsp1800: Declare machine type using DEFINE_TYPES macro

[Philippe Mathieu-Daudé]

Replace DEFINE_MACHINE() by DEFINE_TYPES(), converting the
class_init() handler.

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

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 581b0411e29..6c0f5c6c651 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -55,6 +55,9 @@
 #define ETHLITE_IRQ         1
 #define UARTLITE_IRQ        3
 
+#define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
+            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
+
 static void
 petalogix_s3adsp1800_init(MachineState *machine)
 {
@@ -132,11 +135,21 @@ petalogix_s3adsp1800_init(MachineState *machine)
                            NULL);
 }
 
-static void petalogix_s3adsp1800_machine_init(MachineClass *mc)
+static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
+
     mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
     mc->init = petalogix_s3adsp1800_init;
     mc->is_default = true;
 }
 
-DEFINE_MACHINE("petalogix-s3adsp1800", petalogix_s3adsp1800_machine_init)
+static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
+    {
+        .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
+        .parent         = TYPE_MACHINE,
+        .class_init     = petalogix_s3adsp1800_machine_class_init,
+    },
+};
+
+DEFINE_TYPES(petalogix_s3adsp1800_machine_types)
-- 
2.45.2

Re: [PATCH 05/19] hw/microblaze/s3adsp1800: Declare machine type using DEFINE_TYPES macro

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Replace DEFINE_MACHINE() by DEFINE_TYPES(), converting the
> class_init() handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 05/19] hw/microblaze/s3adsp1800: Declare machine type using DEFINE_TYPES macro

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:05 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Replace DEFINE_MACHINE() by DEFINE_TYPES(), converting the
> class_init() handler.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 581b0411e29..6c0f5c6c651 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -55,6 +55,9 @@
>  #define ETHLITE_IRQ         1
>  #define UARTLITE_IRQ        3
>
> +#define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
> +            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
> +
>  static void
>  petalogix_s3adsp1800_init(MachineState *machine)
>  {
> @@ -132,11 +135,21 @@ petalogix_s3adsp1800_init(MachineState *machine)
>                             NULL);
>  }
>
> -static void petalogix_s3adsp1800_machine_init(MachineClass *mc)
> +static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
>  {
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
>      mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>      mc->init = petalogix_s3adsp1800_init;
>      mc->is_default = true;
>  }
>
> -DEFINE_MACHINE("petalogix-s3adsp1800", petalogix_s3adsp1800_machine_init)
> +static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
> +    {
> +        .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .parent         = TYPE_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init,
> +    },
> +};
> +
> +DEFINE_TYPES(petalogix_s3adsp1800_machine_types)
> --
> 2.45.2
>
>

Re: [PATCH 05/19] hw/microblaze/s3adsp1800: Declare machine type using DEFINE_TYPES macro

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:17PM +0100, Philippe Mathieu-Daudé wrote:
> Replace DEFINE_MACHINE() by DEFINE_TYPES(), converting the
> class_init() handler.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 581b0411e29..6c0f5c6c651 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -55,6 +55,9 @@
>  #define ETHLITE_IRQ         1
>  #define UARTLITE_IRQ        3
>  
> +#define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
> +            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
> +
>  static void
>  petalogix_s3adsp1800_init(MachineState *machine)
>  {
> @@ -132,11 +135,21 @@ petalogix_s3adsp1800_init(MachineState *machine)
>                             NULL);
>  }
>  
> -static void petalogix_s3adsp1800_machine_init(MachineClass *mc)
> +static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
>  {
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
>      mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>      mc->init = petalogix_s3adsp1800_init;
>      mc->is_default = true;
>  }
>  
> -DEFINE_MACHINE("petalogix-s3adsp1800", petalogix_s3adsp1800_machine_init)
> +static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
> +    {
> +        .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .parent         = TYPE_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init,
> +    },
> +};
> +
> +DEFINE_TYPES(petalogix_s3adsp1800_machine_types)
> -- 
> 2.45.2
> 

[PATCH 06/19] hw/microblaze: Fix MemoryRegionOps coding style

[Philippe Mathieu-Daudé]

Fix few MemoryRegionOps style before adding new fields
in the following commits.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/xilinx_uartlite.c | 4 ++--
 hw/intc/xilinx_intc.c     | 4 ++--
 hw/net/xilinx_ethlite.c   | 4 ++--
 hw/timer/xilinx_timer.c   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index f325084f8b9..a69ad769cc4 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -172,8 +172,8 @@ static const MemoryRegionOps uart_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 1,
-        .max_access_size = 4
-    }
+        .max_access_size = 4,
+    },
 };
 
 static Property xilinx_uartlite_properties[] = {
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 6e5012e66eb..2b8246f6206 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -146,8 +146,8 @@ static const MemoryRegionOps pic_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 4,
-        .max_access_size = 4
-    }
+        .max_access_size = 4,
+    },
 };
 
 static void irq_handler(void *opaque, int irq, int level)
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index bd812908085..11eb53c4d60 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -172,8 +172,8 @@ static const MemoryRegionOps eth_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 4,
-        .max_access_size = 4
-    }
+        .max_access_size = 4,
+    },
 };
 
 static bool eth_can_rx(NetClientState *nc)
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 32a9df69e0b..0822345779c 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -195,8 +195,8 @@ static const MemoryRegionOps timer_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
         .min_access_size = 4,
-        .max_access_size = 4
-    }
+        .max_access_size = 4,
+    },
 };
 
 static void timer_hit(void *opaque)
-- 
2.45.2

Re: [PATCH 06/19] hw/microblaze: Fix MemoryRegionOps coding style

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> Fix few MemoryRegionOps style before adding new fields
> in the following commits.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/char/xilinx_uartlite.c | 4 ++--
>   hw/intc/xilinx_intc.c     | 4 ++--
>   hw/net/xilinx_ethlite.c   | 4 ++--
>   hw/timer/xilinx_timer.c   | 4 ++--
>   4 files changed, 8 insertions(+), 8 deletions(-)

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

r~

Re: [PATCH 06/19] hw/microblaze: Fix MemoryRegionOps coding style

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:07 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Fix few MemoryRegionOps style before adding new fields
> in the following commits.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/char/xilinx_uartlite.c | 4 ++--
>  hw/intc/xilinx_intc.c     | 4 ++--
>  hw/net/xilinx_ethlite.c   | 4 ++--
>  hw/timer/xilinx_timer.c   | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
> index f325084f8b9..a69ad769cc4 100644
> --- a/hw/char/xilinx_uartlite.c
> +++ b/hw/char/xilinx_uartlite.c
> @@ -172,8 +172,8 @@ static const MemoryRegionOps uart_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 1,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>
>  static Property xilinx_uartlite_properties[] = {
> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
> index 6e5012e66eb..2b8246f6206 100644
> --- a/hw/intc/xilinx_intc.c
> +++ b/hw/intc/xilinx_intc.c
> @@ -146,8 +146,8 @@ static const MemoryRegionOps pic_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>
>  static void irq_handler(void *opaque, int irq, int level)
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index bd812908085..11eb53c4d60 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -172,8 +172,8 @@ static const MemoryRegionOps eth_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>
>  static bool eth_can_rx(NetClientState *nc)
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 32a9df69e0b..0822345779c 100644
> --- a/hw/timer/xilinx_timer.c
> +++ b/hw/timer/xilinx_timer.c
> @@ -195,8 +195,8 @@ static const MemoryRegionOps timer_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>
>  static void timer_hit(void *opaque)
> --
> 2.45.2
>
>

Re: [PATCH 06/19] hw/microblaze: Fix MemoryRegionOps coding style

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:18PM +0100, Philippe Mathieu-Daudé wrote:
> Fix few MemoryRegionOps style before adding new fields
> in the following commits.

Wasn't aware of this style rule :-)

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/char/xilinx_uartlite.c | 4 ++--
>  hw/intc/xilinx_intc.c     | 4 ++--
>  hw/net/xilinx_ethlite.c   | 4 ++--
>  hw/timer/xilinx_timer.c   | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
> index f325084f8b9..a69ad769cc4 100644
> --- a/hw/char/xilinx_uartlite.c
> +++ b/hw/char/xilinx_uartlite.c
> @@ -172,8 +172,8 @@ static const MemoryRegionOps uart_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 1,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>  
>  static Property xilinx_uartlite_properties[] = {
> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
> index 6e5012e66eb..2b8246f6206 100644
> --- a/hw/intc/xilinx_intc.c
> +++ b/hw/intc/xilinx_intc.c
> @@ -146,8 +146,8 @@ static const MemoryRegionOps pic_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>  
>  static void irq_handler(void *opaque, int irq, int level)
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index bd812908085..11eb53c4d60 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -172,8 +172,8 @@ static const MemoryRegionOps eth_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>  
>  static bool eth_can_rx(NetClientState *nc)
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 32a9df69e0b..0822345779c 100644
> --- a/hw/timer/xilinx_timer.c
> +++ b/hw/timer/xilinx_timer.c
> @@ -195,8 +195,8 @@ static const MemoryRegionOps timer_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +        .max_access_size = 4,
> +    },
>  };
>  
>  static void timer_hit(void *opaque)
> -- 
> 2.45.2
> 

[PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Philippe Mathieu-Daudé]

All these MemoryRegionOps read() and write() handlers are
implemented expecting 32-bit accesses. Clarify that setting
.impl.min/max_access_size fields.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/xilinx_uartlite.c | 4 ++++
 hw/intc/xilinx_intc.c     | 4 ++++
 hw/net/xilinx_ethlite.c   | 4 ++++
 hw/timer/xilinx_timer.c   | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index a69ad769cc4..892efe81fee 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
     .read = uart_read,
     .write = uart_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 1,
         .max_access_size = 4,
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 2b8246f6206..1762b34564e 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -144,6 +144,10 @@ static const MemoryRegionOps pic_ops = {
     .read = pic_read,
     .write = pic_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 11eb53c4d60..ede7c172748 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps eth_ops = {
     .read = eth_read,
     .write = eth_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 0822345779c..28ac95edea1 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -193,6 +193,10 @@ static const MemoryRegionOps timer_ops = {
     .read = timer_read,
     .write = timer_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
-- 
2.45.2

Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> All these MemoryRegionOps read() and write() handlers are
> implemented expecting 32-bit accesses. Clarify that setting
> .impl.min/max_access_size fields.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/char/xilinx_uartlite.c | 4 ++++
>  hw/intc/xilinx_intc.c     | 4 ++++
>  hw/net/xilinx_ethlite.c   | 4 ++++
>  hw/timer/xilinx_timer.c   | 4 ++++
>  4 files changed, 16 insertions(+)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Philippe Mathieu-Daudé]

On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
> All these MemoryRegionOps read() and write() handlers are
> implemented expecting 32-bit accesses. Clarify that setting
> .impl.min/max_access_size fields.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/char/xilinx_uartlite.c | 4 ++++
>   hw/intc/xilinx_intc.c     | 4 ++++
>   hw/net/xilinx_ethlite.c   | 4 ++++
>   hw/timer/xilinx_timer.c   | 4 ++++
>   4 files changed, 16 insertions(+)
> 
> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
> index a69ad769cc4..892efe81fee 100644
> --- a/hw/char/xilinx_uartlite.c
> +++ b/hw/char/xilinx_uartlite.c
> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>       .read = uart_read,
>       .write = uart_write,
>       .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
>       .valid = {
>           .min_access_size = 1,
>           .max_access_size = 4,

To have qtests working I need to squash:

-- >8 --
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 3b92fa5d506..6d9291c8ae2 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
      0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
      0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
      0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
-    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
+    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
      0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
  };

@@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
      0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
      0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
      0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
-    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
+    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
      0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
  };
---

to access the uart by 32-bit instead of 8-bit.

Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Philippe Mathieu-Daudé]

On 5/11/24 23:24, Philippe Mathieu-Daudé wrote:
> On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
>> All these MemoryRegionOps read() and write() handlers are
>> implemented expecting 32-bit accesses. Clarify that setting
>> .impl.min/max_access_size fields.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/char/xilinx_uartlite.c | 4 ++++
>>   hw/intc/xilinx_intc.c     | 4 ++++
>>   hw/net/xilinx_ethlite.c   | 4 ++++
>>   hw/timer/xilinx_timer.c   | 4 ++++
>>   4 files changed, 16 insertions(+)
>>
>> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
>> index a69ad769cc4..892efe81fee 100644
>> --- a/hw/char/xilinx_uartlite.c
>> +++ b/hw/char/xilinx_uartlite.c
>> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>>       .read = uart_read,
>>       .write = uart_write,
>>       .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .impl = {
>> +        .min_access_size = 4,

Odd. The change makes the qtests pass, but here I'm modifying .impl,
not .valid... Since .valid.min_access_size = 1, SBI is a valid
opcode, no need to use SWI.

>> +        .max_access_size = 4,
>> +    },
>>       .valid = {
>>           .min_access_size = 1,
>>           .max_access_size = 4,
> 
> To have qtests working I need to squash:
> 
> -- >8 --
> diff --git a/tests/qtest/boot-serial-test.c 
> b/tests/qtest/boot-serial-test.c
> index 3b92fa5d506..6d9291c8ae2 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
>       0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
>       0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
>       0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
> -    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
> +    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
>       0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
>   };
> 
> @@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
>       0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
>       0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
>       0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
> -    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
> +    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
>       0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
>   };
> ---
> 
> to access the uart by 32-bit instead of 8-bit.

Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Philippe Mathieu-Daudé]

On 5/11/24 23:27, Philippe Mathieu-Daudé wrote:
> On 5/11/24 23:24, Philippe Mathieu-Daudé wrote:
>> On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
>>> All these MemoryRegionOps read() and write() handlers are
>>> implemented expecting 32-bit accesses. Clarify that setting
>>> .impl.min/max_access_size fields.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/char/xilinx_uartlite.c | 4 ++++
>>>   hw/intc/xilinx_intc.c     | 4 ++++
>>>   hw/net/xilinx_ethlite.c   | 4 ++++
>>>   hw/timer/xilinx_timer.c   | 4 ++++
>>>   4 files changed, 16 insertions(+)
>>>
>>> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
>>> index a69ad769cc4..892efe81fee 100644
>>> --- a/hw/char/xilinx_uartlite.c
>>> +++ b/hw/char/xilinx_uartlite.c
>>> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>>>       .read = uart_read,
>>>       .write = uart_write,
>>>       .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .impl = {
>>> +        .min_access_size = 4,
> 
> Odd. The change makes the qtests pass, but here I'm modifying .impl,
> not .valid... Since .valid.min_access_size = 1, SBI is a valid
> opcode, no need to use SWI.

Which proves this device is only mapped in little-endian.

> 
>>> +        .max_access_size = 4,
>>> +    },
>>>       .valid = {
>>>           .min_access_size = 1,
>>>           .max_access_size = 4,
>>
>> To have qtests working I need to squash:
>>
>> -- >8 --
>> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial- 
>> test.c
>> index 3b92fa5d506..6d9291c8ae2 100644
>> --- a/tests/qtest/boot-serial-test.c
>> +++ b/tests/qtest/boot-serial-test.c
>> @@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
>>       0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
>>       0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
>>       0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
>> -    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
>> +    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
>>       0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
>>   };
>>
>> @@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
>>       0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
>>       0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
>>       0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
>> -    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
>> +    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
>>       0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
>>   };
>> ---
>>
>> to access the uart by 32-bit instead of 8-bit.

[PATCH 08/19] hw/microblaze: Propagate CPU endianness to microblaze_load_kernel()

[Philippe Mathieu-Daudé]

Pass vCPU endianness as argument so we can load kernels
with different endianness (different from the qemu-system-binary
builtin one).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/boot.h                     | 4 ++--
 hw/microblaze/boot.c                     | 8 ++++----
 hw/microblaze/petalogix_ml605_mmu.c      | 2 +-
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
 hw/microblaze/xlnx-zynqmp-pmu.c          | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/microblaze/boot.h b/hw/microblaze/boot.h
index 5a8c2f79750..d179a551a69 100644
--- a/hw/microblaze/boot.h
+++ b/hw/microblaze/boot.h
@@ -2,8 +2,8 @@
 #define MICROBLAZE_BOOT_H
 
 
-void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
-                            uint32_t ramsize,
+void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
+                            hwaddr ddr_base, uint32_t ramsize,
                             const char *initrd_filename,
                             const char *dtb_filename,
                             void (*machine_cpu_reset)(MicroBlazeCPU *));
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index ed61e483ee8..3675489fa5b 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -114,8 +114,8 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
     return addr - 0x30000000LL;
 }
 
-void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
-                            uint32_t ramsize,
+void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
+                            hwaddr ddr_base, uint32_t ramsize,
                             const char *initrd_filename,
                             const char *dtb_filename,
                             void (*machine_cpu_reset)(MicroBlazeCPU *))
@@ -144,13 +144,13 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
         /* Boots a kernel elf binary.  */
         kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
                                &entry, NULL, &high, NULL,
-                               TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
+                               !is_little_endian, EM_MICROBLAZE, 0, 0);
         base32 = entry;
         if (base32 == 0xc0000000) {
             kernel_size = load_elf(kernel_filename, NULL,
                                    translate_kernel_address, NULL,
                                    &entry, NULL, NULL, NULL,
-                                   TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
+                                   !is_little_endian, EM_MICROBLAZE, 0, 0);
         }
         /* Always boot into physical ram.  */
         boot_info.bootstrap_pc = (uint32_t)entry;
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 61e47d83988..d2b2109065d 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -204,7 +204,7 @@ petalogix_ml605_init(MachineState *machine)
     cpu->cfg.pvr_regs[5] = 0xc56be000;
     cpu->cfg.pvr_regs[10] = 0x0e000000; /* virtex 6 */
 
-    microblaze_load_kernel(cpu, MEMORY_BASEADDR, ram_size,
+    microblaze_load_kernel(cpu, true, MEMORY_BASEADDR, ram_size,
                            machine->initrd_filename,
                            BINARY_DEVICE_TREE_FILE,
                            NULL);
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 6c0f5c6c651..8110be83715 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -129,7 +129,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
 
     create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
 
-    microblaze_load_kernel(cpu, ddr_base, ram_size,
+    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
                            machine->initrd_filename,
                            BINARY_DEVICE_TREE_FILE,
                            NULL);
diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 567aad47bfc..bdbf7328bf4 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -172,7 +172,7 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
     qdev_realize(DEVICE(pmu), NULL, &error_fatal);
 
     /* Load the kernel */
-    microblaze_load_kernel(&pmu->cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
+    microblaze_load_kernel(&pmu->cpu, true, XLNX_ZYNQMP_PMU_RAM_ADDR,
                            machine->ram_size,
                            machine->initrd_filename,
                            machine->dtb,
-- 
2.45.2

Re: [PATCH 08/19] hw/microblaze: Propagate CPU endianness to microblaze_load_kernel()

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Pass vCPU endianness as argument so we can load kernels
> with different endianness (different from the qemu-system-binary
> builtin one).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/boot.h                     | 4 ++--
>  hw/microblaze/boot.c                     | 8 ++++----
>  hw/microblaze/petalogix_ml605_mmu.c      | 2 +-
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c          | 2 +-
>  5 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 08/19] hw/microblaze: Propagate CPU endianness to microblaze_load_kernel()

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:06 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Pass vCPU endianness as argument so we can load kernels
> with different endianness (different from the qemu-system-binary
> builtin one).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/microblaze/boot.h                     | 4 ++--
>  hw/microblaze/boot.c                     | 8 ++++----
>  hw/microblaze/petalogix_ml605_mmu.c      | 2 +-
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c          | 2 +-
>  5 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/microblaze/boot.h b/hw/microblaze/boot.h
> index 5a8c2f79750..d179a551a69 100644
> --- a/hw/microblaze/boot.h
> +++ b/hw/microblaze/boot.h
> @@ -2,8 +2,8 @@
>  #define MICROBLAZE_BOOT_H
>
>
> -void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> -                            uint32_t ramsize,
> +void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
> +                            hwaddr ddr_base, uint32_t ramsize,
>                              const char *initrd_filename,
>                              const char *dtb_filename,
>                              void (*machine_cpu_reset)(MicroBlazeCPU *));
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index ed61e483ee8..3675489fa5b 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -114,8 +114,8 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return addr - 0x30000000LL;
>  }
>
> -void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> -                            uint32_t ramsize,
> +void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
> +                            hwaddr ddr_base, uint32_t ramsize,
>                              const char *initrd_filename,
>                              const char *dtb_filename,
>                              void (*machine_cpu_reset)(MicroBlazeCPU *))
> @@ -144,13 +144,13 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
>          /* Boots a kernel elf binary.  */
>          kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
>                                 &entry, NULL, &high, NULL,
> -                               TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
> +                               !is_little_endian, EM_MICROBLAZE, 0, 0);
>          base32 = entry;
>          if (base32 == 0xc0000000) {
>              kernel_size = load_elf(kernel_filename, NULL,
>                                     translate_kernel_address, NULL,
>                                     &entry, NULL, NULL, NULL,
> -                                   TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
> +                                   !is_little_endian, EM_MICROBLAZE, 0, 0);
>          }
>          /* Always boot into physical ram.  */
>          boot_info.bootstrap_pc = (uint32_t)entry;
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index 61e47d83988..d2b2109065d 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -204,7 +204,7 @@ petalogix_ml605_init(MachineState *machine)
>      cpu->cfg.pvr_regs[5] = 0xc56be000;
>      cpu->cfg.pvr_regs[10] = 0x0e000000; /* virtex 6 */
>
> -    microblaze_load_kernel(cpu, MEMORY_BASEADDR, ram_size,
> +    microblaze_load_kernel(cpu, true, MEMORY_BASEADDR, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 6c0f5c6c651..8110be83715 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -129,7 +129,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>
>      create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>
> -    microblaze_load_kernel(cpu, ddr_base, ram_size,
> +    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 567aad47bfc..bdbf7328bf4 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -172,7 +172,7 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
>      qdev_realize(DEVICE(pmu), NULL, &error_fatal);
>
>      /* Load the kernel */
> -    microblaze_load_kernel(&pmu->cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
> +    microblaze_load_kernel(&pmu->cpu, true, XLNX_ZYNQMP_PMU_RAM_ADDR,
>                             machine->ram_size,
>                             machine->initrd_filename,
>                             machine->dtb,
> --
> 2.45.2
>
>

Re: [PATCH 08/19] hw/microblaze: Propagate CPU endianness to microblaze_load_kernel()

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:20PM +0100, Philippe Mathieu-Daudé wrote:
> Pass vCPU endianness as argument so we can load kernels
> with different endianness (different from the qemu-system-binary
> builtin one).

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/microblaze/boot.h                     | 4 ++--
>  hw/microblaze/boot.c                     | 8 ++++----
>  hw/microblaze/petalogix_ml605_mmu.c      | 2 +-
>  hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c          | 2 +-
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/microblaze/boot.h b/hw/microblaze/boot.h
> index 5a8c2f79750..d179a551a69 100644
> --- a/hw/microblaze/boot.h
> +++ b/hw/microblaze/boot.h
> @@ -2,8 +2,8 @@
>  #define MICROBLAZE_BOOT_H
>  
>  
> -void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> -                            uint32_t ramsize,
> +void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
> +                            hwaddr ddr_base, uint32_t ramsize,
>                              const char *initrd_filename,
>                              const char *dtb_filename,
>                              void (*machine_cpu_reset)(MicroBlazeCPU *));
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index ed61e483ee8..3675489fa5b 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -114,8 +114,8 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
>      return addr - 0x30000000LL;
>  }
>  
> -void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> -                            uint32_t ramsize,
> +void microblaze_load_kernel(MicroBlazeCPU *cpu, bool is_little_endian,
> +                            hwaddr ddr_base, uint32_t ramsize,
>                              const char *initrd_filename,
>                              const char *dtb_filename,
>                              void (*machine_cpu_reset)(MicroBlazeCPU *))
> @@ -144,13 +144,13 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
>          /* Boots a kernel elf binary.  */
>          kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
>                                 &entry, NULL, &high, NULL,
> -                               TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
> +                               !is_little_endian, EM_MICROBLAZE, 0, 0);
>          base32 = entry;
>          if (base32 == 0xc0000000) {
>              kernel_size = load_elf(kernel_filename, NULL,
>                                     translate_kernel_address, NULL,
>                                     &entry, NULL, NULL, NULL,
> -                                   TARGET_BIG_ENDIAN, EM_MICROBLAZE, 0, 0);
> +                                   !is_little_endian, EM_MICROBLAZE, 0, 0);
>          }
>          /* Always boot into physical ram.  */
>          boot_info.bootstrap_pc = (uint32_t)entry;
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index 61e47d83988..d2b2109065d 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -204,7 +204,7 @@ petalogix_ml605_init(MachineState *machine)
>      cpu->cfg.pvr_regs[5] = 0xc56be000;
>      cpu->cfg.pvr_regs[10] = 0x0e000000; /* virtex 6 */
>  
> -    microblaze_load_kernel(cpu, MEMORY_BASEADDR, ram_size,
> +    microblaze_load_kernel(cpu, true, MEMORY_BASEADDR, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 6c0f5c6c651..8110be83715 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -129,7 +129,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>  
>      create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>  
> -    microblaze_load_kernel(cpu, ddr_base, ram_size,
> +    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
> diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
> index 567aad47bfc..bdbf7328bf4 100644
> --- a/hw/microblaze/xlnx-zynqmp-pmu.c
> +++ b/hw/microblaze/xlnx-zynqmp-pmu.c
> @@ -172,7 +172,7 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
>      qdev_realize(DEVICE(pmu), NULL, &error_fatal);
>  
>      /* Load the kernel */
> -    microblaze_load_kernel(&pmu->cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
> +    microblaze_load_kernel(&pmu->cpu, true, XLNX_ZYNQMP_PMU_RAM_ADDR,
>                             machine->ram_size,
>                             machine->initrd_filename,
>                             machine->dtb,
> -- 
> 2.45.2
> 

[PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

Per the datasheet (reference added in file header, p.9)
'Programming Model' -> 'Register Data Types and Organization':

    "The XPS INTC registers are read as big-endian data"

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/intc/xilinx_intc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 1762b34564e..71f743a1f14 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
@@ -143,12 +146,20 @@ static void pic_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps pic_ops = {
     .read = pic_read,
     .write = pic_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    /* The XPS INTC registers are read as big-endian data. */
+    .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,
     },
-- 
2.45.2

Re: [PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Per the datasheet (reference added in file header, p.9)
> 'Programming Model' -> 'Register Data Types and Organization':
> 
>     "The XPS INTC registers are read as big-endian data"
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/intc/xilinx_intc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:08 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Per the datasheet (reference added in file header, p.9)
> 'Programming Model' -> 'Register Data Types and Organization':
>
>     "The XPS INTC registers are read as big-endian data"
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/intc/xilinx_intc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
> index 1762b34564e..71f743a1f14 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
> @@ -143,12 +146,20 @@ static void pic_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps pic_ops = {
>      .read = pic_read,
>      .write = pic_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    /* The XPS INTC registers are read as big-endian data. */
> +    .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,
>      },
> --
> 2.45.2
>
>

Re: [PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:21PM +0100, Philippe Mathieu-Daudé wrote:
> Per the datasheet (reference added in file header, p.9)
> 'Programming Model' -> 'Register Data Types and Organization':
> 
>     "The XPS INTC registers are read as big-endian data"

Hi Phil,

Some of these devices exist in both big and little endian versions.
So far we've reused the same model by using DEVICE_NATIVE_ENDIAN.

Here's the little endian version:
https://docs.amd.com/v/u/en-US/ds747_axi_intc

Can we have add property to select the endianess?
For the Xilinx use-cases I think it may be a good idea to default it
to little endian and have the big-endian machines explicitly set it.

Cheers,
Edgar


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/intc/xilinx_intc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
> index 1762b34564e..71f743a1f14 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
> @@ -143,12 +146,20 @@ static void pic_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps pic_ops = {
>      .read = pic_read,
>      .write = pic_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    /* The XPS INTC registers are read as big-endian data. */
> +    .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,
>      },
> -- 
> 2.45.2
> 

Re: [PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

+Michal for Linux driver

On 5/11/24 23:08, Edgar E. Iglesias wrote:
> On Tue, Nov 05, 2024 at 02:04:21PM +0100, Philippe Mathieu-Daudé wrote:
>> Per the datasheet (reference added in file header, p.9)
>> 'Programming Model' -> 'Register Data Types and Organization':
>>
>>      "The XPS INTC registers are read as big-endian data"
> 
> Hi Phil,
> 
> Some of these devices exist in both big and little endian versions.
> So far we've reused the same model by using DEVICE_NATIVE_ENDIAN.
> 
> Here's the little endian version:
> https://docs.amd.com/v/u/en-US/ds747_axi_intc

This model is specified as:

- https://docs.amd.com/v/u/en-US/xps_intc
   LogiCORE IP XPS Interrupt Controller (v2.01a)
   DS572 April 19, 2010

Spec is from 2010, you added it in 2009:

   commit 17628bc642260df3a07b9df8b8a9ca7da2e7e87c
   Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
   Date:   Wed May 20 20:11:30 2009 +0200

       xilinx: Add interrupt controller.

The spec is explicit:

   "The XPS INTC registers are read as big-endian data"


The other model you mention is:

- https://docs.amd.com/v/u/en-US/ds747_axi_intc
   LogiCORE IP AXI INTC (v1.04a)
   DS747 June 19, 2013

The spec is more recent than your addition, and contains
the Interrupt Vector Address Register (IVAR) which is not
present in our model.


Indeed the latter seems to extend the former, but they are
not the same and we need some work to model the latter.

The endianness explicit for each model (and is not listed
in the "IP Configurable Design Parameters" tables).


That said, let's look at Linux use. Driver was added in:

   commit eedbdab99fffb8ed71cac75a722088b8ace2583c
   Author: Michal Simek <monstr@monstr.eu>
   Date:   Fri Mar 27 14:25:49 2009 +0100

       microblaze_v8: Interrupt handling and timer support

Using explicit big-endian API:

   +void __init init_IRQ(void)
   +{
   ...
   +       /*
   +        * Disable all external interrupts until they are
   +        * explicity requested.
   +        */
   +       out_be32(intc_baseaddr + IER, 0);
   +
   +       /* Acknowledge any pending interrupts just in case. */
   +       out_be32(intc_baseaddr + IAR, 0xffffffff);
   +
   +       /* Turn on the Master Enable. */
   +       out_be32(intc_baseaddr + MER, MER_HIE | MER_ME);

Then the driver became clever in:

   commit 1aa1243c339d4c902c0f9c1ced45742729a86e6a
   Author: Michal Simek <michal.simek@amd.com>
   Date:   Mon Feb 24 14:56:32 2014 +0100

       microblaze: Make intc driver endian aware

       Detect endianess directly on the hardware and use
       ioread/iowrite functions.

   +static void intc_write32(u32 val, void __iomem *addr)
   +{
   +       iowrite32(val, addr);
   +}
   +
   +static void intc_write32_be(u32 val, void __iomem *addr)
   +{
   +       iowrite32be(val, addr);
   +}

@@ -140,17 +163,25 @@ static int __init xilinx_intc_of_init(struct 
device_node *intc,

   +       write_fn = intc_write32;
   +       read_fn = intc_read32;
   +
           /*
            * Disable all external interrupts until they are
            * explicity requested.
            */
   -       out_be32(intc_baseaddr + IER, 0);
   +       write_fn(0, intc_baseaddr + IER);

           /* Acknowledge any pending interrupts just in case. */
   -       out_be32(intc_baseaddr + IAR, 0xffffffff);
   +       write_fn(0xffffffff, intc_baseaddr + IAR);

           /* Turn on the Master Enable. */
   -       out_be32(intc_baseaddr + MER, MER_HIE | MER_ME);
   +       write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
   +       if (!(read_fn(intc_baseaddr + MER) & (MER_HIE | MER_ME))) {
   +               write_fn = intc_write32_be;
   +               read_fn = intc_read32_be;
   +               write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
   +       }

Interestingly little endianness became the default, although
the driver detect it on init.

This is from 2014, maybe to work with the "LogiCORE IP AXI INTC"
you mentioned, which spec date is 2013.


Indeed when forcing different endianness [*], the Linux kernel used
in our tests (tests/functional/test_microblaze*) does the check
and ends up using the correct INTC endianness:

LE CPU, LE INTC model
pic_write addr=8 val=0
pic_write addr=c val=ffffffff
pic_write addr=1c val=3           <- LE
pic_read 1c=3
pic_write addr=c val=1
pic_write addr=10 val=1
pic_read 18=0

LE CPU, BE INTC model
pic_write addr=8 val=0
pic_write addr=c val=ffffffff
pic_write addr=1c val=3000000     <- LE test
pic_read 1c=0
pic_write addr=1c val=3           <- BE
pic_write addr=c val=1
pic_write addr=10 val=1
pic_read 18=0

BE CPU, BE INTC model
pic_write addr=8 val=0
pic_write addr=c val=ffffffff
pic_write addr=1c val=3000000     <- LE test
pic_read 1c=0
pic_write addr=1c val=3           <- BE
pic_write addr=c val=1
pic_write addr=10 val=1
pic_read 18=0

BE CPU, LE INTC model
pic_write addr=8 val=0
pic_write addr=c val=ffffffff
pic_write addr=1c val=3           <- LE
pic_read 1c=3
pic_write addr=c val=1
pic_write addr=10 val=1
pic_read 18=0


IMHO this patch behavior is correct. Besides, I don't expect
firmwares to be as clever as Linux.

> Can we have add property to select the endianess?
> For the Xilinx use-cases I think it may be a good idea to default it
> to little endian and have the big-endian machines explicitly set it.

What you suggested is implemented in v3:
https://lore.kernel.org/qemu-devel/20241108154317.12129-4-philmd@linaro.org/
but after the analysis, I wonder if it isn't safer to not
make the endianness configurable, but expose as 2 models:
- xlnx,xps_intc (2010) in BE
- xlnx,axi_intc (2013) in LE

> 
> Cheers,
> Edgar
> 
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/intc/xilinx_intc.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
>> index 1762b34564e..71f743a1f14 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
>> @@ -143,12 +146,20 @@ static void pic_write(void *opaque, hwaddr addr,
>>   static const MemoryRegionOps pic_ops = {
>>       .read = pic_read,
>>       .write = pic_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    /* The XPS INTC registers are read as big-endian data. */
>> +    .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,
>>       },
>> -- 
>> 2.45.2
>>

[*] diff used:

-- >8 --
@@ -32,7 +45,7 @@
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index fc55c8afca..883ec685f4 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
  #include "hw/qdev-properties.h"
  #include "qom/object.h"

-#define D(x)
+#define D(x) x

  #define R_ISR       0
  #define R_IPR       1
@@ -105,7 +122,7 @@ static uint64_t pic_read(void *opaque, hwaddr addr, 
unsigned int size)
              break;

      }
-    D(printf("%s %x=%x\n", __func__, addr * 4, r));
+    D(printf("%s %llx=%x\n", __func__, addr * 4, r));
      return r;
  }

@@ -116,7 +133,7 @@ static void pic_write(void *opaque, hwaddr addr,
      uint32_t value = val64;

      addr >>= 2;
-    D(qemu_log("%s addr=%x val=%x\n", __func__, addr * 4, value));
+    D(printf("%s addr=%llx val=%x\n", __func__, addr * 4, value));
      switch (addr)
      {
          case R_IAR:
---

Re: [PATCH 09/19] hw/intc/xilinx_intc: Only expect big-endian accesses

[Michal Simek]

On 11/14/24 23:43, Philippe Mathieu-Daudé wrote:
> +Michal for Linux driver
> 
> On 5/11/24 23:08, Edgar E. Iglesias wrote:
>> On Tue, Nov 05, 2024 at 02:04:21PM +0100, Philippe Mathieu-Daudé wrote:
>>> Per the datasheet (reference added in file header, p.9)
>>> 'Programming Model' -> 'Register Data Types and Organization':
>>>
>>>      "The XPS INTC registers are read as big-endian data"
>>
>> Hi Phil,
>>
>> Some of these devices exist in both big and little endian versions.
>> So far we've reused the same model by using DEVICE_NATIVE_ENDIAN.
>>
>> Here's the little endian version:
>> https://docs.amd.com/v/u/en-US/ds747_axi_intc
> 
> This model is specified as:
> 
> - https://docs.amd.com/v/u/en-US/xps_intc
>    LogiCORE IP XPS Interrupt Controller (v2.01a)
>    DS572 April 19, 2010
> 
> Spec is from 2010, you added it in 2009:
> 
>    commit 17628bc642260df3a07b9df8b8a9ca7da2e7e87c
>    Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>    Date:   Wed May 20 20:11:30 2009 +0200
> 
>        xilinx: Add interrupt controller.
> 
> The spec is explicit:
> 
>    "The XPS INTC registers are read as big-endian data"
> 
> 
> The other model you mention is:
> 
> - https://docs.amd.com/v/u/en-US/ds747_axi_intc
>    LogiCORE IP AXI INTC (v1.04a)
>    DS747 June 19, 2013
> 
> The spec is more recent than your addition, and contains
> the Interrupt Vector Address Register (IVAR) which is not
> present in our model.
> 
> 
> Indeed the latter seems to extend the former, but they are
> not the same and we need some work to model the latter.
> 
> The endianness explicit for each model (and is not listed
> in the "IP Configurable Design Parameters" tables).
> 
> 
> That said, let's look at Linux use. Driver was added in:
> 
>    commit eedbdab99fffb8ed71cac75a722088b8ace2583c
>    Author: Michal Simek <monstr@monstr.eu>
>    Date:   Fri Mar 27 14:25:49 2009 +0100
> 
>        microblaze_v8: Interrupt handling and timer support
> 
> Using explicit big-endian API:
> 
>    +void __init init_IRQ(void)
>    +{
>    ...
>    +       /*
>    +        * Disable all external interrupts until they are
>    +        * explicity requested.
>    +        */
>    +       out_be32(intc_baseaddr + IER, 0);
>    +
>    +       /* Acknowledge any pending interrupts just in case. */
>    +       out_be32(intc_baseaddr + IAR, 0xffffffff);
>    +
>    +       /* Turn on the Master Enable. */
>    +       out_be32(intc_baseaddr + MER, MER_HIE | MER_ME);
> 
> Then the driver became clever in:
> 
>    commit 1aa1243c339d4c902c0f9c1ced45742729a86e6a
>    Author: Michal Simek <michal.simek@amd.com>
>    Date:   Mon Feb 24 14:56:32 2014 +0100
> 
>        microblaze: Make intc driver endian aware
> 
>        Detect endianess directly on the hardware and use
>        ioread/iowrite functions.
> 
>    +static void intc_write32(u32 val, void __iomem *addr)
>    +{
>    +       iowrite32(val, addr);
>    +}
>    +
>    +static void intc_write32_be(u32 val, void __iomem *addr)
>    +{
>    +       iowrite32be(val, addr);
>    +}
> 
> @@ -140,17 +163,25 @@ static int __init xilinx_intc_of_init(struct device_node 
> *intc,
> 
>    +       write_fn = intc_write32;
>    +       read_fn = intc_read32;
>    +
>            /*
>             * Disable all external interrupts until they are
>             * explicity requested.
>             */
>    -       out_be32(intc_baseaddr + IER, 0);
>    +       write_fn(0, intc_baseaddr + IER);
> 
>            /* Acknowledge any pending interrupts just in case. */
>    -       out_be32(intc_baseaddr + IAR, 0xffffffff);
>    +       write_fn(0xffffffff, intc_baseaddr + IAR);
> 
>            /* Turn on the Master Enable. */
>    -       out_be32(intc_baseaddr + MER, MER_HIE | MER_ME);
>    +       write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
>    +       if (!(read_fn(intc_baseaddr + MER) & (MER_HIE | MER_ME))) {
>    +               write_fn = intc_write32_be;
>    +               read_fn = intc_read32_be;
>    +               write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
>    +       }
> 
> Interestingly little endianness became the default, although
> the driver detect it on init.
> 
> This is from 2014, maybe to work with the "LogiCORE IP AXI INTC"
> you mentioned, which spec date is 2013.
> 
> Indeed when forcing different endianness [*], the Linux kernel used
> in our tests (tests/functional/test_microblaze*) does the check
> and ends up using the correct INTC endianness:
> 
> LE CPU, LE INTC model
> pic_write addr=8 val=0
> pic_write addr=c val=ffffffff
> pic_write addr=1c val=3           <- LE
> pic_read 1c=3
> pic_write addr=c val=1
> pic_write addr=10 val=1
> pic_read 18=0
> 
> LE CPU, BE INTC model
> pic_write addr=8 val=0
> pic_write addr=c val=ffffffff
> pic_write addr=1c val=3000000     <- LE test
> pic_read 1c=0
> pic_write addr=1c val=3           <- BE
> pic_write addr=c val=1
> pic_write addr=10 val=1
> pic_read 18=0
> 
> BE CPU, BE INTC model
> pic_write addr=8 val=0
> pic_write addr=c val=ffffffff
> pic_write addr=1c val=3000000     <- LE test
> pic_read 1c=0
> pic_write addr=1c val=3           <- BE
> pic_write addr=c val=1
> pic_write addr=10 val=1
> pic_read 18=0
> 
> BE CPU, LE INTC model
> pic_write addr=8 val=0
> pic_write addr=c val=ffffffff
> pic_write addr=1c val=3           <- LE
> pic_read 1c=3
> pic_write addr=c val=1
> pic_write addr=10 val=1
> pic_read 18=0
> 
> 
> IMHO this patch behavior is correct. Besides, I don't expect
> firmwares to be as clever as Linux.
> 
>> Can we have add property to select the endianess?
>> For the Xilinx use-cases I think it may be a good idea to default it
>> to little endian and have the big-endian machines explicitly set it.
> 
> What you suggested is implemented in v3:
> https://lore.kernel.org/qemu-devel/20241108154317.12129-4-philmd@linaro.org/
> but after the analysis, I wonder if it isn't safer to not
> make the endianness configurable, but expose as 2 models:
> - xlnx,xps_intc (2010) in BE
> - xlnx,axi_intc (2013) in LE

It is a little bit more complicated.
In past everything started as big endian on OPB bus. Then PLB bus still big 
endian and then AXI came and things have been moved to little endian.
That's from bus perspective.

 From CPU perspective itself till AXI microblaze was big endian only. With AXI 
cpu started to be by default little endian but still today you can still 
configured cpu to be big endian (C_ENDIANNESS config) with using AXI bus.

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.

Thanks,
Michal

[PATCH 10/19] hw/timer/xilinx_timer: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

Per the datasheet (reference added in file header, p.10):
'Register Data Types and Organization':

  "The XPS Timer/Counter registers are organized as big-endian data."

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/timer/xilinx_timer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 28ac95edea1..3e272c8bb39 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
@@ -192,7 +195,7 @@ timer_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps timer_ops = {
     .read = timer_read,
     .write = timer_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
     .impl = {
         .min_access_size = 4,
         .max_access_size = 4,
-- 
2.45.2

Re: [PATCH 10/19] hw/timer/xilinx_timer: Only expect big-endian accesses

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Per the datasheet (reference added in file header, p.10):
> 'Register Data Types and Organization':
> 
>   "The XPS Timer/Counter registers are organized as big-endian data."
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/timer/xilinx_timer.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 10/19] hw/timer/xilinx_timer: Only expect big-endian accesses

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:22PM +0100, Philippe Mathieu-Daudé wrote:
> Per the datasheet (reference added in file header, p.10):
> 'Register Data Types and Organization':
> 
>   "The XPS Timer/Counter registers are organized as big-endian data."

Haven't checked but pretty sure this will break things the same way as
for the intc.

Cheers,
Edgar

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/timer/xilinx_timer.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 28ac95edea1..3e272c8bb39 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
> @@ -192,7 +195,7 @@ timer_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps timer_ops = {
>      .read = timer_read,
>      .write = timer_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,
>      .impl = {
>          .min_access_size = 4,
>          .max_access_size = 4,
> -- 
> 2.45.2
> 

[PATCH 11/19] hw/timer/xilinx_timer: Allow down to 8-bit memory access

[Philippe Mathieu-Daudé]

Allow down to 8-bit access, per the datasheet (reference added
in previous commit):

 "Timer Counter registers are accessed as one of the following types:
  • Byte (8 bits)
  • Half word (2 bytes)
  • Word (4 bytes)"

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/timer/xilinx_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 3e272c8bb39..e9498fc7eec 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -201,7 +201,7 @@ static const MemoryRegionOps timer_ops = {
         .max_access_size = 4,
     },
     .valid = {
-        .min_access_size = 4,
+        .min_access_size = 1,
         .max_access_size = 4,
     },
 };
-- 
2.45.2

Re: [PATCH 11/19] hw/timer/xilinx_timer: Allow down to 8-bit memory access

[Anton Johansson via]

On 05/11/24, Philippe Mathieu-Daudé wrote:
> Allow down to 8-bit access, per the datasheet (reference added
> in previous commit):
> 
>  "Timer Counter registers are accessed as one of the following types:
>   • Byte (8 bits)
>   • Half word (2 bytes)
>   • Word (4 bytes)"
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/timer/xilinx_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 11/19] hw/timer/xilinx_timer: Allow down to 8-bit memory access

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:07 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Allow down to 8-bit access, per the datasheet (reference added
> in previous commit):
>
>  "Timer Counter registers are accessed as one of the following types:
>   • Byte (8 bits)
>   • Half word (2 bytes)
>   • Word (4 bytes)"
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/timer/xilinx_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 3e272c8bb39..e9498fc7eec 100644
> --- a/hw/timer/xilinx_timer.c
> +++ b/hw/timer/xilinx_timer.c
> @@ -201,7 +201,7 @@ static const MemoryRegionOps timer_ops = {
>          .max_access_size = 4,
>      },
>      .valid = {
> -        .min_access_size = 4,
> +        .min_access_size = 1,
>          .max_access_size = 4,
>      },
>  };
> --
> 2.45.2
>
>

Re: [PATCH 11/19] hw/timer/xilinx_timer: Allow down to 8-bit memory access

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:23PM +0100, Philippe Mathieu-Daudé wrote:
> Allow down to 8-bit access, per the datasheet (reference added
> in previous commit):
> 
>  "Timer Counter registers are accessed as one of the following types:
>   • Byte (8 bits)
>   • Half word (2 bytes)
>   • Word (4 bytes)"

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/timer/xilinx_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 3e272c8bb39..e9498fc7eec 100644
> --- a/hw/timer/xilinx_timer.c
> +++ b/hw/timer/xilinx_timer.c
> @@ -201,7 +201,7 @@ static const MemoryRegionOps timer_ops = {
>          .max_access_size = 4,
>      },
>      .valid = {
> -        .min_access_size = 4,
> +        .min_access_size = 1,
>          .max_access_size = 4,
>      },
>  };
> -- 
> 2.45.2
> 

[PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

The Xilinx 'ethlite' device was added in commit b43848a100
("xilinx: Add ethlite emulation"), being only built back
then for a big-endian MicroBlaze target (see commit 72b675caac
"microblaze: Hook into the build-system").

I/O endianness access was then clarified in commit d48751ed4f
("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
the 'fix' was to use tswap32(). Since the machine was built as
big-endian target, tswap32() use means the fix was for a little
endian host. While the datasheet (reference added in file header)
is not precise about it, we interpret such change as the device
expects accesses in big-endian order. Besides, this is what other
Xilinx/MicroBlaze devices use (see the 3 previous commits).

Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
property, so if the bus access expect little-endian order we swap
the values. Replace the tswap32() calls accordingly.

Set the property on the single machine using this 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, 17 insertions(+), 4 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 8110be83715..8407dbee12a 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -123,6 +123,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     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);
+    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index ede7c172748..44ef11ebf89 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -3,6 +3,9 @@
  *
  * Copyright (c) 2009 Edgar E. Iglesias.
  *
+ * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
+ * LogiCORE IP XPS Ethernet Lite Media Access Controller
+ *
  * 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
@@ -25,7 +28,6 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qom/object.h"
-#include "exec/tswap.h"
 #include "hw/sysbus.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
@@ -65,6 +67,7 @@ struct xlx_ethlite
     NICState *nic;
     NICConf conf;
 
+    bool access_little_endian;
     uint32_t c_tx_pingpong;
     uint32_t c_rx_pingpong;
     unsigned int txbuf;
@@ -103,9 +106,12 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
             break;
 
         default:
-            r = tswap32(s->regs[addr]);
+            r = s->regs[addr];
             break;
     }
+    if (s->access_little_endian) {
+        bswap32s(&r);
+    }
     return r;
 }
 
@@ -117,6 +123,10 @@ eth_write(void *opaque, hwaddr addr,
     unsigned int base = 0;
     uint32_t value = val64;
 
+    if (s->access_little_endian) {
+        bswap32s(&value);
+    }
+
     addr >>= 2;
     switch (addr) 
     {
@@ -161,7 +171,7 @@ eth_write(void *opaque, hwaddr addr,
             break;
 
         default:
-            s->regs[addr] = tswap32(value);
+            s->regs[addr] = value;
             break;
     }
 }
@@ -169,7 +179,7 @@ eth_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps eth_ops = {
     .read = eth_read,
     .write = eth_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
     .impl = {
         .min_access_size = 4,
         .max_access_size = 4,
@@ -256,6 +266,8 @@ static void xilinx_ethlite_init(Object *obj)
 }
 
 static Property xilinx_ethlite_properties[] = {
+    DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
+                     access_little_endian, false),
     DEFINE_PROP_UINT32("tx-ping-pong", struct xlx_ethlite, c_tx_pingpong, 1),
     DEFINE_PROP_UINT32("rx-ping-pong", struct xlx_ethlite, c_rx_pingpong, 1),
     DEFINE_NIC_PROPERTIES(struct xlx_ethlite, conf),
-- 
2.45.2

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> The Xilinx 'ethlite' device was added in commit b43848a100
> ("xilinx: Add ethlite emulation"), being only built back
> then for a big-endian MicroBlaze target (see commit 72b675caac
> "microblaze: Hook into the build-system").
> 
> I/O endianness access was then clarified in commit d48751ed4f
> ("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
> the 'fix' was to use tswap32(). Since the machine was built as
> big-endian target, tswap32() use means the fix was for a little
> endian host. While the datasheet (reference added in file header)
> is not precise about it, we interpret such change as the device
> expects accesses in big-endian order. Besides, this is what other
> Xilinx/MicroBlaze devices use (see the 3 previous commits).
> 
> Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
> property, so if the bus access expect little-endian order we swap
> the values. Replace the tswap32() calls accordingly.
> 
> Set the property on the single machine using this 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, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 8110be83715..8407dbee12a 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -123,6 +123,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>       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);
> +    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
>       sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index ede7c172748..44ef11ebf89 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -3,6 +3,9 @@
>    *
>    * Copyright (c) 2009 Edgar E. Iglesias.
>    *
> + * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
> + * LogiCORE IP XPS Ethernet Lite Media Access Controller
> + *
>    * 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
> @@ -25,7 +28,6 @@
>   #include "qemu/osdep.h"
>   #include "qemu/module.h"
>   #include "qom/object.h"
> -#include "exec/tswap.h"
>   #include "hw/sysbus.h"
>   #include "hw/irq.h"
>   #include "hw/qdev-properties.h"
> @@ -65,6 +67,7 @@ struct xlx_ethlite
>       NICState *nic;
>       NICConf conf;
>   
> +    bool access_little_endian;
>       uint32_t c_tx_pingpong;
>       uint32_t c_rx_pingpong;
>       unsigned int txbuf;
> @@ -103,9 +106,12 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>               break;
>   
>           default:
> -            r = tswap32(s->regs[addr]);
> +            r = s->regs[addr];
>               break;
>       }
> +    if (s->access_little_endian) {
> +        bswap32s(&r);
> +    }
>       return r;
>   }
>   
> @@ -117,6 +123,10 @@ eth_write(void *opaque, hwaddr addr,
>       unsigned int base = 0;
>       uint32_t value = val64;
>   
> +    if (s->access_little_endian) {
> +        bswap32s(&value);
> +    }
> +
>       addr >>= 2;
>       switch (addr)
>       {
> @@ -161,7 +171,7 @@ eth_write(void *opaque, hwaddr addr,
>               break;
>   
>           default:
> -            s->regs[addr] = tswap32(value);
> +            s->regs[addr] = value;
>               break;
>       }
>   }
> @@ -169,7 +179,7 @@ eth_write(void *opaque, hwaddr addr,
>   static const MemoryRegionOps eth_ops = {
>       .read = eth_read,
>       .write = eth_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,
>       .impl = {
>           .min_access_size = 4,
>           .max_access_size = 4,
> @@ -256,6 +266,8 @@ static void xilinx_ethlite_init(Object *obj)
>   }
>   
>   static Property xilinx_ethlite_properties[] = {
> +    DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
> +                     access_little_endian, false),

I'm not a fan of performing any swapping within device code.
The memory subsystem should do all of it.

A better solution is two tables, eth_ops_{be,le}, which differ only in the setting of 
.endianness.  Handle the property by registering the correct MemoryRegionOps during init.


r~

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

On 5/11/24 13:30, Richard Henderson wrote:
> On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
>> The Xilinx 'ethlite' device was added in commit b43848a100
>> ("xilinx: Add ethlite emulation"), being only built back
>> then for a big-endian MicroBlaze target (see commit 72b675caac
>> "microblaze: Hook into the build-system").
>>
>> I/O endianness access was then clarified in commit d48751ed4f
>> ("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
>> the 'fix' was to use tswap32(). Since the machine was built as
>> big-endian target, tswap32() use means the fix was for a little
>> endian host. While the datasheet (reference added in file header)
>> is not precise about it, we interpret such change as the device
>> expects accesses in big-endian order. Besides, this is what other
>> Xilinx/MicroBlaze devices use (see the 3 previous commits).
>>
>> Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
>> property, so if the bus access expect little-endian order we swap
>> the values. Replace the tswap32() calls accordingly.
>>
>> Set the property on the single machine using this 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, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
>> b/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> index 8110be83715..8407dbee12a 100644
>> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
>> @@ -123,6 +123,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>>       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);
>> +    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
>>       sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
>> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
>> index ede7c172748..44ef11ebf89 100644
>> --- a/hw/net/xilinx_ethlite.c
>> +++ b/hw/net/xilinx_ethlite.c
>> @@ -3,6 +3,9 @@
>>    *
>>    * Copyright (c) 2009 Edgar E. Iglesias.
>>    *
>> + * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
>> + * LogiCORE IP XPS Ethernet Lite Media Access Controller
>> + *
>>    * 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
>> @@ -25,7 +28,6 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/module.h"
>>   #include "qom/object.h"
>> -#include "exec/tswap.h"
>>   #include "hw/sysbus.h"
>>   #include "hw/irq.h"
>>   #include "hw/qdev-properties.h"
>> @@ -65,6 +67,7 @@ struct xlx_ethlite
>>       NICState *nic;
>>       NICConf conf;
>> +    bool access_little_endian;
>>       uint32_t c_tx_pingpong;
>>       uint32_t c_rx_pingpong;
>>       unsigned int txbuf;
>> @@ -103,9 +106,12 @@ eth_read(void *opaque, hwaddr addr, unsigned int 
>> size)
>>               break;
>>           default:
>> -            r = tswap32(s->regs[addr]);
>> +            r = s->regs[addr];
>>               break;
>>       }
>> +    if (s->access_little_endian) {
>> +        bswap32s(&r);
>> +    }
>>       return r;
>>   }
>> @@ -117,6 +123,10 @@ eth_write(void *opaque, hwaddr addr,
>>       unsigned int base = 0;
>>       uint32_t value = val64;
>> +    if (s->access_little_endian) {
>> +        bswap32s(&value);
>> +    }
>> +
>>       addr >>= 2;
>>       switch (addr)
>>       {
>> @@ -161,7 +171,7 @@ eth_write(void *opaque, hwaddr addr,
>>               break;
>>           default:
>> -            s->regs[addr] = tswap32(value);
>> +            s->regs[addr] = value;
>>               break;
>>       }
>>   }
>> @@ -169,7 +179,7 @@ eth_write(void *opaque, hwaddr addr,
>>   static const MemoryRegionOps eth_ops = {
>>       .read = eth_read,
>>       .write = eth_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_BIG_ENDIAN,
>>       .impl = {
>>           .min_access_size = 4,
>>           .max_access_size = 4,
>> @@ -256,6 +266,8 @@ static void xilinx_ethlite_init(Object *obj)
>>   }
>>   static Property xilinx_ethlite_properties[] = {
>> +    DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
>> +                     access_little_endian, false),
> 
> I'm not a fan of performing any swapping within device code.
> The memory subsystem should do all of it.
> 
> A better solution is two tables, eth_ops_{be,le}, which differ only in 
> the setting of .endianness.  Handle the property by registering the 
> correct MemoryRegionOps during init.

Squashing this on top to have the two tables, but we still need to
swap back the effect of memory.c swapping for the binary, so I don't
think this is what you want:

-- >8 --
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 44ef11ebf89..d58757eb919 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -67,6 +67,7 @@ struct xlx_ethlite
      NICState *nic;
      NICConf conf;

+    bool model_little_endian;
      bool access_little_endian;
      uint32_t c_tx_pingpong;
      uint32_t c_rx_pingpong;
@@ -109,7 +110,7 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
              r = s->regs[addr];
              break;
      }
-    if (s->access_little_endian) {
+    if (s->access_little_endian ^ s->model_little_endian) {
          bswap32s(&r);
      }
      return r;
@@ -123,7 +124,7 @@ eth_write(void *opaque, hwaddr addr,
      unsigned int base = 0;
      uint32_t value = val64;

-    if (s->access_little_endian) {
+    if (s->access_little_endian ^ s->model_little_endian) {
          bswap32s(&value);
      }

@@ -176,7 +177,7 @@ eth_write(void *opaque, hwaddr addr,
      }
  }

-static const MemoryRegionOps eth_ops = {
+static const MemoryRegionOps eth_ops_be = {
      .read = eth_read,
      .write = eth_write,
      .endianness = DEVICE_BIG_ENDIAN,
@@ -190,6 +191,20 @@ static const MemoryRegionOps eth_ops = {
      },
  };

+static const MemoryRegionOps eth_ops_le = {
+    .read = eth_read,
+    .write = eth_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
  static bool eth_can_rx(NetClientState *nc)
  {
      struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
@@ -247,6 +262,11 @@ static void xilinx_ethlite_realize(DeviceState 
*dev, Error **errp)
  {
      struct xlx_ethlite *s = XILINX_ETHLITE(dev);

+    memory_region_init_io(&s->mmio, OBJECT(dev),
+                          s->model_little_endian ? &eth_ops_le : 
&eth_ops_be, s,
+                          "xlnx.xps-ethernetlite", R_MAX * 4);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
+
      qemu_macaddr_default_if_unset(&s->conf.macaddr);
      s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
                            object_get_typename(OBJECT(dev)), dev->id,
@@ -259,15 +279,13 @@ static void xilinx_ethlite_init(Object *obj)
      struct xlx_ethlite *s = XILINX_ETHLITE(obj);

      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
-    memory_region_init_io(&s->mmio, obj, &eth_ops, s,
-                          "xlnx.xps-ethernetlite", R_MAX * 4);
-    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
  }

  static Property xilinx_ethlite_properties[] = {
      DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
                       access_little_endian, false),
+    DEFINE_PROP_BOOL("model-little-endian", struct xlx_ethlite,
+                     model_little_endian, false),
      DEFINE_PROP_UINT32("tx-ping-pong", struct xlx_ethlite, 
c_tx_pingpong, 1),
      DEFINE_PROP_UINT32("rx-ping-pong", struct xlx_ethlite, 
c_rx_pingpong, 1),
      DEFINE_NIC_PROPERTIES(struct xlx_ethlite, conf),

---

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Paolo Bonzini]

On 11/5/24 14:04, Philippe Mathieu-Daudé wrote:
> The Xilinx 'ethlite' device was added in commit b43848a100
> ("xilinx: Add ethlite emulation"), being only built back
> then for a big-endian MicroBlaze target (see commit 72b675caac
> "microblaze: Hook into the build-system").
> 
> I/O endianness access was then clarified in commit d48751ed4f
> ("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
> the 'fix' was to use tswap32(). Since the machine was built as
> big-endian target, tswap32() use means the fix was for a little
> endian host. While the datasheet (reference added in file header)
> is not precise about it, we interpret such change as the device
> expects accesses in big-endian order. Besides, this is what other
> Xilinx/MicroBlaze devices use (see the 3 previous commits).
> 
> Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
> property, so if the bus access expect little-endian order we swap
> the values. Replace the tswap32() calls accordingly.
> 
> Set the property on the single machine using this device.

I don't understand.  This machine type is little-endian only and 
expecting inverted accesses, isn't it?  Therefore, all that you need is

> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,

And removing the tswap altogether.  The big-endian petalogix board will 
start getting "correct" values (not swapped anymore).  That's a feature, 
not a bug.

Paolo

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Philippe Mathieu-Daudé]

On 5/11/24 14:18, Paolo Bonzini wrote:
> On 11/5/24 14:04, Philippe Mathieu-Daudé wrote:
>> The Xilinx 'ethlite' device was added in commit b43848a100
>> ("xilinx: Add ethlite emulation"), being only built back
>> then for a big-endian MicroBlaze target (see commit 72b675caac
>> "microblaze: Hook into the build-system").
>>
>> I/O endianness access was then clarified in commit d48751ed4f
>> ("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
>> the 'fix' was to use tswap32(). Since the machine was built as
>> big-endian target, tswap32() use means the fix was for a little
>> endian host. While the datasheet (reference added in file header)
>> is not precise about it, we interpret such change as the device
>> expects accesses in big-endian order. Besides, this is what other
>> Xilinx/MicroBlaze devices use (see the 3 previous commits).
>>
>> Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
>> property, so if the bus access expect little-endian order we swap
>> the values. Replace the tswap32() calls accordingly.
>>
>> Set the property on the single machine using this device.
> 
> I don't understand.  This machine type is little-endian only and 
> expecting inverted accesses, isn't it?  Therefore, all that you need is
> 
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_BIG_ENDIAN,
> 
> And removing the tswap altogether.  The big-endian petalogix board will 
> start getting "correct" values (not swapped anymore).  That's a feature, 
> not a bug.

The feature is memory.c swapping MemoryRegionOps depending on the
*qemu-system binary* target endianness.

We assumed most guest vCPUs run with the same endianness of the binary.

Now we want to swap wrt the vCPU, not the binary. So indeed this patch
effectively undo the memory.c swapping (feature).

I suppose the better way is to modify memory.c, possibly passing MemOp
all over. For HW accel where vCPU endianness is forced to host one,
this would become a no-op. Lot of rework in perspective.

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Paolo Bonzini]

On 11/6/24 00:29, Philippe Mathieu-Daudé wrote:
> We assumed most guest vCPUs run with the same endianness of the binary.
> 
> Now we want to swap wrt the vCPU, not the binary. So indeed this patch
> effectively undo the memory.c swapping (feature).
> 
> I suppose the better way is to modify memory.c, possibly passing MemOp
> all over. For HW accel where vCPU endianness is forced to host one,
> this would become a no-op. Lot of rework in perspective.

It should be much easier than that.  First of all, this is when memory.c
swaps according to DEVICE_*_ENDIAN:

guest      \   host    little-endian         big-endian
little-endian          BIG                   LITTLE, NATIVE
big-endian             BIG, NATIVE           LITTLE

tswap swaps in the two cases marked "NATIVE" (same as DEVICE_NATIVE_ENDIAN).
ldl_le_p swaps in the two cases marked "LITTLE" (same as DEVICE_LITTLE_ENDIAN).
ldl_be_p swaps in the two cases marked "BIG" (same as DEVICE_BIG_ENDIAN).


First of all, current code does different things for RAM vs. the other
registers.  After your patch it's the same, which seems fishy.

Anyway let's focus on RAM for now.  Current code (unconditional tswap +
DEVICE_NATIVE_ENDIAN) always performs an even number of swaps:

guest      \   host    little-endian         big-endian
little-endian          none                  tswap+memory.c
big-endian             tswap+memory.c        none

That's what Edgar says - it's just RAM.



So with your patch the behavior becomes:

guest      \   host    little-endian         big-endian
little-endian          bswap+memory.c        bswap
big-endian             memory.c              none

Behavior changes in the cross-endianness case.  LE-on-LE remains the same.
It seems to break BE hosts since petalogix is a qemu-system-microblazeel board.

If this reasoning is correct, together with DEVICE_BIG_ENDIAN you need
cpu_to_be32 instead of tswap:

guest      \   host    little-endian         big-endian
little-endian          cpu_to_be32+memory.c  none
big-endian             cpu_to_be32+memory.c  none

  

However, things are different for the R_RX* and R_TX* cases.

Before:

guest      \   host    little-endian         big-endian
little-endian          none                  memory.c
big-endian             memory.c              none


Your patch here keeps the same evenness of swaps, even if who
swaps changes:

guest      \   host    little-endian         big-endian
little-endian          bswap+memory.c        bswap
big-endian             memory.c              none

Is this just a change in migration format for the RAM ara?  Then I
guess your patch works, though I'd prefer Richard's suggestion of
flipping the endianness in the MemoryRegionOps.


However, since you said the board is LE-only, maybe the following
also works and seems simpler?

1) use DEVICE_LITTLE_ENDIAN (i.e. always the same perspective as
qemu-microblazeel)

2) use cpu_to_le32 for RAM and nothing for the other registers


But again, maybe I'm completely wrong.

Paolo

Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Edgar E. Iglesias]

On Tue, Nov 05, 2024 at 02:04:24PM +0100, Philippe Mathieu-Daudé wrote:
> The Xilinx 'ethlite' device was added in commit b43848a100
> ("xilinx: Add ethlite emulation"), being only built back
> then for a big-endian MicroBlaze target (see commit 72b675caac
> "microblaze: Hook into the build-system").
> 
> I/O endianness access was then clarified in commit d48751ed4f
> ("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
> the 'fix' was to use tswap32(). Since the machine was built as
> big-endian target, tswap32() use means the fix was for a little
> endian host. While the datasheet (reference added in file header)
> is not precise about it, we interpret such change as the device
> expects accesses in big-endian order. Besides, this is what other
> Xilinx/MicroBlaze devices use (see the 3 previous commits).
> 
> Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
> property, so if the bus access expect little-endian order we swap
> the values. Replace the tswap32() calls accordingly.
> 
> Set the property on the single machine using this device.


I think you're partially correct but not fully. This buffer area is
really a RAM and has no endianess. Problem is back then I don't think
I was a ware of a way to map RAM memory sub regions so we hacked in
byteswaps to swap from host (which usually was little endian) to
big endian. This is because register accesses from CPU to device model
are kept in host endianess. I think the right way to solve this issue
is to map a RAM memory region to represent the BRAM.

Cheers,
Edgar


> 
> 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, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 8110be83715..8407dbee12a 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -123,6 +123,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      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);
> +    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index ede7c172748..44ef11ebf89 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -3,6 +3,9 @@
>   *
>   * Copyright (c) 2009 Edgar E. Iglesias.
>   *
> + * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
> + * LogiCORE IP XPS Ethernet Lite Media Access Controller
> + *
>   * 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
> @@ -25,7 +28,6 @@
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
>  #include "qom/object.h"
> -#include "exec/tswap.h"
>  #include "hw/sysbus.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
> @@ -65,6 +67,7 @@ struct xlx_ethlite
>      NICState *nic;
>      NICConf conf;
>  
> +    bool access_little_endian;
>      uint32_t c_tx_pingpong;
>      uint32_t c_rx_pingpong;
>      unsigned int txbuf;
> @@ -103,9 +106,12 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>              break;
>  
>          default:
> -            r = tswap32(s->regs[addr]);
> +            r = s->regs[addr];
>              break;
>      }
> +    if (s->access_little_endian) {
> +        bswap32s(&r);
> +    }
>      return r;
>  }
>  
> @@ -117,6 +123,10 @@ eth_write(void *opaque, hwaddr addr,
>      unsigned int base = 0;
>      uint32_t value = val64;
>  
> +    if (s->access_little_endian) {
> +        bswap32s(&value);
> +    }
> +
>      addr >>= 2;
>      switch (addr) 
>      {
> @@ -161,7 +171,7 @@ eth_write(void *opaque, hwaddr addr,
>              break;
>  
>          default:
> -            s->regs[addr] = tswap32(value);
> +            s->regs[addr] = value;
>              break;
>      }
>  }
> @@ -169,7 +179,7 @@ eth_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps eth_ops = {
>      .read = eth_read,
>      .write = eth_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,
>      .impl = {
>          .min_access_size = 4,
>          .max_access_size = 4,
> @@ -256,6 +266,8 @@ static void xilinx_ethlite_init(Object *obj)
>  }
>  
>  static Property xilinx_ethlite_properties[] = {
> +    DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
> +                     access_little_endian, false),
>      DEFINE_PROP_UINT32("tx-ping-pong", struct xlx_ethlite, c_tx_pingpong, 1),
>      DEFINE_PROP_UINT32("rx-ping-pong", struct xlx_ethlite, c_rx_pingpong, 1),
>      DEFINE_NIC_PROPERTIES(struct xlx_ethlite, conf),
> -- 
> 2.45.2
> 

[PATCH 13/19] 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>
---
 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 4beaf69e76a..4c25b1e4383 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -779,13 +779,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)
@@ -797,26 +797,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)
@@ -828,14 +828,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)
@@ -845,7 +845,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) {
@@ -929,13 +929,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)
@@ -947,26 +947,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)
@@ -978,14 +978,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)
@@ -1014,7 +1014,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.45.2

Re: [PATCH 13/19] target/microblaze: Explode MO_TExx -> MO_TE | MO_xx

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>   target/microblaze/translate.c | 36 +++++++++++++++++------------------
>   1 file changed, 18 insertions(+), 18 deletions(-)

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

r~

Re: [PATCH 13/19] target/microblaze: Explode MO_TExx -> MO_TE | MO_xx

[Alistair Francis]

On Tue, Nov 5, 2024 at 11:07 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> 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: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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 4beaf69e76a..4c25b1e4383 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -779,13 +779,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)
> @@ -797,26 +797,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)
> @@ -828,14 +828,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)
> @@ -845,7 +845,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) {
> @@ -929,13 +929,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)
> @@ -947,26 +947,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)
> @@ -978,14 +978,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)
> @@ -1014,7 +1014,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.45.2
>
>

[PATCH 14/19] 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>
---
 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 4c25b1e4383..86f3c196189 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -712,6 +712,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.
      *
@@ -779,13 +781,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)
@@ -797,26 +799,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)
@@ -828,14 +830,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)
@@ -862,6 +864,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.
      *
@@ -929,13 +933,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)
@@ -947,26 +951,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)
@@ -978,14 +982,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.45.2

Re: [PATCH 14/19] target/microblaze: Set MO_TE once in do_load() / do_store()

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>   target/microblaze/translate.c | 36 +++++++++++++++++++----------------
>   1 file changed, 20 insertions(+), 16 deletions(-)

I think you could have squashed this with the previous,
but either way,

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

r~

[PATCH 15/19] 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>
---
 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 86f3c196189..0b466db694c 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -707,12 +707,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.
@@ -847,7 +852,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) {
@@ -864,7 +870,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.
@@ -1018,7 +1024,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.45.2

Re: [PATCH 15/19] target/microblaze: Introduce mo_endian() helper

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> mo_endian() returns the target endianness, currently static.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/microblaze/translate.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)

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

r~

[PATCH 16/19] 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>
---
 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 3e5a3e5c605..6d540713eb5 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -412,6 +412,13 @@ void mb_tcg_init(void);
 /* 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 0b466db694c..5595ae4fadb 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -709,7 +709,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,
@@ -1646,7 +1646,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.45.2

Re: [PATCH 16/19] target/microblaze: Consider endianness while translating code

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>   target/microblaze/cpu.h       | 7 +++++++
>   target/microblaze/translate.c | 5 +++--
>   2 files changed, 10 insertions(+), 2 deletions(-)

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

r~

[PATCH 17/19] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 55 +++++++++++++++++++++---
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 8407dbee12a..f5195327b76 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.  */
@@ -123,33 +134,63 @@ petalogix_s3adsp1800_init(MachineState *machine)
     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);
-    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
+    qdev_prop_set_bit(dev, "access-little-endian", pmc->little_endian);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
 
     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_be(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
 
-    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
+    pmc->little_endian = false;
+    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
     mc->init = petalogix_s3adsp1800_init;
+#if TARGET_BIG_ENDIAN
     mc->is_default = true;
+    mc->alias = "petalogix-s3adsp1800";
+#endif
+}
+
+static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
+
+    pmc->little_endian = true;
+    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)";
+    mc->init = petalogix_s3adsp1800_init;
+#if !TARGET_BIG_ENDIAN
+    mc->is_default = true;
+    mc->alias = "petalogix-s3adsp1800";
+#endif
 }
 
 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.45.2

Re: [PATCH 17/19] hw/microblaze: Support various endianness for s3adsp1800 machines

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> -static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
> +static void petalogix_s3adsp1800_machine_class_init_be(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> +    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
>   
> -    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
> +    pmc->little_endian = false;
> +    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
>       mc->init = petalogix_s3adsp1800_init;
> +#if TARGET_BIG_ENDIAN
>       mc->is_default = true;
> +    mc->alias = "petalogix-s3adsp1800";
> +#endif
> +}
> +
> +static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
> +
> +    pmc->little_endian = true;
> +    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)";
> +    mc->init = petalogix_s3adsp1800_init;
> +#if !TARGET_BIG_ENDIAN
> +    mc->is_default = true;
> +    mc->alias = "petalogix-s3adsp1800";
> +#endif
>   }

These can be C if's, instead of preprocessor #if, at which point you can share code.

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->init = petalogix_s3adsp1800_init;
     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);
}

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


r~

[PATCH 18/19] 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>
---
 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 4f692ffdb1a..2b2f7822707 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -17,14 +17,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')
-        file_path = self.ASSET_IMAGE.fetch()
+        file_path = self.ASSET_IMAGE_BE.fetch()
         archive_extract(file_path, self.workdir)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.workdir + '/day17/ballerina.bin')
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index faa3927f2e9..1aee5149fb1 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')
-        file_path = self.ASSET_IMAGE.fetch()
+        file_path = self.ASSET_IMAGE_LE.fetch()
         archive_extract(file_path, self.workdir)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.workdir + '/day13/xmaton.bin')
-- 
2.45.2

Re: [PATCH 18/19] tests/functional: Explicit endianness of microblaze assets

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> 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>
> ---
>   tests/functional/test_microblaze_s3adsp1800.py   | 6 +++---
>   tests/functional/test_microblazeel_s3adsp1800.py | 6 +++---
>   2 files changed, 6 insertions(+), 6 deletions(-)

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

r~

[PATCH 19/19] tests/functional: Add microblaze cross-endianness tests

[Philippe Mathieu-Daudé]

Copy/paste the current tests, but call the opposite endianness
machines, testing:
- petalogix-s3adsp1800-le machine (little-endian CPU) on the
  qemu-system-microblaze binary (big-endian)
- petalogix-s3adsp1800-be machine (big-endian CPU) on the
  qemu-system-microblazeel binary (little-endian).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Clever refactor left for later
---
 .../functional/test_microblaze_s3adsp1800.py  | 21 +++++++++++++++++++
 .../test_microblazeel_s3adsp1800.py           | 19 +++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/tests/functional/test_microblaze_s3adsp1800.py b/tests/functional/test_microblaze_s3adsp1800.py
index 2b2f7822707..7f5e8b6024f 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -36,5 +36,26 @@ 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...
 
+    ASSET_IMAGE_LE = Asset(
+        ('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
+        'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
+
+    def test_microblaze_s3adsp1800_le(self):
+        self.require_netdev('user')
+        self.set_machine('petalogix-s3adsp1800-le')
+        file_path = self.ASSET_IMAGE_LE.fetch()
+        archive_extract(file_path, self.workdir)
+        self.vm.set_console()
+        self.vm.add_args('-kernel', self.workdir + '/day13/xmaton.bin')
+        self.vm.add_args('-nic', 'user,tftp=' + self.workdir + '/day13/')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
+        time.sleep(0.1)
+        exec_command(self, 'root')
+        time.sleep(0.1)
+        exec_command_and_wait_for_pattern(self,
+                'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
+                '821cd3cab8efd16ad6ee5acc3642a8ea')
+
 if __name__ == '__main__':
     QemuSystemTest.main()
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py b/tests/functional/test_microblazeel_s3adsp1800.py
index 1aee5149fb1..60543009bab 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -38,5 +38,24 @@ def test_microblazeel_s3adsp1800_le(self):
                 'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
                 '821cd3cab8efd16ad6ee5acc3642a8ea')
 
+    ASSET_IMAGE_BE = Asset(
+        ('https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/'
+         'day17.tar.xz'),
+        '3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
+
+    def test_microblazeel_s3adsp1800_be(self):
+        self.set_machine('petalogix-s3adsp1800-be')
+        file_path = self.ASSET_IMAGE_BE.fetch()
+        archive_extract(file_path, self.workdir)
+        self.vm.set_console()
+        self.vm.add_args('-kernel', self.workdir + '/day17/ballerina.bin')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'This architecture does not have '
+                                       'kernel memory protection')
+        # Note:
+        # The kernel sometimes gets stuck after the "This architecture ..."
+        # message, that's why we don't test for a later string here. This
+        # needs some investigation by a microblaze wizard one day...
+
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.45.2

Re: [PATCH 19/19] tests/functional: Add microblaze cross-endianness tests

[Richard Henderson]

On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
> Copy/paste the current tests, but call the opposite endianness
> machines, testing:
> - petalogix-s3adsp1800-le machine (little-endian CPU) on the
>    qemu-system-microblaze binary (big-endian)
> - petalogix-s3adsp1800-be machine (big-endian CPU) on the
>    qemu-system-microblazeel binary (little-endian).
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> Clever refactor left for later
> ---
>   .../functional/test_microblaze_s3adsp1800.py  | 21 +++++++++++++++++++
>   .../test_microblazeel_s3adsp1800.py           | 19 +++++++++++++++++
>   2 files changed, 40 insertions(+)

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

r~