[PATCH 0/5] hw: Mark architecture specific devices with specific endianness

[PATCH 0/5] hw: Mark architecture specific devices with specific endianness

[Philippe Mathieu-Daudé]

When a device is only built for an architecture built for
a particular endianness, we can simplify the device to not
use the "target native endianness" definition and directly
use the proper target endianness.

Philippe Mathieu-Daudé (5):
  hw/i386: Mark devices as little-endian
  hw/tricore: Mark devices as little-endian
  hw/m68k: Mark devices as big-endian
  hw/openrisc: Mark devices as big-endian
  hw/sparc: Mark devices as big-endian

 hw/i386/kvm/apic.c              | 2 +-
 hw/i386/pc.c                    | 4 ++--
 hw/i386/vapic.c                 | 2 +-
 hw/i386/xen/xen_apic.c          | 2 +-
 hw/i386/xen/xen_platform.c      | 2 +-
 hw/m68k/mcf5206.c               | 2 +-
 hw/m68k/mcf5208.c               | 6 +++---
 hw/m68k/mcf_intc.c              | 2 +-
 hw/m68k/next-kbd.c              | 2 +-
 hw/openrisc/openrisc_sim.c      | 2 +-
 hw/openrisc/virt.c              | 2 +-
 hw/sparc/sun4m_iommu.c          | 2 +-
 hw/sparc64/sun4u.c              | 2 +-
 hw/tricore/tricore_testdevice.c | 2 +-
 14 files changed, 17 insertions(+), 17 deletions(-)

-- 
2.45.2

[PATCH 1/5] hw/i386: Mark devices as little-endian

[Philippe Mathieu-Daudé]

These devices are only used by the X86 targets, which are only
built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
definition expand to DEVICE_LITTLE_ENDIAN (besides, the
DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
DEVICE_LITTLE_ENDIAN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/kvm/apic.c         | 2 +-
 hw/i386/pc.c               | 4 ++--
 hw/i386/vapic.c            | 2 +-
 hw/i386/xen/xen_apic.c     | 2 +-
 hw/i386/xen/xen_platform.c | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index a72c28e8a7..3a5d69e8d0 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -214,7 +214,7 @@ static void kvm_apic_mem_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps kvm_apic_io_ops = {
     .read = kvm_apic_mem_read,
     .write = kvm_apic_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void kvm_apic_reset(APICCommonState *s)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 830614d930..2259589e0d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1064,7 +1064,7 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
 static const MemoryRegionOps ioport80_io_ops = {
     .write = ioport80_write,
     .read = ioport80_read,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 1,
         .max_access_size = 1,
@@ -1074,7 +1074,7 @@ static const MemoryRegionOps ioport80_io_ops = {
 static const MemoryRegionOps ioportF0_io_ops = {
     .write = ioportF0_write,
     .read = ioportF0_read,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 1,
         .max_access_size = 1,
diff --git a/hw/i386/vapic.c b/hw/i386/vapic.c
index ef7f8b967f..fd906fdf78 100644
--- a/hw/i386/vapic.c
+++ b/hw/i386/vapic.c
@@ -718,7 +718,7 @@ static uint64_t vapic_read(void *opaque, hwaddr addr, unsigned size)
 static const MemoryRegionOps vapic_ops = {
     .write = vapic_write,
     .read = vapic_read,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void vapic_realize(DeviceState *dev, Error **errp)
diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c
index 101e16a766..a94e9005cb 100644
--- a/hw/i386/xen/xen_apic.c
+++ b/hw/i386/xen/xen_apic.c
@@ -36,7 +36,7 @@ static void xen_apic_mem_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps xen_apic_io_ops = {
     .read = xen_apic_mem_read,
     .write = xen_apic_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void xen_apic_realize(DeviceState *dev, Error **errp)
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index ec0e536e85..3b3e745822 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -514,7 +514,7 @@ static void platform_mmio_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps platform_mmio_handler = {
     .read = &platform_mmio_read,
     .write = &platform_mmio_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void platform_mmio_setup(PCIXenPlatformState *d)
-- 
2.45.2

Re: [PATCH 1/5] hw/i386: Mark devices as little-endian

[Richard Henderson]

On 11/6/24 18:46, Philippe Mathieu-Daudé wrote:
> These devices are only used by the X86 targets, which are only
> built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_LITTLE_ENDIAN (besides, the
> DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
> DEVICE_LITTLE_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i386/kvm/apic.c         | 2 +-
>   hw/i386/pc.c               | 4 ++--
>   hw/i386/vapic.c            | 2 +-
>   hw/i386/xen/xen_apic.c     | 2 +-
>   hw/i386/xen/xen_platform.c | 2 +-
>   5 files changed, 6 insertions(+), 6 deletions(-)

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

r~

Re: [PATCH 1/5] hw/i386: Mark devices as little-endian

[Thomas Huth]

Am Wed,  6 Nov 2024 18:46:08 +0000
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> These devices are only used by the X86 targets, which are only
> built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_LITTLE_ENDIAN (besides, the
> DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
> DEVICE_LITTLE_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/i386/kvm/apic.c         | 2 +-
>  hw/i386/pc.c               | 4 ++--
>  hw/i386/vapic.c            | 2 +-
>  hw/i386/xen/xen_apic.c     | 2 +-
>  hw/i386/xen/xen_platform.c | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <huth@tuxfamily.org>

[PATCH 2/5] hw/tricore: Mark devices as little-endian

[Philippe Mathieu-Daudé]

These devices are only used by the TriCore target, which is
only built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
definition expand to DEVICE_LITTLE_ENDIAN (besides, the
DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
DEVICE_LITTLE_ENDIAN.

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

diff --git a/hw/tricore/tricore_testdevice.c b/hw/tricore/tricore_testdevice.c
index ae95c49565..169f5907e9 100644
--- a/hw/tricore/tricore_testdevice.c
+++ b/hw/tricore/tricore_testdevice.c
@@ -47,7 +47,7 @@ static const MemoryRegionOps tricore_testdevice_ops = {
         .min_access_size = 4,
         .max_access_size = 4,
     },
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void tricore_testdevice_init(Object *obj)
-- 
2.45.2

Re: [PATCH 2/5] hw/tricore: Mark devices as little-endian

[Richard Henderson]

On 11/6/24 18:46, Philippe Mathieu-Daudé wrote:
> These devices are only used by the TriCore target, which is
> only built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_LITTLE_ENDIAN (besides, the
> DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
> DEVICE_LITTLE_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/tricore/tricore_testdevice.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

Re: [PATCH 2/5] hw/tricore: Mark devices as little-endian

[Thomas Huth]

Am Wed,  6 Nov 2024 18:46:09 +0000
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> These devices are only used by the TriCore target, which is
> only built as little-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_LITTLE_ENDIAN (besides, the
> DEVICE_BIG_ENDIAN case isn't tested). Simplify directly using
> DEVICE_LITTLE_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/tricore/tricore_testdevice.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Thomas Huth <huth@tuxfamily.org>

[PATCH 3/5] hw/m68k: Mark devices as big-endian

[Philippe Mathieu-Daudé]

These devices are only used by the M68K target, which is only
built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
definition expand to DEVICE_BIG_ENDIAN (besides, the
DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
using DEVICE_BIG_ENDIAN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/mcf5206.c  | 2 +-
 hw/m68k/mcf5208.c  | 6 +++---
 hw/m68k/mcf_intc.c | 2 +-
 hw/m68k/next-kbd.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index 7247cdbe5e..4892924fc2 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -582,7 +582,7 @@ static const MemoryRegionOps m5206_mbar_ops = {
     .write = m5206_mbar_writefn,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static void mcf5206_mbar_realize(DeviceState *dev, Error **errp)
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index e37cd50d18..4166877066 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -155,7 +155,7 @@ static uint64_t m5208_timer_read(void *opaque, hwaddr addr,
 static const MemoryRegionOps m5208_timer_ops = {
     .read = m5208_timer_read,
     .write = m5208_timer_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static uint64_t m5208_sys_read(void *opaque, hwaddr addr,
@@ -192,7 +192,7 @@ static void m5208_sys_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps m5208_sys_ops = {
     .read = m5208_sys_read,
     .write = m5208_sys_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
@@ -224,7 +224,7 @@ static void m5208_rcm_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps m5208_rcm_ops = {
     .read = m5208_rcm_read,
     .write = m5208_rcm_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 9fc30b03ba..5f38c49077 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -166,7 +166,7 @@ static void mcf_intc_reset(DeviceState *dev)
 static const MemoryRegionOps mcf_intc_ops = {
     .read = mcf_intc_read,
     .write = mcf_intc_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static void mcf_intc_instance_init(Object *obj)
diff --git a/hw/m68k/next-kbd.c b/hw/m68k/next-kbd.c
index bc67810f31..4d87edc2de 100644
--- a/hw/m68k/next-kbd.c
+++ b/hw/m68k/next-kbd.c
@@ -163,7 +163,7 @@ static const MemoryRegionOps kbd_ops = {
     .write = kbd_writefn,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static void nextkbd_event(void *opaque, int ch)
-- 
2.45.2

Re: [PATCH 3/5] hw/m68k: Mark devices as big-endian

[Richard Henderson]

On 11/6/24 18:46, Philippe Mathieu-Daudé wrote:
> These devices are only used by the M68K target, which is only
> built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/m68k/mcf5206.c  | 2 +-
>   hw/m68k/mcf5208.c  | 6 +++---
>   hw/m68k/mcf_intc.c | 2 +-
>   hw/m68k/next-kbd.c | 2 +-
>   4 files changed, 6 insertions(+), 6 deletions(-)

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

r~

Re: [PATCH 3/5] hw/m68k: Mark devices as big-endian

[Thomas Huth]

Am Wed,  6 Nov 2024 18:46:10 +0000
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> These devices are only used by the M68K target, which is only
> built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/m68k/mcf5206.c  | 2 +-
>  hw/m68k/mcf5208.c  | 6 +++---
>  hw/m68k/mcf_intc.c | 2 +-
>  hw/m68k/next-kbd.c | 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <huth@tuxfamily.org>

[PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Philippe Mathieu-Daudé]

These devices are only used by the OpenRISC target, which is
only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
definition expand to DEVICE_BIG_ENDIAN (besides, the
DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
using DEVICE_BIG_ENDIAN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/openrisc/openrisc_sim.c | 2 +-
 hw/openrisc/virt.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 9fb63515ef..794c175bdb 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -266,7 +266,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base,
     }
     serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
                    serial_hd(OR1KSIM_UART_COUNT - uart_idx - 1),
-                   DEVICE_NATIVE_ENDIAN);
+                   DEVICE_BIG_ENDIAN);
 
     /* Add device tree node for serial. */
     nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
index 47d2c9bd3c..ede57fe391 100644
--- a/hw/openrisc/virt.c
+++ b/hw/openrisc/virt.c
@@ -236,7 +236,7 @@ static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr base,
     qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
 
     serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
-                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
+                   serial_hd(0), DEVICE_BIG_ENDIAN);
 
     /* Add device tree node for serial. */
     nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
-- 
2.45.2

Re: [PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Richard Henderson]

On 11/6/24 18:46, Philippe Mathieu-Daudé wrote:
> These devices are only used by the OpenRISC target, which is
> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/openrisc/openrisc_sim.c | 2 +-
>   hw/openrisc/virt.c         | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

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


r~

Re: [PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Thomas Huth]

Am Wed,  6 Nov 2024 18:46:11 +0000
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> These devices are only used by the OpenRISC target, which is
> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/openrisc/openrisc_sim.c | 2 +-
>  hw/openrisc/virt.c         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index 9fb63515ef..794c175bdb 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -266,7 +266,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base,
>      }
>      serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
>                     serial_hd(OR1KSIM_UART_COUNT - uart_idx - 1),
> -                   DEVICE_NATIVE_ENDIAN);
> +                   DEVICE_BIG_ENDIAN);
>  
>      /* Add device tree node for serial. */
>      nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> index 47d2c9bd3c..ede57fe391 100644
> --- a/hw/openrisc/virt.c
> +++ b/hw/openrisc/virt.c
> @@ -236,7 +236,7 @@ static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr base,
>      qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
>  
>      serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> -                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
> +                   serial_hd(0), DEVICE_BIG_ENDIAN);
>  
>      /* Add device tree node for serial. */
>      nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);

According to https://openrisc.io/or1k.html the openrisc CPU could be
implemented as little endian, too ... so would it make sense to use
a runtime detected value here instead?

 Thomas

Re: [PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Philippe Mathieu-Daudé]

Hi Thomas,

On 9/11/24 06:42, Thomas Huth wrote:
> Am Wed,  6 Nov 2024 18:46:11 +0000
> schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:
> 
>> These devices are only used by the OpenRISC target, which is
>> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
>> definition expand to DEVICE_BIG_ENDIAN (besides, the
>> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
>> using DEVICE_BIG_ENDIAN.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/openrisc/openrisc_sim.c | 2 +-
>>   hw/openrisc/virt.c         | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)


>> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
>> index 47d2c9bd3c..ede57fe391 100644
>> --- a/hw/openrisc/virt.c
>> +++ b/hw/openrisc/virt.c
>> @@ -236,7 +236,7 @@ static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr base,
>>       qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
>>   
>>       serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
>> -                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
>> +                   serial_hd(0), DEVICE_BIG_ENDIAN);
>>   
>>       /* Add device tree node for serial. */
>>       nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
> 
> According to https://openrisc.io/or1k.html the openrisc CPU could be
> implemented as little endian, too ... so would it make sense to use
> a runtime detected value here instead?

While this patch is a code change, it aims to not introduce any
functional change. We are not building (nor testing) these devices
in a little endian configuration:

$ git grep -l TARGET_BIG_ENDIAN configs/targets/*softmmu*
configs/targets/hppa-softmmu.mak
configs/targets/m68k-softmmu.mak
configs/targets/microblaze-softmmu.mak
configs/targets/mips-softmmu.mak
configs/targets/mips64-softmmu.mak
configs/targets/or1k-softmmu.mak
                 ^^^^
configs/targets/ppc-softmmu.mak
configs/targets/ppc64-softmmu.mak
configs/targets/s390x-softmmu.mak
configs/targets/sh4eb-softmmu.mak
configs/targets/sparc-softmmu.mak
configs/targets/sparc64-softmmu.mak
configs/targets/xtensaeb-softmmu.mak

$ git grep -L TARGET_BIG_ENDIAN configs/targets/*softmmu*
configs/targets/aarch64-softmmu.mak
configs/targets/alpha-softmmu.mak
configs/targets/arm-softmmu.mak
configs/targets/avr-softmmu.mak
configs/targets/i386-softmmu.mak
configs/targets/loongarch64-softmmu.mak
configs/targets/microblazeel-softmmu.mak
configs/targets/mips64el-softmmu.mak
configs/targets/mipsel-softmmu.mak
configs/targets/riscv32-softmmu.mak
configs/targets/riscv64-softmmu.mak
configs/targets/rx-softmmu.mak
configs/targets/sh4-softmmu.mak
configs/targets/tricore-softmmu.mak
configs/targets/x86_64-softmmu.mak
configs/targets/xtensa-softmmu.mak

(no little-endian config here)

Having little-endian OpenRISC is certainly welcomed, but it
has to be done separately, preferably adding test coverage.

Should I rework the commit description to be more precise?

Regards,

Phil.

Re: [PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Richard Henderson]

On 11/9/24 07:58, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 9/11/24 06:42, Thomas Huth wrote:
>> Am Wed,  6 Nov 2024 18:46:11 +0000
>> schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:
>>
>>> These devices are only used by the OpenRISC target, which is
>>> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
>>> definition expand to DEVICE_BIG_ENDIAN (besides, the
>>> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
>>> using DEVICE_BIG_ENDIAN.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/openrisc/openrisc_sim.c | 2 +-
>>>   hw/openrisc/virt.c         | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
>>> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
>>> index 47d2c9bd3c..ede57fe391 100644
>>> --- a/hw/openrisc/virt.c
>>> +++ b/hw/openrisc/virt.c
>>> @@ -236,7 +236,7 @@ static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr 
>>> base,
>>>       qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
>>>       serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
>>> -                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
>>> +                   serial_hd(0), DEVICE_BIG_ENDIAN);
>>>       /* Add device tree node for serial. */
>>>       nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
>>
>> According to https://openrisc.io/or1k.html the openrisc CPU could be
>> implemented as little endian, too ... so would it make sense to use
>> a runtime detected value here instead?
> 
> While this patch is a code change, it aims to not introduce any
> functional change. We are not building (nor testing) these devices
> in a little endian configuration:
> 
> $ git grep -l TARGET_BIG_ENDIAN configs/targets/*softmmu*
> configs/targets/hppa-softmmu.mak
> configs/targets/m68k-softmmu.mak
> configs/targets/microblaze-softmmu.mak
> configs/targets/mips-softmmu.mak
> configs/targets/mips64-softmmu.mak
> configs/targets/or1k-softmmu.mak
>                  ^^^^

The openrisc little-endian control is in a control register: SR[LEE] (which we do not 
implement at present).

So any openrisc little-endian support would look like qemu-system-ppc64.  I would not 
expect devices to switch endianness at all.


r~

Re: [PATCH 4/5] hw/openrisc: Mark devices as big-endian

[Thomas Huth]

Am Sat, 9 Nov 2024 10:08:16 -0800
schrieb Richard Henderson <richard.henderson@linaro.org>:

> On 11/9/24 07:58, Philippe Mathieu-Daudé wrote:
> > Hi Thomas,
> > 
> > On 9/11/24 06:42, Thomas Huth wrote:  
> >> Am Wed,  6 Nov 2024 18:46:11 +0000
> >> schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:
> >>  
> >>> These devices are only used by the OpenRISC target, which is
> >>> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> >>> definition expand to DEVICE_BIG_ENDIAN (besides, the
> >>> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> >>> using DEVICE_BIG_ENDIAN.
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>> ---
> >>>   hw/openrisc/openrisc_sim.c | 2 +-
> >>>   hw/openrisc/virt.c         | 2 +-
> >>>   2 files changed, 2 insertions(+), 2 deletions(-)  
> > 
> >   
> >>> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> >>> index 47d2c9bd3c..ede57fe391 100644
> >>> --- a/hw/openrisc/virt.c
> >>> +++ b/hw/openrisc/virt.c
> >>> @@ -236,7 +236,7 @@ static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr 
> >>> base,
> >>>       qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
> >>>       serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> >>> -                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
> >>> +                   serial_hd(0), DEVICE_BIG_ENDIAN);
> >>>       /* Add device tree node for serial. */
> >>>       nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);  
> >>
> >> According to https://openrisc.io/or1k.html the openrisc CPU could be
> >> implemented as little endian, too ... so would it make sense to use
> >> a runtime detected value here instead?  
> > 
> > While this patch is a code change, it aims to not introduce any
> > functional change. We are not building (nor testing) these devices
> > in a little endian configuration:
> > 
> > $ git grep -l TARGET_BIG_ENDIAN configs/targets/*softmmu*
> > configs/targets/hppa-softmmu.mak
> > configs/targets/m68k-softmmu.mak
> > configs/targets/microblaze-softmmu.mak
> > configs/targets/mips-softmmu.mak
> > configs/targets/mips64-softmmu.mak
> > configs/targets/or1k-softmmu.mak
> >                  ^^^^  
> 
> The openrisc little-endian control is in a control register: SR[LEE] (which we do not 
> implement at present).
> 
> So any openrisc little-endian support would look like qemu-system-ppc64.  I would not 
> expect devices to switch endianness at all.

Ok, thanks, in that case, I think the patch is fine.

FWIW:
Reviewed-by: Thomas Huth <huth@tuxfamily.org>

[PATCH 5/5] hw/sparc: Mark devices as big-endian

[Philippe Mathieu-Daudé]

These devices are only used by the SPARC targets, which are
only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
definition expand to DEVICE_BIG_ENDIAN (besides, the
DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
using DEVICE_BIG_ENDIAN.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sparc/sun4m_iommu.c | 2 +-
 hw/sparc64/sun4u.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sparc/sun4m_iommu.c b/hw/sparc/sun4m_iommu.c
index 6f765e97e4..4486508e3b 100644
--- a/hw/sparc/sun4m_iommu.c
+++ b/hw/sparc/sun4m_iommu.c
@@ -238,7 +238,7 @@ static void iommu_mem_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps iommu_mem_ops = {
     .read = iommu_mem_read,
     .write = iommu_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 541c7f74fa..9d7aebf632 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -254,7 +254,7 @@ static void power_mem_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps power_mem_ops = {
     .read = power_mem_read,
     .write = power_mem_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
-- 
2.45.2

Re: [PATCH 5/5] hw/sparc: Mark devices as big-endian

[Mark Cave-Ayland]

On 06/11/2024 18:46, Philippe Mathieu-Daudé wrote:

> These devices are only used by the SPARC targets, which are
> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sparc/sun4m_iommu.c | 2 +-
>   hw/sparc64/sun4u.c     | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sparc/sun4m_iommu.c b/hw/sparc/sun4m_iommu.c
> index 6f765e97e4..4486508e3b 100644
> --- a/hw/sparc/sun4m_iommu.c
> +++ b/hw/sparc/sun4m_iommu.c
> @@ -238,7 +238,7 @@ static void iommu_mem_write(void *opaque, hwaddr addr,
>   static const MemoryRegionOps iommu_mem_ops = {
>       .read = iommu_mem_read,
>       .write = iommu_mem_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,
>       .valid = {
>           .min_access_size = 4,
>           .max_access_size = 4,
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 541c7f74fa..9d7aebf632 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -254,7 +254,7 @@ static void power_mem_write(void *opaque, hwaddr addr,
>   static const MemoryRegionOps power_mem_ops = {
>       .read = power_mem_read,
>       .write = power_mem_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,
>       .valid = {
>           .min_access_size = 4,
>           .max_access_size = 4,

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 5/5] hw/sparc: Mark devices as big-endian

[Richard Henderson]

On 11/6/24 18:46, Philippe Mathieu-Daudé wrote:
> These devices are only used by the SPARC targets, which are
> only built as big-endian. Therefore the DEVICE_NATIVE_ENDIAN
> definition expand to DEVICE_BIG_ENDIAN (besides, the
> DEVICE_LITTLE_ENDIAN case isn't tested). Simplify directly
> using DEVICE_BIG_ENDIAN.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/sparc/sun4m_iommu.c | 2 +-
>   hw/sparc64/sun4u.c     | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

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

r~

Re: [PATCH 0/5] hw: Mark architecture specific devices with specific endianness

[Philippe Mathieu-Daudé]

On 6/11/24 19:46, Philippe Mathieu-Daudé wrote:
> When a device is only built for an architecture built for
> a particular endianness, we can simplify the device to not
> use the "target native endianness" definition and directly
> use the proper target endianness.
> 
> Philippe Mathieu-Daudé (5):
>    hw/i386: Mark devices as little-endian
>    hw/tricore: Mark devices as little-endian
>    hw/m68k: Mark devices as big-endian
>    hw/openrisc: Mark devices as big-endian
>    hw/sparc: Mark devices as big-endian

I'm queuing this series, better to test it early in the dev cycle.