[PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions

Martin Kröning via qemu development posted 3 patches 3 weeks, 6 days ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Peter Maydell <peter.maydell@linaro.org>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>
[PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Martin Kröning via qemu development 3 weeks, 6 days ago
These functions are needed to support semihosting on CPUs that support
runtime-configurable endianness. They should not be used in other contexts.

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

diff --git a/include/exec/tswap.h b/include/exec/tswap.h
index 9e94fa0021..17ac544454 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
@@ -72,4 +73,39 @@ static inline void tswap64s(uint64_t *s)
 }
 #endif
 
+/*
+ * If we're in semihosting code, have to swap depending on the currently
+ * configured endianness of the CPU. These functions should not be used in
+ * other contexts.
+ */
+#define cpu_internal_needs_bswap(cpu) \
+    (HOST_BIG_ENDIAN != cpu_internal_is_big_endian(cpu))
+
+static inline uint16_t cpu_internal_tswap16(CPUState *cpu, uint16_t s)
+{
+    if (cpu_internal_needs_bswap(cpu)) {
+        return bswap16(s);
+    } else {
+        return s;
+    }
+}
+
+static inline uint32_t cpu_internal_tswap32(CPUState *cpu, uint32_t s)
+{
+    if (cpu_internal_needs_bswap(cpu)) {
+        return bswap32(s);
+    } else {
+        return s;
+    }
+}
+
+static inline uint64_t cpu_internal_tswap64(CPUState *cpu, uint64_t s)
+{
+    if (cpu_internal_needs_bswap(cpu)) {
+        return bswap64(s);
+    } else {
+        return s;
+    }
+}
+
 #endif  /* TSWAP_H */

-- 
Git-155)


Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago
On 11/3/26 17:27, Martin Kröning wrote:
> These functions are needed to support semihosting on CPUs that support
> runtime-configurable endianness. They should not be used in other contexts.

Another note is these function are not target-specific, which is great!

These can also be used outside of semihosting.

Matter of taste I'd have named them using this shorter pattern:

cpu_internal_tswap16() -> tswap16_cpu()

> 
> Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
> ---
>   include/exec/tswap.h | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> index 9e94fa0021..17ac544454 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
> @@ -72,4 +73,39 @@ static inline void tswap64s(uint64_t *s)
>   }
>   #endif
>   
> +/*
> + * If we're in semihosting code, have to swap depending on the currently
> + * configured endianness of the CPU. These functions should not be used in
> + * other contexts.
> + */
> +#define cpu_internal_needs_bswap(cpu) \
> +    (HOST_BIG_ENDIAN != cpu_internal_is_big_endian(cpu))
> +
> +static inline uint16_t cpu_internal_tswap16(CPUState *cpu, uint16_t s)
> +{
> +    if (cpu_internal_needs_bswap(cpu)) {
> +        return bswap16(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint32_t cpu_internal_tswap32(CPUState *cpu, uint32_t s)
> +{
> +    if (cpu_internal_needs_bswap(cpu)) {
> +        return bswap32(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint64_t cpu_internal_tswap64(CPUState *cpu, uint64_t s)
> +{
> +    if (cpu_internal_needs_bswap(cpu)) {
> +        return bswap64(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
>   #endif  /* TSWAP_H */
> 


Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Peter Maydell 3 weeks, 5 days ago
On Thu, 12 Mar 2026 at 19:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 11/3/26 17:27, Martin Kröning wrote:
> > These functions are needed to support semihosting on CPUs that support
> > runtime-configurable endianness. They should not be used in other contexts.
>
> Another note is these function are not target-specific, which is great!
>
> These can also be used outside of semihosting.
>
> Matter of taste I'd have named them using this shorter pattern:
>
> cpu_internal_tswap16() -> tswap16_cpu()

We specifically didn't want to use a name that might tempt
people to use them -- these are only wanted for the very
few niche use cases that are outside the CPU but need to
know information about the CPU's internals, which is basically
legacy-virtio and semihosting.

-- PMM
Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago
On 12/3/26 20:30, Peter Maydell wrote:
> On Thu, 12 Mar 2026 at 19:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 11/3/26 17:27, Martin Kröning wrote:
>>> These functions are needed to support semihosting on CPUs that support
>>> runtime-configurable endianness. They should not be used in other contexts.
>>
>> Another note is these function are not target-specific, which is great!
>>
>> These can also be used outside of semihosting.
>>
>> Matter of taste I'd have named them using this shorter pattern:
>>
>> cpu_internal_tswap16() -> tswap16_cpu()
> 
> We specifically didn't want to use a name that might tempt
> people to use them -- these are only wanted for the very
> few niche use cases that are outside the CPU but need to
> know information about the CPU's internals, which is basically
> legacy-virtio and semihosting.

This was the missing piece to make qtest target-agnostic:

-- >8 --
diff --git a/system/qtest.c b/system/qtest.c
index cf90cd53adb..392c1394656 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -523,18 +523,15 @@ static void qtest_process_command(CharFrontend 
*chr, gchar **words)
              address_space_write(first_cpu->as, addr, 
MEMTXATTRS_UNSPECIFIED,
                                  &data, 1);
          } else if (words[0][5] == 'w') {
-            uint16_t data = value;
-            tswap16s(&data);
+            uint16_t data = cpu_internal_tswap16(first_cpu, value);
              address_space_write(first_cpu->as, addr, 
MEMTXATTRS_UNSPECIFIED,
                                  &data, 2);
          } else if (words[0][5] == 'l') {
-            uint32_t data = value;
-            tswap32s(&data);
+            uint32_t data = cpu_internal_tswap32(first_cpu, value);
              address_space_write(first_cpu->as, addr, 
MEMTXATTRS_UNSPECIFIED,
                                  &data, 4);
          } else if (words[0][5] == 'q') {
-            uint64_t data = value;
-            tswap64s(&data);
+            uint64_t data = cpu_internal_tswap64(first_cpu, value);
              address_space_write(first_cpu->as, addr, 
MEMTXATTRS_UNSPECIFIED,
                                  &data, 8);
          }
---

QTest is already dubious by always accessing the first CPU AS, so
using the first CPU endianness is good enough for me IMO. (Ideally
QTest would support multiple address-spaces).

Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Peter Maydell 3 weeks, 4 days ago
On Thu, 12 Mar 2026 at 19:37, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 12/3/26 20:30, Peter Maydell wrote:
> > On Thu, 12 Mar 2026 at 19:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >> On 11/3/26 17:27, Martin Kröning wrote:
> >>> These functions are needed to support semihosting on CPUs that support
> >>> runtime-configurable endianness. They should not be used in other contexts.
> >>
> >> Another note is these function are not target-specific, which is great!
> >>
> >> These can also be used outside of semihosting.
> >>
> >> Matter of taste I'd have named them using this shorter pattern:
> >>
> >> cpu_internal_tswap16() -> tswap16_cpu()
> >
> > We specifically didn't want to use a name that might tempt
> > people to use them -- these are only wanted for the very
> > few niche use cases that are outside the CPU but need to
> > know information about the CPU's internals, which is basically
> > legacy-virtio and semihosting.
>
> This was the missing piece to make qtest target-agnostic:
>
> -- >8 --
> diff --git a/system/qtest.c b/system/qtest.c
> index cf90cd53adb..392c1394656 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -523,18 +523,15 @@ static void qtest_process_command(CharFrontend
> *chr, gchar **words)
>               address_space_write(first_cpu->as, addr,
> MEMTXATTRS_UNSPECIFIED,
>                                   &data, 1);
>           } else if (words[0][5] == 'w') {
> -            uint16_t data = value;
> -            tswap16s(&data);
> +            uint16_t data = cpu_internal_tswap16(first_cpu, value);

This is a behaviour change, and I don't think we want it.
tswap16s() swaps per the system endianness. cpu_internal_tswap16()
swaps based on the CPU's current configuration. With qtest
that's a bit of a weird thing, because we can never run the
CPU in a way that will make it switch endianness.

This is why we gave these functions the _internal_ bit in
the name -- they really are a very very specific piece
of functionality that you almost never want.

> And these endianness checks in QTest framework are only there to test
> legacy virtio, so we are good, this isn't even another niche use case.

No, these ones are here so that when when a test does a read
or write of non-byte width data (e.g. via "writel 0x8000 0x12345678")
we send it into the memory system in the endianness that it
expects, so that it arrives at the device as "write of width 4
with value 0x12345678". That is, we want to match it to the
system endianness, because that's what we want to cancel out
the swapping done in adjust_endianness() in system/memory.c.

thanks
-- PMM
Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Philippe Mathieu-Daudé 3 weeks, 2 days ago
On 13/3/26 09:51, Peter Maydell wrote:
> On Thu, 12 Mar 2026 at 19:37, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 12/3/26 20:30, Peter Maydell wrote:
>>> On Thu, 12 Mar 2026 at 19:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>>
>>>> On 11/3/26 17:27, Martin Kröning wrote:
>>>>> These functions are needed to support semihosting on CPUs that support
>>>>> runtime-configurable endianness. They should not be used in other contexts.
>>>>
>>>> Another note is these function are not target-specific, which is great!
>>>>
>>>> These can also be used outside of semihosting.
>>>>
>>>> Matter of taste I'd have named them using this shorter pattern:
>>>>
>>>> cpu_internal_tswap16() -> tswap16_cpu()
>>>
>>> We specifically didn't want to use a name that might tempt
>>> people to use them -- these are only wanted for the very
>>> few niche use cases that are outside the CPU but need to
>>> know information about the CPU's internals, which is basically
>>> legacy-virtio and semihosting.
>>
>> This was the missing piece to make qtest target-agnostic:
>>
>> -- >8 --
>> diff --git a/system/qtest.c b/system/qtest.c
>> index cf90cd53adb..392c1394656 100644
>> --- a/system/qtest.c
>> +++ b/system/qtest.c
>> @@ -523,18 +523,15 @@ static void qtest_process_command(CharFrontend
>> *chr, gchar **words)
>>                address_space_write(first_cpu->as, addr,
>> MEMTXATTRS_UNSPECIFIED,
>>                                    &data, 1);
>>            } else if (words[0][5] == 'w') {
>> -            uint16_t data = value;
>> -            tswap16s(&data);
>> +            uint16_t data = cpu_internal_tswap16(first_cpu, value);
> 
> This is a behaviour change, and I don't think we want it.
> tswap16s() swaps per the system endianness. cpu_internal_tswap16()
> swaps based on the CPU's current configuration. With qtest
> that's a bit of a weird thing, because we can never run the
> CPU in a way that will make it switch endianness.
> 
> This is why we gave these functions the _internal_ bit in
> the name -- they really are a very very specific piece
> of functionality that you almost never want.
> 
>> And these endianness checks in QTest framework are only there to test
>> legacy virtio, so we are good, this isn't even another niche use case.
> 
> No, these ones are here so that when when a test does a read
> or write of non-byte width data (e.g. via "writel 0x8000 0x12345678")
> we send it into the memory system in the endianness that it
> expects, so that it arrives at the device as "write of width 4
> with value 0x12345678". That is, we want to match it to the
> system endianness, because that's what we want to cancel out
> the swapping done in adjust_endianness() in system/memory.c.

What do you mean by "system endianness"? The default endianness picked
for a particular qemu-system-foo binary?

Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Peter Maydell 3 weeks, 1 day ago
On Sun, 15 Mar 2026 at 14:07, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 13/3/26 09:51, Peter Maydell wrote:
> > No, these ones are here so that when when a test does a read
> > or write of non-byte width data (e.g. via "writel 0x8000 0x12345678")
> > we send it into the memory system in the endianness that it
> > expects, so that it arrives at the device as "write of width 4
> > with value 0x12345678". That is, we want to match it to the
> > system endianness, because that's what we want to cancel out
> > the swapping done in adjust_endianness() in system/memory.c.
>
> What do you mean by "system endianness"? The default endianness picked
> for a particular qemu-system-foo binary?

I mean whether TARGET_BIG_ENDIAN is set -- the effective
endianness of the whole system, which affects whether
the core memory system thinks it needs to byteswap data
that passes through it.

That's different from what the CPU does internally for things like
Arm CPSR.E. An Arm system is always little-endian at the
system level, regardless of CPSR.E. Setting CPSR.E just makes
the CPU byteswap data before sending it out of the CPU to the
system. (That's how it works in hardware as well as QEMU.)

thanks
-- PMM
Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago
On Thu, 12 Mar 2026 at 20:37, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 12/3/26 20:30, Peter Maydell wrote:
> > On Thu, 12 Mar 2026 at 19:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >> On 11/3/26 17:27, Martin Kröning wrote:
> >>> These functions are needed to support semihosting on CPUs that support
> >>> runtime-configurable endianness. They should not be used in other contexts.
> >>
> >> Another note is these function are not target-specific, which is great!
> >>
> >> These can also be used outside of semihosting.
> >>
> >> Matter of taste I'd have named them using this shorter pattern:
> >>
> >> cpu_internal_tswap16() -> tswap16_cpu()
> >
> > We specifically didn't want to use a name that might tempt
> > people to use them -- these are only wanted for the very
> > few niche use cases that are outside the CPU but need to
> > know information about the CPU's internals, which is basically
> > legacy-virtio and semihosting.
>
> This was the missing piece to make qtest target-agnostic:
>
> -- >8 --
> diff --git a/system/qtest.c b/system/qtest.c
> index cf90cd53adb..392c1394656 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -523,18 +523,15 @@ static void qtest_process_command(CharFrontend
> *chr, gchar **words)
>               address_space_write(first_cpu->as, addr,
> MEMTXATTRS_UNSPECIFIED,
>                                   &data, 1);
>           } else if (words[0][5] == 'w') {
> -            uint16_t data = value;
> -            tswap16s(&data);
> +            uint16_t data = cpu_internal_tswap16(first_cpu, value);
>               address_space_write(first_cpu->as, addr,
> MEMTXATTRS_UNSPECIFIED,
>                                   &data, 2);
>           } else if (words[0][5] == 'l') {
> -            uint32_t data = value;
> -            tswap32s(&data);
> +            uint32_t data = cpu_internal_tswap32(first_cpu, value);
>               address_space_write(first_cpu->as, addr,
> MEMTXATTRS_UNSPECIFIED,
>                                   &data, 4);
>           } else if (words[0][5] == 'q') {
> -            uint64_t data = value;
> -            tswap64s(&data);
> +            uint64_t data = cpu_internal_tswap64(first_cpu, value);
>               address_space_write(first_cpu->as, addr,
> MEMTXATTRS_UNSPECIFIED,
>                                   &data, 8);
>           }
> ---
>
> QTest is already dubious by always accessing the first CPU AS, so
> using the first CPU endianness is good enough for me IMO. (Ideally
> QTest would support multiple address-spaces).

And these endianness checks in QTest framework are only there to test
legacy virtio, so we are good, this isn't even another niche use case.
Re: [PATCH v3 2/3] include/exec: Provide the cpu_internal_tswap() functions
Posted by Peter Maydell 3 weeks, 5 days ago
On Wed, 11 Mar 2026 at 16:28, Martin Kröning
<martin.kroening@eonerc.rwth-aachen.de> wrote:
>
> These functions are needed to support semihosting on CPUs that support
> runtime-configurable endianness. They should not be used in other contexts.
>
> Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM