[PATCH-for-10.1 RESEND v8 6/8] gdbstub/helpers: Replace TARGET_BIG_ENDIAN -> target_big_endian()

Philippe Mathieu-Daudé posted 8 patches 5 months, 1 week ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Beniamino Galvani <b.galvani@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, Helge Deller <deller@gmx.de>, Gerd Hoffmann <kraxel@redhat.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Francisco Iglesias <francisco.iglesias@amd.com>, Vikram Garhwal <vikram.garhwal@bytedance.com>, Jason Wang <jasowang@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Bernhard Beschow <shentey@gmail.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Alexander Graf <agraf@csgraf.de>, Phil Dennis-Jordan <phil@philjordan.eu>, "Alex Bennée" <alex.bennee@linaro.org>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Peter Xu <peterx@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Song Gao <gaosong@loongson.cn>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>
[PATCH-for-10.1 RESEND v8 6/8] gdbstub/helpers: Replace TARGET_BIG_ENDIAN -> target_big_endian()
Posted by Philippe Mathieu-Daudé 5 months, 1 week ago
Check endianness at runtime to remove the target-specific
TARGET_BIG_ENDIAN definition. Use cpu_to_[be,le]XX() from
"qemu/bswap.h" instead of tswapXX() from "exec/tswap.h".

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/gdbstub/helpers.h | 48 +++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/include/gdbstub/helpers.h b/include/gdbstub/helpers.h
index 6f7cc48adcb..b685afac436 100644
--- a/include/gdbstub/helpers.h
+++ b/include/gdbstub/helpers.h
@@ -16,7 +16,8 @@
 #error "gdbstub helpers should only be included by target specific code"
 #endif
 
-#include "exec/tswap.h"
+#include "qemu/bswap.h"
+#include "qemu/target-info.h"
 #include "cpu-param.h"
 
 /*
@@ -33,40 +34,49 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t val)
 
 static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
 {
-    uint16_t to_word = tswap16(val);
-    g_byte_array_append(buf, (uint8_t *) &to_word, 2);
+    if (target_big_endian()) {
+        cpu_to_be16s(&val);
+    } else {
+        cpu_to_le16s(&val);
+    }
+    g_byte_array_append(buf, (uint8_t *) &val, 2);
     return 2;
 }
 
 static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
 {
-    uint32_t to_long = tswap32(val);
-    g_byte_array_append(buf, (uint8_t *) &to_long, 4);
+    if (target_big_endian()) {
+        cpu_to_be32s(&val);
+    } else {
+        cpu_to_le32s(&val);
+    }
+    g_byte_array_append(buf, (uint8_t *) &val, 4);
     return 4;
 }
 
 static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
 {
-    uint64_t to_quad = tswap64(val);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
+    if (target_big_endian()) {
+        cpu_to_be64s(&val);
+    } else {
+        cpu_to_le64s(&val);
+    }
+    g_byte_array_append(buf, (uint8_t *) &val, 8);
     return 8;
 }
 
 static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
                                  uint64_t val_lo)
 {
-    uint64_t to_quad;
-#if TARGET_BIG_ENDIAN
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#else
-    to_quad = tswap64(val_lo);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-    to_quad = tswap64(val_hi);
-    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#endif
+    uint64_t tmp[2];
+    if (target_big_endian()) {
+        tmp[0] = cpu_to_be64(val_hi);
+        tmp[1] = cpu_to_be64(val_lo);
+    } else {
+        tmp[0] = cpu_to_le64(val_lo);
+        tmp[1] = cpu_to_le64(val_hi);
+    }
+    g_byte_array_append(buf, (uint8_t *)&tmp, 16);
     return 16;
 }
 
-- 
2.49.0


Re: [PATCH-for-10.1 RESEND v8 6/8] gdbstub/helpers: Replace TARGET_BIG_ENDIAN -> target_big_endian()
Posted by Manos Pitsidianakis 5 months, 1 week ago
On Wed, Jul 9, 2025 at 12:53 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Check endianness at runtime to remove the target-specific
> TARGET_BIG_ENDIAN definition. Use cpu_to_[be,le]XX() from
> "qemu/bswap.h" instead of tswapXX() from "exec/tswap.h".
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/gdbstub/helpers.h | 48 +++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/include/gdbstub/helpers.h b/include/gdbstub/helpers.h
> index 6f7cc48adcb..b685afac436 100644
> --- a/include/gdbstub/helpers.h
> +++ b/include/gdbstub/helpers.h
> @@ -16,7 +16,8 @@
>  #error "gdbstub helpers should only be included by target specific code"
>  #endif
>
> -#include "exec/tswap.h"
> +#include "qemu/bswap.h"
> +#include "qemu/target-info.h"
>  #include "cpu-param.h"
>
>  /*
> @@ -33,40 +34,49 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t val)
>
>  static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
>  {
> -    uint16_t to_word = tswap16(val);
> -    g_byte_array_append(buf, (uint8_t *) &to_word, 2);
> +    if (target_big_endian()) {
> +        cpu_to_be16s(&val);

Unrelated, but that 's' in 'cpu_to_be16s' is non-obvious, I always
think it's about signedness which is unrelated to endianness. I guess
it stands for "specified". It's more intuitive to call by-value
functions 'into' and by-ref 'to' maybe? e.g. `uint16_t
cpu_into_le16(uint16_t v);` and `void cpu_to_le16(uint16_t *v);`.

> +    } else {
> +        cpu_to_le16s(&val);
> +    }
> +    g_byte_array_append(buf, (uint8_t *) &val, 2);
>      return 2;
>  }
>
>  static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
>  {
> -    uint32_t to_long = tswap32(val);
> -    g_byte_array_append(buf, (uint8_t *) &to_long, 4);
> +    if (target_big_endian()) {
> +        cpu_to_be32s(&val);
> +    } else {
> +        cpu_to_le32s(&val);
> +    }
> +    g_byte_array_append(buf, (uint8_t *) &val, 4);
>      return 4;
>  }
>
>  static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
>  {
> -    uint64_t to_quad = tswap64(val);
> -    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> +    if (target_big_endian()) {
> +        cpu_to_be64s(&val);
> +    } else {
> +        cpu_to_le64s(&val);
> +    }
> +    g_byte_array_append(buf, (uint8_t *) &val, 8);
>      return 8;
>  }
>
>  static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
>                                   uint64_t val_lo)
>  {
> -    uint64_t to_quad;
> -#if TARGET_BIG_ENDIAN
> -    to_quad = tswap64(val_hi);
> -    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -    to_quad = tswap64(val_lo);
> -    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -#else
> -    to_quad = tswap64(val_lo);
> -    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -    to_quad = tswap64(val_hi);
> -    g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -#endif
> +    uint64_t tmp[2];
> +    if (target_big_endian()) {
> +        tmp[0] = cpu_to_be64(val_hi);
> +        tmp[1] = cpu_to_be64(val_lo);
> +    } else {
> +        tmp[0] = cpu_to_le64(val_lo);
> +        tmp[1] = cpu_to_le64(val_hi);
> +    }
> +    g_byte_array_append(buf, (uint8_t *)&tmp, 16);
>      return 16;
>  }
>
> --
> 2.49.0
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>