[PATCH 1/2] include/exec: Provide the cpu_tswap() functions

Martin Kröning via posted 2 patches 4 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>
[PATCH 1/2] include/exec: Provide the cpu_tswap() functions
Posted by Martin Kröning via 4 weeks ago
These functions are needed for CPUs that support runtime-configurable endianness.
In those cases, components such as semihosting need to perform
runtime-dependent byte swaps.

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
---
 include/exec/tswap.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/exec/tswap.h b/include/exec/tswap.h
index 72219e2c43..9aaafb12f3 100644
--- a/include/exec/tswap.h
+++ b/include/exec/tswap.h
@@ -10,6 +10,7 @@
 
 #include "qemu/bswap.h"
 #include "qemu/target-info.h"
+#include "hw/core/cpu.h"
 
 /*
  * If we're in target-specific code, we can hard-code the swapping
@@ -21,6 +22,8 @@
 #define target_needs_bswap()  (HOST_BIG_ENDIAN != target_big_endian())
 #endif /* COMPILING_PER_TARGET */
 
+#define cpu_needs_bswap(cpu)  (HOST_BIG_ENDIAN != cpu_virtio_is_big_endian(cpu))
+
 static inline uint16_t tswap16(uint16_t s)
 {
     if (target_needs_bswap()) {
@@ -48,6 +51,33 @@ static inline uint64_t tswap64(uint64_t s)
     }
 }
 
+static inline uint16_t cpu_tswap16(CPUState *cpu, uint16_t s)
+{
+    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
+        return bswap16(s);
+    } else {
+        return s;
+    }
+}
+
+static inline uint32_t cpu_tswap32(CPUState *cpu, uint32_t s)
+{
+    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
+        return bswap32(s);
+    } else {
+        return s;
+    }
+}
+
+static inline uint64_t cpu_tswap64(CPUState *cpu, uint64_t s)
+{
+    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
+        return bswap64(s);
+    } else {
+        return s;
+    }
+}
+
 static inline void tswap16s(uint16_t *s)
 {
     if (target_needs_bswap()) {

-- 
Git-155)


Re: [PATCH 1/2] include/exec: Provide the cpu_tswap() functions
Posted by Philippe Mathieu-Daudé 2 weeks, 2 days ago
Hi Martin,

On 6/1/26 14:43, Martin Kröning via wrote:
> These functions are needed for CPUs that support runtime-configurable endianness.
> In those cases, components such as semihosting need to perform
> runtime-dependent byte swaps.

Are you targetting user or system emulation?

I suppose user emulation, otherwise you'd have used the
"semihosting/uaccess.h" API.

But then I'm confused because a user process can't change
the CPU endianness...

Can you explain your use case?

> Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
> ---
>   include/exec/tswap.h | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> index 72219e2c43..9aaafb12f3 100644
> --- a/include/exec/tswap.h
> +++ b/include/exec/tswap.h
> @@ -10,6 +10,7 @@
>   
>   #include "qemu/bswap.h"
>   #include "qemu/target-info.h"
> +#include "hw/core/cpu.h"
>   
>   /*
>    * If we're in target-specific code, we can hard-code the swapping
> @@ -21,6 +22,8 @@
>   #define target_needs_bswap()  (HOST_BIG_ENDIAN != target_big_endian())
>   #endif /* COMPILING_PER_TARGET */
>   
> +#define cpu_needs_bswap(cpu)  (HOST_BIG_ENDIAN != cpu_virtio_is_big_endian(cpu))
> +
>   static inline uint16_t tswap16(uint16_t s)
>   {
>       if (target_needs_bswap()) {
> @@ -48,6 +51,33 @@ static inline uint64_t tswap64(uint64_t s)
>       }
>   }
>   
> +static inline uint16_t cpu_tswap16(CPUState *cpu, uint16_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap16(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint32_t cpu_tswap32(CPUState *cpu, uint32_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap32(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint64_t cpu_tswap64(CPUState *cpu, uint64_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap64(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
>   static inline void tswap16s(uint16_t *s)
>   {
>       if (target_needs_bswap()) {
> 


Re: [PATCH 1/2] include/exec: Provide the cpu_tswap() functions
Posted by martin.kroening--- via qemu development 2 weeks, 2 days ago
Hi Philippe,

On 18.01.26, 22:33, Philippe Mathieu-Daudé wrote:

> > These functions are needed for CPUs that support runtime-configurable endianness.
> > In those cases, components such as semihosting need to perform
> > runtime-dependent byte swaps.
>
> Are you targetting user or system emulation?
> 
> I suppose user emulation, otherwise you'd have used the
> "semihosting/uaccess.h" API.
>
> But then I'm confused because a user process can't change
> the CPU endianness...
>
> Can you explain your use case?

Thanks for asking! I am targeting system emulation. My use case is emulating
bare-metal software such as an OS that switches the AArch64 CPU to big-endian
mode during runtime.

`{get,set}_user_u{64,32}` from "semihosting/uaccess.h" currently use
`tswap{32,64}` from "exec/tswap.h", which do not respect runtime-configurable
endianness.

PATCH 1/2 introduces `cpu_tswap{32,64}`, which PATCH 2/2 integrates into
"semihosting/uaccess.h". I can squash those commits if you prefer, of course.
Or did I misunderstand your question?

Alex was worried about expanding the use of `virtio_is_big_endian`:

> Hmm looking at the description:
> 
>     /**
>      * @virtio_is_big_endian: Callback to return %true if a CPU which supports
>      * runtime configurable endianness is currently big-endian.
>      * Non-configurable CPUs can use the default implementation of this method.
>      * This method should not be used by any callers other than the pre-1.0
>      * virtio devices.
>      */
>     bool (*virtio_is_big_endian)(CPUState *cpu);
> 
> I'm not sure if we want to expand the usage of this hack. I think
> Philippe is hoping to get rid of these warts eventually. Of course we
> could rename the method and just provide a way to get the current
> systems endianess.

While not being very familiar with QEMU's source code, something like this
seems necessary to me. I can rename `virtio_is_big_endian` to `is_big_endian`
and `cpu_virtio_is_big_endian` to `cpu_is_big_endian` if you prefer that.

Cheers,
Martin

Re: [PATCH 1/2] include/exec: Provide the cpu_tswap() functions
Posted by Alex Bennée 4 weeks ago
Martin Kröning <martin.kroening@eonerc.rwth-aachen.de> writes:

(adding Philippe to CC)

> These functions are needed for CPUs that support runtime-configurable endianness.
> In those cases, components such as semihosting need to perform
> runtime-dependent byte swaps.
>
> Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
> ---
>  include/exec/tswap.h | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> index 72219e2c43..9aaafb12f3 100644
> --- a/include/exec/tswap.h
> +++ b/include/exec/tswap.h
> @@ -10,6 +10,7 @@
>  
>  #include "qemu/bswap.h"
>  #include "qemu/target-info.h"
> +#include "hw/core/cpu.h"
>  
>  /*
>   * If we're in target-specific code, we can hard-code the swapping
> @@ -21,6 +22,8 @@
>  #define target_needs_bswap()  (HOST_BIG_ENDIAN != target_big_endian())
>  #endif /* COMPILING_PER_TARGET */
>  
> +#define cpu_needs_bswap(cpu)  (HOST_BIG_ENDIAN != cpu_virtio_is_big_endian(cpu))
> +

Hmm looking at the description:

    /**
     * @virtio_is_big_endian: Callback to return %true if a CPU which supports
     * runtime configurable endianness is currently big-endian.
     * Non-configurable CPUs can use the default implementation of this method.
     * This method should not be used by any callers other than the pre-1.0
     * virtio devices.
     */
    bool (*virtio_is_big_endian)(CPUState *cpu);

I'm not sure if we want to expand the usage of this hack. I think
Philippe is hoping to get rid of these warts eventually. Of course we
could rename the method and just provide a way to get the current
systems endianess.


>  static inline uint16_t tswap16(uint16_t s)
>  {
>      if (target_needs_bswap()) {
> @@ -48,6 +51,33 @@ static inline uint64_t tswap64(uint64_t s)
>      }
>  }
>  
> +static inline uint16_t cpu_tswap16(CPUState *cpu, uint16_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap16(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint32_t cpu_tswap32(CPUState *cpu, uint32_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap32(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint64_t cpu_tswap64(CPUState *cpu, uint64_t s)
> +{
> +    if (target_needs_bswap() || cpu_needs_bswap(cpu)) {
> +        return bswap64(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
>  static inline void tswap16s(uint16_t *s)
>  {
>      if (target_needs_bswap()) {

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 1/2] include/exec: Provide the cpu_tswap() functions
Posted by martin.kroening--- via qemu development 3 weeks, 1 day ago
Thanks for responding so swiftly! Sorry for not doing the same. I was thinking
maybe Philippe would want to chime in first.

> > +#define cpu_needs_bswap(cpu) (HOST_BIG_ENDIAN != cpu_virtio_is_big_endian(cpu))
> > +
> 
> 
> Hmm looking at the description:
> 
> 
> /**
> * @virtio_is_big_endian: Callback to return %true if a CPU which supports
> * runtime configurable endianness is currently big-endian.
> * Non-configurable CPUs can use the default implementation of this method.
> * This method should not be used by any callers other than the pre-1.0
> * virtio devices.
> */
> bool (*virtio_is_big_endian)(CPUState *cpu);
> 
> 
> I'm not sure if we want to expand the usage of this hack. I think
> Philippe is hoping to get rid of these warts eventually. Of course we
> could rename the method and just provide a way to get the current
> systems endianess.


I am not very familiar with QEMU's source code, but having a way to get the
current system's endianness sounds necessary to me considering semihosting's
requirements. Should I just rename `virtio_is_big_endian` to `is_big_endian`
and `cpu_virtio_is_big_endian` to `cpu_is_big_endian` or do you prefer
different names?