Hi Phil, Alex,
On 6/27/24 9:26 AM, Philippe Mathieu-Daudé wrote:
On 27/6/24 13:05, Alex Bennée wrote:
Philippe Mathieu-Daudé <phi...@linaro.org> writes:
On 27/6/24 06:13, Gustavo Romero wrote:
Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
are not confined to use only in gdbstub.c.
Signed-off-by: Gustavo Romero <gustavo.rom...@linaro.org>
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
---
gdbstub/internals.h | 2 --
include/exec/gdbstub.h | 5 +++++
include/gdbstub/commands.h | 6 ++++++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 1bd2c4ec2a..77e5ec9a5b 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
extern const GDBFeature gdb_static_features[];
+/**
+ * Return the first attached CPU
+ */
+CPUState *gdb_first_attached_cpu(void);
Alex, it seems dubious to expose the API like that.
IMHO GdbCmdHandler should take a GDBRegisterState argument,
then this would become:
CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
Maybe instead of exposing this we can use user_ctx for something? If we
look at handle_set_reg/handle_get_reg we can see that passes down
gdbserver_state.g_cpu down to the eventual helpers. We could define
something like:
--8<---------------cut here---------------start------------->8---
fixups to avoid get_first_cpu()
5 files changed, 25 insertions(+), 18 deletions(-)
gdbstub/internals.h | 1 +
include/exec/gdbstub.h | 5 -----
include/gdbstub/commands.h | 3 +++
gdbstub/gdbstub.c | 7 ++++++-
target/arm/gdbstub64.c | 27 +++++++++++++++------------
@@ -54,6 +54,8 @@ typedef union GdbCmdVariant {
* "stop reply" packet. The list of commands that accept such response is
* defined at the GDB Remote Serial Protocol documentation. See:
*
https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
+ *
+ * @need_cpu_context: pass current CPU to command via user_ctx.
*/
typedef struct GdbCmdParseEntry {
GdbCmdHandler handler;
@@ -61,6 +63,7 @@ typedef struct GdbCmdParseEntry {
bool cmd_startswith;
const char *schema;
bool allow_stop_reply;
+ bool need_cpu_context;
} GdbCmdParseEntry;
#define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
modified gdbstub/gdbstub.c
@@ -938,6 +938,7 @@ static bool process_string_cmd(const char *data,
for (i = 0; i < num_cmds; i++) {
const GdbCmdParseEntry *cmd = &cmds[i];
+ void *user_ctx = NULL;
g_assert(cmd->handler && cmd->cmd);
if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
@@ -952,8 +953,12 @@ static bool process_string_cmd(const char *data,
}
}
+ if (cmd->need_cpu_context) {
+ user_ctx = (void *) gdbserver_state.g_cpu;
LGTM.
Thanks for the suggestion. I added it to v6.
Cheers,
Gustavo
