Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2019-01-29 Thread Max Filippov 
On Tue, Jan 29, 2019 at 2:05 AM Peter Maydell  wrote:
> > Starting with this commit it is no longer possible to kill QEMU
> > with the 'kill' command from the gdb. This was a nice feature,
> > was this removal intentional, or is it just an implementation
> > bug?
>
> That sounds like a bug. I think with the multiprocess extensions
> available gdb may switch from killing using the 'k' packet to using
> the 'vKill;pid' packet, which we don't implement? Looking at
> what gdb is sending ('set debug remote 1' turns on logging in
> gdb of remote protocol packets) would let us check that theory.

That's correct:

(gdb) kill
Kill the program being debugged? (y or n) y
Sending packet: $vKill;1#6e...Ack
Packet received:
Packet vKill (kill) is NOT supported
Can't kill process

> It's not clear how our implementation should deal with being asked
> to kill just one process if we have more than one, though...

I'll send a fix that restores previous behavior in case of a single
inferior process.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2019-01-29 Thread Peter Maydell 
On Tue, 29 Jan 2019 at 04:56, Max Filippov  wrote:
>
> Hello,
>
> On Fri, Dec 7, 2018 at 1:04 AM Luc Michel  wrote:
> >
> > Change the thread info related packets handling to support multiprocess
> > extension.
> >
> > Add the CPUs class name in the extra info to help differentiate
> > them in multiprocess mode.
> >
> > Signed-off-by: Luc Michel 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Reviewed-by: Edgar E. Iglesias 
> > ---
> >  gdbstub.c | 37 +++--
> >  1 file changed, 27 insertions(+), 10 deletions(-)
>
> Starting with this commit it is no longer possible to kill QEMU
> with the 'kill' command from the gdb. This was a nice feature,
> was this removal intentional, or is it just an implementation
> bug?

That sounds like a bug. I think with the multiprocess extensions
available gdb may switch from killing using the 'k' packet to using
the 'vKill;pid' packet, which we don't implement? Looking at
what gdb is sending ('set debug remote 1' turns on logging in
gdb of remote protocol packets) would let us check that theory.
It's not clear how our implementation should deal with being asked
to kill just one process if we have more than one, though...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2019-01-28 Thread Max Filippov 
Hello,

On Fri, Dec 7, 2018 at 1:04 AM Luc Michel  wrote:
>
> Change the thread info related packets handling to support multiprocess
> extension.
>
> Add the CPUs class name in the extra info to help differentiate
> them in multiprocess mode.
>
> Signed-off-by: Luc Michel 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Edgar E. Iglesias 
> ---
>  gdbstub.c | 37 +++--
>  1 file changed, 27 insertions(+), 10 deletions(-)

Starting with this commit it is no longer possible to kill QEMU
with the 'kill' command from the gdb. This was a nice feature,
was this removal intentional, or is it just an implementation
bug?

> diff --git a/gdbstub.c b/gdbstub.c
> index bea0215f30..770915446a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1267,11 +1267,10 @@ out:
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>  CPUState *cpu;
>  CPUClass *cc;
>  const char *p;
> -uint32_t thread;
>  uint32_t pid, tid;
>  int ch, reg_size, type, res;
>  uint8_t mem_buf[MAX_PACKET_LENGTH];
>  char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
>  char thread_id[16];
> @@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  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 = first_cpu;
> +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%x", 
> cpu_gdb_index(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 = CPU_NEXT(s->query_cpu);
> +s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
>  } else
>  put_packet(s, "l");
>  break;
>  } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> -thread = strtoull(p+16, (char **), 16);
> -cpu = find_cpu(thread);
> +if (read_thread_id(p + 16, , , ) == 
> GDB_READ_THREAD_ERR) {
> +put_packet(s, "E22");
> +break;
> +}
> +cpu = gdb_get_cpu(s, pid, tid);
>  if (cpu != NULL) {
>  cpu_synchronize_state(cpu);
> -/* memtohex() doubles the required space */
> -len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> -   "CPU#%d [%s]", cpu->cpu_index,
> -   cpu->halted ? "halted " : "running");
> +
> +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;
> --
> 2.19.2
>
>


-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2018-12-12 Thread Alistair Francis 

On 07/12/2018 1:01 am, Luc Michel wrote:

Change the thread info related packets handling to support multiprocess
extension.

Add the CPUs class name in the extra info to help differentiate
them in multiprocess mode.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  gdbstub.c | 37 +++--
  1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index bea0215f30..770915446a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1267,11 +1267,10 @@ out:
  static int gdb_handle_packet(GDBState *s, const char *line_buf)
  {
  CPUState *cpu;
  CPUClass *cc;
  const char *p;
-uint32_t thread;
  uint32_t pid, tid;
  int ch, reg_size, type, res;
  uint8_t mem_buf[MAX_PACKET_LENGTH];
  char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
  char thread_id[16];
@@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
  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 = first_cpu;
+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%x", cpu_gdb_index(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 = CPU_NEXT(s->query_cpu);
+s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
  } else
  put_packet(s, "l");
  break;
  } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-thread = strtoull(p+16, (char **), 16);
-cpu = find_cpu(thread);
+if (read_thread_id(p + 16, , , ) == GDB_READ_THREAD_ERR) 
{
+put_packet(s, "E22");
+break;
+}
+cpu = gdb_get_cpu(s, pid, tid);
  if (cpu != NULL) {
  cpu_synchronize_state(cpu);
-/* memtohex() doubles the required space */
-len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-   "CPU#%d [%s]", cpu->cpu_index,
-   cpu->halted ? "halted " : "running");
+
+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;

[Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2018-12-07 Thread Luc Michel 
Change the thread info related packets handling to support multiprocess
extension.

Add the CPUs class name in the extra info to help differentiate
them in multiprocess mode.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Edgar E. Iglesias 
---
 gdbstub.c | 37 +++--
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index bea0215f30..770915446a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1267,11 +1267,10 @@ out:
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
-uint32_t thread;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
@@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 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 = first_cpu;
+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%x", cpu_gdb_index(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 = CPU_NEXT(s->query_cpu);
+s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
 } else
 put_packet(s, "l");
 break;
 } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-thread = strtoull(p+16, (char **), 16);
-cpu = find_cpu(thread);
+if (read_thread_id(p + 16, , , ) == GDB_READ_THREAD_ERR) 
{
+put_packet(s, "E22");
+break;
+}
+cpu = gdb_get_cpu(s, pid, tid);
 if (cpu != NULL) {
 cpu_synchronize_state(cpu);
-/* memtohex() doubles the required space */
-len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-   "CPU#%d [%s]", cpu->cpu_index,
-   cpu->halted ? "halted " : "running");
+
+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;
-- 
2.19.2