qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V3 0/3] Add GDB qAttached support
@ 2013-03-12 17:31 Fabien Chouteau
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 1/3] " Fabien Chouteau
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-12 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: jan.kiszka

v3:
  - split 'Add GDB qAttached support' and 'New option -gdb-opts'

Fabien Chouteau (3):
  Add GDB qAttached support
  Revert "gdbstub: Do not kill target in system emulation mode"
  New option -gdb-opts

 gdbstub.c              |   40 +++++++++++++++++++++++++++++++++++++---
 include/exec/gdbstub.h |    2 ++
 qemu-options.hx        |   17 +++++++++++++++++
 vl.c                   |    3 +++
 4 files changed, 59 insertions(+), 3 deletions(-)

-- 
1.7.9.5

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

* [Qemu-devel] [PATCH V3 1/3] Add GDB qAttached support
  2013-03-12 17:31 [Qemu-devel] [PATCH V3 0/3] Add GDB qAttached support Fabien Chouteau
@ 2013-03-12 17:31 ` Fabien Chouteau
  2013-03-14 19:51   ` [Qemu-devel] [PATCH v4] " Jan Kiszka
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode" Fabien Chouteau
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts Fabien Chouteau
  2 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-12 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: jan.kiszka

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.

This patch implements the requirement described in Jan Kiszka's patch:
"gdbstub: Do not kill target in system emulation mode". The patch can
therefore be reverted.

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

diff --git a/gdbstub.c b/gdbstub.c
index e414ad9..70d54ce 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -42,6 +42,12 @@
 #include "sysemu/kvm.h"
 #include "qemu/bitops.h"
 
+#ifdef CONFIG_USER_ONLY
+static bool gdb_attached; /* false */
+#else
+static bool gdb_attached = true;
+#endif
+
 #ifndef TARGET_CPU_MEMORY_RW_DEBUG
 static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr,
                                          uint8_t *buf, int len, int is_write)
@@ -2491,6 +2497,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             break;
         }
 #endif
+        if (strncmp(p, "Attached", 8) == 0) {
+            put_packet(s, gdb_attached ? "1" : "0");
+            break;
+        }
         /* Unrecognised 'q' command.  */
         goto unknown_command;
 
-- 
1.7.9.5

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

* [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode"
  2013-03-12 17:31 [Qemu-devel] [PATCH V3 0/3] Add GDB qAttached support Fabien Chouteau
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 1/3] " Fabien Chouteau
@ 2013-03-12 17:31 ` Fabien Chouteau
  2013-03-14 19:52   ` Jan Kiszka
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts Fabien Chouteau
  2 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-12 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: jan.kiszka

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 70d54ce..0e94311 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2185,11 +2185,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();
-- 
1.7.9.5

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

* [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-12 17:31 [Qemu-devel] [PATCH V3 0/3] Add GDB qAttached support Fabien Chouteau
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 1/3] " Fabien Chouteau
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode" Fabien Chouteau
@ 2013-03-12 17:31 ` Fabien Chouteau
  2013-03-14  8:44   ` Markus Armbruster
  2 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-12 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: jan.kiszka

We introduce a new command line option. It's a generic option to
customize the gdb server:

-gdb-opts [attached=on|off]

The only parameter for now is "attached".

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 gdbstub.c              |   28 +++++++++++++++++++++++++++-
 include/exec/gdbstub.h |    2 ++
 qemu-options.hx        |   17 +++++++++++++++++
 vl.c                   |    3 +++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index 0e94311..ee57124 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -32,7 +32,6 @@
 #include "monitor/monitor.h"
 #include "char/char.h"
 #include "sysemu/sysemu.h"
-#include "exec/gdbstub.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
@@ -41,6 +40,7 @@
 #include "qemu/sockets.h"
 #include "sysemu/kvm.h"
 #include "qemu/bitops.h"
+#include "exec/gdbstub.h"
 
 #ifdef CONFIG_USER_ONLY
 static bool gdb_attached; /* false */
@@ -3063,3 +3063,29 @@ int gdbserver_start(const char *device)
     return 0;
 }
 #endif
+
+static QemuOptsList qemu_gdb_opts = {
+    .name = "gdb",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_gdb_opts.head),
+    .desc = {
+        {
+            .name = "attached",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
+void gdb_set_opts(const char *opts_str)
+{
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(&qemu_gdb_opts, opts_str, 0);
+    if (!opts) {
+        exit(1);
+    }
+
+    gdb_attached = qemu_opt_get_bool(opts, "attached", gdb_attached);
+
+    qemu_opts_del(opts);
+}
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index ba20afa..86fc18b 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -50,4 +50,6 @@ int gdbserver_start(const char *port);
 /* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */
 extern const char *const xml_builtin[][2];
 
+void gdb_set_opts(const char *opts_str);
+
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index cd76f2a..49a480f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2505,6 +2505,23 @@ within gdb and establish the connection via a pipe:
 @end example
 ETEXI
 
+DEF("gdb-opts", HAS_ARG, QEMU_OPTION_gdb_opts, \
+    "-gdb-opts attached=on|off\n"\
+    "                options for the gdb remote server\n", QEMU_ARCH_ALL)
+STEXI
+@item -gdb-opts @var{dev}
+@findex -gdb-opts
+Set options for the gdb remote server:
+@table @option
+@item attached=on|off:
+
+When 'on' (default), gdb sends a 'detach' command at the end of debugging
+session, otherwise gdb sends a 'kill' command.
+
+@end table
+
+ETEXI
+
 DEF("s", 0, QEMU_OPTION_s, \
     "-s              shorthand for -gdb tcp::" DEFAULT_GDBSTUB_PORT "\n",
     QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index 154f7ba..cce96b6 100644
--- a/vl.c
+++ b/vl.c
@@ -3251,6 +3251,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_gdb:
                 add_device_config(DEV_GDB, optarg);
                 break;
+            case QEMU_OPTION_gdb_opts:
+                gdb_set_opts(optarg);
+                break;
             case QEMU_OPTION_L:
                 data_dir = optarg;
                 break;
-- 
1.7.9.5

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

* Re: [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts Fabien Chouteau
@ 2013-03-14  8:44   ` Markus Armbruster
  2013-03-14 10:59     ` Fabien Chouteau
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2013-03-14  8:44 UTC (permalink / raw)
  To: Fabien Chouteau; +Cc: jan.kiszka, qemu-devel

Fabien Chouteau <chouteau@adacore.com> writes:

> We introduce a new command line option. It's a generic option to
> customize the gdb server:
>
> -gdb-opts [attached=on|off]
>
> The only parameter for now is "attached".
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>

--gdb-opts complements existing --gdb.  You need to use both for full
control.

I figure you do this because you can't extend --gdb, as its argument is
in legacy character device syntax, not QemuOpts.

We had similar cases before, and solved them differently: create a more
general option, then make the old one sugar for the new one.

For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
is in monitor_parse().

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

* Re: [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-14  8:44   ` Markus Armbruster
@ 2013-03-14 10:59     ` Fabien Chouteau
  2013-03-14 12:34       ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-14 10:59 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: jan.kiszka, qemu-devel

On 03/14/2013 09:44 AM, Markus Armbruster wrote:
> Fabien Chouteau <chouteau@adacore.com> writes:
>
>> We introduce a new command line option. It's a generic option to
>> customize the gdb server:
>>
>> -gdb-opts [attached=on|off]
>>
>> The only parameter for now is "attached".
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>
> --gdb-opts complements existing --gdb.  You need to use both for full
> control.
>
> I figure you do this because you can't extend --gdb, as its argument is
> in legacy character device syntax, not QemuOpts.
>

That's right, maybe we can do some string manipulations to handle this case.

-gdb tcp::1234,attached=off

find ',attached={on|off}' and remove it from the string.

> We had similar cases before, and solved them differently: create a more
> general option, then make the old one sugar for the new one.
>
> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
> is in monitor_parse().
>

Something like:

-chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay -gdb-remote chardev=gdb1,attached=off

You still need two options for full control.

Regards,

-- 
Fabien Chouteau

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

* Re: [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-14 10:59     ` Fabien Chouteau
@ 2013-03-14 12:34       ` Markus Armbruster
  2013-03-14 16:33         ` Fabien Chouteau
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2013-03-14 12:34 UTC (permalink / raw)
  To: Fabien Chouteau; +Cc: jan.kiszka, qemu-devel

Fabien Chouteau <chouteau@adacore.com> writes:

> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>> Fabien Chouteau <chouteau@adacore.com> writes:
>>
>>> We introduce a new command line option. It's a generic option to
>>> customize the gdb server:
>>>
>>> -gdb-opts [attached=on|off]
>>>
>>> The only parameter for now is "attached".
>>>
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>
>> --gdb-opts complements existing --gdb.  You need to use both for full
>> control.
>>
>> I figure you do this because you can't extend --gdb, as its argument is
>> in legacy character device syntax, not QemuOpts.
>>
>
> That's right, maybe we can do some string manipulations to handle this case.
>
> -gdb tcp::1234,attached=off
>
> find ',attached={on|off}' and remove it from the string.

That way is madness :)

>> We had similar cases before, and solved them differently: create a more
>> general option, then make the old one sugar for the new one.
>>
>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>> is in monitor_parse().
>>
>
> Something like:
>
> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
> -gdb-remote chardev=gdb1,attached=off
>
> You still need two options for full control.

Yes, but following precedence is good.  Our command line is inconsistent
enough as it is.  Just my two cents.

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

* Re: [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-14 12:34       ` Markus Armbruster
@ 2013-03-14 16:33         ` Fabien Chouteau
  2013-03-19 14:31           ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-14 16:33 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: jan.kiszka, qemu-devel

On 03/14/2013 01:34 PM, Markus Armbruster wrote:
> Fabien Chouteau <chouteau@adacore.com> writes:
> 
>> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>>> Fabien Chouteau <chouteau@adacore.com> writes:
>>>
>>>> We introduce a new command line option. It's a generic option to
>>>> customize the gdb server:
>>>>
>>>> -gdb-opts [attached=on|off]
>>>>
>>>> The only parameter for now is "attached".
>>>>
>>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>>
>>> --gdb-opts complements existing --gdb.  You need to use both for full
>>> control.
>>>
>>> I figure you do this because you can't extend --gdb, as its argument is
>>> in legacy character device syntax, not QemuOpts.
>>>
>>
>> That's right, maybe we can do some string manipulations to handle this case.
>>
>> -gdb tcp::1234,attached=off
>>
>> find ',attached={on|off}' and remove it from the string.
>
> That way is madness :)

Come on, you've seen worse ;)

>>> We had similar cases before, and solved them differently: create a more
>>> general option, then make the old one sugar for the new one.
>>>
>>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>>> is in monitor_parse().
>>>
>>
>> Something like:
>>
>> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
>> -gdb-remote chardev=gdb1,attached=off
>>
>> You still need two options for full control.
> 
> Yes, but following precedence is good.  Our command line is inconsistent
> enough as it is.  Just my two cents.
> 

Fair enough, lets forget about the option this is too much work. I'll
just tweak the sources on our branch.

Regards,

-- 
Fabien Chouteau

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

* [Qemu-devel] [PATCH v4] Add GDB qAttached support
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 1/3] " Fabien Chouteau
@ 2013-03-14 19:51   ` Jan Kiszka
  2013-03-14 21:07     ` Jesse Larrew
  2013-05-01 14:18     ` Jan Kiszka
  0 siblings, 2 replies; 15+ messages in thread
From: Jan Kiszka @ 2013-03-14 19:51 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel@nongnu.org, Fabien Chouteau

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>
---

As Fabien dropped his attempt to make this configurable, let's
preserve the value of exposing this feature to gdb statically.

 gdbstub.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e414ad9..9daee86 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -42,6 +42,12 @@
 #include "sysemu/kvm.h"
 #include "qemu/bitops.h"
 
+#ifdef CONFIG_USER_ONLY
+#define GDB_ATTACHED "0"
+#else
+#define GDB_ATTACHED "1"
+#endif
+
 #ifndef TARGET_CPU_MEMORY_RW_DEBUG
 static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr,
                                          uint8_t *buf, int len, int is_write)
@@ -2491,6 +2497,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             break;
         }
 #endif
+        if (strncmp(p, "Attached", 8) == 0) {
+            put_packet(s, GDB_ATTACHED);
+            break;
+        }
         /* Unrecognised 'q' command.  */
         goto unknown_command;
 
-- 
1.7.3.4

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

* Re: [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode"
  2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode" Fabien Chouteau
@ 2013-03-14 19:52   ` Jan Kiszka
  0 siblings, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2013-03-14 19:52 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel@nongnu.org, Fabien Chouteau

On 2013-03-12 18:31, Fabien Chouteau wrote:
> 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 70d54ce..0e94311 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2185,11 +2185,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();
> 

Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>

Provided that v4 of "Add GDB qAttached support" is applied prior to this.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

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

* Re: [Qemu-devel] [PATCH v4] Add GDB qAttached support
  2013-03-14 19:51   ` [Qemu-devel] [PATCH v4] " Jan Kiszka
@ 2013-03-14 21:07     ` Jesse Larrew
  2013-03-15 12:02       ` Fabien Chouteau
  2013-05-01 14:18     ` Jan Kiszka
  1 sibling, 1 reply; 15+ messages in thread
From: Jesse Larrew @ 2013-03-14 21:07 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel@nongnu.org, Fabien Chouteau

On 03/14/2013 02:51 PM, Jan Kiszka wrote:
> 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>
> ---
> 
> As Fabien dropped his attempt to make this configurable, let's
> preserve the value of exposing this feature to gdb statically.
> 
>  gdbstub.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index e414ad9..9daee86 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -42,6 +42,12 @@
>  #include "sysemu/kvm.h"
>  #include "qemu/bitops.h"
> 
> +#ifdef CONFIG_USER_ONLY
> +#define GDB_ATTACHED "0"
> +#else
> +#define GDB_ATTACHED "1"
> +#endif
> +

Yes, I like the #define better.

>  #ifndef TARGET_CPU_MEMORY_RW_DEBUG
>  static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr,
>                                           uint8_t *buf, int len, int is_write)
> @@ -2491,6 +2497,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>              break;
>          }
>  #endif
> +        if (strncmp(p, "Attached", 8) == 0) {
> +            put_packet(s, GDB_ATTACHED);
> +            break;
> +        }
>          /* Unrecognised 'q' command.  */
>          goto unknown_command;
> 

Reviewed-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

Jesse Larrew
Software Engineer, KVM Team
IBM Linux Technology Center
Phone: (512) 973-2052 (T/L: 363-2052)
jlarrew@linux.vnet.ibm.com

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

* Re: [Qemu-devel] [PATCH v4] Add GDB qAttached support
  2013-03-14 21:07     ` Jesse Larrew
@ 2013-03-15 12:02       ` Fabien Chouteau
  2013-03-19 10:37         ` Fabien Chouteau
  0 siblings, 1 reply; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-15 12:02 UTC (permalink / raw)
  To: Jesse Larrew; +Cc: Jan Kiszka, Anthony Liguori, qemu-devel@nongnu.org

On 03/14/2013 10:07 PM, Jesse Larrew wrote:
> On 03/14/2013 02:51 PM, Jan Kiszka wrote:
>> 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>
>> ---
>>
>> As Fabien dropped his attempt to make this configurable, let's
>> preserve the value of exposing this feature to gdb statically.
>>
>>  gdbstub.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index e414ad9..9daee86 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -42,6 +42,12 @@
>>  #include "sysemu/kvm.h"
>>  #include "qemu/bitops.h"
>>
>> +#ifdef CONFIG_USER_ONLY
>> +#define GDB_ATTACHED "0"
>> +#else
>> +#define GDB_ATTACHED "1"
>> +#endif
>> +
> 
> Yes, I like the #define better.

Right, since we drop reconfigurability, this is more appropriate.


-- 
Fabien Chouteau

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

* Re: [Qemu-devel] [PATCH v4] Add GDB qAttached support
  2013-03-15 12:02       ` Fabien Chouteau
@ 2013-03-19 10:37         ` Fabien Chouteau
  0 siblings, 0 replies; 15+ messages in thread
From: Fabien Chouteau @ 2013-03-19 10:37 UTC (permalink / raw)
  To: Jesse Larrew; +Cc: Jan Kiszka, Anthony Liguori, qemu-devel@nongnu.org

On 03/15/2013 01:02 PM, Fabien Chouteau wrote:
> On 03/14/2013 10:07 PM, Jesse Larrew wrote:
>> On 03/14/2013 02:51 PM, Jan Kiszka wrote:
>>> 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>
>>> ---
>>>
>>> As Fabien dropped his attempt to make this configurable, let's
>>> preserve the value of exposing this feature to gdb statically.
>>>
>>>  gdbstub.c |   10 ++++++++++
>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/gdbstub.c b/gdbstub.c
>>> index e414ad9..9daee86 100644
>>> --- a/gdbstub.c
>>> +++ b/gdbstub.c
>>> @@ -42,6 +42,12 @@
>>>  #include "sysemu/kvm.h"
>>>  #include "qemu/bitops.h"
>>>
>>> +#ifdef CONFIG_USER_ONLY
>>> +#define GDB_ATTACHED "0"
>>> +#else
>>> +#define GDB_ATTACHED "1"
>>> +#endif
>>> +
>>
>> Yes, I like the #define better.
> 
> Right, since we drop reconfigurability, this is more appropriate.
> 
> 

Looks like we have a consensus. Anthony can you please apply this patch and the revert (patch 2/3)?

Thanks in advance,

-- 
Fabien Chouteau

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

* Re: [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts
  2013-03-14 16:33         ` Fabien Chouteau
@ 2013-03-19 14:31           ` Markus Armbruster
  0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2013-03-19 14:31 UTC (permalink / raw)
  To: Fabien Chouteau; +Cc: jan.kiszka, qemu-devel

Fabien Chouteau <chouteau@adacore.com> writes:

> On 03/14/2013 01:34 PM, Markus Armbruster wrote:
>> Fabien Chouteau <chouteau@adacore.com> writes:
>> 
>>> On 03/14/2013 09:44 AM, Markus Armbruster wrote:
>>>> Fabien Chouteau <chouteau@adacore.com> writes:
>>>>
>>>>> We introduce a new command line option. It's a generic option to
>>>>> customize the gdb server:
>>>>>
>>>>> -gdb-opts [attached=on|off]
>>>>>
>>>>> The only parameter for now is "attached".
>>>>>
>>>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>>>
>>>> --gdb-opts complements existing --gdb.  You need to use both for full
>>>> control.
>>>>
>>>> I figure you do this because you can't extend --gdb, as its argument is
>>>> in legacy character device syntax, not QemuOpts.
>>>>
>>>
>>> That's right, maybe we can do some string manipulations to handle this case.
>>>
>>> -gdb tcp::1234,attached=off
>>>
>>> find ',attached={on|off}' and remove it from the string.
>>
>> That way is madness :)
>
> Come on, you've seen worse ;)

Seen?  Perpetrated!  Stared madness in the eye, decided not to come back
for more ;)

>>>> We had similar cases before, and solved them differently: create a more
>>>> general option, then make the old one sugar for the new one.
>>>>
>>>> For instance, --monitor and --qmp are sugar for --mon.  Desugaring code
>>>> is in monitor_parse().
>>>>
>>>
>>> Something like:
>>>
>>> -chardev socket,id=gdb1,host=localhost,port=1234,server,nowait,nodelay
>>> -gdb-remote chardev=gdb1,attached=off
>>>
>>> You still need two options for full control.
>> 
>> Yes, but following precedence is good.  Our command line is inconsistent
>> enough as it is.  Just my two cents.
>> 
>
> Fair enough, lets forget about the option this is too much work. I'll
> just tweak the sources on our branch.

Pity.

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

* Re: [Qemu-devel] [PATCH v4] Add GDB qAttached support
  2013-03-14 19:51   ` [Qemu-devel] [PATCH v4] " Jan Kiszka
  2013-03-14 21:07     ` Jesse Larrew
@ 2013-05-01 14:18     ` Jan Kiszka
  1 sibling, 0 replies; 15+ messages in thread
From: Jan Kiszka @ 2013-05-01 14:18 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel@nongnu.org, Fabien Chouteau

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

On 2013-03-14 20:51, Jan Kiszka wrote:
> 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.
> 

Ping for this and the revert of "gdbstub: Do not kill target in system
emulation mode".

Jan

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> As Fabien dropped his attempt to make this configurable, let's
> preserve the value of exposing this feature to gdb statically.
> 
>  gdbstub.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index e414ad9..9daee86 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -42,6 +42,12 @@
>  #include "sysemu/kvm.h"
>  #include "qemu/bitops.h"
>  
> +#ifdef CONFIG_USER_ONLY
> +#define GDB_ATTACHED "0"
> +#else
> +#define GDB_ATTACHED "1"
> +#endif
> +
>  #ifndef TARGET_CPU_MEMORY_RW_DEBUG
>  static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr,
>                                           uint8_t *buf, int len, int is_write)
> @@ -2491,6 +2497,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>              break;
>          }
>  #endif
> +        if (strncmp(p, "Attached", 8) == 0) {
> +            put_packet(s, GDB_ATTACHED);
> +            break;
> +        }
>          /* Unrecognised 'q' command.  */
>          goto unknown_command;
>  
> 



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

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

end of thread, other threads:[~2013-05-01 14:20 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-12 17:31 [Qemu-devel] [PATCH V3 0/3] Add GDB qAttached support Fabien Chouteau
2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 1/3] " Fabien Chouteau
2013-03-14 19:51   ` [Qemu-devel] [PATCH v4] " Jan Kiszka
2013-03-14 21:07     ` Jesse Larrew
2013-03-15 12:02       ` Fabien Chouteau
2013-03-19 10:37         ` Fabien Chouteau
2013-05-01 14:18     ` Jan Kiszka
2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 2/3] Revert "gdbstub: Do not kill target in system emulation mode" Fabien Chouteau
2013-03-14 19:52   ` Jan Kiszka
2013-03-12 17:31 ` [Qemu-devel] [PATCH V3 3/3] New option -gdb-opts Fabien Chouteau
2013-03-14  8:44   ` Markus Armbruster
2013-03-14 10:59     ` Fabien Chouteau
2013-03-14 12:34       ` Markus Armbruster
2013-03-14 16:33         ` Fabien Chouteau
2013-03-19 14:31           ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).