All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler
@ 2019-05-24 16:00 Jon Doron
  2019-05-24 16:00 ` [Qemu-devel] [PATCH v11 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
                   ` (19 more replies)
  0 siblings, 20 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:00 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 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 | 1753 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 1262 insertions(+), 491 deletions(-)

-- 
2.21.0



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

* [Qemu-devel] [PATCH v11 01/20] gdbstub: Add infrastructure to parse cmd packets
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
@ 2019-05-24 16:00 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:00 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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
  2019-05-24 16:00 ` [Qemu-devel] [PATCH v11 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-27  8:37   ` Alex Bennée
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 03/20] gdbstub: Implement thread_alive (T " Jon Doron
                   ` (17 subsequent siblings)
  19 siblings, 1 reply; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee, Jon Doron

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

diff --git a/gdbstub.c b/gdbstub.c
index e6d895177b..307366b250 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,41 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
     return -1;
 }
 
+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 +1507,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 +1608,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 +1989,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, buf);
         break;
     }
+
+    if (cmd_parser) {
+        /* helper will respond */
+        process_string_cmd(s, NULL, line_buf, cmd_parser, 1);
+    } else {
+        /* unknown command, empty respone */
+        put_packet(s, "");
+    }
+
     return RS_IDLE;
 }
 
-- 
2.21.0



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

* [Qemu-devel] [PATCH v11 03/20] gdbstub: Implement thread_alive (T pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
  2019-05-24 16:00 ` [Qemu-devel] [PATCH v11 01/20] gdbstub: Add infrastructure to parse cmd packets Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 04/20] gdbstub: Implement continue (c " Jon Doron
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 307366b250..b4c4bd4b08 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1493,6 +1493,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;
@@ -1793,17 +1817,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 04/20] gdbstub: Implement continue (c pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (2 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 03/20] gdbstub: Implement thread_alive (T " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 05/20] gdbstub: Implement continue with signal (C " Jon Doron
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 b4c4bd4b08..698d6f558a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1517,6 +1517,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;
@@ -1553,13 +1563,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 05/20] gdbstub: Implement continue with signal (C pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (3 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 04/20] gdbstub: Implement continue (c " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 06/20] gdbstub: Implement set_thread (H " Jon Doron
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 698d6f558a..f74a30da03 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1527,6 +1527,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;
@@ -1574,11 +1593,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 06/20] gdbstub: Implement set_thread (H pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (4 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 05/20] gdbstub: Implement continue with signal (C " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 f74a30da03..129a47230f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1546,6 +1546,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;
@@ -1559,7 +1604,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);
@@ -1822,35 +1866,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] 25+ messages in thread

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

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

diff --git a/gdbstub.c b/gdbstub.c
index 129a47230f..c59a6765cd 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;
@@ -1591,6 +1591,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;
@@ -1846,24 +1892,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 08/20] gdbstub: Implement set register (P pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (6 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 09/20] gdbstub: Implement get register (p " Jon Doron
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 c59a6765cd..1c210d671a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1637,6 +1637,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;
@@ -1881,15 +1902,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 09/20] gdbstub: Implement get register (p pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (7 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 08/20] gdbstub: Implement set register (P " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 10/20] gdbstub: Implement write memory (M " Jon Doron
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 1c210d671a..3284b3e01d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1658,6 +1658,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;
@@ -1887,18 +1917,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 10/20] gdbstub: Implement write memory (M pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (8 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 09/20] gdbstub: Implement get register (p " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 11/20] gdbstub: Implement read memory (m " Jon Doron
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 3284b3e01d..c798d93e22 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1688,6 +1688,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;
@@ -1896,24 +1921,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 11/20] gdbstub: Implement read memory (m pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (9 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 10/20] gdbstub: Implement write memory (M " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 12/20] gdbstub: Implement write all registers (G " Jon Doron
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 c798d93e22..c957b0d8a7 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1713,6 +1713,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;
@@ -1902,22 +1926,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 12/20] gdbstub: Implement write all registers (G pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (10 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 11/20] gdbstub: Implement read memory (m " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 13/20] gdbstub: Implement read all registers (g " Jon Doron
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 c957b0d8a7..1afad31b49 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1737,6 +1737,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;
@@ -1748,7 +1771,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;
 
@@ -1914,16 +1936,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 13/20] gdbstub: Implement read all registers (g pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (11 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 12/20] gdbstub: Implement write all registers (G " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 14/20] gdbstub: Implement file io (F " Jon Doron
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 1afad31b49..781f5882ac 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1760,6 +1760,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;
@@ -1767,7 +1782,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];
@@ -1926,14 +1941,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 14/20] gdbstub: Implement file io (F pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (12 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 13/20] gdbstub: Implement read all registers (g " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 15/20] gdbstub: Implement step (s " Jon Doron
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 781f5882ac..b35acc679c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1775,6 +1775,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;
@@ -1916,28 +1935,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 15/20] gdbstub: Implement step (s pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (13 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 14/20] gdbstub: Implement file io (F " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 16/20] gdbstub: Implement v commands " Jon Doron
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 b35acc679c..a8b81121c5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1794,6 +1794,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;
@@ -1926,13 +1936,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 16/20] gdbstub: Implement v commands with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (14 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 15/20] gdbstub: Implement step (s " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 a8b81121c5..3773b23581 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1804,6 +1804,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;
@@ -1811,7 +1911,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];
@@ -1860,66 +1960,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 17/20] gdbstub: Implement generic set/query (Q/q pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (15 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 16/20] gdbstub: Implement v commands " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 18/20] gdbstub: Implement target halted (? " Jon Doron
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 3773b23581..57a05fd6a4 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
@@ -1904,18 +1896,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);
@@ -2117,183 +2459,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 18/20] gdbstub: Implement target halted (? pkt) with new infra
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (16 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 17/20] gdbstub: Implement generic set/query (Q/q pkt) " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 19/20] gdbstub: Clear unused variables in gdb_handle_packet Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 20/20] gdbstub: Implement qemu physical memory mode Jon Doron
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 57a05fd6a4..097b7d1231 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2251,13 +2251,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);
@@ -2269,15 +2285,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 19/20] gdbstub: Clear unused variables in gdb_handle_packet
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (17 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 18/20] gdbstub: Implement target halted (? " Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 20/20] gdbstub: Implement qemu physical memory mode Jon Doron
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 097b7d1231..9dd934a079 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2270,17 +2270,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;
@@ -2497,8 +2491,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] 25+ messages in thread

* [Qemu-devel] [PATCH v11 20/20] gdbstub: Implement qemu physical memory mode
  2019-05-24 16:00 [Qemu-devel] [PATCH v11 00/20] gdbstub: Refactor command packets handler Jon Doron
                   ` (18 preceding siblings ...)
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 19/20] gdbstub: Clear unused variables in gdb_handle_packet Jon Doron
@ 2019-05-24 16:01 ` Jon Doron
  19 siblings, 0 replies; 25+ messages in thread
From: Jon Doron @ 2019-05-24 16:01 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 9dd934a079..c9269319d8 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);
     }
@@ -2118,8 +2134,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 */
@@ -2201,6 +2245,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[] = {
@@ -2211,6 +2261,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] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra Jon Doron
@ 2019-05-27  8:37   ` Alex Bennée
  2019-05-27  8:54     ` Alex Bennée
  0 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2019-05-27  8: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 | 93 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 53 insertions(+), 40 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index e6d895177b..307366b250 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,41 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>      return -1;
>  }
>
> +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 +1507,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 +1608,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 +1989,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          put_packet(s, buf);
>          break;
>      }
> +
> +    if (cmd_parser) {
> +        /* helper will respond */
> +        process_string_cmd(s, NULL, line_buf, cmd_parser, 1);
> +    } else {
> +        /* unknown command, empty respone */
> +        put_packet(s, "");
> +    }
> +
>      return RS_IDLE;
>  }


--
Alex Bennée


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

* Re: [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-27  8:37   ` Alex Bennée
@ 2019-05-27  8:54     ` Alex Bennée
  2019-05-27  8:58       ` Alex Bennée
  0 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2019-05-27  8:54 UTC (permalink / raw)
  To: Jon Doron; +Cc: qemu-devel


Alex Bennée <alex.bennee@linaro.org> writes:

> Jon Doron <arilou@gmail.com> writes:
>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Hmm although I bisected to this patch which fails on:

09:49 alex@zen/x86_64  [linux.git/master@origin] >gdb ./builds/arm64/vmlinux -x ~/lsrc/qemu.git/tests/guest-debug/test-gdbstub.py
GNU gdb (GDB) 8.3.50.20190424-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Executed .gdbinit
Reading symbols from ./builds/arm64/vmlinux...
Traceback (most recent call last):
  File "/home/alex/lsrc/linux.git/builds/arm64/vmlinux-gdb.py", line 30, in <module>
    import linux.config
ImportError: No module named config
Connecting to remote
0x0000000040000000 in ?? ()
Checking we can step the first few instructions
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: single step in boot code
Checking HW breakpoint works
Hardware assisted breakpoint 1 at 0xffffff8010778f0c: file /home/alex/lsrc/linux.git/init/main.c, line 1068.
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
0x40000000 == {int (void *)} 0xffffff8010778f0c <kernel_init>
FAIL: hbreak @ kernel_init
Setup catch-all for run_init_process
Breakpoint 2 at 0xffffff8010083dc4: file /home/alex/lsrc/linux.git/init/main.c, line 1009.
Breakpoint 3 at 0xffffff8010083e10: file /home/alex/lsrc/linux.git/init/main.c, line 1020.
Checking Normal breakpoint works
Breakpoint 4 at 0xffffff801077b300: file /home/alex/lsrc/linux.git/kernel/sched/completion.c, line 136.
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
0x40000000 == {void (struct completion *)} 0xffffff801077b300 <wait_for_completion> 0
FAIL: break @ wait_for_completion
Checking watchpoint works
Hardware access (read/write) watchpoint 5: *(enum system_states *)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: awatch for system_state (SYSTEM_BOOTING)
Hardware read watchpoint 6: *(enum system_states *)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: rwatch for system_state (SYSTEM_BOOTING)
Hardware watchpoint 7: *(enum system_states *)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: watch for system_state (SYSTEM_BOOTING)
[Inferior 1 (process 1) killed]


>
>> ---
>>  gdbstub.c | 93 +++++++++++++++++++++++++++++++------------------------
>>  1 file changed, 53 insertions(+), 40 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index e6d895177b..307366b250 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,41 @@ static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>>      return -1;
>>  }
>>
>> +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 +1507,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 +1608,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 +1989,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>          put_packet(s, buf);
>>          break;
>>      }
>> +
>> +    if (cmd_parser) {
>> +        /* helper will respond */
>> +        process_string_cmd(s, NULL, line_buf, cmd_parser, 1);
>> +    } else {
>> +        /* unknown command, empty respone */
>> +        put_packet(s, "");
>> +    }
>> +
>>      return RS_IDLE;
>>  }


--
Alex Bennée


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

* Re: [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra
  2019-05-27  8:54     ` Alex Bennée
@ 2019-05-27  8:58       ` Alex Bennée
  0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2019-05-27  8:58 UTC (permalink / raw)
  To: Jon Doron; +Cc: qemu-devel


Alex Bennée <alex.bennee@linaro.org> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Jon Doron <arilou@gmail.com> writes:
>>
>>> Signed-off-by: Jon Doron <arilou@gmail.com>
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> Hmm although I bisected to this patch which fails on:
>
> 09:49 alex@zen/x86_64  [linux.git/master@origin] >gdb ./builds/arm64/vmlinux -x ~/lsrc/qemu.git/tests/guest-debug/test-gdbstub.py
<snip>
> Connecting to remote
> 0x0000000040000000 in ?? ()
> Checking we can step the first few instructions
> warning: Invalid remote reply:
<snip>
>>>      }
>>> +
>>> +    if (cmd_parser) {
>>> +        /* helper will respond */
>>> +        process_string_cmd(s, NULL, line_buf, cmd_parser, 1);
>>> +    } else {
>>> +        /* unknown command, empty respone */
>>> +        put_packet(s, "");
>>> +    }
>>> +

We can't default to this empty response until we have converted the
table otherwise we get strangeness and double responses.

>>>      return RS_IDLE;
>>>  }


--
Alex Bennée


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

* Re: [Qemu-devel] [PATCH v11 07/20] gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
  2019-05-24 16:01 ` [Qemu-devel] [PATCH v11 07/20] gdbstub: Implement breakpoint commands (Z/z " Jon Doron
@ 2019-05-27  9:52   ` Alex Bennée
  0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2019-05-27  9:52 UTC (permalink / raw)
  To: Jon Doron; +Cc: qemu-devel


Jon Doron <arilou@gmail.com> writes:

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

With the fix to avoid double responses this commit still regresses:

10:46 alex@zen/x86_64  [linux.git/master@origin] >gdb ./builds/arm64/vmlinux -x ~/lsrc/qemu.git/tests/guest-debug/test-gdbstub.py
GNU gdb (GDB) 8.3.50.20190424-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Executed .gdbinit
Reading symbols from ./builds/arm64/vmlinux...
Traceback (most recent call last):
  File "/home/alex/lsrc/linux.git/builds/arm64/vmlinux-gdb.py", line 30, in <module>
    import linux.config
ImportError: No module named config
Connecting to remote
0x0000000040000000 in ?? ()
Checking we can step the first few instructions
0x0000000040000004 in ?? ()
0x0000000040000008 in ?? ()
0x000000004000000c in ?? ()
PASS: single step in boot code
Checking HW breakpoint works
Hardware assisted breakpoint 1 at 0xffffff8010778f0c: file /home/alex/lsrc/linux.git/init/main.c, line 1068.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.

Thread 1 hit Breakpoint 1, kernel_init (unused=0x0) at /home/alex/lsrc/linux.git/init/main.c:1068
warning: Source file is more recent than executable.
1068            } else
0xffffff8010778f0c <kernel_init> == {int (void *)} 0xffffff8010778f0c <kernel_init>
warning: Error removing breakpoint 1
PASS: hbreak @ kernel_init

Something might be broken here due to the BP type?

Setup catch-all for run_init_process
Breakpoint 2 at 0xffffff8010083dc4: file /home/alex/lsrc/linux.git/init/main.c, line 1009.
Breakpoint 3 at 0xffffff8010083e10: file /home/alex/lsrc/linux.git/init/main.c, line 1020.
Checking Normal breakpoint works
Breakpoint 4 at 0xffffff801077b300: file /home/alex/lsrc/linux.git/kernel/sched/completion.c, line 136.

Thread 1 received signal SIGTRAP, Trace/breakpoint trap.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
kernel_init (unused=0x0) at /home/alex/lsrc/linux.git/init/main.c:1068
1068            } else
0xffffff8010778f0c <kernel_init> == {void (struct completion *)} 0xffffff801077b300 <wait_for_completion> 0
warning: Error removing breakpoint 4
FAIL: break @ wait_for_completion
Checking watchpoint works
Hardware access (read/write) watchpoint 5: *(enum system_states *)(&system_state)

Thread 1 received signal SIGTRAP, Trace/breakpoint trap.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
kernel_init (unused=0x0) at /home/alex/lsrc/linux.git/init/main.c:1068
1068            } else
FAIL: awatch for system_state (SYSTEM_BOOTING)
Hardware read watchpoint 6: *(enum system_states *)(&system_state)

Thread 1 received signal SIGTRAP, Trace/breakpoint trap.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
kernel_init (unused=0x0) at /home/alex/lsrc/linux.git/init/main.c:1068
1068            } else
FAIL: rwatch for system_state (SYSTEM_BOOTING)
Hardware watchpoint 7: *(enum system_states *)(&system_state)

Thread 1 received signal SIGTRAP, Trace/breakpoint trap.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
kernel_init (unused=0x0) at /home/alex/lsrc/linux.git/init/main.c:1068
1068            } else
FAIL: watch for system_state (SYSTEM_BOOTING)
[Inferior 1 (process 1) killed]


> ---
>  gdbstub.c | 84 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 129a47230f..c59a6765cd 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;
> @@ -1591,6 +1591,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;
> @@ -1846,24 +1892,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] 25+ messages in thread

end of thread, other threads:[~2019-05-27  9:53 UTC | newest]

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

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