All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler
@ 2019-05-29  6:41 Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
                   ` (20 more replies)
  0 siblings, 21 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

This patch series refactors the old gdbstub command packets handler
with a new infrastructure which should ease extending and adding new
and missing gdb command packets.

version 12 changes:
- Fixed a bug during rebase of v10 which broke:
  "Implement breakpoint commands (Z/z pkt) with new infra"
  which basically broke the remove breakpoint command
- Changed gdb_handle_packet call to process_string_cmds with a wrapper
  which handles errors appropriately.
- Patches which require review:
  gdbstub: Implement deatch (D pkt) with new infra
  gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  gdbstub: Implement set register (P pkt) with new infra
  gdbstub: Implement get register (p pkt) with new infra
  gdbstub: Implement file io (F pkt) with new infra
  gdbstub: Implement v commands with new infra
  gdbstub: Implement generic set/query (Q/q pkt) with new infra
  gdbstub: Implement target halted (? pkt) with new infra
  gdbstub: Implement qemu physical memory mode

version 11 changes:
- Add reviewed by tag
- Requires review:
  gdbstub: Implement deatch (D pkt) with new infra
  gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  gdbstub: Implement set register (P pkt) with new infra
  gdbstub: Implement get register (p pkt) with new infra
  gdbstub: Implement file io (F pkt) with new infra
  gdbstub: Implement v commands with new infra
  gdbstub: Implement generic set/query (Q/q pkt) with new infra
  gdbstub: Implement target halted (? pkt) with new infra
  gdbstub: Implement qemu physical memory mode
- Already reviewed:
  gdbstub: Add infrastructure to parse cmd packets
  gdbstub: Implement thread_alive (T pkt) with new infra
  gdbstub: Implement continue (c pkt) with new infra
  gdbstub: Implement continue with signal (C pkt) with new infra
  gdbstub: Implement set_thread (H pkt) with new infra
  gdbstub: Implement write memory (M pkt) with new infra
  gdbstub: Implement read memory (m pkt) with new infra
  gdbstub: Implement write all registers (G pkt) with new infra
  gdbstub: Implement read all registers (g pkt) with new infra
  gdbstub: Implement step (s pkt) with new infra
  gdbstub: Clear unused variables in gdb_handle_packet

version 10 changes:
- Remove kvm added API as this is not really required and can be
  accomplished by defining a coprocessor callback with a system
  specific xml (see: 200bf5b7ffea635079cc05fdfb363372b9544ce7)
- Remove the new QEMU extended commands to read KVM MSRs
- Various fixes from Code Review by Alex Bennee
- Change the QEMU specific command to read physical memory to non-User QEMU 
- Per patch changes:
  gdbstub: Add infrastructure to parse cmd packets
    * remove the union for the flags in GdbCmdParseEntry
  gdbstub: Implement deatch (D pkt) with new infra
    * Changed default handling for error flow / command not found
  gdbstub: Implement continue with signal (C pkt) with new infra
    * Added comment we dont support C sig;[addr] commands
  gdbstub: Implement set_thread (H pkt) with new infra
    * Change num_params check to be equal and not less than
  gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
    * Merged z/Z commands into a single patch
  gdbstub: Implement read memory (m pkt) with new infra
    * Change num_params check to be equal and not less than
  gdbstub: Implement file io (F pkt) with new infra
    * Changed to have a single command parser
  gdbstub: Implement generic set/query (Q/q pkt) with new infra
    * Merged q/Q and qemu.Supported patches into a single patch
  gdbstub: Implement target halted (? pkt) with new infra
    * Removed TODO comment and added a note about it in the commit msg
  gdbstub: Implement qemu physical memory mode
    * Added CONFIG_USER_ONLY where required

version 9 changes:
- checkpatch fixes

version 8 changes:
- Add new command to display the Supported qemu generic query/sets
- kvm: Add API to read/write a MSR
- Add new commands specific for qemu:
  * Command to swap the memory GDB sees to be the physical memory
  * Commands to read and write a MSR

version 7 changes:
- Fixed few checkpatch complaints
- Feedback from Alex Bennee

version 4-6 changes:
- mostly feedback from Richard Henderson

version 3 changes
- Split the single patch to many individual patches for easier reviewing

version 2 changes
- Code convention fixes

Jon Doron (20):
  gdbstub: Add infrastructure to parse cmd packets
  gdbstub: Implement deatch (D pkt) with new infra
  gdbstub: Implement thread_alive (T pkt) with new infra
  gdbstub: Implement continue (c pkt) with new infra
  gdbstub: Implement continue with signal (C pkt) with new infra
  gdbstub: Implement set_thread (H pkt) with new infra
  gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  gdbstub: Implement set register (P pkt) with new infra
  gdbstub: Implement get register (p pkt) with new infra
  gdbstub: Implement write memory (M pkt) with new infra
  gdbstub: Implement read memory (m pkt) with new infra
  gdbstub: Implement write all registers (G pkt) with new infra
  gdbstub: Implement read all registers (g pkt) with new infra
  gdbstub: Implement file io (F pkt) with new infra
  gdbstub: Implement step (s pkt) with new infra
  gdbstub: Implement v commands with new infra
  gdbstub: Implement generic set/query (Q/q pkt) with new infra
  gdbstub: Implement target halted (? pkt) with new infra
  gdbstub: Clear unused variables in gdb_handle_packet
  gdbstub: Implement qemu physical memory mode

 gdbstub.c | 1761 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 1270 insertions(+), 491 deletions(-)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 01/20] gdbstub: Add infrastructure to parse cmd packets
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 195 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index d54abd17cc..e6d895177b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1268,6 +1268,201 @@ out:
     return res;
 }
 
+typedef union GdbCmdVariant {
+    const char *data;
+    uint8_t opcode;
+    unsigned long val_ul;
+    unsigned long long val_ull;
+    struct {
+        GDBThreadIdKind kind;
+        uint32_t pid;
+        uint32_t tid;
+    } thread_id;
+} GdbCmdVariant;
+
+static const char *cmd_next_param(const char *param, const char delimiter)
+{
+    static const char all_delimiters[] = ",;:=";
+    char curr_delimiters[2] = {0};
+    const char *delimiters;
+
+    if (delimiter == '?') {
+        delimiters = all_delimiters;
+    } else if (delimiter == '0') {
+        return strchr(param, '\0');
+    } else if (delimiter == '.' && *param) {
+        return param + 1;
+    } else {
+        curr_delimiters[0] = delimiter;
+        delimiters = curr_delimiters;
+    }
+
+    param += strcspn(param, delimiters);
+    if (*param) {
+        param++;
+    }
+    return param;
+}
+
+static int cmd_parse_params(const char *data, const char *schema,
+                            GdbCmdVariant *params, int *num_params)
+{
+    int curr_param;
+    const char *curr_schema, *curr_data;
+
+    *num_params = 0;
+
+    if (!schema) {
+        return 0;
+    }
+
+    curr_schema = schema;
+    curr_param = 0;
+    curr_data = data;
+    while (curr_schema[0] && curr_schema[1] && *curr_data) {
+        switch (curr_schema[0]) {
+        case 'l':
+            if (qemu_strtoul(curr_data, &curr_data, 16,
+                             &params[curr_param].val_ul)) {
+                return -EINVAL;
+            }
+            curr_param++;
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        case 'L':
+            if (qemu_strtou64(curr_data, &curr_data, 16,
+                              (uint64_t *)&params[curr_param].val_ull)) {
+                return -EINVAL;
+            }
+            curr_param++;
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        case 's':
+            params[curr_param].data = curr_data;
+            curr_param++;
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        case 'o':
+            params[curr_param].opcode = *(uint8_t *)curr_data;
+            curr_param++;
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        case 't':
+            params[curr_param].thread_id.kind =
+                read_thread_id(curr_data, &curr_data,
+                               &params[curr_param].thread_id.pid,
+                               &params[curr_param].thread_id.tid);
+            curr_param++;
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        case '?':
+            curr_data = cmd_next_param(curr_data, curr_schema[1]);
+            break;
+        default:
+            return -EINVAL;
+        }
+        curr_schema += 2;
+    }
+
+    *num_params = curr_param;
+    return 0;
+}
+
+typedef struct GdbCmdContext {
+    GDBState *s;
+    GdbCmdVariant *params;
+    int num_params;
+    uint8_t mem_buf[MAX_PACKET_LENGTH];
+    char str_buf[MAX_PACKET_LENGTH + 1];
+} GdbCmdContext;
+
+typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
+
+/*
+ * cmd_startswith -> cmd is compared using startswith
+ *
+ *
+ * schema definitions:
+ * Each schema parameter entry consists of 2 chars,
+ * the first char represents the parameter type handling
+ * the second char represents the delimiter for the next parameter
+ *
+ * Currently supported schema types:
+ * 'l' -> unsigned long (stored in .val_ul)
+ * 'L' -> unsigned long long (stored in .val_ull)
+ * 's' -> string (stored in .data)
+ * 'o' -> single char (stored in .opcode)
+ * 't' -> thread id (stored in .thread_id)
+ * '?' -> skip according to delimiter
+ *
+ * Currently supported delimiters:
+ * '?' -> Stop at any delimiter (",;:=\0")
+ * '0' -> Stop at "\0"
+ * '.' -> Skip 1 char unless reached "\0"
+ * Any other value is treated as the delimiter value itself
+ */
+typedef struct GdbCmdParseEntry {
+    GdbCmdHandler handler;
+    const char *cmd;
+    bool cmd_startswith;
+    const char *schema;
+} GdbCmdParseEntry;
+
+static inline int startswith(const char *string, const char *pattern)
+{
+  return !strncmp(string, pattern, strlen(pattern));
+}
+
+static int process_string_cmd(
+        GDBState *s, void *user_ctx, const char *data,
+        const GdbCmdParseEntry *cmds, int num_cmds)
+        __attribute__((unused));
+
+static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
+                              const GdbCmdParseEntry *cmds, int num_cmds)
+{
+    int i, schema_len, max_num_params = 0;
+    GdbCmdContext gdb_ctx;
+
+    if (!cmds) {
+        return -1;
+    }
+
+    for (i = 0; i < num_cmds; i++) {
+        const GdbCmdParseEntry *cmd = &cmds[i];
+        g_assert(cmd->handler && cmd->cmd);
+
+        if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
+            (!cmd->cmd_startswith && strcmp(cmd->cmd, data))) {
+            continue;
+        }
+
+        if (cmd->schema) {
+            schema_len = strlen(cmd->schema);
+            if (schema_len % 2) {
+                return -2;
+            }
+
+            max_num_params = schema_len / 2;
+        }
+
+        gdb_ctx.params =
+            (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params);
+        memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
+
+        if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
+                             gdb_ctx.params, &gdb_ctx.num_params)) {
+            return -1;
+        }
+
+        gdb_ctx.s = s;
+        cmd->handler(&gdb_ctx, user_ctx);
+        return 0;
+    }
+
+    return -1;
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:33   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 03/20] gdbstub: Implement thread_alive (T " Jon Doron
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 101 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 40 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e6d895177b..1db322c15a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1413,11 +1413,6 @@ static inline int startswith(const char *string, const char *pattern)
   return !strncmp(string, pattern, strlen(pattern));
 }
 
-static int process_string_cmd(
-        GDBState *s, void *user_ctx, const char *data,
-        const GdbCmdParseEntry *cmds, int num_cmds)
-        __attribute__((unused));
-
 static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
                               const GdbCmdParseEntry *cmds, int num_cmds)
 {
@@ -1463,6 +1458,55 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
     return -1;
 }
 
+static void run_cmd_parser(GDBState *s, const char *data,
+                           const GdbCmdParseEntry *cmd)
+{
+    if (!data) {
+        return;
+    }
+
+    /* In case there was an error during the command parsing we must
+    * send a NULL packet to indicate the command is not supported */
+    if (process_string_cmd(s, NULL, data, cmd, 1)) {
+        put_packet(s, "");
+    }
+}
+
+static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    GDBProcess *process;
+    GDBState *s = gdb_ctx->s;
+    uint32_t pid = 1;
+
+    if (s->multiprocess) {
+        if (!gdb_ctx->num_params) {
+            put_packet(s, "E22");
+            return;
+        }
+
+        pid = gdb_ctx->params[0].val_ul;
+    }
+
+    process = gdb_get_process(s, pid);
+    gdb_process_breakpoint_remove_all(s, process);
+    process->attached = false;
+
+    if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
+        s->c_cpu = gdb_first_attached_cpu(s);
+    }
+
+    if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+        s->g_cpu = gdb_first_attached_cpu(s);
+    }
+
+    if (!s->c_cpu) {
+        /* No more process attached */
+        gdb_syscall_mode = GDB_SYS_DISABLED;
+        gdb_continue(s);
+    }
+    put_packet(s, "OK");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1477,6 +1521,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     uint8_t *registers;
     target_ulong addr, len;
     GDBThreadIdKind thread_kind;
+    const GdbCmdParseEntry *cmd_parser = NULL;
 
     trace_gdbstub_io_command(line_buf);
 
@@ -1577,42 +1622,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         error_report("QEMU: Terminated via GDBstub");
         exit(0);
     case 'D':
-        /* Detach packet */
-        pid = 1;
-
-        if (s->multiprocess) {
-            unsigned long lpid;
-            if (*p != ';') {
-                put_packet(s, "E22");
-                break;
-            }
-
-            if (qemu_strtoul(p + 1, &p, 16, &lpid)) {
-                put_packet(s, "E22");
-                break;
-            }
-
-            pid = lpid;
-        }
-
-        process = gdb_get_process(s, pid);
-        gdb_process_breakpoint_remove_all(s, process);
-        process->attached = false;
-
-        if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
-            s->c_cpu = gdb_first_attached_cpu(s);
-        }
-
-        if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
-            s->g_cpu = gdb_first_attached_cpu(s);
-        }
-
-        if (s->c_cpu == NULL) {
-            /* No more process attached */
-            gdb_syscall_mode = GDB_SYS_DISABLED;
-            gdb_continue(s);
+        {
+            static const GdbCmdParseEntry detach_cmd_desc = {
+                .handler = handle_detach,
+                .cmd = "D",
+                .cmd_startswith = 1,
+                .schema = "?.l0"
+            };
+            cmd_parser = &detach_cmd_desc;
         }
-        put_packet(s, "OK");
         break;
     case 's':
         if (*p != '\0') {
@@ -1985,6 +2003,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, buf);
         break;
     }
+
+    run_cmd_parser(s, line_buf, cmd_parser);
+
     return RS_IDLE;
 }
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 03/20] gdbstub: Implement thread_alive (T pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 04/20] gdbstub: Implement continue (c " Jon Doron
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 1db322c15a..7801f2f260 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1507,6 +1507,30 @@ static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(s, "OK");
 }
 
+static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    CPUState *cpu;
+
+    if (!gdb_ctx->num_params) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    if (gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid,
+                      gdb_ctx->params[0].thread_id.tid);
+    if (!cpu) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    put_packet(gdb_ctx->s, "OK");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1807,17 +1831,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'T':
-        thread_kind = read_thread_id(p, &p, &pid, &tid);
-        if (thread_kind == GDB_READ_THREAD_ERR) {
-            put_packet(s, "E22");
-            break;
-        }
-        cpu = gdb_get_cpu(s, pid, tid);
-
-        if (cpu != NULL) {
-            put_packet(s, "OK");
-        } else {
-            put_packet(s, "E22");
+        {
+            static const GdbCmdParseEntry thread_alive_cmd_desc = {
+                .handler = handle_thread_alive,
+                .cmd = "T",
+                .cmd_startswith = 1,
+                .schema = "t0"
+            };
+            cmd_parser = &thread_alive_cmd_desc;
         }
         break;
     case 'q':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 04/20] gdbstub: Implement continue (c pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (2 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 03/20] gdbstub: Implement thread_alive (T " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 05/20] gdbstub: Implement continue with signal (C " Jon Doron
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 7801f2f260..99b78aa426 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1531,6 +1531,16 @@ static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "OK");
 }
 
+static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (gdb_ctx->num_params) {
+        gdb_set_cpu_pc(gdb_ctx->s, gdb_ctx->params[0].val_ull);
+    }
+
+    gdb_ctx->s->signal = 0;
+    gdb_continue(gdb_ctx->s);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1567,13 +1577,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         gdb_breakpoint_remove_all();
         break;
     case 'c':
-        if (*p != '\0') {
-            addr = strtoull(p, (char **)&p, 16);
-            gdb_set_cpu_pc(s, addr);
+        {
+            static const GdbCmdParseEntry continue_cmd_desc = {
+                .handler = handle_continue,
+                .cmd = "c",
+                .cmd_startswith = 1,
+                .schema = "L0"
+            };
+            cmd_parser = &continue_cmd_desc;
         }
-        s->signal = 0;
-        gdb_continue(s);
-        return RS_IDLE;
+        break;
     case 'C':
         s->signal = gdb_signal_to_target (strtoul(p, (char **)&p, 16));
         if (s->signal == -1)
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 05/20] gdbstub: Implement continue with signal (C pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (3 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 04/20] gdbstub: Implement continue (c " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 06/20] gdbstub: Implement set_thread (H " Jon Doron
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 99b78aa426..5df4d58427 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1541,6 +1541,25 @@ static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue(gdb_ctx->s);
 }
 
+static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    unsigned long signal = 0;
+
+    /*
+     * Note: C sig;[addr] is currently unsupported and we simply
+     *       omit the addr parameter
+     */
+    if (gdb_ctx->num_params) {
+        signal = gdb_ctx->params[0].val_ul;
+    }
+
+    gdb_ctx->s->signal = gdb_signal_to_target(signal);
+    if (gdb_ctx->s->signal == -1) {
+        gdb_ctx->s->signal = 0;
+    }
+    gdb_continue(gdb_ctx->s);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1588,11 +1607,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'C':
-        s->signal = gdb_signal_to_target (strtoul(p, (char **)&p, 16));
-        if (s->signal == -1)
-            s->signal = 0;
-        gdb_continue(s);
-        return RS_IDLE;
+        {
+            static const GdbCmdParseEntry cont_with_sig_cmd_desc = {
+                .handler = handle_cont_with_sig,
+                .cmd = "C",
+                .cmd_startswith = 1,
+                .schema = "l0"
+            };
+            cmd_parser = &cont_with_sig_cmd_desc;
+        }
+        break;
     case 'v':
         if (strncmp(p, "Cont", 4) == 0) {
             p += 4;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 06/20] gdbstub: Implement set_thread (H pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (4 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 05/20] gdbstub: Implement continue with signal (C " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 83 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 5df4d58427..db213cf173 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1560,6 +1560,51 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue(gdb_ctx->s);
 }
 
+static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    CPUState *cpu;
+
+    if (gdb_ctx->num_params != 2) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    if (gdb_ctx->params[1].thread_id.kind == GDB_READ_THREAD_ERR) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    if (gdb_ctx->params[1].thread_id.kind != GDB_ONE_THREAD) {
+        put_packet(gdb_ctx->s, "OK");
+        return;
+    }
+
+    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[1].thread_id.pid,
+                      gdb_ctx->params[1].thread_id.tid);
+    if (!cpu) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    /*
+     * Note: This command is deprecated and modern gdb's will be using the
+     *       vCont command instead.
+     */
+    switch (gdb_ctx->params[0].opcode) {
+    case 'c':
+        gdb_ctx->s->c_cpu = cpu;
+        put_packet(gdb_ctx->s, "OK");
+        break;
+    case 'g':
+        gdb_ctx->s->g_cpu = cpu;
+        put_packet(gdb_ctx->s, "OK");
+        break;
+    default:
+        put_packet(gdb_ctx->s, "E22");
+        break;
+    }
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1573,7 +1618,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     char thread_id[16];
     uint8_t *registers;
     target_ulong addr, len;
-    GDBThreadIdKind thread_kind;
     const GdbCmdParseEntry *cmd_parser = NULL;
 
     trace_gdbstub_io_command(line_buf);
@@ -1836,35 +1880,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet(s, "E22");
         break;
     case 'H':
-        type = *p++;
-
-        thread_kind = read_thread_id(p, &p, &pid, &tid);
-        if (thread_kind == GDB_READ_THREAD_ERR) {
-            put_packet(s, "E22");
-            break;
-        }
-
-        if (thread_kind != GDB_ONE_THREAD) {
-            put_packet(s, "OK");
-            break;
-        }
-        cpu = gdb_get_cpu(s, pid, tid);
-        if (cpu == NULL) {
-            put_packet(s, "E22");
-            break;
-        }
-        switch (type) {
-        case 'c':
-            s->c_cpu = cpu;
-            put_packet(s, "OK");
-            break;
-        case 'g':
-            s->g_cpu = cpu;
-            put_packet(s, "OK");
-            break;
-        default:
-             put_packet(s, "E22");
-             break;
+        {
+            static const GdbCmdParseEntry set_thread_cmd_desc = {
+                .handler = handle_set_thread,
+                .cmd = "H",
+                .cmd_startswith = 1,
+                .schema = "o.t0"
+            };
+            cmd_parser = &set_thread_cmd_desc;
         }
         break;
     case 'T':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (5 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 06/20] gdbstub: Implement set_thread (H " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:34   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P " Jon Doron
                   ` (13 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 86 +++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 19 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index db213cf173..572222bfa4 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -950,7 +950,7 @@ static inline int xlat_gdb_type(CPUState *cpu, int gdbtype)
 }
 #endif
 
-static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
+static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong len)
 {
     CPUState *cpu;
     int err = 0;
@@ -987,7 +987,7 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
     }
 }
 
-static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
+static int gdb_breakpoint_remove(int type, target_ulong addr, target_ulong len)
 {
     CPUState *cpu;
     int err = 0;
@@ -1605,6 +1605,52 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 }
 
+static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int res;
+
+    if (gdb_ctx->num_params != 3) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    res = gdb_breakpoint_insert(gdb_ctx->params[0].val_ul,
+                                gdb_ctx->params[1].val_ull,
+                                gdb_ctx->params[2].val_ull);
+    if (res >= 0) {
+        put_packet(gdb_ctx->s, "OK");
+        return;
+    } else if (res == -ENOSYS) {
+        put_packet(gdb_ctx->s, "");
+        return;
+    }
+
+    put_packet(gdb_ctx->s, "E22");
+}
+
+static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int res;
+
+    if (gdb_ctx->num_params != 3) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    res = gdb_breakpoint_remove(gdb_ctx->params[0].val_ul,
+                                gdb_ctx->params[1].val_ull,
+                                gdb_ctx->params[2].val_ull);
+    if (res >= 0) {
+        put_packet(gdb_ctx->s, "OK");
+        return;
+    } else if (res == -ENOSYS) {
+        put_packet(gdb_ctx->s, "");
+        return;
+    }
+
+    put_packet(gdb_ctx->s, "E22");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1860,24 +1906,26 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, "OK");
         break;
     case 'Z':
+        {
+            static const GdbCmdParseEntry insert_bp_cmd_desc = {
+                .handler = handle_insert_bp,
+                .cmd = "Z",
+                .cmd_startswith = 1,
+                .schema = "l?L?L0"
+            };
+            cmd_parser = &insert_bp_cmd_desc;
+        }
+        break;
     case 'z':
-        type = strtoul(p, (char **)&p, 16);
-        if (*p == ',')
-            p++;
-        addr = strtoull(p, (char **)&p, 16);
-        if (*p == ',')
-            p++;
-        len = strtoull(p, (char **)&p, 16);
-        if (ch == 'Z')
-            res = gdb_breakpoint_insert(addr, len, type);
-        else
-            res = gdb_breakpoint_remove(addr, len, type);
-        if (res >= 0)
-             put_packet(s, "OK");
-        else if (res == -ENOSYS)
-            put_packet(s, "");
-        else
-            put_packet(s, "E22");
+        {
+            static const GdbCmdParseEntry remove_bp_cmd_desc = {
+                .handler = handle_remove_bp,
+                .cmd = "z",
+                .cmd_startswith = 1,
+                .schema = "l?L?L0"
+            };
+            cmd_parser = &remove_bp_cmd_desc;
+        }
         break;
     case 'H':
         {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (6 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:37   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p " Jon Doron
                   ` (12 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 572222bfa4..fc9526b3f5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1651,6 +1651,27 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "E22");
 }
 
+static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int reg_size;
+
+    if (!gdb_has_xml) {
+        put_packet(gdb_ctx->s, "E00");
+        return;
+    }
+
+    if (gdb_ctx->num_params != 2) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    reg_size = strlen(gdb_ctx->params[1].data) / 2;
+    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[1].data, reg_size);
+    gdb_write_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
+                       gdb_ctx->params[0].val_ull);
+    put_packet(gdb_ctx->s, "OK");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1895,15 +1916,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'P':
-        if (!gdb_has_xml)
-            goto unknown_command;
-        addr = strtoull(p, (char **)&p, 16);
-        if (*p == '=')
-            p++;
-        reg_size = strlen(p) / 2;
-        hextomem(mem_buf, p, reg_size);
-        gdb_write_register(s->g_cpu, mem_buf, addr);
-        put_packet(s, "OK");
+        {
+            static const GdbCmdParseEntry set_reg_cmd_desc = {
+                .handler = handle_set_reg,
+                .cmd = "P",
+                .cmd_startswith = 1,
+                .schema = "L?s0"
+            };
+            cmd_parser = &set_reg_cmd_desc;
+        }
         break;
     case 'Z':
         {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (7 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:38   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 10/20] gdbstub: Implement write memory (M " Jon Doron
                   ` (11 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 50 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index fc9526b3f5..07740ec0af 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1672,6 +1672,36 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "OK");
 }
 
+static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int reg_size;
+
+    /*
+     * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
+     * This works, but can be very slow.  Anything new enough to
+     * understand XML also knows how to use this properly.
+     */
+    if (!gdb_has_xml) {
+        put_packet(gdb_ctx->s, "");
+        return;
+    }
+
+    if (!gdb_ctx->num_params) {
+        put_packet(gdb_ctx->s, "E14");
+        return;
+    }
+
+    reg_size = gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
+                                 gdb_ctx->params[0].val_ull);
+    if (!reg_size) {
+        put_packet(gdb_ctx->s, "E14");
+        return;
+    }
+
+    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1901,18 +1931,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'p':
-        /* Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
-           This works, but can be very slow.  Anything new enough to
-           understand XML also knows how to use this properly.  */
-        if (!gdb_has_xml)
-            goto unknown_command;
-        addr = strtoull(p, (char **)&p, 16);
-        reg_size = gdb_read_register(s->g_cpu, mem_buf, addr);
-        if (reg_size) {
-            memtohex(buf, mem_buf, reg_size);
-            put_packet(s, buf);
-        } else {
-            put_packet(s, "E14");
+        {
+            static const GdbCmdParseEntry get_reg_cmd_desc = {
+                .handler = handle_get_reg,
+                .cmd = "p",
+                .cmd_startswith = 1,
+                .schema = "L0"
+            };
+            cmd_parser = &get_reg_cmd_desc;
         }
         break;
     case 'P':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 10/20] gdbstub: Implement write memory (M pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (8 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 11/20] gdbstub: Implement read memory (m " Jon Doron
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 07740ec0af..f2ea5bdd5c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1702,6 +1702,31 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, gdb_ctx->str_buf);
 }
 
+static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (gdb_ctx->num_params != 3) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    /* hextomem() reads 2*len bytes */
+    if (gdb_ctx->params[1].val_ull > strlen(gdb_ctx->params[2].data) / 2) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[2].data,
+             gdb_ctx->params[1].val_ull);
+    if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull,
+                               gdb_ctx->mem_buf,
+                               gdb_ctx->params[1].val_ull, true)) {
+        put_packet(gdb_ctx->s, "E14");
+        return;
+    }
+
+    put_packet(gdb_ctx->s, "OK");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1910,24 +1935,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'M':
-        addr = strtoull(p, (char **)&p, 16);
-        if (*p == ',')
-            p++;
-        len = strtoull(p, (char **)&p, 16);
-        if (*p == ':')
-            p++;
-
-        /* hextomem() reads 2*len bytes */
-        if (len > strlen(p) / 2) {
-            put_packet (s, "E22");
-            break;
-        }
-        hextomem(mem_buf, p, len);
-        if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len,
-                                   true) != 0) {
-            put_packet(s, "E14");
-        } else {
-            put_packet(s, "OK");
+        {
+            static const GdbCmdParseEntry write_mem_cmd_desc = {
+                .handler = handle_write_mem,
+                .cmd = "M",
+                .cmd_startswith = 1,
+                .schema = "L,L:s0"
+            };
+            cmd_parser = &write_mem_cmd_desc;
         }
         break;
     case 'p':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 11/20] gdbstub: Implement read memory (m pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (9 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 10/20] gdbstub: Implement write memory (M " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 12/20] gdbstub: Implement write all registers (G " Jon Doron
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 48 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index f2ea5bdd5c..3d8c2f8f42 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1727,6 +1727,30 @@ static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "OK");
 }
 
+static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (gdb_ctx->num_params != 2) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    /* memtohex() doubles the required space */
+    if (gdb_ctx->params[1].val_ull > MAX_PACKET_LENGTH / 2) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    if (target_memory_rw_debug(gdb_ctx->s->g_cpu, gdb_ctx->params[0].val_ull,
+                               gdb_ctx->mem_buf,
+                               gdb_ctx->params[1].val_ull, false)) {
+        put_packet(gdb_ctx->s, "E14");
+        return;
+    }
+
+    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, gdb_ctx->params[1].val_ull);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1916,22 +1940,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, "OK");
         break;
     case 'm':
-        addr = strtoull(p, (char **)&p, 16);
-        if (*p == ',')
-            p++;
-        len = strtoull(p, NULL, 16);
-
-        /* memtohex() doubles the required space */
-        if (len > MAX_PACKET_LENGTH / 2) {
-            put_packet (s, "E22");
-            break;
-        }
-
-        if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, false) != 0) {
-            put_packet (s, "E14");
-        } else {
-            memtohex(buf, mem_buf, len);
-            put_packet(s, buf);
+        {
+            static const GdbCmdParseEntry read_mem_cmd_desc = {
+                .handler = handle_read_mem,
+                .cmd = "m",
+                .cmd_startswith = 1,
+                .schema = "L,L0"
+            };
+            cmd_parser = &read_mem_cmd_desc;
         }
         break;
     case 'M':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 12/20] gdbstub: Implement write all registers (G pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (10 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 11/20] gdbstub: Implement read memory (m " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 13/20] gdbstub: Implement read all registers (g " Jon Doron
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 3d8c2f8f42..a487e549d1 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1751,6 +1751,29 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, gdb_ctx->str_buf);
 }
 
+static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    target_ulong addr, len;
+    uint8_t *registers;
+    int reg_size;
+
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    cpu_synchronize_state(gdb_ctx->s->g_cpu);
+    registers = gdb_ctx->mem_buf;
+    len = strlen(gdb_ctx->params[0].data) / 2;
+    hextomem(registers, gdb_ctx->params[0].data, len);
+    for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs && len > 0;
+         addr++) {
+        reg_size = gdb_write_register(gdb_ctx->s->g_cpu, registers, addr);
+        len -= reg_size;
+        registers += reg_size;
+    }
+    put_packet(gdb_ctx->s, "OK");
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1762,7 +1785,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     uint8_t mem_buf[MAX_PACKET_LENGTH];
     char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
     char thread_id[16];
-    uint8_t *registers;
     target_ulong addr, len;
     const GdbCmdParseEntry *cmd_parser = NULL;
 
@@ -1928,16 +1950,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, buf);
         break;
     case 'G':
-        cpu_synchronize_state(s->g_cpu);
-        registers = mem_buf;
-        len = strlen(p) / 2;
-        hextomem((uint8_t *)registers, p, len);
-        for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) {
-            reg_size = gdb_write_register(s->g_cpu, registers, addr);
-            len -= reg_size;
-            registers += reg_size;
+        {
+            static const GdbCmdParseEntry write_all_regs_cmd_desc = {
+                .handler = handle_write_all_regs,
+                .cmd = "G",
+                .cmd_startswith = 1,
+                .schema = "s0"
+            };
+            cmd_parser = &write_all_regs_cmd_desc;
         }
-        put_packet(s, "OK");
         break;
     case 'm':
         {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 13/20] gdbstub: Implement read all registers (g pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (11 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 12/20] gdbstub: Implement write all registers (G " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F " Jon Doron
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index a487e549d1..8a401e6527 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1774,6 +1774,21 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "OK");
 }
 
+static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    target_ulong addr, len;
+
+    cpu_synchronize_state(gdb_ctx->s->g_cpu);
+    len = 0;
+    for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs; addr++) {
+        len += gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf + len,
+                                 addr);
+    }
+
+    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1781,7 +1796,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     CPUClass *cc;
     const char *p;
     uint32_t pid, tid;
-    int ch, reg_size, type, res;
+    int ch, type, res;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
     char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
     char thread_id[16];
@@ -1940,14 +1955,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'g':
-        cpu_synchronize_state(s->g_cpu);
-        len = 0;
-        for (addr = 0; addr < s->g_cpu->gdb_num_g_regs; addr++) {
-            reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr);
-            len += reg_size;
+        {
+            static const GdbCmdParseEntry read_all_regs_cmd_desc = {
+                .handler = handle_read_all_regs,
+                .cmd = "g",
+                .cmd_startswith = 1
+            };
+            cmd_parser = &read_all_regs_cmd_desc;
         }
-        memtohex(buf, mem_buf, len);
-        put_packet(s, buf);
         break;
     case 'G':
         {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (12 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 13/20] gdbstub: Implement read all registers (g " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:39   ` Alex Bennée
  2019-05-31 13:41   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 15/20] gdbstub: Implement step (s " Jon Doron
                   ` (6 subsequent siblings)
  20 siblings, 2 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 8a401e6527..ea85966b27 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1789,6 +1789,25 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, gdb_ctx->str_buf);
 }
 
+static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
+        target_ulong ret, err;
+
+        ret = (target_ulong)gdb_ctx->params[0].val_ull;
+        err = (target_ulong)gdb_ctx->params[1].val_ull;
+        gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
+        gdb_ctx->s->current_syscall_cb = NULL;
+    }
+
+    if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') {
+        put_packet(gdb_ctx->s, "T02");
+        return;
+    }
+
+    gdb_continue(gdb_ctx->s);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1930,28 +1949,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         return RS_IDLE;
     case 'F':
         {
-            target_ulong ret;
-            target_ulong err;
-
-            ret = strtoull(p, (char **)&p, 16);
-            if (*p == ',') {
-                p++;
-                err = strtoull(p, (char **)&p, 16);
-            } else {
-                err = 0;
-            }
-            if (*p == ',')
-                p++;
-            type = *p;
-            if (s->current_syscall_cb) {
-                s->current_syscall_cb(s->c_cpu, ret, err);
-                s->current_syscall_cb = NULL;
-            }
-            if (type == 'C') {
-                put_packet(s, "T02");
-            } else {
-                gdb_continue(s);
-            }
+            static const GdbCmdParseEntry file_io_cmd_desc = {
+                .handler = handle_file_io,
+                .cmd = "F",
+                .cmd_startswith = 1,
+                .schema = "L,L,o0"
+            };
+            cmd_parser = &file_io_cmd_desc;
         }
         break;
     case 'g':
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 15/20] gdbstub: Implement step (s pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (13 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands " Jon Doron
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ea85966b27..3fd1a1cddb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1808,6 +1808,16 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue(gdb_ctx->s);
 }
 
+static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (gdb_ctx->num_params) {
+        gdb_set_cpu_pc(gdb_ctx->s, (target_ulong)gdb_ctx->params[0].val_ull);
+    }
+
+    cpu_single_step(gdb_ctx->s->c_cpu, sstep_flags);
+    gdb_continue(gdb_ctx->s);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1940,13 +1950,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 's':
-        if (*p != '\0') {
-            addr = strtoull(p, (char **)&p, 16);
-            gdb_set_cpu_pc(s, addr);
+        {
+            static const GdbCmdParseEntry step_cmd_desc = {
+                .handler = handle_step,
+                .cmd = "s",
+                .cmd_startswith = 1,
+                .schema = "L0"
+            };
+            cmd_parser = &step_cmd_desc;
         }
-        cpu_single_step(s->c_cpu, sstep_flags);
-        gdb_continue(s);
-        return RS_IDLE;
+        break;
     case 'F':
         {
             static const GdbCmdParseEntry file_io_cmd_desc = {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (14 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 15/20] gdbstub: Implement step (s " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 13:42   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
                   ` (4 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 170 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 110 insertions(+), 60 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 3fd1a1cddb..648191a3b0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1818,6 +1818,106 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue(gdb_ctx->s);
 }
 
+static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    put_packet(gdb_ctx->s, "vCont;c;C;s;S");
+}
+
+static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int res;
+
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    res = gdb_handle_vcont(gdb_ctx->s, gdb_ctx->params[0].data);
+    if ((res == -EINVAL) || (res == -ERANGE)) {
+        put_packet(gdb_ctx->s, "E22");
+    } else if (res) {
+        put_packet(gdb_ctx->s, "");
+    }
+}
+
+static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    GDBProcess *process;
+    CPUState *cpu;
+    char thread_id[16];
+
+    pstrcpy(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "E22");
+    if (!gdb_ctx->num_params) {
+        goto cleanup;
+    }
+
+    process = gdb_get_process(gdb_ctx->s, gdb_ctx->params[0].val_ul);
+    if (!process) {
+        goto cleanup;
+    }
+
+    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
+    if (!cpu) {
+        goto cleanup;
+    }
+
+    process->attached = true;
+    gdb_ctx->s->g_cpu = cpu;
+    gdb_ctx->s->c_cpu = cpu;
+
+    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
+             GDB_SIGNAL_TRAP, thread_id);
+cleanup:
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    /* Kill the target */
+    put_packet(gdb_ctx->s, "OK");
+    error_report("QEMU: Terminated via GDBstub");
+    exit(0);
+}
+
+static GdbCmdParseEntry gdb_v_commands_table[] = {
+    /* Order is important if has same prefix */
+    {
+        .handler = handle_v_cont_query,
+        .cmd = "Cont?",
+        .cmd_startswith = 1
+    },
+    {
+        .handler = handle_v_cont,
+        .cmd = "Cont",
+        .cmd_startswith = 1,
+        .schema = "s0"
+    },
+    {
+        .handler = handle_v_attach,
+        .cmd = "Attach;",
+        .cmd_startswith = 1,
+        .schema = "l0"
+    },
+    {
+        .handler = handle_v_kill,
+        .cmd = "Kill;",
+        .cmd_startswith = 1
+    },
+};
+
+static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+                           gdb_v_commands_table,
+                           ARRAY_SIZE(gdb_v_commands_table))) {
+        put_packet(gdb_ctx->s, "");
+    }
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1825,7 +1925,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     CPUClass *cc;
     const char *p;
     uint32_t pid, tid;
-    int ch, type, res;
+    int ch, type;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
     char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
     char thread_id[16];
@@ -1874,66 +1974,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'v':
-        if (strncmp(p, "Cont", 4) == 0) {
-            p += 4;
-            if (*p == '?') {
-                put_packet(s, "vCont;c;C;s;S");
-                break;
-            }
-
-            res = gdb_handle_vcont(s, p);
-
-            if (res) {
-                if ((res == -EINVAL) || (res == -ERANGE)) {
-                    put_packet(s, "E22");
-                    break;
-                }
-                goto unknown_command;
-            }
-            break;
-        } else if (strncmp(p, "Attach;", 7) == 0) {
-            unsigned long pid;
-
-            p += 7;
-
-            if (qemu_strtoul(p, &p, 16, &pid)) {
-                put_packet(s, "E22");
-                break;
-            }
-
-            process = gdb_get_process(s, pid);
-
-            if (process == NULL) {
-                put_packet(s, "E22");
-                break;
-            }
-
-            cpu = get_first_cpu_in_process(s, process);
-
-            if (cpu == NULL) {
-                /* Refuse to attach an empty process */
-                put_packet(s, "E22");
-                break;
-            }
-
-            process->attached = true;
-
-            s->g_cpu = cpu;
-            s->c_cpu = cpu;
-
-            snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
-                     gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
-
-            put_packet(s, buf);
-            break;
-        } else if (strncmp(p, "Kill;", 5) == 0) {
-            /* Kill the target */
-            put_packet(s, "OK");
-            error_report("QEMU: Terminated via GDBstub");
-            exit(0);
-        } else {
-            goto unknown_command;
+        {
+            static const GdbCmdParseEntry v_cmd_desc = {
+                .handler = handle_v_commands,
+                .cmd = "v",
+                .cmd_startswith = 1,
+                .schema = "s0"
+            };
+            cmd_parser = &v_cmd_desc;
         }
+        break;
     case 'k':
         /* Kill the target */
         error_report("QEMU: Terminated via GDBstub");
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (15 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 14:10   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? " Jon Doron
                   ` (3 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

The generic set/query packets contains implementation for varioius
sub-commands which are required for GDB and also additional commands
which are QEMU specific.

To see which QEMU specific commands are available use the command
gdb> maintenance packet qqemu.Supported

Currently the only implemented QEMU specific command is the command
that sets the single step behavior.

gdb> maintenance packet qqemu.sstepbits
Will display the MASK bits used to control the single stepping.

gdb> maintenance packet qqemu.sstep
Will display the current value of the mask used when single stepping.

gdb> maintenance packet Qqemu.sstep:HEX_VALUE
Will change the single step mask.

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 559 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 373 insertions(+), 186 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 648191a3b0..80fe5b2d0c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1130,14 +1130,6 @@ static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
     return GDB_ONE_THREAD;
 }
 
-static int is_query_packet(const char *p, const char *query, char separator)
-{
-    unsigned int query_len = strlen(query);
-
-    return strncmp(p, query, query_len) == 0 &&
-        (p[query_len] == '\0' || p[query_len] == separator);
-}
-
 /**
  * gdb_handle_vcont - Parses and handles a vCont packet.
  * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
@@ -1918,18 +1910,368 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 }
 
-static int gdb_handle_packet(GDBState *s, const char *line_buf)
+static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
+             "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE,
+             SSTEP_NOIRQ, SSTEP_NOTIMER);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    sstep_flags = gdb_ctx->params[0].val_ul;
+    put_packet(gdb_ctx->s, "OK");
+}
+
+static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     CPUState *cpu;
     GDBProcess *process;
+    char thread_id[16];
+
+    /*
+     * "Current thread" remains vague in the spec, so always return
+     * the first thread of the current process (gdb returns the
+     * first thread).
+     */
+    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
+    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
+    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    char thread_id[16];
+
+    if (!gdb_ctx->s->query_cpu) {
+        put_packet(gdb_ctx->s, "l");
+        return;
+    }
+
+    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->query_cpu, thread_id,
+                      sizeof(thread_id));
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    gdb_ctx->s->query_cpu =
+        gdb_next_attached_cpu(gdb_ctx->s, gdb_ctx->s->query_cpu);
+}
+
+static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    gdb_ctx->s->query_cpu = gdb_first_attached_cpu(gdb_ctx->s);
+    handle_query_threads(gdb_ctx, user_ctx);
+}
+
+static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    CPUState *cpu;
+    int len;
+
+    if (!gdb_ctx->num_params ||
+        gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid,
+                      gdb_ctx->params[0].thread_id.tid);
+    if (!cpu) {
+        return;
+    }
+
+    cpu_synchronize_state(cpu);
+
+    if (gdb_ctx->s->multiprocess && (gdb_ctx->s->process_num > 1)) {
+        /* Print the CPU model and name in multiprocess mode */
+        ObjectClass *oc = object_get_class(OBJECT(cpu));
+        const char *cpu_model = object_class_get_name(oc);
+        char *cpu_name = object_get_canonical_path_component(OBJECT(cpu));
+        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
+                       "%s %s [%s]", cpu_model, cpu_name,
+                       cpu->halted ? "halted " : "running");
+        g_free(cpu_name);
+    } else {
+        /* memtohex() doubles the required space */
+        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
+                        "CPU#%d [%s]", cpu->cpu_index,
+                        cpu->halted ? "halted " : "running");
+    }
+    trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf);
+    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+#ifdef CONFIG_USER_ONLY
+static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    TaskState *ts;
+
+    ts = gdb_ctx->s->c_cpu->opaque;
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
+             "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
+             ";Bss=" TARGET_ABI_FMT_lx,
+             ts->info->code_offset,
+             ts->info->data_offset,
+             ts->info->data_offset);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+#else
+static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    int len;
+
+    if (!gdb_ctx->num_params) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    len = strlen(gdb_ctx->params[0].data);
+    if (len % 2) {
+        put_packet(gdb_ctx->s, "E01");
+        return;
+    }
+
+    len = len / 2;
+    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len);
+    gdb_ctx->mem_buf[len++] = 0;
+    qemu_chr_be_write(gdb_ctx->s->mon_chr, gdb_ctx->mem_buf, len);
+    put_packet(gdb_ctx->s, "OK");
+
+}
+#endif
+
+static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
     CPUClass *cc;
+
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "PacketSize=%x",
+             MAX_PACKET_LENGTH);
+    cc = CPU_GET_CLASS(first_cpu);
+    if (cc->gdb_core_xml_file) {
+        pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
+                ";qXfer:features:read+");
+    }
+
+    if (gdb_ctx->num_params &&
+        strstr(gdb_ctx->params[0].data, "multiprocess+")) {
+        gdb_ctx->s->multiprocess = true;
+    }
+
+    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    GDBProcess *process;
+    CPUClass *cc;
+    unsigned long len, total_len, addr;
+    const char *xml;
     const char *p;
-    uint32_t pid, tid;
-    int ch, type;
+
+    if (gdb_ctx->num_params < 3) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
+    cc = CPU_GET_CLASS(gdb_ctx->s->g_cpu);
+    if (!cc->gdb_core_xml_file) {
+        put_packet(gdb_ctx->s, "");
+        return;
+    }
+
+    gdb_has_xml = true;
+    p = gdb_ctx->params[0].data;
+    xml = get_feature_xml(gdb_ctx->s, p, &p, process);
+    if (!xml) {
+        put_packet(gdb_ctx->s, "E00");
+        return;
+    }
+
+    addr = gdb_ctx->params[1].val_ul;
+    len = gdb_ctx->params[2].val_ul;
+    total_len = strlen(xml);
+    if (addr > total_len) {
+        put_packet(gdb_ctx->s, "E00");
+        return;
+    }
+
+    if (len > (MAX_PACKET_LENGTH - 5) / 2) {
+        len = (MAX_PACKET_LENGTH - 5) / 2;
+    }
+
+    if (len < total_len - addr) {
+        gdb_ctx->str_buf[0] = 'm';
+        len = memtox(gdb_ctx->str_buf + 1, xml + addr, len);
+    } else {
+        gdb_ctx->str_buf[0] = 'l';
+        len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr);
+    }
+
+    put_packet_binary(gdb_ctx->s, gdb_ctx->str_buf, len + 1, true);
+}
+
+static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    put_packet(gdb_ctx->s, GDB_ATTACHED);
+}
+
+static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    put_packet(gdb_ctx->s, "sstepbits;sstep");
+}
+
+static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
+    /* Order is important if has same prefix */
+    {
+        .handler = handle_query_qemu_sstepbits,
+        .cmd = "qemu.sstepbits",
+    },
+    {
+        .handler = handle_query_qemu_sstep,
+        .cmd = "qemu.sstep",
+    },
+    {
+        .handler = handle_set_qemu_sstep,
+        .cmd = "qemu.sstep=",
+        .cmd_startswith = 1,
+        .schema = "l0"
+    },
+};
+
+static GdbCmdParseEntry gdb_gen_query_table[] = {
+    {
+        .handler = handle_query_curr_tid,
+        .cmd = "C",
+    },
+    {
+        .handler = handle_query_threads,
+        .cmd = "sThreadInfo",
+    },
+    {
+        .handler = handle_query_first_threads,
+        .cmd = "fThreadInfo",
+    },
+    {
+        .handler = handle_query_thread_extra,
+        .cmd = "ThreadExtraInfo,",
+        .cmd_startswith = 1,
+        .schema = "t0"
+    },
+#ifdef CONFIG_USER_ONLY
+    {
+        .handler = handle_query_offsets,
+        .cmd = "Offsets",
+    },
+#else
+    {
+        .handler = handle_query_rcmd,
+        .cmd = "Rcmd,",
+        .cmd_startswith = 1,
+        .schema = "s0"
+    },
+#endif
+    {
+        .handler = handle_query_supported,
+        .cmd = "Supported:",
+        .cmd_startswith = 1,
+        .schema = "s0"
+    },
+    {
+        .handler = handle_query_supported,
+        .cmd = "Supported",
+        .schema = "s0"
+    },
+    {
+        .handler = handle_query_xfer_features,
+        .cmd = "Xfer:features:read:",
+        .cmd_startswith = 1,
+        .schema = "s:l,l0"
+    },
+    {
+        .handler = handle_query_attached,
+        .cmd = "Attached:",
+        .cmd_startswith = 1
+    },
+    {
+        .handler = handle_query_attached,
+        .cmd = "Attached",
+    },
+    {
+        .handler = handle_query_qemu_supported,
+        .cmd = "qemu.Supported",
+    },
+};
+
+static GdbCmdParseEntry gdb_gen_set_table[] = {
+    /* Order is important if has same prefix */
+    {
+        .handler = handle_set_qemu_sstep,
+        .cmd = "qemu.sstep:",
+        .cmd_startswith = 1,
+        .schema = "l0"
+    },
+};
+
+static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+                            gdb_gen_query_set_common_table,
+                            ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+        return;
+    }
+
+    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+                           gdb_gen_query_table,
+                           ARRAY_SIZE(gdb_gen_query_table))) {
+        put_packet(gdb_ctx->s, "");
+    }
+}
+
+static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (!gdb_ctx->num_params) {
+        return;
+    }
+
+    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+                            gdb_gen_query_set_common_table,
+                            ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+        return;
+    }
+
+    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
+                           gdb_gen_set_table,
+                           ARRAY_SIZE(gdb_gen_set_table))) {
+        put_packet(gdb_ctx->s, "");
+    }
+}
+
+static int gdb_handle_packet(GDBState *s, const char *line_buf)
+{
+    const char *p;
+    int ch;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
     char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
     char thread_id[16];
-    target_ulong addr, len;
     const GdbCmdParseEntry *cmd_parser = NULL;
 
     trace_gdbstub_io_command(line_buf);
@@ -2131,183 +2473,28 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'q':
-    case 'Q':
-        /* parse any 'q' packets here */
-        if (!strcmp(p,"qemu.sstepbits")) {
-            /* Query Breakpoint bit definitions */
-            snprintf(buf, sizeof(buf), "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
-                     SSTEP_ENABLE,
-                     SSTEP_NOIRQ,
-                     SSTEP_NOTIMER);
-            put_packet(s, buf);
-            break;
-        } else if (is_query_packet(p, "qemu.sstep", '=')) {
-            /* Display or change the sstep_flags */
-            p += 10;
-            if (*p != '=') {
-                /* Display current setting */
-                snprintf(buf, sizeof(buf), "0x%x", sstep_flags);
-                put_packet(s, buf);
-                break;
-            }
-            p++;
-            type = strtoul(p, (char **)&p, 16);
-            sstep_flags = type;
-            put_packet(s, "OK");
-            break;
-        } else if (strcmp(p,"C") == 0) {
-            /*
-             * "Current thread" remains vague in the spec, so always return
-             * the first thread of the current process (gdb returns the
-             * first thread).
-             */
-            cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, s->g_cpu));
-            snprintf(buf, sizeof(buf), "QC%s",
-                     gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
-            put_packet(s, buf);
-            break;
-        } else if (strcmp(p,"fThreadInfo") == 0) {
-            s->query_cpu = gdb_first_attached_cpu(s);
-            goto report_cpuinfo;
-        } else if (strcmp(p,"sThreadInfo") == 0) {
-        report_cpuinfo:
-            if (s->query_cpu) {
-                snprintf(buf, sizeof(buf), "m%s",
-                         gdb_fmt_thread_id(s, s->query_cpu,
-                                       thread_id, sizeof(thread_id)));
-                put_packet(s, buf);
-                s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
-            } else
-                put_packet(s, "l");
-            break;
-        } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-            if (read_thread_id(p + 16, &p, &pid, &tid) == GDB_READ_THREAD_ERR) {
-                put_packet(s, "E22");
-                break;
-            }
-            cpu = gdb_get_cpu(s, pid, tid);
-            if (cpu != NULL) {
-                cpu_synchronize_state(cpu);
-
-                if (s->multiprocess && (s->process_num > 1)) {
-                    /* Print the CPU model and name in multiprocess mode */
-                    ObjectClass *oc = object_get_class(OBJECT(cpu));
-                    const char *cpu_model = object_class_get_name(oc);
-                    char *cpu_name =
-                        object_get_canonical_path_component(OBJECT(cpu));
-                    len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-                                   "%s %s [%s]", cpu_model, cpu_name,
-                                   cpu->halted ? "halted " : "running");
-                    g_free(cpu_name);
-                } else {
-                    /* memtohex() doubles the required space */
-                    len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-                                   "CPU#%d [%s]", cpu->cpu_index,
-                                   cpu->halted ? "halted " : "running");
-                }
-                trace_gdbstub_op_extra_info((char *)mem_buf);
-                memtohex(buf, mem_buf, len);
-                put_packet(s, buf);
-            }
-            break;
-        }
-#ifdef CONFIG_USER_ONLY
-        else if (strcmp(p, "Offsets") == 0) {
-            TaskState *ts = s->c_cpu->opaque;
-
-            snprintf(buf, sizeof(buf),
-                     "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
-                     ";Bss=" TARGET_ABI_FMT_lx,
-                     ts->info->code_offset,
-                     ts->info->data_offset,
-                     ts->info->data_offset);
-            put_packet(s, buf);
-            break;
-        }
-#else /* !CONFIG_USER_ONLY */
-        else if (strncmp(p, "Rcmd,", 5) == 0) {
-            int len = strlen(p + 5);
-
-            if ((len % 2) != 0) {
-                put_packet(s, "E01");
-                break;
-            }
-            len = len / 2;
-            hextomem(mem_buf, p + 5, len);
-            mem_buf[len++] = 0;
-            qemu_chr_be_write(s->mon_chr, mem_buf, len);
-            put_packet(s, "OK");
-            break;
-        }
-#endif /* !CONFIG_USER_ONLY */
-        if (is_query_packet(p, "Supported", ':')) {
-            snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
-            cc = CPU_GET_CLASS(first_cpu);
-            if (cc->gdb_core_xml_file != NULL) {
-                pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
-            }
-
-            if (strstr(p, "multiprocess+")) {
-                s->multiprocess = true;
-            }
-            pstrcat(buf, sizeof(buf), ";multiprocess+");
-
-            put_packet(s, buf);
-            break;
-        }
-        if (strncmp(p, "Xfer:features:read:", 19) == 0) {
-            const char *xml;
-            target_ulong total_len;
-
-            process = gdb_get_cpu_process(s, s->g_cpu);
-            cc = CPU_GET_CLASS(s->g_cpu);
-            if (cc->gdb_core_xml_file == NULL) {
-                goto unknown_command;
-            }
-
-            gdb_has_xml = true;
-            p += 19;
-            xml = get_feature_xml(s, p, &p, process);
-            if (!xml) {
-                snprintf(buf, sizeof(buf), "E00");
-                put_packet(s, buf);
-                break;
-            }
-
-            if (*p == ':')
-                p++;
-            addr = strtoul(p, (char **)&p, 16);
-            if (*p == ',')
-                p++;
-            len = strtoul(p, (char **)&p, 16);
-
-            total_len = strlen(xml);
-            if (addr > total_len) {
-                snprintf(buf, sizeof(buf), "E00");
-                put_packet(s, buf);
-                break;
-            }
-            if (len > (MAX_PACKET_LENGTH - 5) / 2)
-                len = (MAX_PACKET_LENGTH - 5) / 2;
-            if (len < total_len - addr) {
-                buf[0] = 'm';
-                len = memtox(buf + 1, xml + addr, len);
-            } else {
-                buf[0] = 'l';
-                len = memtox(buf + 1, xml + addr, total_len - addr);
-            }
-            put_packet_binary(s, buf, len + 1, true);
-            break;
+        {
+            static const GdbCmdParseEntry gen_query_cmd_desc = {
+                .handler = handle_gen_query,
+                .cmd = "q",
+                .cmd_startswith = 1,
+                .schema = "s0"
+            };
+            cmd_parser = &gen_query_cmd_desc;
         }
-        if (is_query_packet(p, "Attached", ':')) {
-            put_packet(s, GDB_ATTACHED);
-            break;
+        break;
+    case 'Q':
+        {
+            static const GdbCmdParseEntry gen_set_cmd_desc = {
+                .handler = handle_gen_set,
+                .cmd = "Q",
+                .cmd_startswith = 1,
+                .schema = "s0"
+            };
+            cmd_parser = &gen_set_cmd_desc;
         }
-        /* Unrecognised 'q' command.  */
-        goto unknown_command;
-
+        break;
     default:
-    unknown_command:
         /* put empty packet */
         buf[0] = '\0';
         put_packet(s, buf);
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? pkt) with new infra
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (16 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 14:11   ` Alex Bennée
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 19/20] gdbstub: Clear unused variables in gdb_handle_packet Jon Doron
                   ` (2 subsequent siblings)
  20 siblings, 1 reply; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Note: The user-mode thread-id has been correctly reported since bd88c780e6

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 80fe5b2d0c..a474f2c755 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2265,13 +2265,29 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 }
 
+static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    char thread_id[16];
+
+    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->c_cpu, thread_id,
+                      sizeof(thread_id));
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
+             GDB_SIGNAL_TRAP, thread_id);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+    /*
+     * Remove all the breakpoints when this query is issued,
+     * because gdb is doing an initial connect and the state
+     * should be cleaned up.
+     */
+    gdb_breakpoint_remove_all();
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     const char *p;
     int ch;
     uint8_t mem_buf[MAX_PACKET_LENGTH];
     char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
-    char thread_id[16];
     const GdbCmdParseEntry *cmd_parser = NULL;
 
     trace_gdbstub_io_command(line_buf);
@@ -2283,15 +2299,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, "OK");
         break;
     case '?':
-        /* TODO: Make this return the correct value for user-mode.  */
-        snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
-                 gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
-        put_packet(s, buf);
-        /* Remove all the breakpoints when this query is issued,
-         * because gdb is doing and initial connect and the state
-         * should be cleaned up.
-         */
-        gdb_breakpoint_remove_all();
+        {
+            static const GdbCmdParseEntry target_halted_cmd_desc = {
+                .handler = handle_target_halt,
+                .cmd = "?",
+                .cmd_startswith = 1
+            };
+            cmd_parser = &target_halted_cmd_desc;
+        }
         break;
     case 'c':
         {
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 19/20] gdbstub: Clear unused variables in gdb_handle_packet
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (17 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? " Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 20/20] gdbstub: Implement qemu physical memory mode Jon Doron
  2019-05-31 14:24 ` [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Alex Bennée
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Signed-off-by: Jon Doron <arilou@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index a474f2c755..a0ff0017f6 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2284,17 +2284,11 @@ static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
-    const char *p;
-    int ch;
-    uint8_t mem_buf[MAX_PACKET_LENGTH];
-    char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
     const GdbCmdParseEntry *cmd_parser = NULL;
 
     trace_gdbstub_io_command(line_buf);
 
-    p = line_buf;
-    ch = *p++;
-    switch(ch) {
+    switch (line_buf[0]) {
     case '!':
         put_packet(s, "OK");
         break;
@@ -2511,8 +2505,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         break;
     default:
         /* put empty packet */
-        buf[0] = '\0';
-        put_packet(s, buf);
+        put_packet(s, "");
         break;
     }
 
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH v12 20/20] gdbstub: Implement qemu physical memory mode
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (18 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 19/20] gdbstub: Clear unused variables in gdb_handle_packet Jon Doron
@ 2019-05-29  6:41 ` Jon Doron
  2019-05-31 14:24 ` [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Alex Bennée
  20 siblings, 0 replies; 31+ messages in thread
From: Jon Doron @ 2019-05-29  6:41 UTC (permalink / raw
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

Add a new query/set which changes the memory GDB sees to physical memory
only.

gdb> maint packet qqemu.PhyMemMode
will reply the current phy_mem_mode state (1 for enabled, 0 for disabled)
gdb> maint packet Qqemu.PhyMemMode:1
Will make GDB read/write only to physical memory, set to 0 to disable

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index a0ff0017f6..d46e21bf70 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -46,11 +46,27 @@
 #define GDB_ATTACHED "1"
 #endif
 
+#ifndef CONFIG_USER_ONLY
+static int phy_memory_mode;
+#endif
+
 static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
                                          uint8_t *buf, int len, bool is_write)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
+    CPUClass *cc;
 
+#ifndef CONFIG_USER_ONLY
+    if (phy_memory_mode) {
+        if (is_write) {
+            cpu_physical_memory_write(addr, buf, len);
+        } else {
+            cpu_physical_memory_read(addr, buf, len);
+        }
+        return 0;
+    }
+#endif
+
+    cc = CPU_GET_CLASS(cpu);
     if (cc->memory_rw_debug) {
         return cc->memory_rw_debug(cpu, addr, buf, len, is_write);
     }
@@ -2132,8 +2148,36 @@ static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
 
 static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-    put_packet(gdb_ctx->s, "sstepbits;sstep");
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "sstepbits;sstep");
+#ifndef CONFIG_USER_ONLY
+    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";PhyMemMode");
+#endif
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+#ifndef CONFIG_USER_ONLY
+static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx,
+                                           void *user_ctx)
+{
+    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "%d", phy_memory_mode);
+    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
+}
+
+static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+    if (!gdb_ctx->num_params) {
+        put_packet(gdb_ctx->s, "E22");
+        return;
+    }
+
+    if (!gdb_ctx->params[0].val_ul) {
+        phy_memory_mode = 0;
+    } else {
+        phy_memory_mode = 1;
+    }
+    put_packet(gdb_ctx->s, "OK");
 }
+#endif
 
 static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
     /* Order is important if has same prefix */
@@ -2215,6 +2259,12 @@ static GdbCmdParseEntry gdb_gen_query_table[] = {
         .handler = handle_query_qemu_supported,
         .cmd = "qemu.Supported",
     },
+#ifndef CONFIG_USER_ONLY
+    {
+        .handler = handle_query_qemu_phy_mem_mode,
+        .cmd = "qemu.PhyMemMode",
+    },
+#endif
 };
 
 static GdbCmdParseEntry gdb_gen_set_table[] = {
@@ -2225,6 +2275,14 @@ static GdbCmdParseEntry gdb_gen_set_table[] = {
         .cmd_startswith = 1,
         .schema = "l0"
     },
+#ifndef CONFIG_USER_ONLY
+    {
+        .handler = handle_set_qemu_phy_mem_mode,
+        .cmd = "qemu.PhyMemMode:",
+        .cmd_startswith = 1,
+        .schema = "l0"
+    },
+#endif
 };
 
 static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
@ 2019-05-31 13:33   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:33 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 101 +++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 61 insertions(+), 40 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index e6d895177b..1db322c15a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1413,11 +1413,6 @@ static inline int startswith(const char *string, const char *pattern)
>    return !strncmp(string, pattern, strlen(pattern));
>  }
>
> -static int process_string_cmd(
> -        GDBState *s, void *user_ctx, const char *data,
> -        const GdbCmdParseEntry *cmds, int num_cmds)
> -        __attribute__((unused));
> -
>  static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>                                const GdbCmdParseEntry *cmds, int num_cmds)
>  {
> @@ -1463,6 +1458,55 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>      return -1;
>  }
>
> +static void run_cmd_parser(GDBState *s, const char *data,
> +                           const GdbCmdParseEntry *cmd)
> +{
> +    if (!data) {
> +        return;
> +    }
> +
> +    /* In case there was an error during the command parsing we must
> +    * send a NULL packet to indicate the command is not supported */
> +    if (process_string_cmd(s, NULL, data, cmd, 1)) {
> +        put_packet(s, "");
> +    }
> +}
> +
> +static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    GDBProcess *process;
> +    GDBState *s = gdb_ctx->s;
> +    uint32_t pid = 1;
> +
> +    if (s->multiprocess) {
> +        if (!gdb_ctx->num_params) {
> +            put_packet(s, "E22");
> +            return;
> +        }
> +
> +        pid = gdb_ctx->params[0].val_ul;
> +    }
> +
> +    process = gdb_get_process(s, pid);
> +    gdb_process_breakpoint_remove_all(s, process);
> +    process->attached = false;
> +
> +    if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
> +        s->c_cpu = gdb_first_attached_cpu(s);
> +    }
> +
> +    if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> +        s->g_cpu = gdb_first_attached_cpu(s);
> +    }
> +
> +    if (!s->c_cpu) {
> +        /* No more process attached */
> +        gdb_syscall_mode = GDB_SYS_DISABLED;
> +        gdb_continue(s);
> +    }
> +    put_packet(s, "OK");
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1477,6 +1521,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>      uint8_t *registers;
>      target_ulong addr, len;
>      GDBThreadIdKind thread_kind;
> +    const GdbCmdParseEntry *cmd_parser = NULL;
>
>      trace_gdbstub_io_command(line_buf);
>
> @@ -1577,42 +1622,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          error_report("QEMU: Terminated via GDBstub");
>          exit(0);
>      case 'D':
> -        /* Detach packet */
> -        pid = 1;
> -
> -        if (s->multiprocess) {
> -            unsigned long lpid;
> -            if (*p != ';') {
> -                put_packet(s, "E22");
> -                break;
> -            }
> -
> -            if (qemu_strtoul(p + 1, &p, 16, &lpid)) {
> -                put_packet(s, "E22");
> -                break;
> -            }
> -
> -            pid = lpid;
> -        }
> -
> -        process = gdb_get_process(s, pid);
> -        gdb_process_breakpoint_remove_all(s, process);
> -        process->attached = false;
> -
> -        if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
> -            s->c_cpu = gdb_first_attached_cpu(s);
> -        }
> -
> -        if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> -            s->g_cpu = gdb_first_attached_cpu(s);
> -        }
> -
> -        if (s->c_cpu == NULL) {
> -            /* No more process attached */
> -            gdb_syscall_mode = GDB_SYS_DISABLED;
> -            gdb_continue(s);
> +        {
> +            static const GdbCmdParseEntry detach_cmd_desc = {
> +                .handler = handle_detach,
> +                .cmd = "D",
> +                .cmd_startswith = 1,
> +                .schema = "?.l0"
> +            };
> +            cmd_parser = &detach_cmd_desc;
>          }
> -        put_packet(s, "OK");
>          break;
>      case 's':
>          if (*p != '\0') {
> @@ -1985,6 +2003,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          put_packet(s, buf);
>          break;
>      }
> +
> +    run_cmd_parser(s, line_buf, cmd_parser);
> +
>      return RS_IDLE;
>  }


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
@ 2019-05-31 13:34   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:34 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 86 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 67 insertions(+), 19 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index db213cf173..572222bfa4 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -950,7 +950,7 @@ static inline int xlat_gdb_type(CPUState *cpu, int gdbtype)
>  }
>  #endif
>
> -static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
> +static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong len)
>  {
>      CPUState *cpu;
>      int err = 0;
> @@ -987,7 +987,7 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
>      }
>  }
>
> -static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
> +static int gdb_breakpoint_remove(int type, target_ulong addr, target_ulong len)
>  {
>      CPUState *cpu;
>      int err = 0;
> @@ -1605,6 +1605,52 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
>      }
>  }
>
> +static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int res;
> +
> +    if (gdb_ctx->num_params != 3) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    res = gdb_breakpoint_insert(gdb_ctx->params[0].val_ul,
> +                                gdb_ctx->params[1].val_ull,
> +                                gdb_ctx->params[2].val_ull);
> +    if (res >= 0) {
> +        put_packet(gdb_ctx->s, "OK");
> +        return;
> +    } else if (res == -ENOSYS) {
> +        put_packet(gdb_ctx->s, "");
> +        return;
> +    }
> +
> +    put_packet(gdb_ctx->s, "E22");
> +}
> +
> +static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int res;
> +
> +    if (gdb_ctx->num_params != 3) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    res = gdb_breakpoint_remove(gdb_ctx->params[0].val_ul,
> +                                gdb_ctx->params[1].val_ull,
> +                                gdb_ctx->params[2].val_ull);
> +    if (res >= 0) {
> +        put_packet(gdb_ctx->s, "OK");
> +        return;
> +    } else if (res == -ENOSYS) {
> +        put_packet(gdb_ctx->s, "");
> +        return;
> +    }
> +
> +    put_packet(gdb_ctx->s, "E22");
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1860,24 +1906,26 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          put_packet(s, "OK");
>          break;
>      case 'Z':
> +        {
> +            static const GdbCmdParseEntry insert_bp_cmd_desc = {
> +                .handler = handle_insert_bp,
> +                .cmd = "Z",
> +                .cmd_startswith = 1,
> +                .schema = "l?L?L0"
> +            };
> +            cmd_parser = &insert_bp_cmd_desc;
> +        }
> +        break;
>      case 'z':
> -        type = strtoul(p, (char **)&p, 16);
> -        if (*p == ',')
> -            p++;
> -        addr = strtoull(p, (char **)&p, 16);
> -        if (*p == ',')
> -            p++;
> -        len = strtoull(p, (char **)&p, 16);
> -        if (ch == 'Z')
> -            res = gdb_breakpoint_insert(addr, len, type);
> -        else
> -            res = gdb_breakpoint_remove(addr, len, type);
> -        if (res >= 0)
> -             put_packet(s, "OK");
> -        else if (res == -ENOSYS)
> -            put_packet(s, "");
> -        else
> -            put_packet(s, "E22");
> +        {
> +            static const GdbCmdParseEntry remove_bp_cmd_desc = {
> +                .handler = handle_remove_bp,
> +                .cmd = "z",
> +                .cmd_startswith = 1,
> +                .schema = "l?L?L0"
> +            };
> +            cmd_parser = &remove_bp_cmd_desc;
> +        }
>          break;
>      case 'H':
>          {


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P " Jon Doron
@ 2019-05-31 13:37   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:37 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 572222bfa4..fc9526b3f5 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1651,6 +1651,27 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
>      put_packet(gdb_ctx->s, "E22");
>  }
>
> +static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int reg_size;
> +
> +    if (!gdb_has_xml) {
> +        put_packet(gdb_ctx->s, "E00");
> +        return;
> +    }
> +
> +    if (gdb_ctx->num_params != 2) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    reg_size = strlen(gdb_ctx->params[1].data) / 2;
> +    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[1].data, reg_size);
> +    gdb_write_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
> +                       gdb_ctx->params[0].val_ull);
> +    put_packet(gdb_ctx->s, "OK");
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1895,15 +1916,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          }
>          break;
>      case 'P':
> -        if (!gdb_has_xml)
> -            goto unknown_command;
> -        addr = strtoull(p, (char **)&p, 16);
> -        if (*p == '=')
> -            p++;
> -        reg_size = strlen(p) / 2;
> -        hextomem(mem_buf, p, reg_size);
> -        gdb_write_register(s->g_cpu, mem_buf, addr);
> -        put_packet(s, "OK");
> +        {
> +            static const GdbCmdParseEntry set_reg_cmd_desc = {
> +                .handler = handle_set_reg,
> +                .cmd = "P",
> +                .cmd_startswith = 1,
> +                .schema = "L?s0"
> +            };
> +            cmd_parser = &set_reg_cmd_desc;
> +        }
>          break;
>      case 'Z':
>          {


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p " Jon Doron
@ 2019-05-31 13:38   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:38 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 50 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index fc9526b3f5..07740ec0af 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1672,6 +1672,36 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
>      put_packet(gdb_ctx->s, "OK");
>  }
>
> +static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int reg_size;
> +
> +    /*
> +     * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
> +     * This works, but can be very slow.  Anything new enough to
> +     * understand XML also knows how to use this properly.
> +     */
> +    if (!gdb_has_xml) {
> +        put_packet(gdb_ctx->s, "");
> +        return;
> +    }
> +
> +    if (!gdb_ctx->num_params) {
> +        put_packet(gdb_ctx->s, "E14");
> +        return;
> +    }
> +
> +    reg_size = gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf,
> +                                 gdb_ctx->params[0].val_ull);
> +    if (!reg_size) {
> +        put_packet(gdb_ctx->s, "E14");
> +        return;
> +    }
> +
> +    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, reg_size);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1901,18 +1931,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          }
>          break;
>      case 'p':
> -        /* Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
> -           This works, but can be very slow.  Anything new enough to
> -           understand XML also knows how to use this properly.  */
> -        if (!gdb_has_xml)
> -            goto unknown_command;
> -        addr = strtoull(p, (char **)&p, 16);
> -        reg_size = gdb_read_register(s->g_cpu, mem_buf, addr);
> -        if (reg_size) {
> -            memtohex(buf, mem_buf, reg_size);
> -            put_packet(s, buf);
> -        } else {
> -            put_packet(s, "E14");
> +        {
> +            static const GdbCmdParseEntry get_reg_cmd_desc = {
> +                .handler = handle_get_reg,
> +                .cmd = "p",
> +                .cmd_startswith = 1,
> +                .schema = "L0"
> +            };
> +            cmd_parser = &get_reg_cmd_desc;
>          }
>          break;
>      case 'P':


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F " Jon Doron
@ 2019-05-31 13:39   ` Alex Bennée
  2019-05-31 13:41   ` Alex Bennée
  1 sibling, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:39 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 48 ++++++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 8a401e6527..ea85966b27 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1789,6 +1789,25 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
>      put_packet(gdb_ctx->s, gdb_ctx->str_buf);
>  }
>
> +static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
> +        target_ulong ret, err;
> +
> +        ret = (target_ulong)gdb_ctx->params[0].val_ull;
> +        err = (target_ulong)gdb_ctx->params[1].val_ull;
> +        gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
> +        gdb_ctx->s->current_syscall_cb = NULL;
> +    }
> +
> +    if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') {
> +        put_packet(gdb_ctx->s, "T02");
> +        return;
> +    }
> +
> +    gdb_continue(gdb_ctx->s);
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1930,28 +1949,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          return RS_IDLE;
>      case 'F':
>          {
> -            target_ulong ret;
> -            target_ulong err;
> -
> -            ret = strtoull(p, (char **)&p, 16);
> -            if (*p == ',') {
> -                p++;
> -                err = strtoull(p, (char **)&p, 16);
> -            } else {
> -                err = 0;
> -            }
> -            if (*p == ',')
> -                p++;
> -            type = *p;
> -            if (s->current_syscall_cb) {
> -                s->current_syscall_cb(s->c_cpu, ret, err);
> -                s->current_syscall_cb = NULL;
> -            }
> -            if (type == 'C') {
> -                put_packet(s, "T02");
> -            } else {
> -                gdb_continue(s);
> -            }
> +            static const GdbCmdParseEntry file_io_cmd_desc = {
> +                .handler = handle_file_io,
> +                .cmd = "F",
> +                .cmd_startswith = 1,
> +                .schema = "L,L,o0"
> +            };
> +            cmd_parser = &file_io_cmd_desc;
>          }
>          break;
>      case 'g':


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F " Jon Doron
  2019-05-31 13:39   ` Alex Bennée
@ 2019-05-31 13:41   ` Alex Bennée
  1 sibling, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:41 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 48 ++++++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 8a401e6527..ea85966b27 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1789,6 +1789,25 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
>      put_packet(gdb_ctx->s, gdb_ctx->str_buf);
>  }
>
> +static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
> +        target_ulong ret, err;
> +
> +        ret = (target_ulong)gdb_ctx->params[0].val_ull;
> +        err = (target_ulong)gdb_ctx->params[1].val_ull;
> +        gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
> +        gdb_ctx->s->current_syscall_cb = NULL;
> +    }
> +
> +    if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') {
> +        put_packet(gdb_ctx->s, "T02");
> +        return;
> +    }
> +
> +    gdb_continue(gdb_ctx->s);
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1930,28 +1949,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          return RS_IDLE;
>      case 'F':
>          {
> -            target_ulong ret;
> -            target_ulong err;
> -
> -            ret = strtoull(p, (char **)&p, 16);
> -            if (*p == ',') {
> -                p++;
> -                err = strtoull(p, (char **)&p, 16);
> -            } else {
> -                err = 0;
> -            }
> -            if (*p == ',')
> -                p++;
> -            type = *p;
> -            if (s->current_syscall_cb) {
> -                s->current_syscall_cb(s->c_cpu, ret, err);
> -                s->current_syscall_cb = NULL;
> -            }
> -            if (type == 'C') {
> -                put_packet(s, "T02");
> -            } else {
> -                gdb_continue(s);
> -            }
> +            static const GdbCmdParseEntry file_io_cmd_desc = {
> +                .handler = handle_file_io,
> +                .cmd = "F",
> +                .cmd_startswith = 1,
> +                .schema = "L,L,o0"
> +            };
> +            cmd_parser = &file_io_cmd_desc;
>          }
>          break;
>      case 'g':


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands " Jon Doron
@ 2019-05-31 13:42   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 13:42 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 170 +++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 110 insertions(+), 60 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 3fd1a1cddb..648191a3b0 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1818,6 +1818,106 @@ static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
>      gdb_continue(gdb_ctx->s);
>  }
>
> +static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    put_packet(gdb_ctx->s, "vCont;c;C;s;S");
> +}
> +
> +static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int res;
> +
> +    if (!gdb_ctx->num_params) {
> +        return;
> +    }
> +
> +    res = gdb_handle_vcont(gdb_ctx->s, gdb_ctx->params[0].data);
> +    if ((res == -EINVAL) || (res == -ERANGE)) {
> +        put_packet(gdb_ctx->s, "E22");
> +    } else if (res) {
> +        put_packet(gdb_ctx->s, "");
> +    }
> +}
> +
> +static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    GDBProcess *process;
> +    CPUState *cpu;
> +    char thread_id[16];
> +
> +    pstrcpy(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "E22");
> +    if (!gdb_ctx->num_params) {
> +        goto cleanup;
> +    }
> +
> +    process = gdb_get_process(gdb_ctx->s, gdb_ctx->params[0].val_ul);
> +    if (!process) {
> +        goto cleanup;
> +    }
> +
> +    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
> +    if (!cpu) {
> +        goto cleanup;
> +    }
> +
> +    process->attached = true;
> +    gdb_ctx->s->g_cpu = cpu;
> +    gdb_ctx->s->c_cpu = cpu;
> +
> +    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
> +             GDB_SIGNAL_TRAP, thread_id);
> +cleanup:
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    /* Kill the target */
> +    put_packet(gdb_ctx->s, "OK");
> +    error_report("QEMU: Terminated via GDBstub");
> +    exit(0);
> +}
> +
> +static GdbCmdParseEntry gdb_v_commands_table[] = {
> +    /* Order is important if has same prefix */
> +    {
> +        .handler = handle_v_cont_query,
> +        .cmd = "Cont?",
> +        .cmd_startswith = 1
> +    },
> +    {
> +        .handler = handle_v_cont,
> +        .cmd = "Cont",
> +        .cmd_startswith = 1,
> +        .schema = "s0"
> +    },
> +    {
> +        .handler = handle_v_attach,
> +        .cmd = "Attach;",
> +        .cmd_startswith = 1,
> +        .schema = "l0"
> +    },
> +    {
> +        .handler = handle_v_kill,
> +        .cmd = "Kill;",
> +        .cmd_startswith = 1
> +    },
> +};
> +
> +static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (!gdb_ctx->num_params) {
> +        return;
> +    }
> +
> +    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
> +                           gdb_v_commands_table,
> +                           ARRAY_SIZE(gdb_v_commands_table))) {
> +        put_packet(gdb_ctx->s, "");
> +    }
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> @@ -1825,7 +1925,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>      CPUClass *cc;
>      const char *p;
>      uint32_t pid, tid;
> -    int ch, type, res;
> +    int ch, type;
>      uint8_t mem_buf[MAX_PACKET_LENGTH];
>      char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
>      char thread_id[16];
> @@ -1874,66 +1974,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          }
>          break;
>      case 'v':
> -        if (strncmp(p, "Cont", 4) == 0) {
> -            p += 4;
> -            if (*p == '?') {
> -                put_packet(s, "vCont;c;C;s;S");
> -                break;
> -            }
> -
> -            res = gdb_handle_vcont(s, p);
> -
> -            if (res) {
> -                if ((res == -EINVAL) || (res == -ERANGE)) {
> -                    put_packet(s, "E22");
> -                    break;
> -                }
> -                goto unknown_command;
> -            }
> -            break;
> -        } else if (strncmp(p, "Attach;", 7) == 0) {
> -            unsigned long pid;
> -
> -            p += 7;
> -
> -            if (qemu_strtoul(p, &p, 16, &pid)) {
> -                put_packet(s, "E22");
> -                break;
> -            }
> -
> -            process = gdb_get_process(s, pid);
> -
> -            if (process == NULL) {
> -                put_packet(s, "E22");
> -                break;
> -            }
> -
> -            cpu = get_first_cpu_in_process(s, process);
> -
> -            if (cpu == NULL) {
> -                /* Refuse to attach an empty process */
> -                put_packet(s, "E22");
> -                break;
> -            }
> -
> -            process->attached = true;
> -
> -            s->g_cpu = cpu;
> -            s->c_cpu = cpu;
> -
> -            snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
> -                     gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
> -
> -            put_packet(s, buf);
> -            break;
> -        } else if (strncmp(p, "Kill;", 5) == 0) {
> -            /* Kill the target */
> -            put_packet(s, "OK");
> -            error_report("QEMU: Terminated via GDBstub");
> -            exit(0);
> -        } else {
> -            goto unknown_command;
> +        {
> +            static const GdbCmdParseEntry v_cmd_desc = {
> +                .handler = handle_v_commands,
> +                .cmd = "v",
> +                .cmd_startswith = 1,
> +                .schema = "s0"
> +            };
> +            cmd_parser = &v_cmd_desc;
>          }
> +        break;
>      case 'k':
>          /* Kill the target */
>          error_report("QEMU: Terminated via GDBstub");


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
@ 2019-05-31 14:10   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 14:10 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> The generic set/query packets contains implementation for varioius
> sub-commands which are required for GDB and also additional commands
> which are QEMU specific.
>
> To see which QEMU specific commands are available use the command
> gdb> maintenance packet qqemu.Supported
>
> Currently the only implemented QEMU specific command is the command
> that sets the single step behavior.
>
> gdb> maintenance packet qqemu.sstepbits
> Will display the MASK bits used to control the single stepping.
>
> gdb> maintenance packet qqemu.sstep
> Will display the current value of the mask used when single stepping.
>
> gdb> maintenance packet Qqemu.sstep:HEX_VALUE
> Will change the single step mask.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  gdbstub.c | 559 ++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 373 insertions(+), 186 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 648191a3b0..80fe5b2d0c 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1130,14 +1130,6 @@ static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
>      return GDB_ONE_THREAD;
>  }
>
> -static int is_query_packet(const char *p, const char *query, char separator)
> -{
> -    unsigned int query_len = strlen(query);
> -
> -    return strncmp(p, query, query_len) == 0 &&
> -        (p[query_len] == '\0' || p[query_len] == separator);
> -}
> -
>  /**
>   * gdb_handle_vcont - Parses and handles a vCont packet.
>   * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
> @@ -1918,18 +1910,368 @@ static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
>      }
>  }
>
> -static int gdb_handle_packet(GDBState *s, const char *line_buf)
> +static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
> +             "ENABLE=%x,NOIRQ=%x,NOTIMER=%x", SSTEP_ENABLE,
> +             SSTEP_NOIRQ, SSTEP_NOTIMER);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (!gdb_ctx->num_params) {
> +        return;
> +    }
> +
> +    sstep_flags = gdb_ctx->params[0].val_ul;
> +    put_packet(gdb_ctx->s, "OK");
> +}
> +
> +static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "0x%x", sstep_flags);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
>  {
>      CPUState *cpu;
>      GDBProcess *process;
> +    char thread_id[16];
> +
> +    /*
> +     * "Current thread" remains vague in the spec, so always return
> +     * the first thread of the current process (gdb returns the
> +     * first thread).
> +     */
> +    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
> +    cpu = get_first_cpu_in_process(gdb_ctx->s, process);
> +    gdb_fmt_thread_id(gdb_ctx->s, cpu, thread_id, sizeof(thread_id));
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "QC%s", thread_id);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    char thread_id[16];
> +
> +    if (!gdb_ctx->s->query_cpu) {
> +        put_packet(gdb_ctx->s, "l");
> +        return;
> +    }
> +
> +    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->query_cpu, thread_id,
> +                      sizeof(thread_id));
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "m%s", thread_id);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +    gdb_ctx->s->query_cpu =
> +        gdb_next_attached_cpu(gdb_ctx->s, gdb_ctx->s->query_cpu);
> +}
> +
> +static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    gdb_ctx->s->query_cpu = gdb_first_attached_cpu(gdb_ctx->s);
> +    handle_query_threads(gdb_ctx, user_ctx);
> +}
> +
> +static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    CPUState *cpu;
> +    int len;
> +
> +    if (!gdb_ctx->num_params ||
> +        gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    cpu = gdb_get_cpu(gdb_ctx->s, gdb_ctx->params[0].thread_id.pid,
> +                      gdb_ctx->params[0].thread_id.tid);
> +    if (!cpu) {
> +        return;
> +    }
> +
> +    cpu_synchronize_state(cpu);
> +
> +    if (gdb_ctx->s->multiprocess && (gdb_ctx->s->process_num > 1)) {
> +        /* Print the CPU model and name in multiprocess mode */
> +        ObjectClass *oc = object_get_class(OBJECT(cpu));
> +        const char *cpu_model = object_class_get_name(oc);
> +        char *cpu_name = object_get_canonical_path_component(OBJECT(cpu));
> +        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
> +                       "%s %s [%s]", cpu_model, cpu_name,
> +                       cpu->halted ? "halted " : "running");
> +        g_free(cpu_name);
> +    } else {
> +        /* memtohex() doubles the required space */
> +        len = snprintf((char *)gdb_ctx->mem_buf, sizeof(gdb_ctx->str_buf) / 2,
> +                        "CPU#%d [%s]", cpu->cpu_index,
> +                        cpu->halted ? "halted " : "running");
> +    }
> +    trace_gdbstub_op_extra_info((char *)gdb_ctx->mem_buf);
> +    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +#ifdef CONFIG_USER_ONLY
> +static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    TaskState *ts;
> +
> +    ts = gdb_ctx->s->c_cpu->opaque;
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
> +             "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
> +             ";Bss=" TARGET_ABI_FMT_lx,
> +             ts->info->code_offset,
> +             ts->info->data_offset,
> +             ts->info->data_offset);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +#else
> +static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    int len;
> +
> +    if (!gdb_ctx->num_params) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    len = strlen(gdb_ctx->params[0].data);
> +    if (len % 2) {
> +        put_packet(gdb_ctx->s, "E01");
> +        return;
> +    }
> +
> +    len = len / 2;
> +    hextomem(gdb_ctx->mem_buf, gdb_ctx->params[0].data, len);
> +    gdb_ctx->mem_buf[len++] = 0;
> +    qemu_chr_be_write(gdb_ctx->s->mon_chr, gdb_ctx->mem_buf, len);
> +    put_packet(gdb_ctx->s, "OK");
> +
> +}
> +#endif
> +
> +static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
>      CPUClass *cc;
> +
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "PacketSize=%x",
> +             MAX_PACKET_LENGTH);
> +    cc = CPU_GET_CLASS(first_cpu);
> +    if (cc->gdb_core_xml_file) {
> +        pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf),
> +                ";qXfer:features:read+");
> +    }
> +
> +    if (gdb_ctx->num_params &&
> +        strstr(gdb_ctx->params[0].data, "multiprocess+")) {
> +        gdb_ctx->s->multiprocess = true;
> +    }
> +
> +    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +}
> +
> +static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    GDBProcess *process;
> +    CPUClass *cc;
> +    unsigned long len, total_len, addr;
> +    const char *xml;
>      const char *p;
> -    uint32_t pid, tid;
> -    int ch, type;
> +
> +    if (gdb_ctx->num_params < 3) {
> +        put_packet(gdb_ctx->s, "E22");
> +        return;
> +    }
> +
> +    process = gdb_get_cpu_process(gdb_ctx->s, gdb_ctx->s->g_cpu);
> +    cc = CPU_GET_CLASS(gdb_ctx->s->g_cpu);
> +    if (!cc->gdb_core_xml_file) {
> +        put_packet(gdb_ctx->s, "");
> +        return;
> +    }
> +
> +    gdb_has_xml = true;
> +    p = gdb_ctx->params[0].data;
> +    xml = get_feature_xml(gdb_ctx->s, p, &p, process);
> +    if (!xml) {
> +        put_packet(gdb_ctx->s, "E00");
> +        return;
> +    }
> +
> +    addr = gdb_ctx->params[1].val_ul;
> +    len = gdb_ctx->params[2].val_ul;
> +    total_len = strlen(xml);
> +    if (addr > total_len) {
> +        put_packet(gdb_ctx->s, "E00");
> +        return;
> +    }
> +
> +    if (len > (MAX_PACKET_LENGTH - 5) / 2) {
> +        len = (MAX_PACKET_LENGTH - 5) / 2;
> +    }
> +
> +    if (len < total_len - addr) {
> +        gdb_ctx->str_buf[0] = 'm';
> +        len = memtox(gdb_ctx->str_buf + 1, xml + addr, len);
> +    } else {
> +        gdb_ctx->str_buf[0] = 'l';
> +        len = memtox(gdb_ctx->str_buf + 1, xml + addr, total_len - addr);
> +    }
> +
> +    put_packet_binary(gdb_ctx->s, gdb_ctx->str_buf, len + 1, true);
> +}
> +
> +static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    put_packet(gdb_ctx->s, GDB_ATTACHED);
> +}
> +
> +static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    put_packet(gdb_ctx->s, "sstepbits;sstep");
> +}
> +
> +static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
> +    /* Order is important if has same prefix */
> +    {
> +        .handler = handle_query_qemu_sstepbits,
> +        .cmd = "qemu.sstepbits",
> +    },
> +    {
> +        .handler = handle_query_qemu_sstep,
> +        .cmd = "qemu.sstep",
> +    },
> +    {
> +        .handler = handle_set_qemu_sstep,
> +        .cmd = "qemu.sstep=",
> +        .cmd_startswith = 1,
> +        .schema = "l0"
> +    },
> +};
> +
> +static GdbCmdParseEntry gdb_gen_query_table[] = {
> +    {
> +        .handler = handle_query_curr_tid,
> +        .cmd = "C",
> +    },
> +    {
> +        .handler = handle_query_threads,
> +        .cmd = "sThreadInfo",
> +    },
> +    {
> +        .handler = handle_query_first_threads,
> +        .cmd = "fThreadInfo",
> +    },
> +    {
> +        .handler = handle_query_thread_extra,
> +        .cmd = "ThreadExtraInfo,",
> +        .cmd_startswith = 1,
> +        .schema = "t0"
> +    },
> +#ifdef CONFIG_USER_ONLY
> +    {
> +        .handler = handle_query_offsets,
> +        .cmd = "Offsets",
> +    },
> +#else
> +    {
> +        .handler = handle_query_rcmd,
> +        .cmd = "Rcmd,",
> +        .cmd_startswith = 1,
> +        .schema = "s0"
> +    },
> +#endif
> +    {
> +        .handler = handle_query_supported,
> +        .cmd = "Supported:",
> +        .cmd_startswith = 1,
> +        .schema = "s0"
> +    },
> +    {
> +        .handler = handle_query_supported,
> +        .cmd = "Supported",
> +        .schema = "s0"
> +    },
> +    {
> +        .handler = handle_query_xfer_features,
> +        .cmd = "Xfer:features:read:",
> +        .cmd_startswith = 1,
> +        .schema = "s:l,l0"
> +    },
> +    {
> +        .handler = handle_query_attached,
> +        .cmd = "Attached:",
> +        .cmd_startswith = 1
> +    },
> +    {
> +        .handler = handle_query_attached,
> +        .cmd = "Attached",
> +    },
> +    {
> +        .handler = handle_query_qemu_supported,
> +        .cmd = "qemu.Supported",
> +    },
> +};
> +
> +static GdbCmdParseEntry gdb_gen_set_table[] = {
> +    /* Order is important if has same prefix */
> +    {
> +        .handler = handle_set_qemu_sstep,
> +        .cmd = "qemu.sstep:",
> +        .cmd_startswith = 1,
> +        .schema = "l0"
> +    },
> +};
> +
> +static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (!gdb_ctx->num_params) {
> +        return;
> +    }
> +
> +    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
> +                            gdb_gen_query_set_common_table,
> +                            ARRAY_SIZE(gdb_gen_query_set_common_table))) {
> +        return;
> +    }
> +
> +    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
> +                           gdb_gen_query_table,
> +                           ARRAY_SIZE(gdb_gen_query_table))) {
> +        put_packet(gdb_ctx->s, "");
> +    }
> +}
> +
> +static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    if (!gdb_ctx->num_params) {
> +        return;
> +    }
> +
> +    if (!process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
> +                            gdb_gen_query_set_common_table,
> +                            ARRAY_SIZE(gdb_gen_query_set_common_table))) {
> +        return;
> +    }
> +
> +    if (process_string_cmd(gdb_ctx->s, NULL, gdb_ctx->params[0].data,
> +                           gdb_gen_set_table,
> +                           ARRAY_SIZE(gdb_gen_set_table))) {
> +        put_packet(gdb_ctx->s, "");
> +    }
> +}
> +
> +static int gdb_handle_packet(GDBState *s, const char *line_buf)
> +{
> +    const char *p;
> +    int ch;
>      uint8_t mem_buf[MAX_PACKET_LENGTH];
>      char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
>      char thread_id[16];
> -    target_ulong addr, len;
>      const GdbCmdParseEntry *cmd_parser = NULL;
>
>      trace_gdbstub_io_command(line_buf);
> @@ -2131,183 +2473,28 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          }
>          break;
>      case 'q':
> -    case 'Q':
> -        /* parse any 'q' packets here */
> -        if (!strcmp(p,"qemu.sstepbits")) {
> -            /* Query Breakpoint bit definitions */
> -            snprintf(buf, sizeof(buf), "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
> -                     SSTEP_ENABLE,
> -                     SSTEP_NOIRQ,
> -                     SSTEP_NOTIMER);
> -            put_packet(s, buf);
> -            break;
> -        } else if (is_query_packet(p, "qemu.sstep", '=')) {
> -            /* Display or change the sstep_flags */
> -            p += 10;
> -            if (*p != '=') {
> -                /* Display current setting */
> -                snprintf(buf, sizeof(buf), "0x%x", sstep_flags);
> -                put_packet(s, buf);
> -                break;
> -            }
> -            p++;
> -            type = strtoul(p, (char **)&p, 16);
> -            sstep_flags = type;
> -            put_packet(s, "OK");
> -            break;
> -        } else if (strcmp(p,"C") == 0) {
> -            /*
> -             * "Current thread" remains vague in the spec, so always return
> -             * the first thread of the current process (gdb returns the
> -             * first thread).
> -             */
> -            cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, s->g_cpu));
> -            snprintf(buf, sizeof(buf), "QC%s",
> -                     gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
> -            put_packet(s, buf);
> -            break;
> -        } else if (strcmp(p,"fThreadInfo") == 0) {
> -            s->query_cpu = gdb_first_attached_cpu(s);
> -            goto report_cpuinfo;
> -        } else if (strcmp(p,"sThreadInfo") == 0) {
> -        report_cpuinfo:
> -            if (s->query_cpu) {
> -                snprintf(buf, sizeof(buf), "m%s",
> -                         gdb_fmt_thread_id(s, s->query_cpu,
> -                                       thread_id, sizeof(thread_id)));
> -                put_packet(s, buf);
> -                s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
> -            } else
> -                put_packet(s, "l");
> -            break;
> -        } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> -            if (read_thread_id(p + 16, &p, &pid, &tid) == GDB_READ_THREAD_ERR) {
> -                put_packet(s, "E22");
> -                break;
> -            }
> -            cpu = gdb_get_cpu(s, pid, tid);
> -            if (cpu != NULL) {
> -                cpu_synchronize_state(cpu);
> -
> -                if (s->multiprocess && (s->process_num > 1)) {
> -                    /* Print the CPU model and name in multiprocess mode */
> -                    ObjectClass *oc = object_get_class(OBJECT(cpu));
> -                    const char *cpu_model = object_class_get_name(oc);
> -                    char *cpu_name =
> -                        object_get_canonical_path_component(OBJECT(cpu));
> -                    len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> -                                   "%s %s [%s]", cpu_model, cpu_name,
> -                                   cpu->halted ? "halted " : "running");
> -                    g_free(cpu_name);
> -                } else {
> -                    /* memtohex() doubles the required space */
> -                    len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> -                                   "CPU#%d [%s]", cpu->cpu_index,
> -                                   cpu->halted ? "halted " : "running");
> -                }
> -                trace_gdbstub_op_extra_info((char *)mem_buf);
> -                memtohex(buf, mem_buf, len);
> -                put_packet(s, buf);
> -            }
> -            break;
> -        }
> -#ifdef CONFIG_USER_ONLY
> -        else if (strcmp(p, "Offsets") == 0) {
> -            TaskState *ts = s->c_cpu->opaque;
> -
> -            snprintf(buf, sizeof(buf),
> -                     "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
> -                     ";Bss=" TARGET_ABI_FMT_lx,
> -                     ts->info->code_offset,
> -                     ts->info->data_offset,
> -                     ts->info->data_offset);
> -            put_packet(s, buf);
> -            break;
> -        }
> -#else /* !CONFIG_USER_ONLY */
> -        else if (strncmp(p, "Rcmd,", 5) == 0) {
> -            int len = strlen(p + 5);
> -
> -            if ((len % 2) != 0) {
> -                put_packet(s, "E01");
> -                break;
> -            }
> -            len = len / 2;
> -            hextomem(mem_buf, p + 5, len);
> -            mem_buf[len++] = 0;
> -            qemu_chr_be_write(s->mon_chr, mem_buf, len);
> -            put_packet(s, "OK");
> -            break;
> -        }
> -#endif /* !CONFIG_USER_ONLY */
> -        if (is_query_packet(p, "Supported", ':')) {
> -            snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
> -            cc = CPU_GET_CLASS(first_cpu);
> -            if (cc->gdb_core_xml_file != NULL) {
> -                pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
> -            }
> -
> -            if (strstr(p, "multiprocess+")) {
> -                s->multiprocess = true;
> -            }
> -            pstrcat(buf, sizeof(buf), ";multiprocess+");
> -
> -            put_packet(s, buf);
> -            break;
> -        }
> -        if (strncmp(p, "Xfer:features:read:", 19) == 0) {
> -            const char *xml;
> -            target_ulong total_len;
> -
> -            process = gdb_get_cpu_process(s, s->g_cpu);
> -            cc = CPU_GET_CLASS(s->g_cpu);
> -            if (cc->gdb_core_xml_file == NULL) {
> -                goto unknown_command;
> -            }
> -
> -            gdb_has_xml = true;
> -            p += 19;
> -            xml = get_feature_xml(s, p, &p, process);
> -            if (!xml) {
> -                snprintf(buf, sizeof(buf), "E00");
> -                put_packet(s, buf);
> -                break;
> -            }
> -
> -            if (*p == ':')
> -                p++;
> -            addr = strtoul(p, (char **)&p, 16);
> -            if (*p == ',')
> -                p++;
> -            len = strtoul(p, (char **)&p, 16);
> -
> -            total_len = strlen(xml);
> -            if (addr > total_len) {
> -                snprintf(buf, sizeof(buf), "E00");
> -                put_packet(s, buf);
> -                break;
> -            }
> -            if (len > (MAX_PACKET_LENGTH - 5) / 2)
> -                len = (MAX_PACKET_LENGTH - 5) / 2;
> -            if (len < total_len - addr) {
> -                buf[0] = 'm';
> -                len = memtox(buf + 1, xml + addr, len);
> -            } else {
> -                buf[0] = 'l';
> -                len = memtox(buf + 1, xml + addr, total_len - addr);
> -            }
> -            put_packet_binary(s, buf, len + 1, true);
> -            break;
> +        {
> +            static const GdbCmdParseEntry gen_query_cmd_desc = {
> +                .handler = handle_gen_query,
> +                .cmd = "q",
> +                .cmd_startswith = 1,
> +                .schema = "s0"
> +            };
> +            cmd_parser = &gen_query_cmd_desc;
>          }
> -        if (is_query_packet(p, "Attached", ':')) {
> -            put_packet(s, GDB_ATTACHED);
> -            break;
> +        break;
> +    case 'Q':
> +        {
> +            static const GdbCmdParseEntry gen_set_cmd_desc = {
> +                .handler = handle_gen_set,
> +                .cmd = "Q",
> +                .cmd_startswith = 1,
> +                .schema = "s0"
> +            };
> +            cmd_parser = &gen_set_cmd_desc;
>          }
> -        /* Unrecognised 'q' command.  */
> -        goto unknown_command;
> -
> +        break;
>      default:
> -    unknown_command:
>          /* put empty packet */
>          buf[0] = '\0';
>          put_packet(s, buf);


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? pkt) with new infra
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? " Jon Doron
@ 2019-05-31 14:11   ` Alex Bennée
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 14:11 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> Note: The user-mode thread-id has been correctly reported since bd88c780e6
>
> Signed-off-by: Jon Doron <arilou@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 80fe5b2d0c..a474f2c755 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2265,13 +2265,29 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
>      }
>  }
>
> +static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
> +{
> +    char thread_id[16];
> +
> +    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->c_cpu, thread_id,
> +                      sizeof(thread_id));
> +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
> +             GDB_SIGNAL_TRAP, thread_id);
> +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> +    /*
> +     * Remove all the breakpoints when this query is issued,
> +     * because gdb is doing an initial connect and the state
> +     * should be cleaned up.
> +     */
> +    gdb_breakpoint_remove_all();
> +}
> +
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      const char *p;
>      int ch;
>      uint8_t mem_buf[MAX_PACKET_LENGTH];
>      char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
> -    char thread_id[16];
>      const GdbCmdParseEntry *cmd_parser = NULL;
>
>      trace_gdbstub_io_command(line_buf);
> @@ -2283,15 +2299,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          put_packet(s, "OK");
>          break;
>      case '?':
> -        /* TODO: Make this return the correct value for user-mode.  */
> -        snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
> -                 gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
> -        put_packet(s, buf);
> -        /* Remove all the breakpoints when this query is issued,
> -         * because gdb is doing and initial connect and the state
> -         * should be cleaned up.
> -         */
> -        gdb_breakpoint_remove_all();
> +        {
> +            static const GdbCmdParseEntry target_halted_cmd_desc = {
> +                .handler = handle_target_halt,
> +                .cmd = "?",
> +                .cmd_startswith = 1
> +            };
> +            cmd_parser = &target_halted_cmd_desc;
> +        }
>          break;
>      case 'c':
>          {


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler
  2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (19 preceding siblings ...)
  2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 20/20] gdbstub: Implement qemu physical memory mode Jon Doron
@ 2019-05-31 14:24 ` Alex Bennée
  20 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2019-05-31 14:24 UTC (permalink / raw
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

> This patch series refactors the old gdbstub command packets handler
> with a new infrastructure which should ease extending and adding new
> and missing gdb command packets.

And a:

Tested-by: Alex Bennée <alex.bennee@linaro.org>

for the whole series. I'll see about preparing a PR next week.

>
> version 12 changes:
> - Fixed a bug during rebase of v10 which broke:
>   "Implement breakpoint commands (Z/z pkt) with new infra"
>   which basically broke the remove breakpoint command
> - Changed gdb_handle_packet call to process_string_cmds with a wrapper
>   which handles errors appropriately.
> - Patches which require review:
>   gdbstub: Implement deatch (D pkt) with new infra
>   gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
>   gdbstub: Implement set register (P pkt) with new infra
>   gdbstub: Implement get register (p pkt) with new infra
>   gdbstub: Implement file io (F pkt) with new infra
>   gdbstub: Implement v commands with new infra
>   gdbstub: Implement generic set/query (Q/q pkt) with new infra
>   gdbstub: Implement target halted (? pkt) with new infra
>   gdbstub: Implement qemu physical memory mode
>
> version 11 changes:
> - Add reviewed by tag
> - Requires review:
>   gdbstub: Implement deatch (D pkt) with new infra
>   gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
>   gdbstub: Implement set register (P pkt) with new infra
>   gdbstub: Implement get register (p pkt) with new infra
>   gdbstub: Implement file io (F pkt) with new infra
>   gdbstub: Implement v commands with new infra
>   gdbstub: Implement generic set/query (Q/q pkt) with new infra
>   gdbstub: Implement target halted (? pkt) with new infra
>   gdbstub: Implement qemu physical memory mode
> - Already reviewed:
>   gdbstub: Add infrastructure to parse cmd packets
>   gdbstub: Implement thread_alive (T pkt) with new infra
>   gdbstub: Implement continue (c pkt) with new infra
>   gdbstub: Implement continue with signal (C pkt) with new infra
>   gdbstub: Implement set_thread (H pkt) with new infra
>   gdbstub: Implement write memory (M pkt) with new infra
>   gdbstub: Implement read memory (m pkt) with new infra
>   gdbstub: Implement write all registers (G pkt) with new infra
>   gdbstub: Implement read all registers (g pkt) with new infra
>   gdbstub: Implement step (s pkt) with new infra
>   gdbstub: Clear unused variables in gdb_handle_packet
>
> version 10 changes:
> - Remove kvm added API as this is not really required and can be
>   accomplished by defining a coprocessor callback with a system
>   specific xml (see: 200bf5b7ffea635079cc05fdfb363372b9544ce7)
> - Remove the new QEMU extended commands to read KVM MSRs
> - Various fixes from Code Review by Alex Bennee
> - Change the QEMU specific command to read physical memory to non-User QEMU
> - Per patch changes:
>   gdbstub: Add infrastructure to parse cmd packets
>     * remove the union for the flags in GdbCmdParseEntry
>   gdbstub: Implement deatch (D pkt) with new infra
>     * Changed default handling for error flow / command not found
>   gdbstub: Implement continue with signal (C pkt) with new infra
>     * Added comment we dont support C sig;[addr] commands
>   gdbstub: Implement set_thread (H pkt) with new infra
>     * Change num_params check to be equal and not less than
>   gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
>     * Merged z/Z commands into a single patch
>   gdbstub: Implement read memory (m pkt) with new infra
>     * Change num_params check to be equal and not less than
>   gdbstub: Implement file io (F pkt) with new infra
>     * Changed to have a single command parser
>   gdbstub: Implement generic set/query (Q/q pkt) with new infra
>     * Merged q/Q and qemu.Supported patches into a single patch
>   gdbstub: Implement target halted (? pkt) with new infra
>     * Removed TODO comment and added a note about it in the commit msg
>   gdbstub: Implement qemu physical memory mode
>     * Added CONFIG_USER_ONLY where required
>
> version 9 changes:
> - checkpatch fixes
>
> version 8 changes:
> - Add new command to display the Supported qemu generic query/sets
> - kvm: Add API to read/write a MSR
> - Add new commands specific for qemu:
>   * Command to swap the memory GDB sees to be the physical memory
>   * Commands to read and write a MSR
>
> version 7 changes:
> - Fixed few checkpatch complaints
> - Feedback from Alex Bennee
>
> version 4-6 changes:
> - mostly feedback from Richard Henderson
>
> version 3 changes
> - Split the single patch to many individual patches for easier reviewing
>
> version 2 changes
> - Code convention fixes
>
> Jon Doron (20):
>   gdbstub: Add infrastructure to parse cmd packets
>   gdbstub: Implement deatch (D pkt) with new infra
>   gdbstub: Implement thread_alive (T pkt) with new infra
>   gdbstub: Implement continue (c pkt) with new infra
>   gdbstub: Implement continue with signal (C pkt) with new infra
>   gdbstub: Implement set_thread (H pkt) with new infra
>   gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
>   gdbstub: Implement set register (P pkt) with new infra
>   gdbstub: Implement get register (p pkt) with new infra
>   gdbstub: Implement write memory (M pkt) with new infra
>   gdbstub: Implement read memory (m pkt) with new infra
>   gdbstub: Implement write all registers (G pkt) with new infra
>   gdbstub: Implement read all registers (g pkt) with new infra
>   gdbstub: Implement file io (F pkt) with new infra
>   gdbstub: Implement step (s pkt) with new infra
>   gdbstub: Implement v commands with new infra
>   gdbstub: Implement generic set/query (Q/q pkt) with new infra
>   gdbstub: Implement target halted (? pkt) with new infra
>   gdbstub: Clear unused variables in gdb_handle_packet
>   gdbstub: Implement qemu physical memory mode
>
>  gdbstub.c | 1761 ++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 1270 insertions(+), 491 deletions(-)


--
Alex Bennée


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2019-05-31 14:25 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-29  6:41 [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
2019-05-31 13:33   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 03/20] gdbstub: Implement thread_alive (T " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 04/20] gdbstub: Implement continue (c " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 05/20] gdbstub: Implement continue with signal (C " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 06/20] gdbstub: Implement set_thread (H " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
2019-05-31 13:34   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 08/20] gdbstub: Implement set register (P " Jon Doron
2019-05-31 13:37   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 09/20] gdbstub: Implement get register (p " Jon Doron
2019-05-31 13:38   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 10/20] gdbstub: Implement write memory (M " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 11/20] gdbstub: Implement read memory (m " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 12/20] gdbstub: Implement write all registers (G " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 13/20] gdbstub: Implement read all registers (g " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 14/20] gdbstub: Implement file io (F " Jon Doron
2019-05-31 13:39   ` Alex Bennée
2019-05-31 13:41   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 15/20] gdbstub: Implement step (s " Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 16/20] gdbstub: Implement v commands " Jon Doron
2019-05-31 13:42   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
2019-05-31 14:10   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 18/20] gdbstub: Implement target halted (? " Jon Doron
2019-05-31 14:11   ` Alex Bennée
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 19/20] gdbstub: Clear unused variables in gdb_handle_packet Jon Doron
2019-05-29  6:41 ` [Qemu-devel] [PATCH v12 20/20] gdbstub: Implement qemu physical memory mode Jon Doron
2019-05-31 14:24 ` [Qemu-devel] [PATCH v12 00/20] gdbstub: Refactor command packets handler Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.