On 3/19/25 11:22, Alex Bennée wrote:
The current helper.h functions rely on hard coded assumptions about
target endianess to use the tswap macros. We also end up double
swapping a bunch of values if the target can run in multiple endianess
modes. Avoid this by getting the target to pass the endianess and size
via a MemOp and fixing up appropriately.

Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
---
  include/gdbstub/registers.h | 30 ++++++++++++++++++++++++++++++
  gdbstub/gdbstub.c           | 22 ++++++++++++++++++++++
  2 files changed, 52 insertions(+)
  create mode 100644 include/gdbstub/registers.h

diff --git a/include/gdbstub/registers.h b/include/gdbstub/registers.h
new file mode 100644
index 0000000000..4abc7a6ae7
--- /dev/null
+++ b/include/gdbstub/registers.h
@@ -0,0 +1,30 @@
+/*
+ * GDB Common Register Helpers
+ *
+ * Copyright (c) 2025 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef GDB_REGISTERS_H
+#define GDB_REGISTERS_H
+
+#include "exec/memop.h"
+
+/**
+ * gdb_get_register_value() - get register value for gdb
+ * mo: size and endian MemOp
+ * buf: GByteArray to store in target order
+ * val: pointer to value in host order
+ *
+ * This replaces the previous legacy read functions with a single
+ * function to handle all sizes. Passing @mo allows the target mode to
+ * be taken into account and avoids using hard coded tswap() macros.
+ *
+ * Returns the number of bytes written to the array.
+ */
+int gdb_get_register_value(MemOp op, GByteArray *buf, uint8_t *val);
+
+#endif /* GDB_REGISTERS_H */
+
+
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 282e13e163..3d7b1028e4 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -32,6 +32,7 @@
  #include "exec/gdbstub.h"
  #include "gdbstub/commands.h"
  #include "gdbstub/syscalls.h"
+#include "gdbstub/registers.h"
  #ifdef CONFIG_USER_ONLY
  #include "accel/tcg/vcpu-state.h"
  #include "gdbstub/user.h"
@@ -45,6 +46,7 @@
  #include "system/runstate.h"
  #include "exec/replay-core.h"
  #include "exec/hwaddr.h"
+#include "exec/memop.h"
#include "internals.h" @@ -551,6 +553,26 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
      return 0;
  }
+/*
+ * Target helper function to read value into GByteArray, target
+ * supplies the size and target endianess via the MemOp.
+ */
+int gdb_get_register_value(MemOp op, GByteArray *buf, uint8_t *val)
+{
+    size_t bytes = memop_size(op);
+
+    if (op & MO_BSWAP) {
+        for ( int i = bytes ; i > 0; i--) {
+            g_byte_array_append(buf, &val[i - 1], 1);
+        };
+    } else {
+        g_byte_array_append(buf, val, bytes);
+    }
+
+    return bytes;
+}
+
+
  static void gdb_register_feature(CPUState *cpu, int base_reg,
                                   gdb_get_reg_cb get_reg, gdb_set_reg_cb 
set_reg,
                                   const GDBFeature *feature)

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

Reply via email to