[Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Jan Kiszka]

This addresses the review comments on the previews two patches to add
qAttached support. No longer trivial, so maybe you can pick it up,
Peter.

Jan


CC: Fabien Chouteau <chouteau@adacore.com>
CC: Jan Kiszka <jan.kiszka@siemens.com>

Fabien Chouteau (1):
  Revert "gdbstub: Do not kill target in system emulation mode"

Jan Kiszka (3):
  gdbstub: Fix qOffsets packet detection
  gdbstub: Introduce and is is_query_packet helper
  Add GDB qAttached support

 gdbstub.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/4] gdbstub: Fix qOffsets packet detection

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

qOffsets has no additional optional parameters. So match the complete
string to avoid stumbling over possible future commands with identical
prefix.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index e4a1a79..cd25d1a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1107,7 +1107,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             break;
         }
 #ifdef CONFIG_USER_ONLY
-        else if (strncmp(p, "Offsets", 7) == 0) {
+        else if (strcmp(p, "Offsets") == 0) {
             TaskState *ts = s->c_cpu->opaque;
 
             snprintf(buf, sizeof(buf),
-- 
2.1.4

[Qemu-devel] [PATCH 2/4] gdbstub: Introduce and is is_query_packet helper

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This helper supports parsing of query packets with optional extensions.
The separator can be specified so that we can use it already for both
qqemu.sstep[=] and qSupported[:feature].

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index cd25d1a..91cc785 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -769,6 +769,14 @@ static CPUState *find_cpu(uint32_t thread_id)
     return NULL;
 }
 
+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);
+}
+
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
     CPUState *cpu;
@@ -1062,7 +1070,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
                      SSTEP_NOTIMER);
             put_packet(s, buf);
             break;
-        } else if (strncmp(p,"qemu.sstep",10) == 0) {
+        } else if (is_query_packet(p, "qemu.sstep", '=')) {
             /* Display or change the sstep_flags */
             p += 10;
             if (*p != '=') {
@@ -1135,7 +1143,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             break;
         }
 #endif /* !CONFIG_USER_ONLY */
-        if (strncmp(p, "Supported", 9) == 0) {
+        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) {
-- 
2.1.4

[Qemu-devel] [PATCH 3/4] Add GDB qAttached support

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

With this patch QEMU handles qAttached request from gdb. When QEMU
replies 1, GDB sends a "detach" command at the end of a debugging
session otherwise GDB sends "kill".

The default value for qAttached is 1 on system emulation and 0 on user
emulation.

Based on original version by Fabien Chouteau.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 91cc785..efa29b2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -41,6 +41,12 @@
 #include "qemu/sockets.h"
 #include "sysemu/kvm.h"
 
+#ifdef CONFIG_USER_ONLY
+#define GDB_ATTACHED "0"
+#else
+#define GDB_ATTACHED "1"
+#endif
+
 static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
                                          uint8_t *buf, int len, bool is_write)
 {
@@ -1195,6 +1201,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet_binary(s, buf, len + 1);
             break;
         }
+        if (is_query_packet(p, "Attached", ':')) {
+            put_packet(s, GDB_ATTACHED);
+            break;
+        }
         /* Unrecognised 'q' command.  */
         goto unknown_command;
 
-- 
2.1.4

[Qemu-devel] [PATCH 4/4] Revert "gdbstub: Do not kill target in system emulation mode"

[Jan Kiszka]

From: Fabien Chouteau <chouteau@adacore.com>

The requirements described in this patch are implemented by "Add GDB
qAttached support".

This reverts commit 00e94dbc7fd0110b0555d59592b004333adfb4b8.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 gdbstub.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index efa29b2..0f2d371 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -888,11 +888,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             goto unknown_command;
         }
     case 'k':
-#ifdef CONFIG_USER_ONLY
         /* Kill the target */
         fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
         exit(0);
-#endif
     case 'D':
         /* Detach packet */
         gdb_breakpoint_remove_all();
-- 
2.1.4

Re: [Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Pedro Alves]

On 02/07/2015 08:38 AM, Jan Kiszka wrote:
> This addresses the review comments on the previews two patches to add
> qAttached support. No longer trivial, so maybe you can pick it up,
> Peter.

Excellent, thanks for doing this Jan.  Looks good to me.

Thanks,
Pedro Alves

Re: [Qemu-devel] [PATCH 1/4] gdbstub: Fix qOffsets packet detection

[Peter Maydell]

On 7 February 2015 at 08:38, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> qOffsets has no additional optional parameters. So match the complete
> string to avoid stumbling over possible future commands with identical
> prefix.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  gdbstub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index e4a1a79..cd25d1a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1107,7 +1107,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>              break;
>          }
>  #ifdef CONFIG_USER_ONLY
> -        else if (strncmp(p, "Offsets", 7) == 0) {
> +        else if (strcmp(p, "Offsets") == 0) {
>              TaskState *ts = s->c_cpu->opaque;
>
>              snprintf(buf, sizeof(buf),

We could write this as
 if (is_query_packet(p, "Offsets", 0))
but straight strcmp() works fine too.

-- PMM

Re: [Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Peter Maydell]

On 7 February 2015 at 08:38, Jan Kiszka <jan.kiszka@web.de> wrote:
> This addresses the review comments on the previews two patches to add
> qAttached support. No longer trivial, so maybe you can pick it up,
> Peter.
>
> Jan
>
>
> CC: Fabien Chouteau <chouteau@adacore.com>
> CC: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

If you can find a tree for this to go through it might be
faster than trying to get me to apply it directly...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On 2015-02-12 05:19, Peter Maydell wrote:
> On 7 February 2015 at 08:38, Jan Kiszka <jan.kiszka@web.de> wrote:
>> This addresses the review comments on the previews two patches to add
>> qAttached support. No longer trivial, so maybe you can pick it up,
>> Peter.
>>
>> Jan
>>
>>
>> CC: Fabien Chouteau <chouteau@adacore.com>
>> CC: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> If you can find a tree for this to go through it might be
> faster than trying to get me to apply it directly...

If you have a suggestion for such a tree, I'll try. Otherwise: ping
(still applies cleanly).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 0/4] gdbstub: improve query packet parsing, add qAttached support

[Peter Maydell]

On 24 May 2015 at 16:30, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2015-02-12 05:19, Peter Maydell wrote:
>> On 7 February 2015 at 08:38, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> This addresses the review comments on the previews two patches to add
>>> qAttached support. No longer trivial, so maybe you can pick it up,
>>> Peter.
>>>
>>> Jan
>>>
>>>
>>> CC: Fabien Chouteau <chouteau@adacore.com>
>>> CC: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> If you can find a tree for this to go through it might be
>> faster than trying to get me to apply it directly...
>
> If you have a suggestion for such a tree, I'll try. Otherwise: ping
> (still applies cleanly).

Thanks, applied to master.

-- PMM