[PATCH 00/13] hw: Add ld/st_endian() APIs

[PATCH 00/13] hw: Add ld/st_endian() APIs

[Philippe Mathieu-Daudé]

In preparation of heterogeneous machines, remove knowledge of the
target endianness in generic hw/ code. Move it to the machine level.

Philippe Mathieu-Daudé (13):
  qemu/bswap: Introduce ld/st_endian_p() API
  hw/virtio/virtio-access: Use the ld/st_endian_p() API
  target/arm/ptw: Use the ld/st_endian_p() API
  hw/mips: Pass BlCpuCfg argument to bootloader API
  hw/mips: Add cpu_is_bigendian field to BlCpuCfg structure
  tests/tcg/plugins: Use the ld/st_endian_p() API
  hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry
  hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()
  exec/memory_ldst_phys: Introduce ld/st_endian_phys() API
  hw/virtio/virtio-access: Use ld/st_endian_phys() API
  hw/pci/pci_device: Add PCI_DMA_DEFINE_LDST_END() macro
  hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API
  hw/net/tulip: Use ld/st_endian_pci_dma() API

 include/hw/mips/bootloader.h        |  18 +++-
 include/hw/pci/pci_device.h         |  35 +++++--
 include/hw/virtio/virtio-access.h   |  63 ++----------
 include/qemu/bswap.h                |  19 ++++
 include/exec/memory_ldst_phys.h.inc |  66 ++++++++++++
 hw/mips/bootloader.c                | 152 +++++++++++++++-------------
 hw/mips/boston.c                    |   9 +-
 hw/mips/fuloong2e.c                 |   3 +-
 hw/mips/malta.c                     |  21 ++--
 hw/net/tulip.c                      |  32 ++----
 hw/xtensa/xtfpga.c                  |  18 ++--
 target/arm/ptw.c                    |  19 +---
 tests/tcg/plugins/mem.c             |  24 ++---
 13 files changed, 269 insertions(+), 210 deletions(-)

-- 
2.45.2

[PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Philippe Mathieu-Daudé]

Introduce the ld/st_endian_p() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_p() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update docstring regexp
---
 include/qemu/bswap.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ad22910a5d..ec813a756d 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
 #undef le_bswaps
 #undef be_bswaps
 
+#define lduw_endian_p(big_endian, p) \
+                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
+#define ldsw_endian_p(big_endian, p) \
+                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
+#define ldl_endian_p(big_endian, p) \
+                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
+#define ldq_endian_p(big_endian, p) \
+                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
+#define stw_endian_p(big_endian, p, v) \
+                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
+#define stl_endian_p(big_endian, p, v) \
+                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
+#define stq_endian_p(big_endian, p, v) \
+                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
+#define ldn_endian_p(big_endian, p, sz) \
+                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
+#define stn_endian_p(big_endian, p, sz, v) \
+                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
+
 #endif /* BSWAP_H */
-- 
2.45.2

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_p() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_p() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/qemu/bswap.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index ad22910a5d..ec813a756d 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
>   #undef le_bswaps
>   #undef be_bswaps
>   
> +#define lduw_endian_p(big_endian, p) \
> +                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
> +#define ldsw_endian_p(big_endian, p) \
> +                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
> +#define ldl_endian_p(big_endian, p) \
> +                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
> +#define ldq_endian_p(big_endian, p) \
> +                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
> +#define stw_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
> +#define stl_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
> +#define stq_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
> +#define ldn_endian_p(big_endian, p, sz) \
> +                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
> +#define stn_endian_p(big_endian, p, sz, v) \
> +                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
> +
>   #endif /* BSWAP_H */

May it be useful to have extra parenthesis around macro value to prevent 
any issue when using it?
((big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v))

Else,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Philippe Mathieu-Daudé]

On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_p() API, which takes an extra

Alternatively we could use ld/st_te_p() since we already
have ld/st_he_p() for host endianness.

> boolean argument to dispatch to ld/st_{be,le}_p() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/qemu/bswap.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index ad22910a5d..ec813a756d 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
>   #undef le_bswaps
>   #undef be_bswaps
>   
> +#define lduw_endian_p(big_endian, p) \
> +                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
> +#define ldsw_endian_p(big_endian, p) \
> +                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
> +#define ldl_endian_p(big_endian, p) \
> +                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
> +#define ldq_endian_p(big_endian, p) \
> +                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
> +#define stw_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
> +#define stl_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
> +#define stq_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
> +#define ldn_endian_p(big_endian, p, sz) \
> +                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
> +#define stn_endian_p(big_endian, p, sz, v) \
> +                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
> +
>   #endif /* BSWAP_H */

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Richard Henderson]

On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>> Introduce the ld/st_endian_p() API, which takes an extra
> 
> Alternatively we could use ld/st_te_p() since we already
> have ld/st_he_p() for host endianness.

That's what ld/st_p are -- target-specific, in exec/cpu-all.h.


r~

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Philippe Mathieu-Daudé]

On 3/10/24 23:28, Richard Henderson wrote:
> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>> Introduce the ld/st_endian_p() API, which takes an extra
>>
>> Alternatively we could use ld/st_te_p() since we already
>> have ld/st_he_p() for host endianness.
> 
> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.

They are indeed *target-specific*, so we can not use them in
target-agnostic code.

By explicitly passing the endianness, ld/st_endian_p() API is
target-agnostic.

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Richard Henderson]

On 10/3/24 14:34, Philippe Mathieu-Daudé wrote:
> On 3/10/24 23:28, Richard Henderson wrote:
>> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>>> Introduce the ld/st_endian_p() API, which takes an extra
>>>
>>> Alternatively we could use ld/st_te_p() since we already
>>> have ld/st_he_p() for host endianness.
>>
>> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.
> 
> They are indeed *target-specific*, so we can not use them in
> target-agnostic code.
> 
> By explicitly passing the endianness, ld/st_endian_p() API is
> target-agnostic.

Then I miss whatever you meant here re st_te_p().
Care to elaborate?


r~

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

[Philippe Mathieu-Daudé]

On 3/10/24 23:37, Richard Henderson wrote:
> On 10/3/24 14:34, Philippe Mathieu-Daudé wrote:
>> On 3/10/24 23:28, Richard Henderson wrote:
>>> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>>>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>>>> Introduce the ld/st_endian_p() API, which takes an extra
>>>>
>>>> Alternatively we could use ld/st_te_p() since we already
>>>> have ld/st_he_p() for host endianness.
>>>
>>> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.
>>
>> They are indeed *target-specific*, so we can not use them in
>> target-agnostic code.
>>
>> By explicitly passing the endianness, ld/st_endian_p() API is
>> target-agnostic.
> 
> Then I miss whatever you meant here re st_te_p().
> Care to elaborate?

I might had a bad start by adding this now endian-agnostic API
before removing the current endian-specific one.

Goal is instead of having machine code build twice, one for each
endianness, the same machine will be built once, but registering
2x machines. Endianness being a machine property, propagated to
the vCPUs and HW.

Instead of the following target-specific API:

   #if TARGET_BIG_ENDIAN
   #define stl_p(p, v) stl_be_p(p, v)
   #else
   #define stl_p(p, v) stl_le_p(p, v)
   #endif

I'm suggesting this target-agnostic one:

   #define stl_endian_p(big_endian, p, v) \
                       (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)

[PATCH 02/13] hw/virtio/virtio-access: Use the ld/st_endian_p() API

[Philippe Mathieu-Daudé]

Refactor to use the recently introduced ld/st_endian_p() API
No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-access.h | 36 ++++++-------------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 07aae69042..b920874be8 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -95,56 +95,32 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
 
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        stw_be_p(ptr, v);
-    } else {
-        stw_le_p(ptr, v);
-    }
+    stw_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
 }
 
 static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        stl_be_p(ptr, v);
-    } else {
-        stl_le_p(ptr, v);
-    }
+    stl_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
 }
 
 static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        stq_be_p(ptr, v);
-    } else {
-        stq_le_p(ptr, v);
-    }
+    stq_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
 }
 
 static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        return lduw_be_p(ptr);
-    } else {
-        return lduw_le_p(ptr);
-    }
+    return lduw_endian_p(virtio_access_is_big_endian(vdev), ptr);
 }
 
 static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        return ldl_be_p(ptr);
-    } else {
-        return ldl_le_p(ptr);
-    }
+    return ldl_endian_p(virtio_access_is_big_endian(vdev), ptr);
 }
 
 static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
 {
-    if (virtio_access_is_big_endian(vdev)) {
-        return ldq_be_p(ptr);
-    } else {
-        return ldq_le_p(ptr);
-    }
+    return ldq_endian_p(virtio_access_is_big_endian(vdev), ptr);
 }
 
 static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
-- 
2.45.2

Re: [PATCH 02/13] hw/virtio/virtio-access: Use the ld/st_endian_p() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_p() API
> No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/virtio/virtio-access.h | 36 ++++++-------------------------
>   1 file changed, 6 insertions(+), 30 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index 07aae69042..b920874be8 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -95,56 +95,32 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
>   
>   static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        stw_be_p(ptr, v);
> -    } else {
> -        stw_le_p(ptr, v);
> -    }
> +    stw_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
>   }
>   
>   static inline void virtio_stl_p(VirtIODevice *vdev, void *ptr, uint32_t v)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        stl_be_p(ptr, v);
> -    } else {
> -        stl_le_p(ptr, v);
> -    }
> +    stl_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
>   }
>   
>   static inline void virtio_stq_p(VirtIODevice *vdev, void *ptr, uint64_t v)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        stq_be_p(ptr, v);
> -    } else {
> -        stq_le_p(ptr, v);
> -    }
> +    stq_endian_p(virtio_access_is_big_endian(vdev), ptr, v);
>   }
>   
>   static inline int virtio_lduw_p(VirtIODevice *vdev, const void *ptr)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return lduw_be_p(ptr);
> -    } else {
> -        return lduw_le_p(ptr);
> -    }
> +    return lduw_endian_p(virtio_access_is_big_endian(vdev), ptr);
>   }
>   
>   static inline int virtio_ldl_p(VirtIODevice *vdev, const void *ptr)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return ldl_be_p(ptr);
> -    } else {
> -        return ldl_le_p(ptr);
> -    }
> +    return ldl_endian_p(virtio_access_is_big_endian(vdev), ptr);
>   }
>   
>   static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
>   {
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return ldq_be_p(ptr);
> -    } else {
> -        return ldq_le_p(ptr);
> -    }
> +    return ldq_endian_p(virtio_access_is_big_endian(vdev), ptr);
>   }
>   
>   static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 03/13] target/arm/ptw: Use the ld/st_endian_p() API

[Philippe Mathieu-Daudé]

Refactor to use the recently introduced ld/st_endian_p() API
No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/ptw.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index defd6b84de..a1a6b1fec3 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -699,11 +699,7 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw,
             data = le64_to_cpu(data);
         }
 #else
-        if (ptw->out_be) {
-            data = ldq_be_p(host);
-        } else {
-            data = ldq_le_p(host);
-        }
+        data = ldq_endian_p(ptw->out_be, host);
 #endif
     } else {
         /* Page tables are in MMIO. */
@@ -860,16 +856,9 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,
     if (!locked) {
         bql_lock();
     }
-    if (ptw->out_be) {
-        cur_val = ldq_be_p(host);
-        if (cur_val == old_val) {
-            stq_be_p(host, new_val);
-        }
-    } else {
-        cur_val = ldq_le_p(host);
-        if (cur_val == old_val) {
-            stq_le_p(host, new_val);
-        }
+    cur_val = ldq_endian_p(ptw->out_be, host);
+    if (cur_val == old_val) {
+        stq_endian_p(ptw->out_be, host, new_val);
     }
     if (!locked) {
         bql_unlock();
-- 
2.45.2

Re: [PATCH 03/13] target/arm/ptw: Use the ld/st_endian_p() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_p() API
> No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/ptw.c | 19 ++++---------------
>   1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index defd6b84de..a1a6b1fec3 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -699,11 +699,7 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw,
>               data = le64_to_cpu(data);
>           }
>   #else
> -        if (ptw->out_be) {
> -            data = ldq_be_p(host);
> -        } else {
> -            data = ldq_le_p(host);
> -        }
> +        data = ldq_endian_p(ptw->out_be, host);
>   #endif
>       } else {
>           /* Page tables are in MMIO. */
> @@ -860,16 +856,9 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,
>       if (!locked) {
>           bql_lock();
>       }
> -    if (ptw->out_be) {
> -        cur_val = ldq_be_p(host);
> -        if (cur_val == old_val) {
> -            stq_be_p(host, new_val);
> -        }
> -    } else {
> -        cur_val = ldq_le_p(host);
> -        if (cur_val == old_val) {
> -            stq_le_p(host, new_val);
> -        }
> +    cur_val = ldq_endian_p(ptw->out_be, host);
> +    if (cur_val == old_val) {
> +        stq_endian_p(ptw->out_be, host, new_val);
>       }
>       if (!locked) {
>           bql_unlock();

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 04/13] hw/mips: Pass BlCpuCfg argument to bootloader API

[Philippe Mathieu-Daudé]

In preparation to pass endianness and target word size to
the bootloader API, introduce an empty BlCpuCfg structure
and propagate it to the MIPS bootloader methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/mips/bootloader.h |  17 +++--
 hw/mips/bootloader.c         | 142 +++++++++++++++++++----------------
 hw/mips/boston.c             |   9 ++-
 hw/mips/fuloong2e.c          |   3 +-
 hw/mips/malta.c              |  21 +++---
 5 files changed, 109 insertions(+), 83 deletions(-)

diff --git a/include/hw/mips/bootloader.h b/include/hw/mips/bootloader.h
index c32f6c2835..744eb11d0e 100644
--- a/include/hw/mips/bootloader.h
+++ b/include/hw/mips/bootloader.h
@@ -10,17 +10,24 @@
 #define HW_MIPS_BOOTLOADER_H
 
 #include "exec/cpu-defs.h"
+#include "exec/target_long.h"
 
-void bl_gen_jump_to(void **ptr, target_ulong jump_addr);
-void bl_gen_jump_kernel(void **ptr,
+typedef struct bl_cpu_cfg {
+} BlCpuCfg;
+
+void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr);
+void bl_gen_jump_kernel(const BlCpuCfg *cfg, void **ptr,
                         bool set_sp, target_ulong sp,
                         bool set_a0, target_ulong a0,
                         bool set_a1, target_ulong a1,
                         bool set_a2, target_ulong a2,
                         bool set_a3, target_ulong a3,
                         target_ulong kernel_addr);
-void bl_gen_write_ulong(void **ptr, target_ulong addr, target_ulong val);
-void bl_gen_write_u32(void **ptr, target_ulong addr, uint32_t val);
-void bl_gen_write_u64(void **ptr, target_ulong addr, uint64_t val);
+void bl_gen_write_ulong(const BlCpuCfg *cfg, void **ptr,
+                        target_ulong addr, target_ulong val);
+void bl_gen_write_u32(const BlCpuCfg *cfg, void **ptr,
+                      target_ulong addr, uint32_t val);
+void bl_gen_write_u64(const BlCpuCfg *cfg, void **ptr,
+                      target_ulong addr, uint64_t val);
 
 #endif
diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
index 1dd6ef2096..ee1a1c4f20 100644
--- a/hw/mips/bootloader.c
+++ b/hw/mips/bootloader.c
@@ -54,7 +54,7 @@ static bool bootcpu_supports_isa(uint64_t isa_mask)
     return cpu_supports_isa(&MIPS_CPU(first_cpu)->env, isa_mask);
 }
 
-static void st_nm32_p(void **ptr, uint32_t insn)
+static void st_nm32_p(const BlCpuCfg *cfg, void **ptr, uint32_t insn)
 {
     uint16_t *p = *ptr;
 
@@ -67,10 +67,10 @@ static void st_nm32_p(void **ptr, uint32_t insn)
 }
 
 /* Base types */
-static void bl_gen_nop(void **ptr)
+static void bl_gen_nop(const BlCpuCfg *cfg, void **ptr)
 {
     if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
-        st_nm32_p(ptr, 0x8000c000);
+        st_nm32_p(cfg, ptr, 0x8000c000);
     } else {
         uint32_t *p = *ptr;
 
@@ -80,7 +80,8 @@ static void bl_gen_nop(void **ptr)
     }
 }
 
-static void bl_gen_r_type(void **ptr, uint8_t opcode,
+static void bl_gen_r_type(const BlCpuCfg *cfg,
+                          void **ptr, uint8_t opcode,
                           bl_reg rs, bl_reg rt, bl_reg rd,
                           uint8_t shift, uint8_t funct)
 {
@@ -100,7 +101,8 @@ static void bl_gen_r_type(void **ptr, uint8_t opcode,
     *ptr = p;
 }
 
-static void bl_gen_i_type(void **ptr, uint8_t opcode,
+static void bl_gen_i_type(const BlCpuCfg *cfg,
+                          void **ptr, uint8_t opcode,
                           bl_reg rs, bl_reg rt, uint16_t imm)
 {
     uint32_t *p = *ptr;
@@ -118,16 +120,17 @@ static void bl_gen_i_type(void **ptr, uint8_t opcode,
 }
 
 /* Single instructions */
-static void bl_gen_dsll(void **p, bl_reg rd, bl_reg rt, uint8_t sa)
+static void bl_gen_dsll(const BlCpuCfg *cfg, void **p,
+                        bl_reg rd, bl_reg rt, uint8_t sa)
 {
     if (bootcpu_supports_isa(ISA_MIPS3)) {
-        bl_gen_r_type(p, 0, 0, rt, rd, sa, 0x38);
+        bl_gen_r_type(cfg, p, 0, 0, rt, rd, sa, 0x38);
     } else {
         g_assert_not_reached(); /* unsupported */
     }
 }
 
-static void bl_gen_jalr(void **p, bl_reg rs)
+static void bl_gen_jalr(const BlCpuCfg *cfg, void **p, bl_reg rs)
 {
     if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
         uint32_t insn = 0;
@@ -136,13 +139,14 @@ static void bl_gen_jalr(void **p, bl_reg rs)
         insn = deposit32(insn, 21, 5, BL_REG_RA);
         insn = deposit32(insn, 16, 5, rs);
 
-        st_nm32_p(p, insn);
+        st_nm32_p(cfg, p, insn);
     } else {
-        bl_gen_r_type(p, 0, rs, 0, BL_REG_RA, 0, 0x09);
+        bl_gen_r_type(cfg, p, 0, rs, 0, BL_REG_RA, 0, 0x09);
     }
 }
 
-static void bl_gen_lui_nm(void **ptr, bl_reg rt, uint32_t imm20)
+static void bl_gen_lui_nm(const BlCpuCfg *cfg, void **ptr,
+                          bl_reg rt, uint32_t imm20)
 {
     uint32_t insn = 0;
 
@@ -153,16 +157,18 @@ static void bl_gen_lui_nm(void **ptr, bl_reg rt, uint32_t imm20)
     insn = deposit32(insn, 2, 10, extract32(imm20, 9, 10));
     insn = deposit32(insn, 0, 1, sextract32(imm20, 19, 1));
 
-    st_nm32_p(ptr, insn);
+    st_nm32_p(cfg, ptr, insn);
 }
 
-static void bl_gen_lui(void **p, bl_reg rt, uint16_t imm)
+static void bl_gen_lui(const BlCpuCfg *cfg, void **p,
+                       bl_reg rt, uint16_t imm)
 {
     /* R6: It's a alias of AUI with RS = 0 */
-    bl_gen_i_type(p, 0x0f, 0, rt, imm);
+    bl_gen_i_type(cfg, p, 0x0f, 0, rt, imm);
 }
 
-static void bl_gen_ori_nm(void **ptr, bl_reg rt, bl_reg rs, uint16_t imm12)
+static void bl_gen_ori_nm(const BlCpuCfg *cfg, void **ptr,
+                          bl_reg rt, bl_reg rs, uint16_t imm12)
 {
     uint32_t insn = 0;
 
@@ -172,15 +178,17 @@ static void bl_gen_ori_nm(void **ptr, bl_reg rt, bl_reg rs, uint16_t imm12)
     insn = deposit32(insn, 16, 5, rs);
     insn = deposit32(insn, 0, 12, imm12);
 
-    st_nm32_p(ptr, insn);
+    st_nm32_p(cfg, ptr, insn);
 }
 
-static void bl_gen_ori(void **p, bl_reg rt, bl_reg rs, uint16_t imm)
+static void bl_gen_ori(const BlCpuCfg *cfg, void **p,
+                       bl_reg rt, bl_reg rs, uint16_t imm)
 {
-    bl_gen_i_type(p, 0x0d, rs, rt, imm);
+    bl_gen_i_type(cfg, p, 0x0d, rs, rt, imm);
 }
 
-static void bl_gen_sw_nm(void **ptr, bl_reg rt, uint8_t rs, uint16_t ofs12)
+static void bl_gen_sw_nm(const BlCpuCfg *cfg, void **ptr,
+                         bl_reg rt, uint8_t rs, uint16_t ofs12)
 {
     uint32_t insn = 0;
 
@@ -191,66 +199,71 @@ static void bl_gen_sw_nm(void **ptr, bl_reg rt, uint8_t rs, uint16_t ofs12)
     insn = deposit32(insn, 12, 4, 0b1001);
     insn = deposit32(insn, 0, 12, ofs12);
 
-    st_nm32_p(ptr, insn);
+    st_nm32_p(cfg, ptr, insn);
 }
 
-static void bl_gen_sw(void **p, bl_reg rt, uint8_t base, uint16_t offset)
+static void bl_gen_sw(const BlCpuCfg *cfg, void **p,
+                      bl_reg rt, uint8_t base, uint16_t offset)
 {
     if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
-        bl_gen_sw_nm(p, rt, base, offset);
+        bl_gen_sw_nm(cfg, p, rt, base, offset);
     } else {
-        bl_gen_i_type(p, 0x2b, base, rt, offset);
+        bl_gen_i_type(cfg, p, 0x2b, base, rt, offset);
     }
 }
 
-static void bl_gen_sd(void **p, bl_reg rt, uint8_t base, uint16_t offset)
+static void bl_gen_sd(const BlCpuCfg *cfg, void **p,
+                      bl_reg rt, uint8_t base, uint16_t offset)
 {
     if (bootcpu_supports_isa(ISA_MIPS3)) {
-        bl_gen_i_type(p, 0x3f, base, rt, offset);
+        bl_gen_i_type(cfg, p, 0x3f, base, rt, offset);
     } else {
         g_assert_not_reached(); /* unsupported */
     }
 }
 
 /* Pseudo instructions */
-static void bl_gen_li(void **p, bl_reg rt, uint32_t imm)
+static void bl_gen_li(const BlCpuCfg *cfg, void **p,
+                      bl_reg rt, uint32_t imm)
 {
     if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
-        bl_gen_lui_nm(p, rt, extract32(imm, 12, 20));
-        bl_gen_ori_nm(p, rt, rt, extract32(imm, 0, 12));
+        bl_gen_lui_nm(cfg, p, rt, extract32(imm, 12, 20));
+        bl_gen_ori_nm(cfg, p, rt, rt, extract32(imm, 0, 12));
     } else {
-        bl_gen_lui(p, rt, extract32(imm, 16, 16));
-        bl_gen_ori(p, rt, rt, extract32(imm, 0, 16));
+        bl_gen_lui(cfg, p, rt, extract32(imm, 16, 16));
+        bl_gen_ori(cfg, p, rt, rt, extract32(imm, 0, 16));
     }
 }
 
-static void bl_gen_dli(void **p, bl_reg rt, uint64_t imm)
+static void bl_gen_dli(const BlCpuCfg *cfg, void **p,
+                       bl_reg rt, uint64_t imm)
 {
-    bl_gen_li(p, rt, extract64(imm, 32, 32));
-    bl_gen_dsll(p, rt, rt, 16);
-    bl_gen_ori(p, rt, rt, extract64(imm, 16, 16));
-    bl_gen_dsll(p, rt, rt, 16);
-    bl_gen_ori(p, rt, rt, extract64(imm, 0, 16));
+    bl_gen_li(cfg, p, rt, extract64(imm, 32, 32));
+    bl_gen_dsll(cfg, p, rt, rt, 16);
+    bl_gen_ori(cfg, p, rt, rt, extract64(imm, 16, 16));
+    bl_gen_dsll(cfg, p, rt, rt, 16);
+    bl_gen_ori(cfg, p, rt, rt, extract64(imm, 0, 16));
 }
 
-static void bl_gen_load_ulong(void **p, bl_reg rt, target_ulong imm)
+static void bl_gen_load_ulong(const BlCpuCfg *cfg, void **p,
+                              bl_reg rt, target_ulong imm)
 {
     if (bootcpu_supports_isa(ISA_MIPS3)) {
-        bl_gen_dli(p, rt, imm); /* 64bit */
+        bl_gen_dli(cfg, p, rt, imm); /* 64bit */
     } else {
-        bl_gen_li(p, rt, imm); /* 32bit */
+        bl_gen_li(cfg, p, rt, imm); /* 32bit */
     }
 }
 
 /* Helpers */
-void bl_gen_jump_to(void **p, target_ulong jump_addr)
+void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr)
 {
-    bl_gen_load_ulong(p, BL_REG_T9, jump_addr);
-    bl_gen_jalr(p, BL_REG_T9);
-    bl_gen_nop(p); /* delay slot */
+    bl_gen_load_ulong(cfg, p, BL_REG_T9, jump_addr);
+    bl_gen_jalr(cfg, p, BL_REG_T9);
+    bl_gen_nop(cfg, p); /* delay slot */
 }
 
-void bl_gen_jump_kernel(void **p,
+void bl_gen_jump_kernel(const BlCpuCfg *cfg, void **p,
                         bool set_sp, target_ulong sp,
                         bool set_a0, target_ulong a0,
                         bool set_a1, target_ulong a1,
@@ -259,45 +272,48 @@ void bl_gen_jump_kernel(void **p,
                         target_ulong kernel_addr)
 {
     if (set_sp) {
-        bl_gen_load_ulong(p, BL_REG_SP, sp);
+        bl_gen_load_ulong(cfg, p, BL_REG_SP, sp);
     }
     if (set_a0) {
-        bl_gen_load_ulong(p, BL_REG_A0, a0);
+        bl_gen_load_ulong(cfg, p, BL_REG_A0, a0);
     }
     if (set_a1) {
-        bl_gen_load_ulong(p, BL_REG_A1, a1);
+        bl_gen_load_ulong(cfg, p, BL_REG_A1, a1);
     }
     if (set_a2) {
-        bl_gen_load_ulong(p, BL_REG_A2, a2);
+        bl_gen_load_ulong(cfg, p, BL_REG_A2, a2);
     }
     if (set_a3) {
-        bl_gen_load_ulong(p, BL_REG_A3, a3);
+        bl_gen_load_ulong(cfg, p, BL_REG_A3, a3);
     }
 
-    bl_gen_jump_to(p, kernel_addr);
+    bl_gen_jump_to(cfg, p, kernel_addr);
 }
 
-void bl_gen_write_ulong(void **p, target_ulong addr, target_ulong val)
+void bl_gen_write_ulong(const BlCpuCfg *cfg, void **p,
+                        target_ulong addr, target_ulong val)
 {
-    bl_gen_load_ulong(p, BL_REG_K0, val);
-    bl_gen_load_ulong(p, BL_REG_K1, addr);
+    bl_gen_load_ulong(cfg, p, BL_REG_K0, val);
+    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
     if (bootcpu_supports_isa(ISA_MIPS3)) {
-        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
+        bl_gen_sd(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
     } else {
-        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
+        bl_gen_sw(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
     }
 }
 
-void bl_gen_write_u32(void **p, target_ulong addr, uint32_t val)
+void bl_gen_write_u32(const BlCpuCfg *cfg, void **p,
+                      target_ulong addr, uint32_t val)
 {
-    bl_gen_li(p, BL_REG_K0, val);
-    bl_gen_load_ulong(p, BL_REG_K1, addr);
-    bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
+    bl_gen_li(cfg, p, BL_REG_K0, val);
+    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
+    bl_gen_sw(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
 }
 
-void bl_gen_write_u64(void **p, target_ulong addr, uint64_t val)
+void bl_gen_write_u64(const BlCpuCfg *cfg, void **p,
+                      target_ulong addr, uint64_t val)
 {
-    bl_gen_dli(p, BL_REG_K0, val);
-    bl_gen_load_ulong(p, BL_REG_K1, addr);
-    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
+    bl_gen_dli(cfg, p, BL_REG_K0, val);
+    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
+    bl_gen_sd(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
 }
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 1b44fb354c..8e210876e1 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -325,23 +325,24 @@ type_init(boston_register_types)
 
 static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
 {
+    const BlCpuCfg bl_cfg = { };
     uint64_t regaddr;
 
     /* Move CM GCRs */
     regaddr = cpu_mips_phys_to_kseg1(NULL, GCR_BASE_ADDR + GCR_BASE_OFS),
-    bl_gen_write_ulong(&p, regaddr,
+    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
                        boston_memmap[BOSTON_CM].base);
 
     /* Move & enable GIC GCRs */
     regaddr = cpu_mips_phys_to_kseg1(NULL, boston_memmap[BOSTON_CM].base
                                            + GCR_GIC_BASE_OFS),
-    bl_gen_write_ulong(&p, regaddr,
+    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
                        boston_memmap[BOSTON_GIC].base | GCR_GIC_BASE_GICEN_MSK);
 
     /* Move & enable CPC GCRs */
     regaddr = cpu_mips_phys_to_kseg1(NULL, boston_memmap[BOSTON_CM].base
                                            + GCR_CPC_BASE_OFS),
-    bl_gen_write_ulong(&p, regaddr,
+    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
                        boston_memmap[BOSTON_CPC].base | GCR_CPC_BASE_CPCEN_MSK);
 
     /*
@@ -352,7 +353,7 @@ static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
      * a2/$6 = 0
      * a3/$7 = 0
      */
-    bl_gen_jump_kernel(&p,
+    bl_gen_jump_kernel(&bl_cfg, &p,
                        true, 0, true, (int32_t)-2,
                        true, fdt_addr, true, 0, true, 0,
                        kernel_entry);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 6e4303ba47..a989637d3b 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -165,6 +165,7 @@ static uint64_t load_kernel(MIPSCPU *cpu)
 static void write_bootloader(CPUMIPSState *env, uint8_t *base,
                              uint64_t kernel_addr)
 {
+    const BlCpuCfg bl_cfg = { };
     uint32_t *p;
 
     /* Small bootloader */
@@ -178,7 +179,7 @@ static void write_bootloader(CPUMIPSState *env, uint8_t *base,
     /* Second part of the bootloader */
     p = (uint32_t *)(base + 0x040);
 
-    bl_gen_jump_kernel((void **)&p,
+    bl_gen_jump_kernel(&bl_cfg, (void **)&p,
                        true, ENVP_VADDR - 64,
                        true, 2, true, ENVP_VADDR,
                        true, ENVP_VADDR + 8,
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 664a2ae0a9..fc485cc884 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -624,6 +624,7 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
     static const char pci_pins_cfg[PCI_NUM_PINS] = {
         10, 10, 11, 11 /* PIIX IRQRC[A:D] */
     };
+    const BlCpuCfg bl_cfg = { };
 
     /* Bus endianness is always reversed */
 #if TARGET_BIG_ENDIAN
@@ -635,29 +636,29 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
     /* setup MEM-to-PCI0 mapping as done by YAMON */
 
     /* move GT64120 registers from 0x14000000 to 0x1be00000 */
-    bl_gen_write_u32(p, /* GT_ISD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_ISD */
                      cpu_mips_phys_to_kseg1(NULL, 0x14000000 + 0x68),
                      cpu_to_gt32(0x1be00000 << 3));
 
     /* setup PCI0 io window to 0x18000000-0x181fffff */
-    bl_gen_write_u32(p, /* GT_PCI0IOLD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0IOLD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x48),
                      cpu_to_gt32(0x18000000 << 3));
-    bl_gen_write_u32(p, /* GT_PCI0IOHD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0IOHD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x50),
                      cpu_to_gt32(0x08000000 << 3));
 
     /* setup PCI0 mem windows */
-    bl_gen_write_u32(p, /* GT_PCI0M0LD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M0LD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x58),
                      cpu_to_gt32(0x10000000 << 3));
-    bl_gen_write_u32(p, /* GT_PCI0M0HD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M0HD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x60),
                      cpu_to_gt32(0x07e00000 << 3));
-    bl_gen_write_u32(p, /* GT_PCI0M1LD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M1LD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x80),
                      cpu_to_gt32(0x18200000 << 3));
-    bl_gen_write_u32(p, /* GT_PCI0M1HD */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M1HD */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x88),
                      cpu_to_gt32(0x0bc00000 << 3));
 
@@ -668,16 +669,16 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
      * Load the PIIX IRQC[A:D] routing config address, then
      * write routing configuration to the config data register.
      */
-    bl_gen_write_u32(p, /* GT_PCI0_CFGADDR */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0_CFGADDR */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcf8),
                      tswap32((1 << 31) /* ConfigEn */
                              | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8
                              | PIIX_PIRQCA));
-    bl_gen_write_u32(p, /* GT_PCI0_CFGDATA */
+    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0_CFGDATA */
                      cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcfc),
                      tswap32(ldl_be_p(pci_pins_cfg)));
 
-    bl_gen_jump_kernel(p,
+    bl_gen_jump_kernel(&bl_cfg, p,
                        true, ENVP_VADDR - 64,
                        /*
                         * If semihosting is used, arguments have already
-- 
2.45.2

Re: [PATCH 04/13] hw/mips: Pass BlCpuCfg argument to bootloader API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> In preparation to pass endianness and target word size to
> the bootloader API, introduce an empty BlCpuCfg structure
> and propagate it to the MIPS bootloader methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/mips/bootloader.h |  17 +++--
>   hw/mips/bootloader.c         | 142 +++++++++++++++++++----------------
>   hw/mips/boston.c             |   9 ++-
>   hw/mips/fuloong2e.c          |   3 +-
>   hw/mips/malta.c              |  21 +++---
>   5 files changed, 109 insertions(+), 83 deletions(-)
> 
> diff --git a/include/hw/mips/bootloader.h b/include/hw/mips/bootloader.h
> index c32f6c2835..744eb11d0e 100644
> --- a/include/hw/mips/bootloader.h
> +++ b/include/hw/mips/bootloader.h
> @@ -10,17 +10,24 @@
>   #define HW_MIPS_BOOTLOADER_H
>   
>   #include "exec/cpu-defs.h"
> +#include "exec/target_long.h"
>   
> -void bl_gen_jump_to(void **ptr, target_ulong jump_addr);
> -void bl_gen_jump_kernel(void **ptr,
> +typedef struct bl_cpu_cfg {
> +} BlCpuCfg;
> +
> +void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr);
> +void bl_gen_jump_kernel(const BlCpuCfg *cfg, void **ptr,
>                           bool set_sp, target_ulong sp,
>                           bool set_a0, target_ulong a0,
>                           bool set_a1, target_ulong a1,
>                           bool set_a2, target_ulong a2,
>                           bool set_a3, target_ulong a3,
>                           target_ulong kernel_addr);
> -void bl_gen_write_ulong(void **ptr, target_ulong addr, target_ulong val);
> -void bl_gen_write_u32(void **ptr, target_ulong addr, uint32_t val);
> -void bl_gen_write_u64(void **ptr, target_ulong addr, uint64_t val);
> +void bl_gen_write_ulong(const BlCpuCfg *cfg, void **ptr,
> +                        target_ulong addr, target_ulong val);
> +void bl_gen_write_u32(const BlCpuCfg *cfg, void **ptr,
> +                      target_ulong addr, uint32_t val);
> +void bl_gen_write_u64(const BlCpuCfg *cfg, void **ptr,
> +                      target_ulong addr, uint64_t val);
>   
>   #endif
> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
> index 1dd6ef2096..ee1a1c4f20 100644
> --- a/hw/mips/bootloader.c
> +++ b/hw/mips/bootloader.c
> @@ -54,7 +54,7 @@ static bool bootcpu_supports_isa(uint64_t isa_mask)
>       return cpu_supports_isa(&MIPS_CPU(first_cpu)->env, isa_mask);
>   }
>   
> -static void st_nm32_p(void **ptr, uint32_t insn)
> +static void st_nm32_p(const BlCpuCfg *cfg, void **ptr, uint32_t insn)
>   {
>       uint16_t *p = *ptr;
>   
> @@ -67,10 +67,10 @@ static void st_nm32_p(void **ptr, uint32_t insn)
>   }
>   
>   /* Base types */
> -static void bl_gen_nop(void **ptr)
> +static void bl_gen_nop(const BlCpuCfg *cfg, void **ptr)
>   {
>       if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
> -        st_nm32_p(ptr, 0x8000c000);
> +        st_nm32_p(cfg, ptr, 0x8000c000);
>       } else {
>           uint32_t *p = *ptr;
>   
> @@ -80,7 +80,8 @@ static void bl_gen_nop(void **ptr)
>       }
>   }
>   
> -static void bl_gen_r_type(void **ptr, uint8_t opcode,
> +static void bl_gen_r_type(const BlCpuCfg *cfg,
> +                          void **ptr, uint8_t opcode,
>                             bl_reg rs, bl_reg rt, bl_reg rd,
>                             uint8_t shift, uint8_t funct)
>   {
> @@ -100,7 +101,8 @@ static void bl_gen_r_type(void **ptr, uint8_t opcode,
>       *ptr = p;
>   }
>   
> -static void bl_gen_i_type(void **ptr, uint8_t opcode,
> +static void bl_gen_i_type(const BlCpuCfg *cfg,
> +                          void **ptr, uint8_t opcode,
>                             bl_reg rs, bl_reg rt, uint16_t imm)
>   {
>       uint32_t *p = *ptr;
> @@ -118,16 +120,17 @@ static void bl_gen_i_type(void **ptr, uint8_t opcode,
>   }
>   
>   /* Single instructions */
> -static void bl_gen_dsll(void **p, bl_reg rd, bl_reg rt, uint8_t sa)
> +static void bl_gen_dsll(const BlCpuCfg *cfg, void **p,
> +                        bl_reg rd, bl_reg rt, uint8_t sa)
>   {
>       if (bootcpu_supports_isa(ISA_MIPS3)) {
> -        bl_gen_r_type(p, 0, 0, rt, rd, sa, 0x38);
> +        bl_gen_r_type(cfg, p, 0, 0, rt, rd, sa, 0x38);
>       } else {
>           g_assert_not_reached(); /* unsupported */
>       }
>   }
>   
> -static void bl_gen_jalr(void **p, bl_reg rs)
> +static void bl_gen_jalr(const BlCpuCfg *cfg, void **p, bl_reg rs)
>   {
>       if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
>           uint32_t insn = 0;
> @@ -136,13 +139,14 @@ static void bl_gen_jalr(void **p, bl_reg rs)
>           insn = deposit32(insn, 21, 5, BL_REG_RA);
>           insn = deposit32(insn, 16, 5, rs);
>   
> -        st_nm32_p(p, insn);
> +        st_nm32_p(cfg, p, insn);
>       } else {
> -        bl_gen_r_type(p, 0, rs, 0, BL_REG_RA, 0, 0x09);
> +        bl_gen_r_type(cfg, p, 0, rs, 0, BL_REG_RA, 0, 0x09);
>       }
>   }
>   
> -static void bl_gen_lui_nm(void **ptr, bl_reg rt, uint32_t imm20)
> +static void bl_gen_lui_nm(const BlCpuCfg *cfg, void **ptr,
> +                          bl_reg rt, uint32_t imm20)
>   {
>       uint32_t insn = 0;
>   
> @@ -153,16 +157,18 @@ static void bl_gen_lui_nm(void **ptr, bl_reg rt, uint32_t imm20)
>       insn = deposit32(insn, 2, 10, extract32(imm20, 9, 10));
>       insn = deposit32(insn, 0, 1, sextract32(imm20, 19, 1));
>   
> -    st_nm32_p(ptr, insn);
> +    st_nm32_p(cfg, ptr, insn);
>   }
>   
> -static void bl_gen_lui(void **p, bl_reg rt, uint16_t imm)
> +static void bl_gen_lui(const BlCpuCfg *cfg, void **p,
> +                       bl_reg rt, uint16_t imm)
>   {
>       /* R6: It's a alias of AUI with RS = 0 */
> -    bl_gen_i_type(p, 0x0f, 0, rt, imm);
> +    bl_gen_i_type(cfg, p, 0x0f, 0, rt, imm);
>   }
>   
> -static void bl_gen_ori_nm(void **ptr, bl_reg rt, bl_reg rs, uint16_t imm12)
> +static void bl_gen_ori_nm(const BlCpuCfg *cfg, void **ptr,
> +                          bl_reg rt, bl_reg rs, uint16_t imm12)
>   {
>       uint32_t insn = 0;
>   
> @@ -172,15 +178,17 @@ static void bl_gen_ori_nm(void **ptr, bl_reg rt, bl_reg rs, uint16_t imm12)
>       insn = deposit32(insn, 16, 5, rs);
>       insn = deposit32(insn, 0, 12, imm12);
>   
> -    st_nm32_p(ptr, insn);
> +    st_nm32_p(cfg, ptr, insn);
>   }
>   
> -static void bl_gen_ori(void **p, bl_reg rt, bl_reg rs, uint16_t imm)
> +static void bl_gen_ori(const BlCpuCfg *cfg, void **p,
> +                       bl_reg rt, bl_reg rs, uint16_t imm)
>   {
> -    bl_gen_i_type(p, 0x0d, rs, rt, imm);
> +    bl_gen_i_type(cfg, p, 0x0d, rs, rt, imm);
>   }
>   
> -static void bl_gen_sw_nm(void **ptr, bl_reg rt, uint8_t rs, uint16_t ofs12)
> +static void bl_gen_sw_nm(const BlCpuCfg *cfg, void **ptr,
> +                         bl_reg rt, uint8_t rs, uint16_t ofs12)
>   {
>       uint32_t insn = 0;
>   
> @@ -191,66 +199,71 @@ static void bl_gen_sw_nm(void **ptr, bl_reg rt, uint8_t rs, uint16_t ofs12)
>       insn = deposit32(insn, 12, 4, 0b1001);
>       insn = deposit32(insn, 0, 12, ofs12);
>   
> -    st_nm32_p(ptr, insn);
> +    st_nm32_p(cfg, ptr, insn);
>   }
>   
> -static void bl_gen_sw(void **p, bl_reg rt, uint8_t base, uint16_t offset)
> +static void bl_gen_sw(const BlCpuCfg *cfg, void **p,
> +                      bl_reg rt, uint8_t base, uint16_t offset)
>   {
>       if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
> -        bl_gen_sw_nm(p, rt, base, offset);
> +        bl_gen_sw_nm(cfg, p, rt, base, offset);
>       } else {
> -        bl_gen_i_type(p, 0x2b, base, rt, offset);
> +        bl_gen_i_type(cfg, p, 0x2b, base, rt, offset);
>       }
>   }
>   
> -static void bl_gen_sd(void **p, bl_reg rt, uint8_t base, uint16_t offset)
> +static void bl_gen_sd(const BlCpuCfg *cfg, void **p,
> +                      bl_reg rt, uint8_t base, uint16_t offset)
>   {
>       if (bootcpu_supports_isa(ISA_MIPS3)) {
> -        bl_gen_i_type(p, 0x3f, base, rt, offset);
> +        bl_gen_i_type(cfg, p, 0x3f, base, rt, offset);
>       } else {
>           g_assert_not_reached(); /* unsupported */
>       }
>   }
>   
>   /* Pseudo instructions */
> -static void bl_gen_li(void **p, bl_reg rt, uint32_t imm)
> +static void bl_gen_li(const BlCpuCfg *cfg, void **p,
> +                      bl_reg rt, uint32_t imm)
>   {
>       if (bootcpu_supports_isa(ISA_NANOMIPS32)) {
> -        bl_gen_lui_nm(p, rt, extract32(imm, 12, 20));
> -        bl_gen_ori_nm(p, rt, rt, extract32(imm, 0, 12));
> +        bl_gen_lui_nm(cfg, p, rt, extract32(imm, 12, 20));
> +        bl_gen_ori_nm(cfg, p, rt, rt, extract32(imm, 0, 12));
>       } else {
> -        bl_gen_lui(p, rt, extract32(imm, 16, 16));
> -        bl_gen_ori(p, rt, rt, extract32(imm, 0, 16));
> +        bl_gen_lui(cfg, p, rt, extract32(imm, 16, 16));
> +        bl_gen_ori(cfg, p, rt, rt, extract32(imm, 0, 16));
>       }
>   }
>   
> -static void bl_gen_dli(void **p, bl_reg rt, uint64_t imm)
> +static void bl_gen_dli(const BlCpuCfg *cfg, void **p,
> +                       bl_reg rt, uint64_t imm)
>   {
> -    bl_gen_li(p, rt, extract64(imm, 32, 32));
> -    bl_gen_dsll(p, rt, rt, 16);
> -    bl_gen_ori(p, rt, rt, extract64(imm, 16, 16));
> -    bl_gen_dsll(p, rt, rt, 16);
> -    bl_gen_ori(p, rt, rt, extract64(imm, 0, 16));
> +    bl_gen_li(cfg, p, rt, extract64(imm, 32, 32));
> +    bl_gen_dsll(cfg, p, rt, rt, 16);
> +    bl_gen_ori(cfg, p, rt, rt, extract64(imm, 16, 16));
> +    bl_gen_dsll(cfg, p, rt, rt, 16);
> +    bl_gen_ori(cfg, p, rt, rt, extract64(imm, 0, 16));
>   }
>   
> -static void bl_gen_load_ulong(void **p, bl_reg rt, target_ulong imm)
> +static void bl_gen_load_ulong(const BlCpuCfg *cfg, void **p,
> +                              bl_reg rt, target_ulong imm)
>   {
>       if (bootcpu_supports_isa(ISA_MIPS3)) {
> -        bl_gen_dli(p, rt, imm); /* 64bit */
> +        bl_gen_dli(cfg, p, rt, imm); /* 64bit */
>       } else {
> -        bl_gen_li(p, rt, imm); /* 32bit */
> +        bl_gen_li(cfg, p, rt, imm); /* 32bit */
>       }
>   }
>   
>   /* Helpers */
> -void bl_gen_jump_to(void **p, target_ulong jump_addr)
> +void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr)
>   {
> -    bl_gen_load_ulong(p, BL_REG_T9, jump_addr);
> -    bl_gen_jalr(p, BL_REG_T9);
> -    bl_gen_nop(p); /* delay slot */
> +    bl_gen_load_ulong(cfg, p, BL_REG_T9, jump_addr);
> +    bl_gen_jalr(cfg, p, BL_REG_T9);
> +    bl_gen_nop(cfg, p); /* delay slot */
>   }
>   
> -void bl_gen_jump_kernel(void **p,
> +void bl_gen_jump_kernel(const BlCpuCfg *cfg, void **p,
>                           bool set_sp, target_ulong sp,
>                           bool set_a0, target_ulong a0,
>                           bool set_a1, target_ulong a1,
> @@ -259,45 +272,48 @@ void bl_gen_jump_kernel(void **p,
>                           target_ulong kernel_addr)
>   {
>       if (set_sp) {
> -        bl_gen_load_ulong(p, BL_REG_SP, sp);
> +        bl_gen_load_ulong(cfg, p, BL_REG_SP, sp);
>       }
>       if (set_a0) {
> -        bl_gen_load_ulong(p, BL_REG_A0, a0);
> +        bl_gen_load_ulong(cfg, p, BL_REG_A0, a0);
>       }
>       if (set_a1) {
> -        bl_gen_load_ulong(p, BL_REG_A1, a1);
> +        bl_gen_load_ulong(cfg, p, BL_REG_A1, a1);
>       }
>       if (set_a2) {
> -        bl_gen_load_ulong(p, BL_REG_A2, a2);
> +        bl_gen_load_ulong(cfg, p, BL_REG_A2, a2);
>       }
>       if (set_a3) {
> -        bl_gen_load_ulong(p, BL_REG_A3, a3);
> +        bl_gen_load_ulong(cfg, p, BL_REG_A3, a3);
>       }
>   
> -    bl_gen_jump_to(p, kernel_addr);
> +    bl_gen_jump_to(cfg, p, kernel_addr);
>   }
>   
> -void bl_gen_write_ulong(void **p, target_ulong addr, target_ulong val)
> +void bl_gen_write_ulong(const BlCpuCfg *cfg, void **p,
> +                        target_ulong addr, target_ulong val)
>   {
> -    bl_gen_load_ulong(p, BL_REG_K0, val);
> -    bl_gen_load_ulong(p, BL_REG_K1, addr);
> +    bl_gen_load_ulong(cfg, p, BL_REG_K0, val);
> +    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
>       if (bootcpu_supports_isa(ISA_MIPS3)) {
> -        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
> +        bl_gen_sd(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
>       } else {
> -        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
> +        bl_gen_sw(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
>       }
>   }
>   
> -void bl_gen_write_u32(void **p, target_ulong addr, uint32_t val)
> +void bl_gen_write_u32(const BlCpuCfg *cfg, void **p,
> +                      target_ulong addr, uint32_t val)
>   {
> -    bl_gen_li(p, BL_REG_K0, val);
> -    bl_gen_load_ulong(p, BL_REG_K1, addr);
> -    bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
> +    bl_gen_li(cfg, p, BL_REG_K0, val);
> +    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
> +    bl_gen_sw(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
>   }
>   
> -void bl_gen_write_u64(void **p, target_ulong addr, uint64_t val)
> +void bl_gen_write_u64(const BlCpuCfg *cfg, void **p,
> +                      target_ulong addr, uint64_t val)
>   {
> -    bl_gen_dli(p, BL_REG_K0, val);
> -    bl_gen_load_ulong(p, BL_REG_K1, addr);
> -    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
> +    bl_gen_dli(cfg, p, BL_REG_K0, val);
> +    bl_gen_load_ulong(cfg, p, BL_REG_K1, addr);
> +    bl_gen_sd(cfg, p, BL_REG_K0, BL_REG_K1, 0x0);
>   }
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 1b44fb354c..8e210876e1 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -325,23 +325,24 @@ type_init(boston_register_types)
>   
>   static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
>   {
> +    const BlCpuCfg bl_cfg = { };
>       uint64_t regaddr;
>   
>       /* Move CM GCRs */
>       regaddr = cpu_mips_phys_to_kseg1(NULL, GCR_BASE_ADDR + GCR_BASE_OFS),
> -    bl_gen_write_ulong(&p, regaddr,
> +    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
>                          boston_memmap[BOSTON_CM].base);
>   
>       /* Move & enable GIC GCRs */
>       regaddr = cpu_mips_phys_to_kseg1(NULL, boston_memmap[BOSTON_CM].base
>                                              + GCR_GIC_BASE_OFS),
> -    bl_gen_write_ulong(&p, regaddr,
> +    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
>                          boston_memmap[BOSTON_GIC].base | GCR_GIC_BASE_GICEN_MSK);
>   
>       /* Move & enable CPC GCRs */
>       regaddr = cpu_mips_phys_to_kseg1(NULL, boston_memmap[BOSTON_CM].base
>                                              + GCR_CPC_BASE_OFS),
> -    bl_gen_write_ulong(&p, regaddr,
> +    bl_gen_write_ulong(&bl_cfg, &p, regaddr,
>                          boston_memmap[BOSTON_CPC].base | GCR_CPC_BASE_CPCEN_MSK);
>   
>       /*
> @@ -352,7 +353,7 @@ static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
>        * a2/$6 = 0
>        * a3/$7 = 0
>        */
> -    bl_gen_jump_kernel(&p,
> +    bl_gen_jump_kernel(&bl_cfg, &p,
>                          true, 0, true, (int32_t)-2,
>                          true, fdt_addr, true, 0, true, 0,
>                          kernel_entry);
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 6e4303ba47..a989637d3b 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -165,6 +165,7 @@ static uint64_t load_kernel(MIPSCPU *cpu)
>   static void write_bootloader(CPUMIPSState *env, uint8_t *base,
>                                uint64_t kernel_addr)
>   {
> +    const BlCpuCfg bl_cfg = { };
>       uint32_t *p;
>   
>       /* Small bootloader */
> @@ -178,7 +179,7 @@ static void write_bootloader(CPUMIPSState *env, uint8_t *base,
>       /* Second part of the bootloader */
>       p = (uint32_t *)(base + 0x040);
>   
> -    bl_gen_jump_kernel((void **)&p,
> +    bl_gen_jump_kernel(&bl_cfg, (void **)&p,
>                          true, ENVP_VADDR - 64,
>                          true, 2, true, ENVP_VADDR,
>                          true, ENVP_VADDR + 8,
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index 664a2ae0a9..fc485cc884 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -624,6 +624,7 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
>       static const char pci_pins_cfg[PCI_NUM_PINS] = {
>           10, 10, 11, 11 /* PIIX IRQRC[A:D] */
>       };
> +    const BlCpuCfg bl_cfg = { };
>   
>       /* Bus endianness is always reversed */
>   #if TARGET_BIG_ENDIAN
> @@ -635,29 +636,29 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
>       /* setup MEM-to-PCI0 mapping as done by YAMON */
>   
>       /* move GT64120 registers from 0x14000000 to 0x1be00000 */
> -    bl_gen_write_u32(p, /* GT_ISD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_ISD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x14000000 + 0x68),
>                        cpu_to_gt32(0x1be00000 << 3));
>   
>       /* setup PCI0 io window to 0x18000000-0x181fffff */
> -    bl_gen_write_u32(p, /* GT_PCI0IOLD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0IOLD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x48),
>                        cpu_to_gt32(0x18000000 << 3));
> -    bl_gen_write_u32(p, /* GT_PCI0IOHD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0IOHD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x50),
>                        cpu_to_gt32(0x08000000 << 3));
>   
>       /* setup PCI0 mem windows */
> -    bl_gen_write_u32(p, /* GT_PCI0M0LD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M0LD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x58),
>                        cpu_to_gt32(0x10000000 << 3));
> -    bl_gen_write_u32(p, /* GT_PCI0M0HD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M0HD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x60),
>                        cpu_to_gt32(0x07e00000 << 3));
> -    bl_gen_write_u32(p, /* GT_PCI0M1LD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M1LD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x80),
>                        cpu_to_gt32(0x18200000 << 3));
> -    bl_gen_write_u32(p, /* GT_PCI0M1HD */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0M1HD */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0x88),
>                        cpu_to_gt32(0x0bc00000 << 3));
>   
> @@ -668,16 +669,16 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
>        * Load the PIIX IRQC[A:D] routing config address, then
>        * write routing configuration to the config data register.
>        */
> -    bl_gen_write_u32(p, /* GT_PCI0_CFGADDR */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0_CFGADDR */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcf8),
>                        tswap32((1 << 31) /* ConfigEn */
>                                | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8
>                                | PIIX_PIRQCA));
> -    bl_gen_write_u32(p, /* GT_PCI0_CFGDATA */
> +    bl_gen_write_u32(&bl_cfg, p, /* GT_PCI0_CFGDATA */
>                        cpu_mips_phys_to_kseg1(NULL, 0x1be00000 + 0xcfc),
>                        tswap32(ldl_be_p(pci_pins_cfg)));
>   
> -    bl_gen_jump_kernel(p,
> +    bl_gen_jump_kernel(&bl_cfg, p,
>                          true, ENVP_VADDR - 64,
>                          /*
>                           * If semihosting is used, arguments have already

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 05/13] hw/mips: Add cpu_is_bigendian field to BlCpuCfg structure

[Philippe Mathieu-Daudé]

Add the BlCpuCfg::cpu_is_bigendian field, initialize it in
machine code. Bootloader API use the ld/st_endian_p() to
dispatch to target endianness.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/mips/bootloader.h |  1 +
 hw/mips/bootloader.c         | 10 +++++-----
 hw/mips/boston.c             |  2 +-
 hw/mips/fuloong2e.c          |  2 +-
 hw/mips/malta.c              |  2 +-
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/hw/mips/bootloader.h b/include/hw/mips/bootloader.h
index 744eb11d0e..ef778a38d0 100644
--- a/include/hw/mips/bootloader.h
+++ b/include/hw/mips/bootloader.h
@@ -13,6 +13,7 @@
 #include "exec/target_long.h"
 
 typedef struct bl_cpu_cfg {
+    bool cpu_is_bigendian;
 } BlCpuCfg;
 
 void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr);
diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
index ee1a1c4f20..258cc5d8c8 100644
--- a/hw/mips/bootloader.c
+++ b/hw/mips/bootloader.c
@@ -58,9 +58,9 @@ static void st_nm32_p(const BlCpuCfg *cfg, void **ptr, uint32_t insn)
 {
     uint16_t *p = *ptr;
 
-    stw_p(p, insn >> 16);
+    stw_endian_p(cfg->cpu_is_bigendian, p, insn >> 16);
     p++;
-    stw_p(p, insn >> 0);
+    stw_endian_p(cfg->cpu_is_bigendian, p, insn >> 0);
     p++;
 
     *ptr = p;
@@ -74,7 +74,7 @@ static void bl_gen_nop(const BlCpuCfg *cfg, void **ptr)
     } else {
         uint32_t *p = *ptr;
 
-        stl_p(p, 0);
+        stl_endian_p(cfg->cpu_is_bigendian, p, 0);
         p++;
         *ptr = p;
     }
@@ -95,7 +95,7 @@ static void bl_gen_r_type(const BlCpuCfg *cfg,
     insn = deposit32(insn, 6, 5, shift);
     insn = deposit32(insn, 0, 6, funct);
 
-    stl_p(p, insn);
+    stl_endian_p(cfg->cpu_is_bigendian, p, insn);
     p++;
 
     *ptr = p;
@@ -113,7 +113,7 @@ static void bl_gen_i_type(const BlCpuCfg *cfg,
     insn = deposit32(insn, 16, 5, rt);
     insn = deposit32(insn, 0, 16, imm);
 
-    stl_p(p, insn);
+    stl_endian_p(cfg->cpu_is_bigendian, p, insn);
     p++;
 
     *ptr = p;
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 8e210876e1..d4dd242d0d 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -325,7 +325,7 @@ type_init(boston_register_types)
 
 static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
 {
-    const BlCpuCfg bl_cfg = { };
+    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = TARGET_BIG_ENDIAN };
     uint64_t regaddr;
 
     /* Move CM GCRs */
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index a989637d3b..4fe5108845 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -165,7 +165,7 @@ static uint64_t load_kernel(MIPSCPU *cpu)
 static void write_bootloader(CPUMIPSState *env, uint8_t *base,
                              uint64_t kernel_addr)
 {
-    const BlCpuCfg bl_cfg = { };
+    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = false };
     uint32_t *p;
 
     /* Small bootloader */
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index fc485cc884..6e73c896ff 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -624,7 +624,7 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
     static const char pci_pins_cfg[PCI_NUM_PINS] = {
         10, 10, 11, 11 /* PIIX IRQRC[A:D] */
     };
-    const BlCpuCfg bl_cfg = { };
+    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = TARGET_BIG_ENDIAN };
 
     /* Bus endianness is always reversed */
 #if TARGET_BIG_ENDIAN
-- 
2.45.2

Re: [PATCH 05/13] hw/mips: Add cpu_is_bigendian field to BlCpuCfg structure

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Add the BlCpuCfg::cpu_is_bigendian field, initialize it in
> machine code. Bootloader API use the ld/st_endian_p() to
> dispatch to target endianness.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/mips/bootloader.h |  1 +
>   hw/mips/bootloader.c         | 10 +++++-----
>   hw/mips/boston.c             |  2 +-
>   hw/mips/fuloong2e.c          |  2 +-
>   hw/mips/malta.c              |  2 +-
>   5 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/mips/bootloader.h b/include/hw/mips/bootloader.h
> index 744eb11d0e..ef778a38d0 100644
> --- a/include/hw/mips/bootloader.h
> +++ b/include/hw/mips/bootloader.h
> @@ -13,6 +13,7 @@
>   #include "exec/target_long.h"
>   
>   typedef struct bl_cpu_cfg {
> +    bool cpu_is_bigendian;
>   } BlCpuCfg;
>   
>   void bl_gen_jump_to(const BlCpuCfg *cfg, void **p, target_ulong jump_addr);
> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
> index ee1a1c4f20..258cc5d8c8 100644
> --- a/hw/mips/bootloader.c
> +++ b/hw/mips/bootloader.c
> @@ -58,9 +58,9 @@ static void st_nm32_p(const BlCpuCfg *cfg, void **ptr, uint32_t insn)
>   {
>       uint16_t *p = *ptr;
>   
> -    stw_p(p, insn >> 16);
> +    stw_endian_p(cfg->cpu_is_bigendian, p, insn >> 16);
>       p++;
> -    stw_p(p, insn >> 0);
> +    stw_endian_p(cfg->cpu_is_bigendian, p, insn >> 0);
>       p++;
>   
>       *ptr = p;
> @@ -74,7 +74,7 @@ static void bl_gen_nop(const BlCpuCfg *cfg, void **ptr)
>       } else {
>           uint32_t *p = *ptr;
>   
> -        stl_p(p, 0);
> +        stl_endian_p(cfg->cpu_is_bigendian, p, 0);
>           p++;
>           *ptr = p;
>       }
> @@ -95,7 +95,7 @@ static void bl_gen_r_type(const BlCpuCfg *cfg,
>       insn = deposit32(insn, 6, 5, shift);
>       insn = deposit32(insn, 0, 6, funct);
>   
> -    stl_p(p, insn);
> +    stl_endian_p(cfg->cpu_is_bigendian, p, insn);
>       p++;
>   
>       *ptr = p;
> @@ -113,7 +113,7 @@ static void bl_gen_i_type(const BlCpuCfg *cfg,
>       insn = deposit32(insn, 16, 5, rt);
>       insn = deposit32(insn, 0, 16, imm);
>   
> -    stl_p(p, insn);
> +    stl_endian_p(cfg->cpu_is_bigendian, p, insn);
>       p++;
>   
>       *ptr = p;
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index 8e210876e1..d4dd242d0d 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -325,7 +325,7 @@ type_init(boston_register_types)
>   
>   static void gen_firmware(void *p, hwaddr kernel_entry, hwaddr fdt_addr)
>   {
> -    const BlCpuCfg bl_cfg = { };
> +    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = TARGET_BIG_ENDIAN };
>       uint64_t regaddr;
>   
>       /* Move CM GCRs */
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index a989637d3b..4fe5108845 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -165,7 +165,7 @@ static uint64_t load_kernel(MIPSCPU *cpu)
>   static void write_bootloader(CPUMIPSState *env, uint8_t *base,
>                                uint64_t kernel_addr)
>   {
> -    const BlCpuCfg bl_cfg = { };
> +    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = false };
>       uint32_t *p;
>   
>       /* Small bootloader */
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index fc485cc884..6e73c896ff 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -624,7 +624,7 @@ static void bl_setup_gt64120_jump_kernel(void **p, uint64_t run_addr,
>       static const char pci_pins_cfg[PCI_NUM_PINS] = {
>           10, 10, 11, 11 /* PIIX IRQRC[A:D] */
>       };
> -    const BlCpuCfg bl_cfg = { };
> +    const BlCpuCfg bl_cfg = { .cpu_is_bigendian = TARGET_BIG_ENDIAN };
>   
>       /* Bus endianness is always reversed */
>   #if TARGET_BIG_ENDIAN

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 06/13] tests/tcg/plugins: Use the ld/st_endian_p() API

[Philippe Mathieu-Daudé]

Refactor to use the recently introduced ld/st_endian_p() API
No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/tcg/plugins/mem.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
index b0fa8a9f27..3586d05587 100644
--- a/tests/tcg/plugins/mem.c
+++ b/tests/tcg/plugins/mem.c
@@ -163,13 +163,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
     {
         uint16_t *p = (uint16_t *) &ri->data[offset];
         if (is_store) {
-            if (be) {
-                stw_be_p(p, value.data.u16);
-            } else {
-                stw_le_p(p, value.data.u16);
-            }
+            stw_endian_p(be, p, value.data.u16);
         } else {
-            uint16_t val = be ? lduw_be_p(p) : lduw_le_p(p);
+            uint16_t val = lduw_endian_p(be, p);
             unseen_data = val != value.data.u16;
         }
         break;
@@ -178,13 +174,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
     {
         uint32_t *p = (uint32_t *) &ri->data[offset];
         if (is_store) {
-            if (be) {
-                stl_be_p(p, value.data.u32);
-            } else {
-                stl_le_p(p, value.data.u32);
-            }
+            stl_endian_p(be, p, value.data.u32);
         } else {
-            uint32_t val = be ? ldl_be_p(p) : ldl_le_p(p);
+            uint32_t val = ldl_endian_p(be, p);
             unseen_data = val != value.data.u32;
         }
         break;
@@ -193,13 +185,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
     {
         uint64_t *p = (uint64_t *) &ri->data[offset];
         if (is_store) {
-            if (be) {
-                stq_be_p(p, value.data.u64);
-            } else {
-                stq_le_p(p, value.data.u64);
-            }
+            stq_endian_p(be, p, value.data.u64);
         } else {
-            uint64_t val = be ? ldq_be_p(p) : ldq_le_p(p);
+            uint64_t val = ldq_endian_p(be, p);
             unseen_data = val != value.data.u64;
         }
         break;
-- 
2.45.2

Re: [PATCH 06/13] tests/tcg/plugins: Use the ld/st_endian_p() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_p() API
> No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/tcg/plugins/mem.c | 24 ++++++------------------
>   1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
> index b0fa8a9f27..3586d05587 100644
> --- a/tests/tcg/plugins/mem.c
> +++ b/tests/tcg/plugins/mem.c
> @@ -163,13 +163,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
>       {
>           uint16_t *p = (uint16_t *) &ri->data[offset];
>           if (is_store) {
> -            if (be) {
> -                stw_be_p(p, value.data.u16);
> -            } else {
> -                stw_le_p(p, value.data.u16);
> -            }
> +            stw_endian_p(be, p, value.data.u16);
>           } else {
> -            uint16_t val = be ? lduw_be_p(p) : lduw_le_p(p);
> +            uint16_t val = lduw_endian_p(be, p);
>               unseen_data = val != value.data.u16;
>           }
>           break;
> @@ -178,13 +174,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
>       {
>           uint32_t *p = (uint32_t *) &ri->data[offset];
>           if (is_store) {
> -            if (be) {
> -                stl_be_p(p, value.data.u32);
> -            } else {
> -                stl_le_p(p, value.data.u32);
> -            }
> +            stl_endian_p(be, p, value.data.u32);
>           } else {
> -            uint32_t val = be ? ldl_be_p(p) : ldl_le_p(p);
> +            uint32_t val = ldl_endian_p(be, p);
>               unseen_data = val != value.data.u32;
>           }
>           break;
> @@ -193,13 +185,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
>       {
>           uint64_t *p = (uint64_t *) &ri->data[offset];
>           if (is_store) {
> -            if (be) {
> -                stq_be_p(p, value.data.u64);
> -            } else {
> -                stq_le_p(p, value.data.u64);
> -            }
> +            stq_endian_p(be, p, value.data.u64);
>           } else {
> -            uint64_t val = be ? ldq_be_p(p) : ldq_le_p(p);
> +            uint64_t val = ldq_endian_p(be, p);
>               unseen_data = val != value.data.u64;
>           }
>           break;

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 06/13] tests/tcg/plugins: Use the ld/st_endian_p() API

[Alex Bennée]

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

> Refactor to use the recently introduced ld/st_endian_p() API
> No logical change intended.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 07/13] hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry

[Philippe Mathieu-Daudé]

Move code evaluation from preprocessor to compiler so
both if() ladders are processed. Mostly style change.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/xtensa/xtfpga.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 955e8867a3..228f00b045 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -415,8 +415,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
             }
         }
         if (entry_point != env->pc) {
-            uint8_t boot[] = {
-#if TARGET_BIG_ENDIAN
+            uint8_t boot_be[] = {
                 0x60, 0x00, 0x08,       /* j    1f */
                 0x00,                   /* .literal_position */
                 0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */
@@ -425,7 +424,8 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
                 0x10, 0xff, 0xfe,       /* l32r a0, entry_pc */
                 0x12, 0xff, 0xfe,       /* l32r a2, entry_a2 */
                 0x0a, 0x00, 0x00,       /* jx   a0 */
-#else
+            };
+            uint8_t boot_le[] = {
                 0x06, 0x02, 0x00,       /* j    1f */
                 0x00,                   /* .literal_position */
                 0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */
@@ -434,14 +434,16 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
                 0x01, 0xfe, 0xff,       /* l32r a0, entry_pc */
                 0x21, 0xfe, 0xff,       /* l32r a2, entry_a2 */
                 0xa0, 0x00, 0x00,       /* jx   a0 */
-#endif
             };
+            const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
+                                                     : sizeof(boot_le);
+            uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
             uint32_t entry_pc = tswap32(entry_point);
             uint32_t entry_a2 = tswap32(tagptr);
 
             memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
             memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
-            cpu_physical_memory_write(env->pc, boot, sizeof(boot));
+            cpu_physical_memory_write(env->pc, boot, boot_sz);
         }
     } else {
         if (flash) {
-- 
2.45.2

Re: [PATCH 07/13] hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry

[Thomas Huth]

On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
> Move code evaluation from preprocessor to compiler so
> both if() ladders are processed. Mostly style change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xtensa/xtfpga.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)

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

Re: [PATCH 07/13] hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Move code evaluation from preprocessor to compiler so
> both if() ladders are processed. Mostly style change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xtensa/xtfpga.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index 955e8867a3..228f00b045 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -415,8 +415,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>               }
>           }
>           if (entry_point != env->pc) {
> -            uint8_t boot[] = {
> -#if TARGET_BIG_ENDIAN
> +            uint8_t boot_be[] = {
>                   0x60, 0x00, 0x08,       /* j    1f */
>                   0x00,                   /* .literal_position */
>                   0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */
> @@ -425,7 +424,8 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>                   0x10, 0xff, 0xfe,       /* l32r a0, entry_pc */
>                   0x12, 0xff, 0xfe,       /* l32r a2, entry_a2 */
>                   0x0a, 0x00, 0x00,       /* jx   a0 */
> -#else
> +            };
> +            uint8_t boot_le[] = {
>                   0x06, 0x02, 0x00,       /* j    1f */
>                   0x00,                   /* .literal_position */
>                   0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */
> @@ -434,14 +434,16 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>                   0x01, 0xfe, 0xff,       /* l32r a0, entry_pc */
>                   0x21, 0xfe, 0xff,       /* l32r a2, entry_a2 */
>                   0xa0, 0x00, 0x00,       /* jx   a0 */
> -#endif
>               };
> +            const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
> +                                                     : sizeof(boot_le);
> +            uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>               uint32_t entry_pc = tswap32(entry_point);
>               uint32_t entry_a2 = tswap32(tagptr);
>   
>               memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>               memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
> -            cpu_physical_memory_write(env->pc, boot, sizeof(boot));
> +            cpu_physical_memory_write(env->pc, boot, boot_sz);
>           }
>       } else {
>           if (flash) {

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 07/13] hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry

[Richard Henderson]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Move code evaluation from preprocessor to compiler so
> both if() ladders are processed. Mostly style change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xtensa/xtfpga.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index 955e8867a3..228f00b045 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -415,8 +415,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>               }
>           }
>           if (entry_point != env->pc) {
> -            uint8_t boot[] = {
> -#if TARGET_BIG_ENDIAN
> +            uint8_t boot_be[] = {
>                   0x60, 0x00, 0x08,       /* j    1f */
>                   0x00,                   /* .literal_position */
>                   0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */
> @@ -425,7 +424,8 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>                   0x10, 0xff, 0xfe,       /* l32r a0, entry_pc */
>                   0x12, 0xff, 0xfe,       /* l32r a2, entry_a2 */
>                   0x0a, 0x00, 0x00,       /* jx   a0 */
> -#else
> +            };
> +            uint8_t boot_le[] = {

Fix these to use static const?


r~

Re: [PATCH 07/13] hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry

[Philippe Mathieu-Daudé]

On 3/10/24 18:35, Richard Henderson wrote:
> On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
>> Move code evaluation from preprocessor to compiler so
>> both if() ladders are processed. Mostly style change.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/xtensa/xtfpga.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>> index 955e8867a3..228f00b045 100644
>> --- a/hw/xtensa/xtfpga.c
>> +++ b/hw/xtensa/xtfpga.c
>> @@ -415,8 +415,7 @@ static void xtfpga_init(const XtfpgaBoardDesc 
>> *board, MachineState *machine)
>>               }
>>           }
>>           if (entry_point != env->pc) {
>> -            uint8_t boot[] = {
>> -#if TARGET_BIG_ENDIAN
>> +            uint8_t boot_be[] = {
>>                   0x60, 0x00, 0x08,       /* j    1f */
>>                   0x00,                   /* .literal_position */
>>                   0x00, 0x00, 0x00, 0x00, /* .literal entry_pc */

                                              ^^^^^^^^^^^^^^^^

>> @@ -425,7 +424,8 @@ static void xtfpga_init(const XtfpgaBoardDesc 
>> *board, MachineState *machine)
>>                   0x10, 0xff, 0xfe,       /* l32r a0, entry_pc */
>>                   0x12, 0xff, 0xfe,       /* l32r a2, entry_a2 */
>>                   0x0a, 0x00, 0x00,       /* jx   a0 */
>> -#else
>> +            };
>> +            uint8_t boot_le[] = {
> 
> Fix these to use static const?

We use memcpy() to up update $pc/$a2 in place.

> 
> 
> r~

[PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

Replace a pair of memcpy() + tswap32() by stl_endian_p(),
which also swap the value using target endianness.

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

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 228f00b045..521fe84b01 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
             const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
                                                      : sizeof(boot_le);
             uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
-            uint32_t entry_pc = tswap32(entry_point);
-            uint32_t entry_a2 = tswap32(tagptr);
 
-            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
-            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
+            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
+            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
             cpu_physical_memory_write(env->pc, boot, boot_sz);
         }
     } else {
-- 
2.45.2

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Thomas Huth]

On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
> which also swap the value using target endianness.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xtensa/xtfpga.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index 228f00b045..521fe84b01 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>               const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
>                                                        : sizeof(boot_le);
>               uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
> -            uint32_t entry_pc = tswap32(entry_point);
> -            uint32_t entry_a2 = tswap32(tagptr);
>   
> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);

Why don't you simply use stl_p() here?

  Thomas

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

On 30/9/24 16:32, Thomas Huth wrote:
> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>> which also swap the value using target endianness.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/xtensa/xtfpga.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>> index 228f00b045..521fe84b01 100644
>> --- a/hw/xtensa/xtfpga.c
>> +++ b/hw/xtensa/xtfpga.c
>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc 
>> *board, MachineState *machine)
>>               const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
>>                                                        : sizeof(boot_le);
>>               uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>> -            uint32_t entry_pc = tswap32(entry_point);
>> -            uint32_t entry_a2 = tswap32(tagptr);
>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
> 
> Why don't you simply use stl_p() here?

We want to remove the tswap32() calls...

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Pierrick Bouvier]

On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
> On 30/9/24 16:32, Thomas Huth wrote:
>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>> which also swap the value using target endianness.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/xtensa/xtfpga.c | 6 ++----
>>>    1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>> index 228f00b045..521fe84b01 100644
>>> --- a/hw/xtensa/xtfpga.c
>>> +++ b/hw/xtensa/xtfpga.c
>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>> *board, MachineState *machine)
>>>                const size_t boot_sz = TARGET_BIG_ENDIAN ? sizeof(boot_be)
>>>                                                         : sizeof(boot_le);
>>>                uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>>> -            uint32_t entry_pc = tswap32(entry_point);
>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>
>> Why don't you simply use stl_p() here?
> 
> We want to remove the tswap32() calls...
> 

I think is point is that you could directly use stl_be_p, instead of 
stl_endian_p(TARGET_BIT_ENDIAN, ...).
I don't know if your intent is to make be/le variant "private" and 
relies only on endian_p though.

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

On 3/10/24 18:04, Pierrick Bouvier wrote:
> On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
>> On 30/9/24 16:32, Thomas Huth wrote:
>>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>>> which also swap the value using target endianness.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    hw/xtensa/xtfpga.c | 6 ++----
>>>>    1 file changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>>> index 228f00b045..521fe84b01 100644
>>>> --- a/hw/xtensa/xtfpga.c
>>>> +++ b/hw/xtensa/xtfpga.c
>>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>>> *board, MachineState *machine)
>>>>                const size_t boot_sz = TARGET_BIG_ENDIAN ? 
>>>> sizeof(boot_be)
>>>>                                                         : 
>>>> sizeof(boot_le);
>>>>                uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>>>> -            uint32_t entry_pc = tswap32(entry_point);
>>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>>
>>> Why don't you simply use stl_p() here?
>>
>> We want to remove the tswap32() calls...
>>
> 
> I think is point is that you could directly use stl_be_p, instead of 
> stl_endian_p(TARGET_BIT_ENDIAN, ...).

TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.

The following change isn't worth it:

   if (TARGET_BIG_ENDIAN) {
     stl_be_p(boot + 8, tagptr);
   } else {
     stl_le_p(boot + 8, tagptr);
   }

Maybe I'm missing Thomas point, as the xtfpga machines are available
for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).

> I don't know if your intent is to make be/le variant "private" and 
> relies only on endian_p though.

My intent is to enforce endian agnostic API uses when possible, and
use LE/BE specific variant when it is known at build time.

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Pierrick Bouvier]

On 10/3/24 13:48, Philippe Mathieu-Daudé wrote:
> On 3/10/24 18:04, Pierrick Bouvier wrote:
>> On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
>>> On 30/9/24 16:32, Thomas Huth wrote:
>>>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>>>> which also swap the value using target endianness.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>>     hw/xtensa/xtfpga.c | 6 ++----
>>>>>     1 file changed, 2 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>>>> index 228f00b045..521fe84b01 100644
>>>>> --- a/hw/xtensa/xtfpga.c
>>>>> +++ b/hw/xtensa/xtfpga.c
>>>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>>>> *board, MachineState *machine)
>>>>>                 const size_t boot_sz = TARGET_BIG_ENDIAN ?
>>>>> sizeof(boot_be)
>>>>>                                                          :
>>>>> sizeof(boot_le);
>>>>>                 uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>>>>> -            uint32_t entry_pc = tswap32(entry_point);
>>>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>>>
>>>> Why don't you simply use stl_p() here?
>>>
>>> We want to remove the tswap32() calls...
>>>
>>
>> I think is point is that you could directly use stl_be_p, instead of
>> stl_endian_p(TARGET_BIT_ENDIAN, ...).
> 
> TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.
> 
> The following change isn't worth it:
> 
>     if (TARGET_BIG_ENDIAN) {
>       stl_be_p(boot + 8, tagptr);
>     } else {
>       stl_le_p(boot + 8, tagptr);
>     }
> 
> Maybe I'm missing Thomas point, as the xtfpga machines are available
> for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).
> 
>> I don't know if your intent is to make be/le variant "private" and
>> relies only on endian_p though.
> 
> My intent is to enforce endian agnostic API uses when possible, and
> use LE/BE specific variant when it is known at build time.

Oh ok, it's me who missed your point then.
For some reason, I thought we were always calling big endian variant.

Thus, your implementation makes totally sense.

Let's see if Thomas meant something different.
Else,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Pierrick Bouvier]

On 10/3/24 14:31, Pierrick Bouvier wrote:
> On 10/3/24 13:48, Philippe Mathieu-Daudé wrote:
>> On 3/10/24 18:04, Pierrick Bouvier wrote:
>>> On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
>>>> On 30/9/24 16:32, Thomas Huth wrote:
>>>>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>>>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>>>>> which also swap the value using target endianness.
>>>>>>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>> ---
>>>>>>      hw/xtensa/xtfpga.c | 6 ++----
>>>>>>      1 file changed, 2 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>>>>> index 228f00b045..521fe84b01 100644
>>>>>> --- a/hw/xtensa/xtfpga.c
>>>>>> +++ b/hw/xtensa/xtfpga.c
>>>>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>>>>> *board, MachineState *machine)
>>>>>>                  const size_t boot_sz = TARGET_BIG_ENDIAN ?
>>>>>> sizeof(boot_be)
>>>>>>                                                           :
>>>>>> sizeof(boot_le);
>>>>>>                  uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>>>>>> -            uint32_t entry_pc = tswap32(entry_point);
>>>>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>>>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>>>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>>>>
>>>>> Why don't you simply use stl_p() here?
>>>>
>>>> We want to remove the tswap32() calls...
>>>>
>>>
>>> I think is point is that you could directly use stl_be_p, instead of
>>> stl_endian_p(TARGET_BIT_ENDIAN, ...).
>>
>> TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.
>>
>> The following change isn't worth it:
>>
>>      if (TARGET_BIG_ENDIAN) {
>>        stl_be_p(boot + 8, tagptr);
>>      } else {
>>        stl_le_p(boot + 8, tagptr);
>>      }
>>
>> Maybe I'm missing Thomas point, as the xtfpga machines are available
>> for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).
>>
>>> I don't know if your intent is to make be/le variant "private" and
>>> relies only on endian_p though.
>>
>> My intent is to enforce endian agnostic API uses when possible, and
>> use LE/BE specific variant when it is known at build time.
> 
> Oh ok, it's me who missed your point then.
> For some reason, I thought we were always calling big endian variant.
> 
> Thus, your implementation makes totally sense.
> 
> Let's see if Thomas meant something different.
> Else,
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Looking more closely,
stl_p is already correctly defined when you know at compile time your 
target endianness. So Thomas was referring to this.

https://gitlab.com/qemu-project/qemu/-/blame/master/include/exec/cpu-all.h?ref_type=heads#L49

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Philippe Mathieu-Daudé]

On 3/10/24 23:34, Pierrick Bouvier wrote:
> On 10/3/24 14:31, Pierrick Bouvier wrote:
>> On 10/3/24 13:48, Philippe Mathieu-Daudé wrote:
>>> On 3/10/24 18:04, Pierrick Bouvier wrote:
>>>> On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
>>>>> On 30/9/24 16:32, Thomas Huth wrote:
>>>>>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>>>>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>>>>>> which also swap the value using target endianness.
>>>>>>>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>>> ---
>>>>>>>      hw/xtensa/xtfpga.c | 6 ++----
>>>>>>>      1 file changed, 2 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>>>>>> index 228f00b045..521fe84b01 100644
>>>>>>> --- a/hw/xtensa/xtfpga.c
>>>>>>> +++ b/hw/xtensa/xtfpga.c
>>>>>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>>>>>> *board, MachineState *machine)
>>>>>>>                  const size_t boot_sz = TARGET_BIG_ENDIAN ?
>>>>>>> sizeof(boot_be)
>>>>>>>                                                           :
>>>>>>> sizeof(boot_le);
>>>>>>>                  uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : 
>>>>>>> boot_le;
>>>>>>> -            uint32_t entry_pc = tswap32(entry_point);
>>>>>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>>>>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>>>>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>>>>>
>>>>>> Why don't you simply use stl_p() here?
>>>>>
>>>>> We want to remove the tswap32() calls...
>>>>>
>>>>
>>>> I think is point is that you could directly use stl_be_p, instead of
>>>> stl_endian_p(TARGET_BIT_ENDIAN, ...).
>>>
>>> TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.
>>>
>>> The following change isn't worth it:
>>>
>>>      if (TARGET_BIG_ENDIAN) {
>>>        stl_be_p(boot + 8, tagptr);
>>>      } else {
>>>        stl_le_p(boot + 8, tagptr);
>>>      }
>>>
>>> Maybe I'm missing Thomas point, as the xtfpga machines are available
>>> for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).
>>>
>>>> I don't know if your intent is to make be/le variant "private" and
>>>> relies only on endian_p though.
>>>
>>> My intent is to enforce endian agnostic API uses when possible, and
>>> use LE/BE specific variant when it is known at build time.
>>
>> Oh ok, it's me who missed your point then.
>> For some reason, I thought we were always calling big endian variant.
>>
>> Thus, your implementation makes totally sense.
>>
>> Let's see if Thomas meant something different.
>> Else,
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Looking more closely,
> stl_p is already correctly defined when you know at compile time your 
> target endianness. So Thomas was referring to this.
> 
> https://gitlab.com/qemu-project/qemu/-/blame/master/include/exec/cpu-all.h?ref_type=heads#L49

OK I guess I'm seeing Thomas point now; this series cover was not clear
enough. The goal is to remove TARGET_BIG_ENDIAN so we can build half
objects and do a little step toward the single binary.

Maybe I need to kill the stl_p & Co functions first, or do it while
suggesting this target agnostic replacement API.

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Thomas Huth]

On 03/10/2024 23.40, Philippe Mathieu-Daudé wrote:
> On 3/10/24 23:34, Pierrick Bouvier wrote:
>> On 10/3/24 14:31, Pierrick Bouvier wrote:
>>> On 10/3/24 13:48, Philippe Mathieu-Daudé wrote:
>>>> On 3/10/24 18:04, Pierrick Bouvier wrote:
>>>>> On 10/3/24 09:02, Philippe Mathieu-Daudé wrote:
>>>>>> On 30/9/24 16:32, Thomas Huth wrote:
>>>>>>> On 30/09/2024 09.34, Philippe Mathieu-Daudé wrote:
>>>>>>>> Replace a pair of memcpy() + tswap32() by stl_endian_p(),
>>>>>>>> which also swap the value using target endianness.
>>>>>>>>
>>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>>>> ---
>>>>>>>>      hw/xtensa/xtfpga.c | 6 ++----
>>>>>>>>      1 file changed, 2 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>>>>>>> index 228f00b045..521fe84b01 100644
>>>>>>>> --- a/hw/xtensa/xtfpga.c
>>>>>>>> +++ b/hw/xtensa/xtfpga.c
>>>>>>>> @@ -438,11 +438,9 @@ static void xtfpga_init(const XtfpgaBoardDesc
>>>>>>>> *board, MachineState *machine)
>>>>>>>>                  const size_t boot_sz = TARGET_BIG_ENDIAN ?
>>>>>>>> sizeof(boot_be)
>>>>>>>>                                                           :
>>>>>>>> sizeof(boot_le);
>>>>>>>>                  uint8_t *boot = TARGET_BIG_ENDIAN ? boot_be : boot_le;
>>>>>>>> -            uint32_t entry_pc = tswap32(entry_point);
>>>>>>>> -            uint32_t entry_a2 = tswap32(tagptr);
>>>>>>>> -            memcpy(boot + 4, &entry_pc, sizeof(entry_pc));
>>>>>>>> -            memcpy(boot + 8, &entry_a2, sizeof(entry_a2));
>>>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 4, entry_point);
>>>>>>>> +            stl_endian_p(TARGET_BIG_ENDIAN, boot + 8, tagptr);
>>>>>>>
>>>>>>> Why don't you simply use stl_p() here?
>>>>>>
>>>>>> We want to remove the tswap32() calls...
>>>>>>
>>>>>
>>>>> I think is point is that you could directly use stl_be_p, instead of
>>>>> stl_endian_p(TARGET_BIT_ENDIAN, ...).
>>>>
>>>> TARGET_BIG_ENDIAN is defined as 0 on little endian, and 1 on big one.
>>>>
>>>> The following change isn't worth it:
>>>>
>>>>      if (TARGET_BIG_ENDIAN) {
>>>>        stl_be_p(boot + 8, tagptr);
>>>>      } else {
>>>>        stl_le_p(boot + 8, tagptr);
>>>>      }
>>>>
>>>> Maybe I'm missing Thomas point, as the xtfpga machines are available
>>>> for both xtensa-softmmu (LE) and xtensaeb-softmmu (BE).
>>>>
>>>>> I don't know if your intent is to make be/le variant "private" and
>>>>> relies only on endian_p though.
>>>>
>>>> My intent is to enforce endian agnostic API uses when possible, and
>>>> use LE/BE specific variant when it is known at build time.
>>>
>>> Oh ok, it's me who missed your point then.
>>> For some reason, I thought we were always calling big endian variant.
>>>
>>> Thus, your implementation makes totally sense.
>>>
>>> Let's see if Thomas meant something different.
>>> Else,
>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Looking more closely,
>> stl_p is already correctly defined when you know at compile time your 
>> target endianness. So Thomas was referring to this.
>>
>> https://gitlab.com/qemu-project/qemu/-/blame/master/include/exec/cpu- 
>> all.h?ref_type=heads#L49
> 
> OK I guess I'm seeing Thomas point now; this series cover was not clear
> enough. The goal is to remove TARGET_BIG_ENDIAN so we can build half
> objects and do a little step toward the single binary.

Ok, that piece of information was missing in the patch description, indeed.
But the tswap function should already work with common code, see the 
target_needs_bswap() stuff in include/exec/tswap.h, so maybe this change 
here even is not needed at all?

  Thomas

Re: [PATCH 08/13] hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()

[Richard Henderson]

On 10/3/24 23:44, Thomas Huth wrote:
> On 03/10/2024 23.40, Philippe Mathieu-Daudé wrote:
>> OK I guess I'm seeing Thomas point now; this series cover was not clear
>> enough. The goal is to remove TARGET_BIG_ENDIAN so we can build half
>> objects and do a little step toward the single binary.
> 
> Ok, that piece of information was missing in the patch description, indeed.
> But the tswap function should already work with common code, see the target_needs_bswap() 
> stuff in include/exec/tswap.h, so maybe this change here even is not needed at all?

The current practice of target_needs_bswap() merely restricts the scope of 
TARGET_BIG_ENDIAN, allowing devices to be built once.  It still requires TARGET_BIG_ENDIAN 
to exist.

For the goal of single-binary, target_needs_bswap() cannot exist, since there may be no 
one correct answer across the system.  We will need the sort of infrastructure that Phil 
is adding: endianness being chosen by the machine model and/or the instantiated devices.

What is left is deciding on the exact API to use.  The ld/st_endian_p interface with a 
boolean seems reasonable.  It interacts well with TARGET_BIG_ENDIAN as an intermediate 
step, as well as with the virtio legacy cpu mode dependent callback.


r~

[PATCH 09/13] exec/memory_ldst_phys: Introduce ld/st_endian_phys() API

[Philippe Mathieu-Daudé]

Introduce the ld/st_endian_phys() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_phys() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update docstring regexp
---
 include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
index ecd678610d..8ea162b40d 100644
--- a/include/exec/memory_ldst_phys.h.inc
+++ b/include/exec/memory_ldst_phys.h.inc
@@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                                MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
+                                                      ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
+                                                 MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
+                                                 MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline uint32_t glue(ldl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
 {
     return glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
@@ -86,6 +96,16 @@ static inline uint32_t glue(ldl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                               MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
+                                                     ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline uint64_t glue(ldq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
 {
     return glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
@@ -98,6 +118,16 @@ static inline uint64_t glue(ldq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                               MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint32_t glue(ldq_endian_phys, SUFFIX)(bool big_endian,
+                                                     ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_ldq_be, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline void glue(stb_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint8_t val)
 {
     glue(address_space_stb, SUFFIX)(ARG1, addr, val,
@@ -116,6 +146,18 @@ static inline void glue(stw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint16_t va
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline void glue(stw_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint16_t val)
+{
+    if (big_endian) {
+        glue(address_space_stw_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stw_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
+
 static inline void glue(stl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t val)
 {
     glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
@@ -128,6 +170,18 @@ static inline void glue(stl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t va
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline void glue(stl_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint32_t val)
+{
+    if (big_endian) {
+        glue(address_space_stl_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
+
 static inline void glue(stq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t val)
 {
     glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
@@ -139,6 +193,18 @@ static inline void glue(stq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t va
     glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
+
+static inline void glue(stq_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint64_t val)
+{
+    if (big_endian) {
+        glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
 #endif
 
 #undef ARG1_DECL
-- 
2.45.2

Re: [PATCH 09/13] exec/memory_ldst_phys: Introduce ld/st_endian_phys() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline uint32_t glue(ldl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>   {
>       return glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> @@ -86,6 +96,16 @@ static inline uint32_t glue(ldl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                 MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline uint64_t glue(ldq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>   {
>       return glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
> @@ -98,6 +118,16 @@ static inline uint64_t glue(ldq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                 MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint32_t glue(ldq_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldq_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline void glue(stb_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint8_t val)
>   {
>       glue(address_space_stb, SUFFIX)(ARG1, addr, val,
> @@ -116,6 +146,18 @@ static inline void glue(stw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint16_t va
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline void glue(stw_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint16_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stw_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stw_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
> +
>   static inline void glue(stl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t val)
>   {
>       glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
> @@ -128,6 +170,18 @@ static inline void glue(stl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t va
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline void glue(stl_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint32_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stl_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
> +
>   static inline void glue(stq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t val)
>   {
>       glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
> @@ -139,6 +193,18 @@ static inline void glue(stq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t va
>       glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
> +
> +static inline void glue(stq_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint64_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
>   #endif
>   
>   #undef ARG1_DECL

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 09/13] exec/memory_ldst_phys: Introduce ld/st_endian_phys() API

[Richard Henderson]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +

This is backward, using the le function for big_endian true.

> +static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}

Likewise.


r~

Re: [PATCH 09/13] exec/memory_ldst_phys: Introduce ld/st_endian_phys() API

[Richard Henderson]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}

Endian swap aside, I think you should expose this at the address_space_* level first, 
where the internals already have

     return glue(address_space_lduw_internal, SUFFIX)(ARG1, addr, attrs, result,
                                                      DEVICE_LITTLE_ENDIAN);

then you can pass big_endian directly (perhaps frobbed into DEVICE_* space).

That leaves one unconditional function call instead of two conditional calls.


r~

[PATCH 10/13] hw/virtio/virtio-access: Use ld/st_endian_phys() API

[Philippe Mathieu-Daudé]

Refactor to use the recently introduced ld/st_endian_phys() API.
No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-access.h | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index b920874be8..37a42407ea 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -43,30 +43,21 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
-        return lduw_be_phys(dma_as, pa);
-    }
-    return lduw_le_phys(dma_as, pa);
+    return lduw_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
 }
 
 static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
-        return ldl_be_phys(dma_as, pa);
-    }
-    return ldl_le_phys(dma_as, pa);
+    return ldl_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
 }
 
 static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
-        return ldq_be_phys(dma_as, pa);
-    }
-    return ldq_le_phys(dma_as, pa);
+    return ldq_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
 }
 
 static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
@@ -74,11 +65,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
-        stw_be_phys(dma_as, pa, value);
-    } else {
-        stw_le_phys(dma_as, pa, value);
-    }
+    stw_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa, value);
 }
 
 static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
@@ -86,11 +73,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
 {
     AddressSpace *dma_as = vdev->dma_as;
 
-    if (virtio_access_is_big_endian(vdev)) {
-        stl_be_phys(dma_as, pa, value);
-    } else {
-        stl_le_phys(dma_as, pa, value);
-    }
+    stl_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa, value);
 }
 
 static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)
-- 
2.45.2

Re: [PATCH 10/13] hw/virtio/virtio-access: Use ld/st_endian_phys() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_phys() API.
> No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/virtio/virtio-access.h | 27 +++++----------------------
>   1 file changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index b920874be8..37a42407ea 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -43,30 +43,21 @@ static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
>   {
>       AddressSpace *dma_as = vdev->dma_as;
>   
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return lduw_be_phys(dma_as, pa);
> -    }
> -    return lduw_le_phys(dma_as, pa);
> +    return lduw_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
>   }
>   
>   static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
>   {
>       AddressSpace *dma_as = vdev->dma_as;
>   
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return ldl_be_phys(dma_as, pa);
> -    }
> -    return ldl_le_phys(dma_as, pa);
> +    return ldl_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
>   }
>   
>   static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
>   {
>       AddressSpace *dma_as = vdev->dma_as;
>   
> -    if (virtio_access_is_big_endian(vdev)) {
> -        return ldq_be_phys(dma_as, pa);
> -    }
> -    return ldq_le_phys(dma_as, pa);
> +    return ldq_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa);
>   }
>   
>   static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
> @@ -74,11 +65,7 @@ static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
>   {
>       AddressSpace *dma_as = vdev->dma_as;
>   
> -    if (virtio_access_is_big_endian(vdev)) {
> -        stw_be_phys(dma_as, pa, value);
> -    } else {
> -        stw_le_phys(dma_as, pa, value);
> -    }
> +    stw_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa, value);
>   }
>   
>   static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
> @@ -86,11 +73,7 @@ static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
>   {
>       AddressSpace *dma_as = vdev->dma_as;
>   
> -    if (virtio_access_is_big_endian(vdev)) {
> -        stl_be_phys(dma_as, pa, value);
> -    } else {
> -        stl_le_phys(dma_as, pa, value);
> -    }
> +    stl_endian_phys(virtio_access_is_big_endian(vdev), dma_as, pa, value);
>   }
>   
>   static inline void virtio_stw_p(VirtIODevice *vdev, void *ptr, uint16_t v)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 11/13] hw/pci/pci_device: Add PCI_DMA_DEFINE_LDST_END() macro

[Philippe Mathieu-Daudé]

Define both endianness variants with a single macro.
Useful to add yet other endian specific definitions
in the next commit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/pci/pci_device.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 91df40f989..ff619241a4 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -298,13 +298,14 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
         return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
     }
 
+#define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
+    PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
+    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
+
 PCI_DMA_DEFINE_LDST(ub, b, 8);
-PCI_DMA_DEFINE_LDST(uw_le, w_le, 16)
-PCI_DMA_DEFINE_LDST(l_le, l_le, 32);
-PCI_DMA_DEFINE_LDST(q_le, q_le, 64);
-PCI_DMA_DEFINE_LDST(uw_be, w_be, 16)
-PCI_DMA_DEFINE_LDST(l_be, l_be, 32);
-PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
+PCI_DMA_DEFINE_LDST_END(uw, w, 16)
+PCI_DMA_DEFINE_LDST_END(l,  l, 32)
+PCI_DMA_DEFINE_LDST_END(q,  q, 64)
 
 #undef PCI_DMA_DEFINE_LDST
 
-- 
2.45.2

Re: [PATCH 11/13] hw/pci/pci_device: Add PCI_DMA_DEFINE_LDST_END() macro

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Define both endianness variants with a single macro.
> Useful to add yet other endian specific definitions
> in the next commit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/pci/pci_device.h | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index 91df40f989..ff619241a4 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -298,13 +298,14 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>           return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
>       }
>   
> +#define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
> +    PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
> +    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
> +
>   PCI_DMA_DEFINE_LDST(ub, b, 8);
> -PCI_DMA_DEFINE_LDST(uw_le, w_le, 16)
> -PCI_DMA_DEFINE_LDST(l_le, l_le, 32);
> -PCI_DMA_DEFINE_LDST(q_le, q_le, 64);
> -PCI_DMA_DEFINE_LDST(uw_be, w_be, 16)
> -PCI_DMA_DEFINE_LDST(l_be, l_be, 32);
> -PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
> +PCI_DMA_DEFINE_LDST_END(uw, w, 16)
> +PCI_DMA_DEFINE_LDST_END(l,  l, 32)
> +PCI_DMA_DEFINE_LDST_END(q,  q, 64)
>   
>   #undef PCI_DMA_DEFINE_LDST
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API

[Philippe Mathieu-Daudé]

Introduce the ld/st_endian_pci_dma() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update docstring regexp
---
 include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index ff619241a4..dc9b17dded 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
 
 #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
     PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
-    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
+    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
+    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
+                                                      PCIDevice *dev, \
+                                                      dma_addr_t addr, \
+                                                      uint##_bits##_t *val, \
+                                                      MemTxAttrs attrs) \
+    { \
+        AddressSpace *pci_as = pci_get_address_space(dev); \
+        return is_big_endian \
+               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
+               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
+    } \
+    static inline MemTxResult st##_s##_endian_pci_dma(bool is_big_endian, \
+                                                      PCIDevice *dev, \
+                                                      dma_addr_t addr, \
+                                                      uint##_bits##_t val, \
+                                                      MemTxAttrs attrs) \
+    { \
+        AddressSpace *pci_as = pci_get_address_space(dev); \
+        return is_big_endian \
+               ? st##_s##_be_dma(pci_as, addr, val, attrs) \
+               : st##_s##_le_dma(pci_as, addr, val, attrs); \
+    }
 
 PCI_DMA_DEFINE_LDST(ub, b, 8);
 PCI_DMA_DEFINE_LDST_END(uw, w, 16)
-- 
2.45.2

Re: [PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_pci_dma() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ff619241a4..dc9b17dded 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>   
>   #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
>       PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
> -    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
> +    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
> +    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t *val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
> +               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
> +    } \
> +    static inline MemTxResult st##_s##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? st##_s##_be_dma(pci_as, addr, val, attrs) \
> +               : st##_s##_le_dma(pci_as, addr, val, attrs); \
> +    }
>   
>   PCI_DMA_DEFINE_LDST(ub, b, 8);
>   PCI_DMA_DEFINE_LDST_END(uw, w, 16)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 12/13] hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API

[Richard Henderson]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_pci_dma() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_pci_dma() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/hw/pci/pci_device.h | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ff619241a4..dc9b17dded 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -300,7 +300,29 @@ static inline MemTxResult pci_dma_write(PCIDevice *dev, dma_addr_t addr,
>   
>   #define PCI_DMA_DEFINE_LDST_END(_l, _s, _bits) \
>       PCI_DMA_DEFINE_LDST(_l##_le, _s##_le, _bits) \
> -    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits)
> +    PCI_DMA_DEFINE_LDST(_l##_be, _s##_be, _bits) \
> +    static inline MemTxResult ld##_l##_endian_pci_dma(bool is_big_endian, \
> +                                                      PCIDevice *dev, \
> +                                                      dma_addr_t addr, \
> +                                                      uint##_bits##_t *val, \
> +                                                      MemTxAttrs attrs) \
> +    { \
> +        AddressSpace *pci_as = pci_get_address_space(dev); \
> +        return is_big_endian \
> +               ? ld##_l##_be_dma(pci_as, addr, val, attrs) \
> +               : ld##_l##_le_dma(pci_as, addr, val, attrs); \
> +    } \

Like the address_space_* functions, I think the endianness is being handled at the wrong 
level here.  Improve sysemu/dma.h first.


r~

[PATCH 13/13] hw/net/tulip: Use ld/st_endian_pci_dma() API

[Philippe Mathieu-Daudé]

Refactor to use the recently introduced ld/st_endian_pci_dma()
API. No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/tulip.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 9df3e17162..6c67958da7 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p,
         struct tulip_descriptor *desc)
 {
     const MemTxAttrs attrs = { .memory = true };
+    bool use_big_endian = s->csr[0] & CSR0_DBO;
 
-    if (s->csr[0] & CSR0_DBO) {
-        ldl_be_pci_dma(&s->dev, p, &desc->status, attrs);
-        ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs);
-        ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
-        ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
-    } else {
-        ldl_le_pci_dma(&s->dev, p, &desc->status, attrs);
-        ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs);
-        ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
-        ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
-    }
+    ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs);
+    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs);
+    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs);
+    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs);
 }
 
 static void tulip_desc_write(TULIPState *s, hwaddr p,
         struct tulip_descriptor *desc)
 {
     const MemTxAttrs attrs = { .memory = true };
+    bool use_big_endian = s->csr[0] & CSR0_DBO;
 
-    if (s->csr[0] & CSR0_DBO) {
-        stl_be_pci_dma(&s->dev, p, desc->status, attrs);
-        stl_be_pci_dma(&s->dev, p + 4, desc->control, attrs);
-        stl_be_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs);
-        stl_be_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs);
-    } else {
-        stl_le_pci_dma(&s->dev, p, desc->status, attrs);
-        stl_le_pci_dma(&s->dev, p + 4, desc->control, attrs);
-        stl_le_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs);
-        stl_le_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs);
-    }
+    stl_endian_pci_dma(use_big_endian, &s->dev, p, desc->status, attrs);
+    stl_endian_pci_dma(use_big_endian, &s->dev, p + 4, desc->control, attrs);
+    stl_endian_pci_dma(use_big_endian, &s->dev, p + 8, desc->buf_addr1, attrs);
+    stl_endian_pci_dma(use_big_endian, &s->dev, p + 12, desc->buf_addr2, attrs);
 }
 
 static void tulip_update_int(TULIPState *s)
-- 
2.45.2

Re: [PATCH 13/13] hw/net/tulip: Use ld/st_endian_pci_dma() API

[Pierrick Bouvier]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_pci_dma()
> API. No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/tulip.c | 32 ++++++++++----------------------
>   1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 9df3e17162..6c67958da7 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p,
>           struct tulip_descriptor *desc)
>   {
>       const MemTxAttrs attrs = { .memory = true };
> +    bool use_big_endian = s->csr[0] & CSR0_DBO;
>   
> -    if (s->csr[0] & CSR0_DBO) {
> -        ldl_be_pci_dma(&s->dev, p, &desc->status, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
> -    } else {
> -        ldl_le_pci_dma(&s->dev, p, &desc->status, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
> -    }
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs);
>   }
>   
>   static void tulip_desc_write(TULIPState *s, hwaddr p,
>           struct tulip_descriptor *desc)
>   {
>       const MemTxAttrs attrs = { .memory = true };
> +    bool use_big_endian = s->csr[0] & CSR0_DBO;
>   
> -    if (s->csr[0] & CSR0_DBO) {
> -        stl_be_pci_dma(&s->dev, p, desc->status, attrs);
> -        stl_be_pci_dma(&s->dev, p + 4, desc->control, attrs);
> -        stl_be_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs);
> -        stl_be_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs);
> -    } else {
> -        stl_le_pci_dma(&s->dev, p, desc->status, attrs);
> -        stl_le_pci_dma(&s->dev, p + 4, desc->control, attrs);
> -        stl_le_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs);
> -        stl_le_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs);
> -    }
> +    stl_endian_pci_dma(use_big_endian, &s->dev, p, desc->status, attrs);
> +    stl_endian_pci_dma(use_big_endian, &s->dev, p + 4, desc->control, attrs);
> +    stl_endian_pci_dma(use_big_endian, &s->dev, p + 8, desc->buf_addr1, attrs);
> +    stl_endian_pci_dma(use_big_endian, &s->dev, p + 12, desc->buf_addr2, attrs);
>   }
>   
>   static void tulip_update_int(TULIPState *s)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 13/13] hw/net/tulip: Use ld/st_endian_pci_dma() API

[Richard Henderson]

On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Refactor to use the recently introduced ld/st_endian_pci_dma()
> API. No logical change intended.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/tulip.c | 32 ++++++++++----------------------
>   1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 9df3e17162..6c67958da7 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p,
>           struct tulip_descriptor *desc)
>   {
>       const MemTxAttrs attrs = { .memory = true };
> +    bool use_big_endian = s->csr[0] & CSR0_DBO;
>   
> -    if (s->csr[0] & CSR0_DBO) {
> -        ldl_be_pci_dma(&s->dev, p, &desc->status, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
> -        ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
> -    } else {
> -        ldl_le_pci_dma(&s->dev, p, &desc->status, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs);
> -        ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs);
> -    }
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs);
> +    ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs);

I don't know if I'm keen on replacing 1 conditional with 4.
I had the same thought vs patch 3, in target/arm/ptw.c.

I suppose it's not exactly performance sensative code, and the compiler might be able to 
do something, given that the conditional is invariant, but it strikes me as untidy.

Anyone else care to weigh in?


r~