From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41406) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g70rY-0002Ze-7b for qemu-devel@nongnu.org; Mon, 01 Oct 2018 12:16:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g70rV-0003aK-0S for qemu-devel@nongnu.org; Mon, 01 Oct 2018 12:16:00 -0400 Received: from mail-wr1-f67.google.com ([209.85.221.67]:42541) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1g70rU-0003YF-PK for qemu-devel@nongnu.org; Mon, 01 Oct 2018 12:15:56 -0400 Received: by mail-wr1-f67.google.com with SMTP id b11-v6so14620576wru.9 for ; Mon, 01 Oct 2018 09:15:56 -0700 (PDT) References: <20181001115704.701-1-luc.michel@greensocs.com> <20181001115704.701-2-luc.michel@greensocs.com> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Message-ID: Date: Mon, 1 Oct 2018 18:15:53 +0200 MIME-Version: 1.0 In-Reply-To: <20181001115704.701-2-luc.michel@greensocs.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 01/15] gdbstub: introduce GDB processes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luc Michel , qemu-devel@nongnu.org Cc: Peter Maydell , alistair@alistair23.me, mark.burton@greensocs.com, =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , saipava@xilinx.com, edgari@xilinx.com, qemu-arm@nongnu.org Hi Luc, On 01/10/2018 13:56, Luc Michel wrote: > Add a structure GDBProcess that represent processes from the GDB > semantic point of view. > > CPUs can be split into different processes, by grouping them under a QOM > container named after the GDB_CPU_GROUP_NAME macro (`gdb-group[*]'). > Each occurrence of such a container implies the existence of the > corresponding process in the GDB stub. The gdb_cpu_group_container_get() > function can be used to create a new container. > > When no such container are found, all the CPUs are put in a unique GDB > process (create_unique_process()). This is also the case when compiled > in user mode, where multi-processes do not make much sense for now. > > Signed-off-by: Luc Michel > --- > include/exec/gdbstub.h | 8 +++++ > gdbstub.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+) > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > index 08363969c1..a3e4159bf4 100644 > --- a/include/exec/gdbstub.h > +++ b/include/exec/gdbstub.h > @@ -1,8 +1,10 @@ > #ifndef GDBSTUB_H > #define GDBSTUB_H > > +#include "qom/object.h" > + > #define DEFAULT_GDBSTUB_PORT "1234" > > /* GDB breakpoint/watchpoint types */ > #define GDB_BREAKPOINT_SW 0 > #define GDB_BREAKPOINT_HW 1 > @@ -129,6 +131,12 @@ void gdbserver_cleanup(void); > extern bool gdb_has_xml; > > /* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */ > extern const char *const xml_builtin[][2]; > > +#define GDB_CPU_GROUP_NAME "gdb-group" > + > +static inline Object *gdb_cpu_group_container_get(Object *parent) > +{ > + return container_get(parent, "/" GDB_CPU_GROUP_NAME "[*]"); > +} > #endif > diff --git a/gdbstub.c b/gdbstub.c > index d6ab95006c..5c86218f49 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -295,10 +295,17 @@ typedef struct GDBRegisterState { > gdb_reg_cb set_reg; > const char *xml; > struct GDBRegisterState *next; > } GDBRegisterState; > > +typedef struct GDBProcess { > + uint32_t pid; > + bool attached; > + > + char target_xml[1024]; I'd add this field in the patch #7 "support to Xfer:features:read:" where you start using it. > +} GDBProcess; > + > enum RSState { > RS_INACTIVE, > RS_IDLE, > RS_GETLINE, > RS_GETLINE_ESC, > @@ -323,10 +330,13 @@ typedef struct GDBState { > int running_state; > #else > CharBackend chr; > Chardev *mon_chr; > #endif > + bool multiprocess; > + GDBProcess *processes; > + int process_num; > char syscall_buf[256]; > gdb_syscall_complete_cb current_syscall_cb; > } GDBState; > > /* By default use no IRQs and no timers while single stepping so as to > @@ -1750,10 +1760,24 @@ void gdb_exit(CPUArchState *env, int code) > #ifndef CONFIG_USER_ONLY > qemu_chr_fe_deinit(&s->chr, true); > #endif > } > > +/* > + * Create a unique process containing all the CPUs. > + */ > +static void create_unique_process(GDBState *s) > +{ > + GDBProcess *process; > + > + s->processes = g_malloc0(sizeof(GDBProcess)); > + s->process_num = 1; > + process = &s->processes[0]; > + > + process->pid = 1; > +} > + > #ifdef CONFIG_USER_ONLY > int > gdb_handlesig(CPUState *cpu, int sig) > { > GDBState *s; > @@ -1847,10 +1871,11 @@ static bool gdb_accept(void) > } > > s = g_malloc0(sizeof(GDBState)); > s->c_cpu = first_cpu; > s->g_cpu = first_cpu; > + create_unique_process(s); > s->fd = fd; > gdb_has_xml = false; > > gdbserver_state = s; > return true; > @@ -2003,10 +2028,48 @@ static const TypeInfo char_gdb_type_info = { > .name = TYPE_CHARDEV_GDB, > .parent = TYPE_CHARDEV, > .class_init = char_gdb_class_init, > }; > > +static void create_processes(GDBState *s) > +{ > + Object *container; > + int i = 0; > + char process_str[16]; > + > + container = object_resolve_path(GDB_CPU_GROUP_NAME "[0]", NULL); > + > + while (container) { > + s->processes = g_renew(GDBProcess, s->processes, i + 1); > + > + GDBProcess *process = &s->processes[i]; > + > + /* GDB process IDs -1 and 0 are reserved */ > + process->pid = i + 1; > + process->attached = false; > + process->target_xml[0] = '\0'; > + > + i++; > + snprintf(process_str, sizeof(process_str), GDB_CPU_GROUP_NAME "[%d]", i); > + container = object_resolve_path(process_str, NULL); > + } > + > + if (!s->processes) { > + /* No CPU group specified by the machine */ > + create_unique_process(s); > + } else { > + s->process_num = i; > + } > +} > + > +static void cleanup_processes(GDBState *s) > +{ > + g_free(s->processes); > + s->process_num = 0; > + s->processes = NULL; > +} > + > int gdbserver_start(const char *device) > { > trace_gdbstub_op_start(device); > > GDBState *s; > @@ -2055,15 +2118,19 @@ int gdbserver_start(const char *device) > NULL, &error_abort); > monitor_init(mon_chr, 0); > } else { > qemu_chr_fe_deinit(&s->chr, true); > mon_chr = s->mon_chr; > + cleanup_processes(s); > memset(s, 0, sizeof(GDBState)); > s->mon_chr = mon_chr; > } > s->c_cpu = first_cpu; > s->g_cpu = first_cpu; > + > + create_processes(s); > + > if (chr) { > qemu_chr_fe_init(&s->chr, chr, &error_abort); > qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive, > gdb_chr_event, NULL, NULL, NULL, true); > } >