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.
Reviewed-by: Pierrick Bouvier <[email protected]> Signed-off-by: Alex Bennée <[email protected]> --- v2 - use unsigned consistently - fix some rouge whitespace - add typed reg32/64 wrappers - pass void * to underlying helper to avoid casting --- include/gdbstub/registers.h | 55 +++++++++++++++++++++++++++++++++++++ gdbstub/gdbstub.c | 23 ++++++++++++++++ 2 files changed, 78 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..2220f58efe --- /dev/null +++ b/include/gdbstub/registers.h @@ -0,0 +1,55 @@ +/* + * 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. + * + * There are wrapper functions for the common sizes you can use to + * keep type checking. + * + * Returns the number of bytes written to the array. + */ +int gdb_get_register_value(MemOp op, GByteArray *buf, void *val); + +/** + * gdb_get_reg32_value() - type checked wrapper for gdb_get_register_value() + * mo: size and endian MemOp + * buf: GByteArray to store in target order + * val: pointer to uint32_t value in host order + */ +static inline int gdb_get_reg32_value(MemOp op, GByteArray *buf, uint32_t *val) { + g_assert((op & MO_SIZE) == MO_32); + return gdb_get_register_value(op, buf, val); +} + +/** + * gdb_get_reg64_value() - type checked wrapper for gdb_get_register_value() + * mo: size and endian MemOp + * buf: GByteArray to store in target order + * val: pointer to uint32_t value in host order + */ +static inline int gdb_get_reg64_value(MemOp op, GByteArray *buf, uint64_t *val) { + g_assert((op & MO_SIZE) == MO_64); + return gdb_get_register_value(op, buf, val); +} + +#endif /* GDB_REGISTERS_H */ + + diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index b6d5e11e03..e799fdc019 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,27 @@ 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, void *val) +{ + unsigned bytes = memop_size(op); + + if (op & MO_BSWAP) { + uint8_t *ptr = &((uint8_t *) val)[bytes - 1]; + for (unsigned i = bytes; i > 0; i--) { + g_byte_array_append(buf, ptr--, 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) -- 2.39.5
