qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).