* [Qemu-devel] [PATCH v6 01/16] hw/cpu: introduce CPU clusters
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 02/16] gdbstub: introduce GDB processes Luc Michel
` (15 subsequent siblings)
16 siblings, 0 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
This commit adds the cpu-cluster type. It aims at gathering CPUs from
the same cluster in a machine.
For now it only has a `cluster-id` property.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
include/hw/cpu/cluster.h | 38 ++++++++++++++++++++++++++
hw/cpu/cluster.c | 59 ++++++++++++++++++++++++++++++++++++++++
MAINTAINERS | 2 ++
hw/cpu/Makefile.objs | 2 +-
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 include/hw/cpu/cluster.h
create mode 100644 hw/cpu/cluster.c
diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
new file mode 100644
index 0000000000..11f50d5f6b
--- /dev/null
+++ b/include/hw/cpu/cluster.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * <http://www.gnu.org/licenses/gpl-2.0.html>
+ */
+#ifndef HW_CPU_CLUSTER_H
+#define HW_CPU_CLUSTER_H
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+#define TYPE_CPU_CLUSTER "cpu-cluster"
+#define CPU_CLUSTER(obj) \
+ OBJECT_CHECK(CPUClusterState, (obj), TYPE_CPU_CLUSTER)
+
+typedef struct CPUClusterState {
+ /*< private >*/
+ DeviceState parent_obj;
+
+ /*< public >*/
+ uint32_t cluster_id;
+} CPUClusterState;
+
+#endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
new file mode 100644
index 0000000000..e0ffd76152
--- /dev/null
+++ b/hw/cpu/cluster.c
@@ -0,0 +1,59 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * <http://www.gnu.org/licenses/gpl-2.0.html>
+ */
+
+#include "qemu/osdep.h"
+#include "hw/cpu/cluster.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+
+static void cpu_cluster_init(Object *obj)
+{
+ static uint32_t cluster_id_auto_increment;
+ CPUClusterState *cluster = CPU_CLUSTER(obj);
+
+ cluster->cluster_id = cluster_id_auto_increment++;
+}
+
+static Property cpu_cluster_properties[] = {
+ DEFINE_PROP_UINT32("cluster-id", CPUClusterState, cluster_id, 0),
+ DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpu_cluster_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->props = cpu_cluster_properties;
+}
+
+static const TypeInfo cpu_cluster_type_info = {
+ .name = TYPE_CPU_CLUSTER,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(CPUClusterState),
+ .instance_init = cpu_cluster_init,
+ .class_init = cpu_cluster_class_init,
+};
+
+static void cpu_cluster_register_types(void)
+{
+ type_register_static(&cpu_cluster_type_info);
+}
+
+type_init(cpu_cluster_register_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b8db618f5..3e7a0b09c0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1029,11 +1029,13 @@ Machine core
M: Eduardo Habkost <ehabkost@redhat.com>
M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
S: Supported
F: hw/core/machine.c
F: hw/core/null-machine.c
+F: hw/cpu/cluster.c
F: include/hw/boards.h
+F: include/hw/cpu/cluster.h
T: git https://github.com/ehabkost/qemu.git machine-next
Xtensa Machines
---------------
sim
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index cd52d20b65..8db9e8a7b3 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,5 +1,5 @@
obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
obj-$(CONFIG_REALVIEW) += realview_mpcore.o
obj-$(CONFIG_A9MPCORE) += a9mpcore.o
obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-common-obj-y += core.o
+common-obj-y += core.o cluster.o
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH v6 02/16] gdbstub: introduce GDB processes
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 01/16] hw/cpu: introduce CPU clusters Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 03/16] gdbstub: add multiprocess support to '?' packets Luc Michel
` (14 subsequent siblings)
16 siblings, 0 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
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
different cpu-cluster objects. Each occurrence of a cpu-cluster object
implies the existence of the corresponding process in the GDB stub. The
GDB process ID is derived from the corresponding cluster ID as follows:
GDB PID = cluster ID + 1
This is because PIDs -1 and 0 are reserved in GDB and cannot be used by
processes.
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 <luc.michel@greensocs.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
gdbstub.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index c4e4f9f082..26f5a7449a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -27,10 +27,11 @@
#include "monitor/monitor.h"
#include "chardev/char.h"
#include "chardev/char-fe.h"
#include "sysemu/sysemu.h"
#include "exec/gdbstub.h"
+#include "hw/cpu/cluster.h"
#endif
#define MAX_PACKET_LENGTH 4096
#include "qemu/sockets.h"
@@ -294,10 +295,15 @@ typedef struct GDBRegisterState {
gdb_reg_cb set_reg;
const char *xml;
struct GDBRegisterState *next;
} GDBRegisterState;
+typedef struct GDBProcess {
+ uint32_t pid;
+ bool attached;
+} GDBProcess;
+
enum RSState {
RS_INACTIVE,
RS_IDLE,
RS_GETLINE,
RS_GETLINE_ESC,
@@ -322,10 +328,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
@@ -1749,10 +1758,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;
@@ -1846,10 +1869,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;
@@ -2002,10 +2026,69 @@ static const TypeInfo char_gdb_type_info = {
.name = TYPE_CHARDEV_GDB,
.parent = TYPE_CHARDEV,
.class_init = char_gdb_class_init,
};
+static int find_cpu_clusters(Object *child, void *opaque)
+{
+ if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+ GDBState *s = (GDBState *) opaque;
+ CPUClusterState *cluster = CPU_CLUSTER(child);
+ GDBProcess *process;
+
+ s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
+
+ process = &s->processes[s->process_num - 1];
+
+ /*
+ * GDB process IDs -1 and 0 are reserved. To avoid subtle errors at
+ * runtime, we enforce here that the machine does not use a cluster ID
+ * that would lead to PID 0. */
+ assert(process->pid != UINT32_MAX);
+ process->pid = cluster->cluster_id + 1;
+ process->attached = false;
+
+ return 0;
+ }
+
+ return object_child_foreach(child, find_cpu_clusters, opaque);
+}
+
+static int pid_order(const void *a, const void *b)
+{
+ GDBProcess *pa = (GDBProcess *) a;
+ GDBProcess *pb = (GDBProcess *) b;
+
+ if (pa->pid < pb->pid) {
+ return -1;
+ } else if (pa->pid > pb->pid) {
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
+static void create_processes(GDBState *s)
+{
+ object_child_foreach(object_get_root(), find_cpu_clusters, s);
+
+ if (!s->processes) {
+ /* No CPU cluster specified by the machine */
+ create_unique_process(s);
+ } else {
+ /* Sort by PID */
+ qsort(s->processes, s->process_num, sizeof(s->processes[0]), pid_order);
+ }
+}
+
+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;
@@ -2058,15 +2141,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);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH v6 03/16] gdbstub: add multiprocess support to '?' packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 01/16] hw/cpu: introduce CPU clusters Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 02/16] gdbstub: introduce GDB processes Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
` (13 subsequent siblings)
16 siblings, 0 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
The gdb_get_cpu_pid() function does the PID lookup for the given CPU. It
checks if the CPU is a direct child of a CPU cluster. If it is, the
returned PID is the cluster ID plus one (cluster IDs start at 0, GDB
PIDs at 1). When the CPU is not a child of such a container, the PID of
the first process is returned.
The gdb_fmt_thread_id() function generates the string to be used to identify
a given thread, in a response packet for the peer. This function
supports generating thread IDs when multiprocess mode is enabled (in the
form `p<pid>.<tid>').
Use them in the reply to a '?' request.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
gdbstub.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 26f5a7449a..4fbc05dfe3 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -638,10 +638,52 @@ static int memtox(char *buf, const char *mem, int len)
}
}
return p - buf;
}
+static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
+{
+#ifndef CONFIG_USER_ONLY
+ gchar *path, *name;
+ Object *obj;
+ CPUClusterState *cluster;
+ uint32_t ret;
+
+ path = object_get_canonical_path(OBJECT(cpu));
+ name = object_get_canonical_path_component(OBJECT(cpu));
+
+ if (path == NULL) {
+ ret = s->processes[0].pid;
+ goto out;
+ }
+
+ /*
+ * Retrieve the CPU parent path by removing the last '/' and the CPU name
+ * from the CPU canonical path. */
+ path[strlen(path) - strlen(name) - 1] = '\0';
+
+ obj = object_resolve_path_type(path, TYPE_CPU_CLUSTER, NULL);
+
+ if (obj == NULL) {
+ ret = s->processes[0].pid;
+ goto out;
+ }
+
+ cluster = CPU_CLUSTER(obj);
+ ret = cluster->cluster_id + 1;
+
+out:
+ g_free(name);
+ g_free(path);
+
+ return ret;
+
+#else
+ return s->processes[0].pid;
+#endif
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
{
size_t len;
int i;
@@ -907,10 +949,23 @@ static CPUState *find_cpu(uint32_t thread_id)
}
return NULL;
}
+static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
+ char *buf, size_t buf_size)
+{
+ if (s->multiprocess) {
+ snprintf(buf, buf_size, "p%02x.%02x",
+ gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+ } else {
+ snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+ }
+
+ return buf;
+}
+
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 &&
@@ -1018,22 +1073,23 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
const char *p;
uint32_t thread;
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];
uint8_t *registers;
target_ulong addr, len;
trace_gdbstub_io_command(line_buf);
p = line_buf;
ch = *p++;
switch(ch) {
case '?':
/* TODO: Make this return the correct value for user-mode. */
- snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP,
- cpu_gdb_index(s->c_cpu));
+ 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.
*/
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (2 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 03/16] gdbstub: add multiprocess support to '?' packets Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-16 9:51 ` Edgar E. Iglesias
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
` (12 subsequent siblings)
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add a couple of helper functions to cope with GDB threads and processes.
The gdb_get_process() function looks for a process given a pid.
The gdb_get_cpu() function returns the CPU corresponding to the (pid,
tid) pair given as parameters.
The read_thread_id() function parses the thread-id sent by the peer.
This function supports the multiprocess extension thread-id syntax. The
return value specifies if the parsing failed, or if a special case was
encountered (all processes or all threads).
Use them in 'H' and 'T' packets handling to support the multiprocess
extension.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 134 insertions(+), 18 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 4fbc05dfe3..fa2b7077b2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -680,10 +680,73 @@ out:
#else
return s->processes[0].pid;
#endif
}
+static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
+{
+ int i;
+
+ if (!pid) {
+ /* 0 means any process, we take the first one */
+ return &s->processes[0];
+ }
+
+ for (i = 0; i < s->process_num; i++) {
+ if (s->processes[i].pid == pid) {
+ return &s->processes[i];
+ }
+ }
+
+ return NULL;
+}
+
+static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
+{
+ return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
+}
+
+static CPUState *find_cpu(uint32_t thread_id)
+{
+ CPUState *cpu;
+
+ CPU_FOREACH(cpu) {
+ if (cpu_gdb_index(cpu) == thread_id) {
+ return cpu;
+ }
+ }
+
+ return NULL;
+}
+
+static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
+{
+ GDBProcess *process;
+ CPUState *cpu = find_cpu(tid);
+
+ if (!tid) {
+ /* 0 means any thread, we take the first one */
+ tid = 1;
+ }
+
+ if (cpu == NULL) {
+ return NULL;
+ }
+
+ process = gdb_get_cpu_process(s, cpu);
+
+ if (process->pid != pid) {
+ return NULL;
+ }
+
+ if (!process->attached) {
+ return NULL;
+ }
+
+ return cpu;
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
{
size_t len;
int i;
@@ -936,23 +999,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
cpu_synchronize_state(cpu);
cpu_set_pc(cpu, pc);
}
-static CPUState *find_cpu(uint32_t thread_id)
-{
- CPUState *cpu;
-
- CPU_FOREACH(cpu) {
- if (cpu_gdb_index(cpu) == thread_id) {
- return cpu;
- }
- }
-
- return NULL;
-}
-
static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
char *buf, size_t buf_size)
{
if (s->multiprocess) {
snprintf(buf, buf_size, "p%02x.%02x",
@@ -962,10 +1012,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
}
return buf;
}
+typedef enum GDBThreadIdKind {
+ GDB_ONE_THREAD = 0,
+ GDB_ALL_THREADS, /* One process, all threads */
+ GDB_ALL_PROCESSES,
+ GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
+ uint32_t *pid, uint32_t *tid)
+{
+ unsigned long p, t;
+ int ret;
+
+ if (*buf == 'p') {
+ buf++;
+ ret = qemu_strtoul(buf, &buf, 16, &p);
+
+ if (ret) {
+ return GDB_READ_THREAD_ERR;
+ }
+
+ /* Skip '.' */
+ buf++;
+ } else {
+ p = 1;
+ }
+
+ ret = qemu_strtoul(buf, &buf, 16, &t);
+
+ if (ret) {
+ return GDB_READ_THREAD_ERR;
+ }
+
+ *end_buf = buf;
+
+ if (p == -1) {
+ return GDB_ALL_PROCESSES;
+ }
+
+ if (pid) {
+ *pid = p;
+ }
+
+ if (t == -1) {
+ return GDB_ALL_THREADS;
+ }
+
+ if (tid) {
+ *tid = t;
+ }
+
+ 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 &&
@@ -1070,16 +1174,18 @@ 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];
uint8_t *registers;
target_ulong addr, len;
+ GDBThreadIdKind thread_kind;
trace_gdbstub_io_command(line_buf);
p = line_buf;
ch = *p++;
@@ -1283,16 +1389,22 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
else
put_packet(s, "E22");
break;
case 'H':
type = *p++;
- thread = strtoull(p, (char **)&p, 16);
- if (thread == -1 || thread == 0) {
+
+ 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 = find_cpu(thread);
+ cpu = gdb_get_cpu(s, pid, tid);
if (cpu == NULL) {
put_packet(s, "E22");
break;
}
switch (type) {
@@ -1308,12 +1420,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
put_packet(s, "E22");
break;
}
break;
case 'T':
- thread = strtoull(p, (char **)&p, 16);
- cpu = find_cpu(thread);
+ 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");
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
@ 2018-11-16 9:51 ` Edgar E. Iglesias
2018-11-19 8:17 ` Luc Michel
0 siblings, 1 reply; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 9:51 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:55AM +0100, Luc Michel wrote:
> Add a couple of helper functions to cope with GDB threads and processes.
>
> The gdb_get_process() function looks for a process given a pid.
>
> The gdb_get_cpu() function returns the CPU corresponding to the (pid,
> tid) pair given as parameters.
>
> The read_thread_id() function parses the thread-id sent by the peer.
> This function supports the multiprocess extension thread-id syntax. The
> return value specifies if the parsing failed, or if a special case was
> encountered (all processes or all threads).
>
> Use them in 'H' and 'T' packets handling to support the multiprocess
> extension.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Hi Luc,
> ---
> gdbstub.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 134 insertions(+), 18 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 4fbc05dfe3..fa2b7077b2 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -680,10 +680,73 @@ out:
> #else
> return s->processes[0].pid;
> #endif
> }
>
> +static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
> +{
> + int i;
> +
> + if (!pid) {
> + /* 0 means any process, we take the first one */
> + return &s->processes[0];
> + }
> +
> + for (i = 0; i < s->process_num; i++) {
> + if (s->processes[i].pid == pid) {
> + return &s->processes[i];
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
> +{
> + return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
> +}
> +
> +static CPUState *find_cpu(uint32_t thread_id)
> +{
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + if (cpu_gdb_index(cpu) == thread_id) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> +{
> + GDBProcess *process;
> + CPUState *cpu = find_cpu(tid);
> +
> + if (!tid) {
> + /* 0 means any thread, we take the first one */
> + tid = 1;
> + }
> +
> + if (cpu == NULL) {
> + return NULL;
> + }
Not sure about this. If tid is zero, you fix up the wildcard by setting tid to one.
Shouldn't you also retry find_cpu(tid) in that case?
Otherwise, tid doesn't seem to be used after the wildcard fixup.
Other than that, I think this looks good:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> +
> + process = gdb_get_cpu_process(s, cpu);
> +
> + if (process->pid != pid) {
> + return NULL;
> + }
> +
> + if (!process->attached) {
> + return NULL;
> + }
> +
> + return cpu;
> +}
> +
> static const char *get_feature_xml(const char *p, const char **newp,
> CPUClass *cc)
> {
> size_t len;
> int i;
> @@ -936,23 +999,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
>
> cpu_synchronize_state(cpu);
> cpu_set_pc(cpu, pc);
> }
>
> -static CPUState *find_cpu(uint32_t thread_id)
> -{
> - CPUState *cpu;
> -
> - CPU_FOREACH(cpu) {
> - if (cpu_gdb_index(cpu) == thread_id) {
> - return cpu;
> - }
> - }
> -
> - return NULL;
> -}
> -
> static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
> char *buf, size_t buf_size)
> {
> if (s->multiprocess) {
> snprintf(buf, buf_size, "p%02x.%02x",
> @@ -962,10 +1012,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
> }
>
> return buf;
> }
>
> +typedef enum GDBThreadIdKind {
> + GDB_ONE_THREAD = 0,
> + GDB_ALL_THREADS, /* One process, all threads */
> + GDB_ALL_PROCESSES,
> + GDB_READ_THREAD_ERR
> +} GDBThreadIdKind;
> +
> +static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
> + uint32_t *pid, uint32_t *tid)
> +{
> + unsigned long p, t;
> + int ret;
> +
> + if (*buf == 'p') {
> + buf++;
> + ret = qemu_strtoul(buf, &buf, 16, &p);
> +
> + if (ret) {
> + return GDB_READ_THREAD_ERR;
> + }
> +
> + /* Skip '.' */
> + buf++;
> + } else {
> + p = 1;
> + }
> +
> + ret = qemu_strtoul(buf, &buf, 16, &t);
> +
> + if (ret) {
> + return GDB_READ_THREAD_ERR;
> + }
> +
> + *end_buf = buf;
> +
> + if (p == -1) {
> + return GDB_ALL_PROCESSES;
> + }
> +
> + if (pid) {
> + *pid = p;
> + }
> +
> + if (t == -1) {
> + return GDB_ALL_THREADS;
> + }
> +
> + if (tid) {
> + *tid = t;
> + }
> +
> + 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 &&
> @@ -1070,16 +1174,18 @@ 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];
> uint8_t *registers;
> target_ulong addr, len;
> + GDBThreadIdKind thread_kind;
>
> trace_gdbstub_io_command(line_buf);
>
> p = line_buf;
> ch = *p++;
> @@ -1283,16 +1389,22 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> else
> put_packet(s, "E22");
> break;
> case 'H':
> type = *p++;
> - thread = strtoull(p, (char **)&p, 16);
> - if (thread == -1 || thread == 0) {
> +
> + 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 = find_cpu(thread);
> + cpu = gdb_get_cpu(s, pid, tid);
> if (cpu == NULL) {
> put_packet(s, "E22");
> break;
> }
> switch (type) {
> @@ -1308,12 +1420,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> put_packet(s, "E22");
> break;
> }
> break;
> case 'T':
> - thread = strtoull(p, (char **)&p, 16);
> - cpu = find_cpu(thread);
> + 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");
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets
2018-11-16 9:51 ` Edgar E. Iglesias
@ 2018-11-19 8:17 ` Luc Michel
0 siblings, 0 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-19 8:17 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On 11/16/18 10:51 AM, Edgar E. Iglesias wrote:
> On Thu, Nov 15, 2018 at 10:41:55AM +0100, Luc Michel wrote:
>> Add a couple of helper functions to cope with GDB threads and processes.
>>
>> The gdb_get_process() function looks for a process given a pid.
>>
>> The gdb_get_cpu() function returns the CPU corresponding to the (pid,
>> tid) pair given as parameters.
>>
>> The read_thread_id() function parses the thread-id sent by the peer.
>> This function supports the multiprocess extension thread-id syntax. The
>> return value specifies if the parsing failed, or if a special case was
>> encountered (all processes or all threads).
>>
>> Use them in 'H' and 'T' packets handling to support the multiprocess
>> extension.
>>
>> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>
> Hi Luc,
>
>
>
>> ---
>> gdbstub.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 134 insertions(+), 18 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 4fbc05dfe3..fa2b7077b2 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -680,10 +680,73 @@ out:
>> #else
>> return s->processes[0].pid;
>> #endif
>> }
>>
>> +static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
>> +{
>> + int i;
>> +
>> + if (!pid) {
>> + /* 0 means any process, we take the first one */
>> + return &s->processes[0];
>> + }
>> +
>> + for (i = 0; i < s->process_num; i++) {
>> + if (s->processes[i].pid == pid) {
>> + return &s->processes[i];
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
>> +{
>> + return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
>> +}
>> +
>> +static CPUState *find_cpu(uint32_t thread_id)
>> +{
>> + CPUState *cpu;
>> +
>> + CPU_FOREACH(cpu) {
>> + if (cpu_gdb_index(cpu) == thread_id) {
>> + return cpu;
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
>> +{
>> + GDBProcess *process;
>> + CPUState *cpu = find_cpu(tid);
>> +
>> + if (!tid) {
>> + /* 0 means any thread, we take the first one */
>> + tid = 1;
>> + }
>> +
>> + if (cpu == NULL) {
>> + return NULL;
>> + }
>
> Not sure about this. If tid is zero, you fix up the wildcard by setting tid to one.
> Shouldn't you also retry find_cpu(tid) in that case?
>
> Otherwise, tid doesn't seem to be used after the wildcard fixup.
Thanks for catching this. It's a refactoring mistake. It will be fixed
in the next re-roll.
--
Luc
>
> Other than that, I think this looks good:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
>
>
>> +
>> + process = gdb_get_cpu_process(s, cpu);
>> +
>> + if (process->pid != pid) {
>> + return NULL;
>> + }
>> +
>> + if (!process->attached) {
>> + return NULL;
>> + }
>> +
>> + return cpu;
>> +}
>> +
>> static const char *get_feature_xml(const char *p, const char **newp,
>> CPUClass *cc)
>> {
>> size_t len;
>> int i;
>> @@ -936,23 +999,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
>>
>> cpu_synchronize_state(cpu);
>> cpu_set_pc(cpu, pc);
>> }
>>
>> -static CPUState *find_cpu(uint32_t thread_id)
>> -{
>> - CPUState *cpu;
>> -
>> - CPU_FOREACH(cpu) {
>> - if (cpu_gdb_index(cpu) == thread_id) {
>> - return cpu;
>> - }
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
>> char *buf, size_t buf_size)
>> {
>> if (s->multiprocess) {
>> snprintf(buf, buf_size, "p%02x.%02x",
>> @@ -962,10 +1012,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
>> }
>>
>> return buf;
>> }
>>
>> +typedef enum GDBThreadIdKind {
>> + GDB_ONE_THREAD = 0,
>> + GDB_ALL_THREADS, /* One process, all threads */
>> + GDB_ALL_PROCESSES,
>> + GDB_READ_THREAD_ERR
>> +} GDBThreadIdKind;
>> +
>> +static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
>> + uint32_t *pid, uint32_t *tid)
>> +{
>> + unsigned long p, t;
>> + int ret;
>> +
>> + if (*buf == 'p') {
>> + buf++;
>> + ret = qemu_strtoul(buf, &buf, 16, &p);
>> +
>> + if (ret) {
>> + return GDB_READ_THREAD_ERR;
>> + }
>> +
>> + /* Skip '.' */
>> + buf++;
>> + } else {
>> + p = 1;
>> + }
>> +
>> + ret = qemu_strtoul(buf, &buf, 16, &t);
>> +
>> + if (ret) {
>> + return GDB_READ_THREAD_ERR;
>> + }
>> +
>> + *end_buf = buf;
>> +
>> + if (p == -1) {
>> + return GDB_ALL_PROCESSES;
>> + }
>> +
>> + if (pid) {
>> + *pid = p;
>> + }
>> +
>> + if (t == -1) {
>> + return GDB_ALL_THREADS;
>> + }
>> +
>> + if (tid) {
>> + *tid = t;
>> + }
>> +
>> + 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 &&
>> @@ -1070,16 +1174,18 @@ 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];
>> uint8_t *registers;
>> target_ulong addr, len;
>> + GDBThreadIdKind thread_kind;
>>
>> trace_gdbstub_io_command(line_buf);
>>
>> p = line_buf;
>> ch = *p++;
>> @@ -1283,16 +1389,22 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>> else
>> put_packet(s, "E22");
>> break;
>> case 'H':
>> type = *p++;
>> - thread = strtoull(p, (char **)&p, 16);
>> - if (thread == -1 || thread == 0) {
>> +
>> + 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 = find_cpu(thread);
>> + cpu = gdb_get_cpu(s, pid, tid);
>> if (cpu == NULL) {
>> put_packet(s, "E22");
>> break;
>> }
>> switch (type) {
>> @@ -1308,12 +1420,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>> put_packet(s, "E22");
>> break;
>> }
>> break;
>> case 'T':
>> - thread = strtoull(p, (char **)&p, 16);
>> - cpu = find_cpu(thread);
>> + 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");
>> --
>> 2.19.1
>>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (3 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-16 9:57 ` Edgar E. Iglesias
2018-11-16 21:51 ` Alistair Francis
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 06/16] gdbstub: add multiprocess support to 'sC' packets Luc Michel
` (11 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
the CPUs in currently attached processes.
Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
iterate over CPUs of a given process.
Use them to add multiprocess extension support to vCont packets.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 102 insertions(+), 15 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index fa2b7077b2..f1ec3481cb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -716,10 +716,40 @@ static CPUState *find_cpu(uint32_t thread_id)
}
return NULL;
}
+static CPUState *get_first_cpu_in_process(const GDBState *s,
+ GDBProcess *process)
+{
+ CPUState *cpu;
+
+ CPU_FOREACH(cpu) {
+ if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+ return cpu;
+ }
+ }
+
+ return NULL;
+}
+
+static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+{
+ uint32_t pid = gdb_get_cpu_pid(s, cpu);
+ cpu = CPU_NEXT(cpu);
+
+ while (cpu) {
+ if (gdb_get_cpu_pid(s, cpu) == pid) {
+ break;
+ }
+
+ cpu = CPU_NEXT(cpu);
+ }
+
+ return cpu;
+}
+
static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
{
GDBProcess *process;
CPUState *cpu = find_cpu(tid);
@@ -743,10 +773,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
}
return cpu;
}
+/* Return the cpu following @cpu, while ignoring
+ * unattached processes.
+ */
+static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
+{
+ cpu = CPU_NEXT(cpu);
+
+ while (cpu) {
+ if (gdb_get_cpu_process(s, cpu)->attached) {
+ break;
+ }
+
+ cpu = CPU_NEXT(cpu);
+ }
+
+ return cpu;
+}
+
+/* Return the first attached cpu */
+static CPUState *gdb_first_cpu(const GDBState *s)
+{
+ CPUState *cpu = first_cpu;
+ GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+ if (!process->attached) {
+ return gdb_next_cpu(s, cpu);
+ }
+
+ return cpu;
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
{
size_t len;
int i;
@@ -1081,14 +1142,16 @@ static int is_query_packet(const char *p, const char *query, char separator)
* returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
* a format error, 0 on success.
*/
static int gdb_handle_vcont(GDBState *s, const char *p)
{
- int res, idx, signal = 0;
+ int res, signal = 0;
char cur_action;
char *newstates;
unsigned long tmp;
+ uint32_t pid, tid;
+ GDBProcess *process;
CPUState *cpu;
#ifdef CONFIG_USER_ONLY
int max_cpus = 1; /* global variable max_cpus exists only in system mode */
CPU_FOREACH(cpu) {
@@ -1127,29 +1190,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
} else if (cur_action != 'c' && cur_action != 's') {
/* unknown/invalid/unsupported command */
res = -ENOTSUP;
goto out;
}
- /* thread specification. special values: (none), -1 = all; 0 = any */
- if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
- if (*p == ':') {
- p += 3;
- }
- for (idx = 0; idx < max_cpus; idx++) {
- if (newstates[idx] == 1) {
- newstates[idx] = cur_action;
+
+ if (*p++ != ':') {
+ res = -ENOTSUP;
+ goto out;
+ }
+
+ switch (read_thread_id(p, &p, &pid, &tid)) {
+ case GDB_READ_THREAD_ERR:
+ res = -EINVAL;
+ goto out;
+
+ case GDB_ALL_PROCESSES:
+ cpu = gdb_first_cpu(s);
+ while (cpu) {
+ if (newstates[cpu->cpu_index] == 1) {
+ newstates[cpu->cpu_index] = cur_action;
}
+
+ cpu = gdb_next_cpu(s, cpu);
}
- } else if (*p == ':') {
- p++;
- res = qemu_strtoul(p, &p, 16, &tmp);
- if (res) {
+ break;
+
+ case GDB_ALL_THREADS:
+ process = gdb_get_process(s, pid);
+
+ if (!process->attached) {
+ res = -EINVAL;
goto out;
}
- /* 0 means any thread, so we pick the first valid CPU */
- cpu = tmp ? find_cpu(tmp) : first_cpu;
+ cpu = get_first_cpu_in_process(s, process);
+ while (cpu) {
+ if (newstates[cpu->cpu_index] == 1) {
+ newstates[cpu->cpu_index] = cur_action;
+ }
+
+ cpu = gdb_next_cpu_in_process(s, cpu);
+ }
+ break;
+
+ case GDB_ONE_THREAD:
+ cpu = gdb_get_cpu(s, pid, tid);
/* invalid CPU/thread specified */
if (!cpu) {
res = -EINVAL;
goto out;
@@ -1157,10 +1243,11 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
/* only use if no previous match occourred */
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
}
+ break;
}
}
s->signal = signal;
gdb_continue_partial(s, newstates);
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
@ 2018-11-16 9:57 ` Edgar E. Iglesias
2018-11-16 21:51 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 9:57 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:56AM +0100, Luc Michel wrote:
> Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
> the CPUs in currently attached processes.
>
> Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
> iterate over CPUs of a given process.
>
> Use them to add multiprocess extension support to vCont packets.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> gdbstub.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 102 insertions(+), 15 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index fa2b7077b2..f1ec3481cb 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -716,10 +716,40 @@ static CPUState *find_cpu(uint32_t thread_id)
> }
>
> return NULL;
> }
>
> +static CPUState *get_first_cpu_in_process(const GDBState *s,
> + GDBProcess *process)
> +{
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + if (gdb_get_cpu_pid(s, cpu) == process->pid) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
> +{
> + uint32_t pid = gdb_get_cpu_pid(s, cpu);
> + cpu = CPU_NEXT(cpu);
> +
> + while (cpu) {
> + if (gdb_get_cpu_pid(s, cpu) == pid) {
> + break;
> + }
> +
> + cpu = CPU_NEXT(cpu);
> + }
> +
> + return cpu;
> +}
> +
> static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> {
> GDBProcess *process;
> CPUState *cpu = find_cpu(tid);
>
> @@ -743,10 +773,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> }
>
> return cpu;
> }
>
> +/* Return the cpu following @cpu, while ignoring
> + * unattached processes.
> + */
> +static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
Perhaps a better name would be gdb_next_attached_cpu() ?
With that name change:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> +{
> + cpu = CPU_NEXT(cpu);
> +
> + while (cpu) {
> + if (gdb_get_cpu_process(s, cpu)->attached) {
> + break;
> + }
> +
> + cpu = CPU_NEXT(cpu);
> + }
> +
> + return cpu;
> +}
> +
> +/* Return the first attached cpu */
> +static CPUState *gdb_first_cpu(const GDBState *s)
> +{
> + CPUState *cpu = first_cpu;
> + GDBProcess *process = gdb_get_cpu_process(s, cpu);
> +
> + if (!process->attached) {
> + return gdb_next_cpu(s, cpu);
> + }
> +
> + return cpu;
> +}
> +
> static const char *get_feature_xml(const char *p, const char **newp,
> CPUClass *cc)
> {
> size_t len;
> int i;
> @@ -1081,14 +1142,16 @@ static int is_query_packet(const char *p, const char *query, char separator)
> * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
> * a format error, 0 on success.
> */
> static int gdb_handle_vcont(GDBState *s, const char *p)
> {
> - int res, idx, signal = 0;
> + int res, signal = 0;
> char cur_action;
> char *newstates;
> unsigned long tmp;
> + uint32_t pid, tid;
> + GDBProcess *process;
> CPUState *cpu;
> #ifdef CONFIG_USER_ONLY
> int max_cpus = 1; /* global variable max_cpus exists only in system mode */
>
> CPU_FOREACH(cpu) {
> @@ -1127,29 +1190,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
> } else if (cur_action != 'c' && cur_action != 's') {
> /* unknown/invalid/unsupported command */
> res = -ENOTSUP;
> goto out;
> }
> - /* thread specification. special values: (none), -1 = all; 0 = any */
> - if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
> - if (*p == ':') {
> - p += 3;
> - }
> - for (idx = 0; idx < max_cpus; idx++) {
> - if (newstates[idx] == 1) {
> - newstates[idx] = cur_action;
> +
> + if (*p++ != ':') {
> + res = -ENOTSUP;
> + goto out;
> + }
> +
> + switch (read_thread_id(p, &p, &pid, &tid)) {
> + case GDB_READ_THREAD_ERR:
> + res = -EINVAL;
> + goto out;
> +
> + case GDB_ALL_PROCESSES:
> + cpu = gdb_first_cpu(s);
> + while (cpu) {
> + if (newstates[cpu->cpu_index] == 1) {
> + newstates[cpu->cpu_index] = cur_action;
> }
> +
> + cpu = gdb_next_cpu(s, cpu);
> }
> - } else if (*p == ':') {
> - p++;
> - res = qemu_strtoul(p, &p, 16, &tmp);
> - if (res) {
> + break;
> +
> + case GDB_ALL_THREADS:
> + process = gdb_get_process(s, pid);
> +
> + if (!process->attached) {
> + res = -EINVAL;
> goto out;
> }
>
> - /* 0 means any thread, so we pick the first valid CPU */
> - cpu = tmp ? find_cpu(tmp) : first_cpu;
> + cpu = get_first_cpu_in_process(s, process);
> + while (cpu) {
> + if (newstates[cpu->cpu_index] == 1) {
> + newstates[cpu->cpu_index] = cur_action;
> + }
> +
> + cpu = gdb_next_cpu_in_process(s, cpu);
> + }
> + break;
> +
> + case GDB_ONE_THREAD:
> + cpu = gdb_get_cpu(s, pid, tid);
>
> /* invalid CPU/thread specified */
> if (!cpu) {
> res = -EINVAL;
> goto out;
> @@ -1157,10 +1243,11 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
>
> /* only use if no previous match occourred */
> if (newstates[cpu->cpu_index] == 1) {
> newstates[cpu->cpu_index] = cur_action;
> }
> + break;
> }
> }
> s->signal = signal;
> gdb_continue_partial(s, newstates);
>
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
2018-11-16 9:57 ` Edgar E. Iglesias
@ 2018-11-16 21:51 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 21:51 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:41 am, Luc Michel wrote:
> Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
> the CPUs in currently attached processes.
>
> Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
> iterate over CPUs of a given process.
>
> Use them to add multiprocess extension support to vCont packets.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 102 insertions(+), 15 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index fa2b7077b2..f1ec3481cb 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -716,10 +716,40 @@ static CPUState *find_cpu(uint32_t thread_id)
> }
>
> return NULL;
> }
>
> +static CPUState *get_first_cpu_in_process(const GDBState *s,
> + GDBProcess *process)
> +{
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + if (gdb_get_cpu_pid(s, cpu) == process->pid) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
> +{
> + uint32_t pid = gdb_get_cpu_pid(s, cpu);
> + cpu = CPU_NEXT(cpu);
> +
> + while (cpu) {
> + if (gdb_get_cpu_pid(s, cpu) == pid) {
> + break;
> + }
> +
> + cpu = CPU_NEXT(cpu);
> + }
> +
> + return cpu;
> +}
> +
> static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> {
> GDBProcess *process;
> CPUState *cpu = find_cpu(tid);
>
> @@ -743,10 +773,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> }
>
> return cpu;
> }
>
> +/* Return the cpu following @cpu, while ignoring
> + * unattached processes.
> + */
> +static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
> +{
> + cpu = CPU_NEXT(cpu);
> +
> + while (cpu) {
> + if (gdb_get_cpu_process(s, cpu)->attached) {
> + break;
> + }
> +
> + cpu = CPU_NEXT(cpu);
> + }
> +
> + return cpu;
> +}
> +
> +/* Return the first attached cpu */
> +static CPUState *gdb_first_cpu(const GDBState *s)
> +{
> + CPUState *cpu = first_cpu;
> + GDBProcess *process = gdb_get_cpu_process(s, cpu);
> +
> + if (!process->attached) {
> + return gdb_next_cpu(s, cpu);
> + }
> +
> + return cpu;
> +}
> +
> static const char *get_feature_xml(const char *p, const char **newp,
> CPUClass *cc)
> {
> size_t len;
> int i;
> @@ -1081,14 +1142,16 @@ static int is_query_packet(const char *p, const char *query, char separator)
> * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
> * a format error, 0 on success.
> */
> static int gdb_handle_vcont(GDBState *s, const char *p)
> {
> - int res, idx, signal = 0;
> + int res, signal = 0;
> char cur_action;
> char *newstates;
> unsigned long tmp;
> + uint32_t pid, tid;
> + GDBProcess *process;
> CPUState *cpu;
> #ifdef CONFIG_USER_ONLY
> int max_cpus = 1; /* global variable max_cpus exists only in system mode */
>
> CPU_FOREACH(cpu) {
> @@ -1127,29 +1190,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
> } else if (cur_action != 'c' && cur_action != 's') {
> /* unknown/invalid/unsupported command */
> res = -ENOTSUP;
> goto out;
> }
> - /* thread specification. special values: (none), -1 = all; 0 = any */
> - if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
> - if (*p == ':') {
> - p += 3;
> - }
> - for (idx = 0; idx < max_cpus; idx++) {
> - if (newstates[idx] == 1) {
> - newstates[idx] = cur_action;
> +
> + if (*p++ != ':') {
> + res = -ENOTSUP;
> + goto out;
> + }
> +
> + switch (read_thread_id(p, &p, &pid, &tid)) {
> + case GDB_READ_THREAD_ERR:
> + res = -EINVAL;
> + goto out;
> +
> + case GDB_ALL_PROCESSES:
> + cpu = gdb_first_cpu(s);
> + while (cpu) {
> + if (newstates[cpu->cpu_index] == 1) {
> + newstates[cpu->cpu_index] = cur_action;
> }
> +
> + cpu = gdb_next_cpu(s, cpu);
> }
> - } else if (*p == ':') {
> - p++;
> - res = qemu_strtoul(p, &p, 16, &tmp);
> - if (res) {
> + break;
> +
> + case GDB_ALL_THREADS:
> + process = gdb_get_process(s, pid);
> +
> + if (!process->attached) {
> + res = -EINVAL;
> goto out;
> }
>
> - /* 0 means any thread, so we pick the first valid CPU */
> - cpu = tmp ? find_cpu(tmp) : first_cpu;
> + cpu = get_first_cpu_in_process(s, process);
> + while (cpu) {
> + if (newstates[cpu->cpu_index] == 1) {
> + newstates[cpu->cpu_index] = cur_action;
> + }
> +
> + cpu = gdb_next_cpu_in_process(s, cpu);
> + }
> + break;
> +
> + case GDB_ONE_THREAD:
> + cpu = gdb_get_cpu(s, pid, tid);
>
> /* invalid CPU/thread specified */
> if (!cpu) {
> res = -EINVAL;
> goto out;
> @@ -1157,10 +1243,11 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
>
> /* only use if no previous match occourred */
> if (newstates[cpu->cpu_index] == 1) {
> newstates[cpu->cpu_index] = cur_action;
> }
> + break;
> }
> }
> s->signal = signal;
> gdb_continue_partial(s, newstates);
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 06/16] gdbstub: add multiprocess support to 'sC' packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (4 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-16 9:57 ` Edgar E. Iglesias
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
` (10 subsequent siblings)
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Change the sC packet handling to support the multiprocess extension.
Instead of returning the first thread, we return the first thread of the
current process.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index f1ec3481cb..d19b0137e8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1546,13 +1546,18 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
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 CPU (gdb returns the first thread). */
- put_packet(s, "QC1");
+ /* "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 = first_cpu;
goto report_cpuinfo;
} else if (strcmp(p,"sThreadInfo") == 0) {
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 06/16] gdbstub: add multiprocess support to 'sC' packets
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 06/16] gdbstub: add multiprocess support to 'sC' packets Luc Michel
@ 2018-11-16 9:57 ` Edgar E. Iglesias
0 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 9:57 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:57AM +0100, Luc Michel wrote:
> Change the sC packet handling to support the multiprocess extension.
> Instead of returning the first thread, we return the first thread of the
> current process.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index f1ec3481cb..d19b0137e8 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1546,13 +1546,18 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> 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 CPU (gdb returns the first thread). */
> - put_packet(s, "QC1");
> + /* "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 = first_cpu;
> goto report_cpuinfo;
> } else if (strcmp(p,"sThreadInfo") == 0) {
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (5 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 06/16] gdbstub: add multiprocess support to 'sC' packets Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-16 10:04 ` Edgar E. Iglesias
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
` (9 subsequent siblings)
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
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 <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index d19b0137e8..292dee8914 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1260,11 +1260,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];
@@ -1556,30 +1555,46 @@ 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_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_cpu(s, s->query_cpu);
} else
put_packet(s, "l");
break;
} else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
- thread = strtoull(p+16, (char **)&p, 16);
- cpu = find_cpu(thread);
+ 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);
- /* 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 in multiprocess mode */
+ ObjectClass *oc = object_get_class(OBJECT(cpu));
+ const char *cpu_model = object_class_get_name(oc);
+ len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+ "CPU#%d %s [%s]", cpu->cpu_index,
+ cpu_model,
+ cpu->halted ? "halted " : "running");
+ } 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.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
@ 2018-11-16 10:04 ` Edgar E. Iglesias
2018-11-19 10:12 ` Luc Michel
0 siblings, 1 reply; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:04 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:58AM +0100, 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 <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> gdbstub.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index d19b0137e8..292dee8914 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1260,11 +1260,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];
> @@ -1556,30 +1555,46 @@ 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_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_cpu(s, s->query_cpu);
> } else
> put_packet(s, "l");
> break;
> } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> - thread = strtoull(p+16, (char **)&p, 16);
> - cpu = find_cpu(thread);
> + 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);
> - /* 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 in multiprocess mode */
> + ObjectClass *oc = object_get_class(OBJECT(cpu));
> + const char *cpu_model = object_class_get_name(oc);
> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> + "CPU#%d %s [%s]", cpu->cpu_index,
> + cpu_model,
> + cpu->halted ? "halted " : "running");
I wonder if we could also print a friendly name here deducted from QOM?
In some of our use-cases we have an array of MicroBlazes that all live
in different HW subsystems and are named differently (e.g CSU, PMU, PMC,
PSM etc).
Instead of just seeing a list of MicroBlaze cores it may be more useful
to see the actual core name of some sort, e.g:
Instead of:
CPU#0 MicroBlaze [running]
CPU#1 MicroBlaze [running]
CPU#2 MicroBlaze [running]
CPU#3 MicroBlaze [running]
Perhaps something like:
CPU#0 MicroBlaze PMU [running]
CPU#1 MicroBlaze PMC-PPU0 [running]
CPU#2 MicroBlaze PMC-PPU1 [running]
CPU#3 MicroBlaze PSM [running]
Any thoughts on that?
Thanks,
Edgar
> + } 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.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-11-16 10:04 ` Edgar E. Iglesias
@ 2018-11-19 10:12 ` Luc Michel
2018-11-19 16:44 ` Edgar E. Iglesias
0 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-19 10:12 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On 11/16/18 11:04 AM, Edgar E. Iglesias wrote:
> On Thu, Nov 15, 2018 at 10:41:58AM +0100, 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 <luc.michel@greensocs.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> gdbstub.c | 35 +++++++++++++++++++++++++----------
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index d19b0137e8..292dee8914 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -1260,11 +1260,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];
>> @@ -1556,30 +1555,46 @@ 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_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_cpu(s, s->query_cpu);
>> } else
>> put_packet(s, "l");
>> break;
>> } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
>> - thread = strtoull(p+16, (char **)&p, 16);
>> - cpu = find_cpu(thread);
>> + 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);
>> - /* 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 in multiprocess mode */
>> + ObjectClass *oc = object_get_class(OBJECT(cpu));
>> + const char *cpu_model = object_class_get_name(oc);
>> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
>> + "CPU#%d %s [%s]", cpu->cpu_index,
>> + cpu_model,
>> + cpu->halted ? "halted " : "running");
>
>
>
> I wonder if we could also print a friendly name here deducted from QOM?
> In some of our use-cases we have an array of MicroBlazes that all live
> in different HW subsystems and are named differently (e.g CSU, PMU, PMC,
> PSM etc).
>
> Instead of just seeing a list of MicroBlaze cores it may be more useful
> to see the actual core name of some sort, e.g:
>
> Instead of:
> CPU#0 MicroBlaze [running]
> CPU#1 MicroBlaze [running]
> CPU#2 MicroBlaze [running]
> CPU#3 MicroBlaze [running]
>
> Perhaps something like:
> CPU#0 MicroBlaze PMU [running]
> CPU#1 MicroBlaze PMC-PPU0 [running]
> CPU#2 MicroBlaze PMC-PPU1 [running]
> CPU#3 MicroBlaze PSM [running]
>
> Any thoughts on that?
I wanted to avoid the ThreadExtraInfo packet to become too much cluttered.
Here are some tests adding the component part of the CPU canonical name:
(gdb) info threads
Id Target Id Frame
1.1 Thread 1.1 (CPU#0 cortex-a53-arm-cpu apu-cpu[0] [running])
0x0000000000000000 in ?? ()
1.2 Thread 1.2 (CPU#1 cortex-a53-arm-cpu apu-cpu[1] [halted ])
0x0000000000000000 in ?? ()
1.3 Thread 1.3 (CPU#2 cortex-a53-arm-cpu apu-cpu[2] [halted ])
0x0000000000000000 in ?? ()
1.4 Thread 1.4 (CPU#3 cortex-a53-arm-cpu apu-cpu[3] [halted ])
0x0000000000000000 in ?? ()
* 2.1 Thread 2.5 (CPU#4 cortex-r5f-arm-cpu rpu-cpu[0] [halted ])
0xffff0000 in ?? ()
2.2 Thread 2.6 (CPU#5 cortex-r5f-arm-cpu rpu-cpu[1] [halted ])
0xffff0000 in ?? ()
The model name takes quite some room. The interesting info are `arm` and
`cortex-xxx`, but AFAIK there is no way of extracting that for a CPU
generically.
In this case, having the component part of the canonical name is ok
because self-explanatory. However we could encounter cases where the
parent name would be necessary to discriminate the CPUs, something like:
cluster[0]/cpu[0]
/cpu[1]
cluster[1]/cpu[0]
/cpu[1]
...
The "safest" way would be to have the whole path:
(gdb) info threads
Id Target Id Frame
1.1 Thread 1.1 (CPU#0 cortex-a53-arm-cpu
/machine/soc/apu-cluster/apu-cpu[0] [running]) 0x0000000000000000 in ?? ()
1.2 Thread 1.2 (CPU#1 cortex-a53-arm-cpu
/machine/soc/apu-cluster/apu-cpu[1] [halted ]) 0x0000000000000000 in ?? ()
1.3 Thread 1.3 (CPU#2 cortex-a53-arm-cpu
/machine/soc/apu-cluster/apu-cpu[2] [halted ]) 0x0000000000000000 in ?? ()
1.4 Thread 1.4 (CPU#3 cortex-a53-arm-cpu
/machine/soc/apu-cluster/apu-cpu[3] [halted ]) 0x0000000000000000 in ?? ()
* 2.1 Thread 2.5 (CPU#4 cortex-r5f-arm-cpu
/machine/soc/rpu-cluster/rpu-cpu[0] [halted ]) 0xffff0000 in ?? ()
2.2 Thread 2.6 (CPU#5 cortex-r5f-arm-cpu
/machine/soc/rpu-cluster/rpu-cpu[1] [halted ]) 0xffff0000 in ?? ()
But that becomes really cluttered... We could also remove the CPU model
completely.
What are your thoughts?
Thanks,
Luc
>
> Thanks,
> Edgar
>
>> + } 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.1
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-11-19 10:12 ` Luc Michel
@ 2018-11-19 16:44 ` Edgar E. Iglesias
0 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-19 16:44 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Mon, Nov 19, 2018 at 11:12:45AM +0100, Luc Michel wrote:
>
>
> On 11/16/18 11:04 AM, Edgar E. Iglesias wrote:
> > On Thu, Nov 15, 2018 at 10:41:58AM +0100, 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 <luc.michel@greensocs.com>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >> gdbstub.c | 35 +++++++++++++++++++++++++----------
> >> 1 file changed, 25 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/gdbstub.c b/gdbstub.c
> >> index d19b0137e8..292dee8914 100644
> >> --- a/gdbstub.c
> >> +++ b/gdbstub.c
> >> @@ -1260,11 +1260,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];
> >> @@ -1556,30 +1555,46 @@ 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_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_cpu(s, s->query_cpu);
> >> } else
> >> put_packet(s, "l");
> >> break;
> >> } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> >> - thread = strtoull(p+16, (char **)&p, 16);
> >> - cpu = find_cpu(thread);
> >> + 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);
> >> - /* 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 in multiprocess mode */
> >> + ObjectClass *oc = object_get_class(OBJECT(cpu));
> >> + const char *cpu_model = object_class_get_name(oc);
> >> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> >> + "CPU#%d %s [%s]", cpu->cpu_index,
> >> + cpu_model,
> >> + cpu->halted ? "halted " : "running");
> >
> >
> >
> > I wonder if we could also print a friendly name here deducted from QOM?
> > In some of our use-cases we have an array of MicroBlazes that all live
> > in different HW subsystems and are named differently (e.g CSU, PMU, PMC,
> > PSM etc).
> >
> > Instead of just seeing a list of MicroBlaze cores it may be more useful
> > to see the actual core name of some sort, e.g:
> >
> > Instead of:
> > CPU#0 MicroBlaze [running]
> > CPU#1 MicroBlaze [running]
> > CPU#2 MicroBlaze [running]
> > CPU#3 MicroBlaze [running]
> >
> > Perhaps something like:
> > CPU#0 MicroBlaze PMU [running]
> > CPU#1 MicroBlaze PMC-PPU0 [running]
> > CPU#2 MicroBlaze PMC-PPU1 [running]
> > CPU#3 MicroBlaze PSM [running]
> >
> > Any thoughts on that?
> I wanted to avoid the ThreadExtraInfo packet to become too much cluttered.
>
> Here are some tests adding the component part of the CPU canonical name:
>
> (gdb) info threads
> Id Target Id Frame
> 1.1 Thread 1.1 (CPU#0 cortex-a53-arm-cpu apu-cpu[0] [running])
> 0x0000000000000000 in ?? ()
> 1.2 Thread 1.2 (CPU#1 cortex-a53-arm-cpu apu-cpu[1] [halted ])
> 0x0000000000000000 in ?? ()
> 1.3 Thread 1.3 (CPU#2 cortex-a53-arm-cpu apu-cpu[2] [halted ])
> 0x0000000000000000 in ?? ()
> 1.4 Thread 1.4 (CPU#3 cortex-a53-arm-cpu apu-cpu[3] [halted ])
> 0x0000000000000000 in ?? ()
> * 2.1 Thread 2.5 (CPU#4 cortex-r5f-arm-cpu rpu-cpu[0] [halted ])
> 0xffff0000 in ?? ()
> 2.2 Thread 2.6 (CPU#5 cortex-r5f-arm-cpu rpu-cpu[1] [halted ])
> 0xffff0000 in ?? ()
>
> The model name takes quite some room. The interesting info are `arm` and
> `cortex-xxx`, but AFAIK there is no way of extracting that for a CPU
> generically.
>
> In this case, having the component part of the canonical name is ok
> because self-explanatory. However we could encounter cases where the
> parent name would be necessary to discriminate the CPUs, something like:
> cluster[0]/cpu[0]
> /cpu[1]
> cluster[1]/cpu[0]
> /cpu[1]
> ...
>
> The "safest" way would be to have the whole path:
>
> (gdb) info threads
> Id Target Id Frame
> 1.1 Thread 1.1 (CPU#0 cortex-a53-arm-cpu
> /machine/soc/apu-cluster/apu-cpu[0] [running]) 0x0000000000000000 in ?? ()
> 1.2 Thread 1.2 (CPU#1 cortex-a53-arm-cpu
> /machine/soc/apu-cluster/apu-cpu[1] [halted ]) 0x0000000000000000 in ?? ()
> 1.3 Thread 1.3 (CPU#2 cortex-a53-arm-cpu
> /machine/soc/apu-cluster/apu-cpu[2] [halted ]) 0x0000000000000000 in ?? ()
> 1.4 Thread 1.4 (CPU#3 cortex-a53-arm-cpu
> /machine/soc/apu-cluster/apu-cpu[3] [halted ]) 0x0000000000000000 in ?? ()
> * 2.1 Thread 2.5 (CPU#4 cortex-r5f-arm-cpu
> /machine/soc/rpu-cluster/rpu-cpu[0] [halted ]) 0xffff0000 in ?? ()
> 2.2 Thread 2.6 (CPU#5 cortex-r5f-arm-cpu
> /machine/soc/rpu-cluster/rpu-cpu[1] [halted ]) 0xffff0000 in ?? ()
>
> But that becomes really cluttered... We could also remove the CPU model
> completely.
>
> What are your thoughts?
Thanks Luc,
Not sure...
(CPU#0 cortex-a53-arm-cpu apu-cpu[0] [running])
Looks a little long but I think still the better option here. Would be interesting to hear others opinion.
Also, would it make sense to remove the CPU#X alltogether here?
It's not of much use in GDB since we controll things by process and thread anyway...
Cheers,
Edgar
>
> Thanks,
> Luc
>
> >
> > Thanks,
> > Edgar
> >
> >> + } 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.1
> >>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read:
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (6 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
@ 2018-11-15 9:41 ` Luc Michel
2018-11-16 10:06 ` Edgar E. Iglesias
2018-11-16 21:54 ` Alistair Francis
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
` (8 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:41 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Change the Xfer:features:read: packet handling to support the
multiprocess extension. This packet is used to request the XML
description of the CPU. In multiprocess mode, different descriptions can
be sent for different processes.
This function now takes the process to send the description for as a
parameter, and use a buffer in the process structure to store the
generated description.
It takes the first CPU of the process to generate the description.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 292dee8914..4cbc81ae5b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
} GDBRegisterState;
typedef struct GDBProcess {
uint32_t pid;
bool attached;
+
+ char target_xml[1024];
} GDBProcess;
enum RSState {
RS_INACTIVE,
RS_IDLE,
@@ -804,55 +806,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
}
return cpu;
}
-static const char *get_feature_xml(const char *p, const char **newp,
- CPUClass *cc)
+static const char *get_feature_xml(const GDBState *s, const char *p,
+ const char **newp, GDBProcess *process)
{
size_t len;
int i;
const char *name;
- static char target_xml[1024];
+ CPUState *cpu = get_first_cpu_in_process(s, process);
+ CPUClass *cc = CPU_GET_CLASS(cpu);
len = 0;
while (p[len] && p[len] != ':')
len++;
*newp = p + len;
name = NULL;
if (strncmp(p, "target.xml", len) == 0) {
+ char *buf = process->target_xml;
+ const size_t buf_sz = sizeof(process->target_xml);
+
/* Generate the XML description for this CPU. */
- if (!target_xml[0]) {
+ if (!buf[0]) {
GDBRegisterState *r;
- CPUState *cpu = first_cpu;
- pstrcat(target_xml, sizeof(target_xml),
+ pstrcat(buf, buf_sz,
"<?xml version=\"1.0\"?>"
"<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
"<target>");
if (cc->gdb_arch_name) {
gchar *arch = cc->gdb_arch_name(cpu);
- pstrcat(target_xml, sizeof(target_xml), "<architecture>");
- pstrcat(target_xml, sizeof(target_xml), arch);
- pstrcat(target_xml, sizeof(target_xml), "</architecture>");
+ pstrcat(buf, buf_sz, "<architecture>");
+ pstrcat(buf, buf_sz, arch);
+ pstrcat(buf, buf_sz, "</architecture>");
g_free(arch);
}
- pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
- pstrcat(target_xml, sizeof(target_xml), cc->gdb_core_xml_file);
- pstrcat(target_xml, sizeof(target_xml), "\"/>");
+ pstrcat(buf, buf_sz, "<xi:include href=\"");
+ pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
+ pstrcat(buf, buf_sz, "\"/>");
for (r = cpu->gdb_regs; r; r = r->next) {
- pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
- pstrcat(target_xml, sizeof(target_xml), r->xml);
- pstrcat(target_xml, sizeof(target_xml), "\"/>");
+ pstrcat(buf, buf_sz, "<xi:include href=\"");
+ pstrcat(buf, buf_sz, r->xml);
+ pstrcat(buf, buf_sz, "\"/>");
}
- pstrcat(target_xml, sizeof(target_xml), "</target>");
+ pstrcat(buf, buf_sz, "</target>");
}
- return target_xml;
+ return buf;
}
if (cc->gdb_get_dynamic_xml) {
- CPUState *cpu = first_cpu;
char *xmlname = g_strndup(p, len);
const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
g_free(xmlname);
if (xml) {
@@ -1258,10 +1262,11 @@ out:
}
static int gdb_handle_packet(GDBState *s, const char *line_buf)
{
CPUState *cpu;
+ GDBProcess *process;
CPUClass *cc;
const char *p;
uint32_t pid, tid;
int ch, reg_size, type, res;
uint8_t mem_buf[MAX_PACKET_LENGTH];
@@ -1639,18 +1644,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
}
if (strncmp(p, "Xfer:features:read:", 19) == 0) {
const char *xml;
target_ulong total_len;
- cc = CPU_GET_CLASS(first_cpu);
+ 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(p, &p, cc);
+ xml = get_feature_xml(s, p, &p, process);
if (!xml) {
snprintf(buf, sizeof(buf), "E00");
put_packet(s, buf);
break;
}
@@ -2323,10 +2329,11 @@ static int find_cpu_clusters(Object *child, void *opaque)
* runtime, we enforce here that the machine does not use a cluster ID
* that would lead to PID 0. */
assert(process->pid != UINT32_MAX);
process->pid = cluster->cluster_id + 1;
process->attached = false;
+ process->target_xml[0] = '\0';
return 0;
}
return object_child_foreach(child, find_cpu_clusters, opaque);
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read:
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
@ 2018-11-16 10:06 ` Edgar E. Iglesias
2018-11-16 21:54 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:06 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:59AM +0100, Luc Michel wrote:
> Change the Xfer:features:read: packet handling to support the
> multiprocess extension. This packet is used to request the XML
> description of the CPU. In multiprocess mode, different descriptions can
> be sent for different processes.
>
> This function now takes the process to send the description for as a
> parameter, and use a buffer in the process structure to store the
> generated description.
>
> It takes the first CPU of the process to generate the description.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 47 +++++++++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 292dee8914..4cbc81ae5b 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
> } GDBRegisterState;
>
> typedef struct GDBProcess {
> uint32_t pid;
> bool attached;
> +
> + char target_xml[1024];
> } GDBProcess;
>
> enum RSState {
> RS_INACTIVE,
> RS_IDLE,
> @@ -804,55 +806,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
> }
>
> return cpu;
> }
>
> -static const char *get_feature_xml(const char *p, const char **newp,
> - CPUClass *cc)
> +static const char *get_feature_xml(const GDBState *s, const char *p,
> + const char **newp, GDBProcess *process)
> {
> size_t len;
> int i;
> const char *name;
> - static char target_xml[1024];
> + CPUState *cpu = get_first_cpu_in_process(s, process);
> + CPUClass *cc = CPU_GET_CLASS(cpu);
>
> len = 0;
> while (p[len] && p[len] != ':')
> len++;
> *newp = p + len;
>
> name = NULL;
> if (strncmp(p, "target.xml", len) == 0) {
> + char *buf = process->target_xml;
> + const size_t buf_sz = sizeof(process->target_xml);
> +
> /* Generate the XML description for this CPU. */
> - if (!target_xml[0]) {
> + if (!buf[0]) {
> GDBRegisterState *r;
> - CPUState *cpu = first_cpu;
>
> - pstrcat(target_xml, sizeof(target_xml),
> + pstrcat(buf, buf_sz,
> "<?xml version=\"1.0\"?>"
> "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> "<target>");
> if (cc->gdb_arch_name) {
> gchar *arch = cc->gdb_arch_name(cpu);
> - pstrcat(target_xml, sizeof(target_xml), "<architecture>");
> - pstrcat(target_xml, sizeof(target_xml), arch);
> - pstrcat(target_xml, sizeof(target_xml), "</architecture>");
> + pstrcat(buf, buf_sz, "<architecture>");
> + pstrcat(buf, buf_sz, arch);
> + pstrcat(buf, buf_sz, "</architecture>");
> g_free(arch);
> }
> - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> - pstrcat(target_xml, sizeof(target_xml), cc->gdb_core_xml_file);
> - pstrcat(target_xml, sizeof(target_xml), "\"/>");
> + pstrcat(buf, buf_sz, "<xi:include href=\"");
> + pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
> + pstrcat(buf, buf_sz, "\"/>");
> for (r = cpu->gdb_regs; r; r = r->next) {
> - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> - pstrcat(target_xml, sizeof(target_xml), r->xml);
> - pstrcat(target_xml, sizeof(target_xml), "\"/>");
> + pstrcat(buf, buf_sz, "<xi:include href=\"");
> + pstrcat(buf, buf_sz, r->xml);
> + pstrcat(buf, buf_sz, "\"/>");
> }
> - pstrcat(target_xml, sizeof(target_xml), "</target>");
> + pstrcat(buf, buf_sz, "</target>");
> }
> - return target_xml;
> + return buf;
> }
> if (cc->gdb_get_dynamic_xml) {
> - CPUState *cpu = first_cpu;
> char *xmlname = g_strndup(p, len);
> const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
>
> g_free(xmlname);
> if (xml) {
> @@ -1258,10 +1262,11 @@ out:
> }
>
> static int gdb_handle_packet(GDBState *s, const char *line_buf)
> {
> CPUState *cpu;
> + GDBProcess *process;
> CPUClass *cc;
> const char *p;
> uint32_t pid, tid;
> int ch, reg_size, type, res;
> uint8_t mem_buf[MAX_PACKET_LENGTH];
> @@ -1639,18 +1644,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> }
> if (strncmp(p, "Xfer:features:read:", 19) == 0) {
> const char *xml;
> target_ulong total_len;
>
> - cc = CPU_GET_CLASS(first_cpu);
> + 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(p, &p, cc);
> + xml = get_feature_xml(s, p, &p, process);
> if (!xml) {
> snprintf(buf, sizeof(buf), "E00");
> put_packet(s, buf);
> break;
> }
> @@ -2323,10 +2329,11 @@ static int find_cpu_clusters(Object *child, void *opaque)
> * runtime, we enforce here that the machine does not use a cluster ID
> * that would lead to PID 0. */
> assert(process->pid != UINT32_MAX);
> process->pid = cluster->cluster_id + 1;
> process->attached = false;
> + process->target_xml[0] = '\0';
>
> return 0;
> }
>
> return object_child_foreach(child, find_cpu_clusters, opaque);
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read:
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
2018-11-16 10:06 ` Edgar E. Iglesias
@ 2018-11-16 21:54 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 21:54 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:41 am, Luc Michel wrote:
> Change the Xfer:features:read: packet handling to support the
> multiprocess extension. This packet is used to request the XML
> description of the CPU. In multiprocess mode, different descriptions can
> be sent for different processes.
>
> This function now takes the process to send the description for as a
> parameter, and use a buffer in the process structure to store the
> generated description.
>
> It takes the first CPU of the process to generate the description.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 47 +++++++++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 292dee8914..4cbc81ae5b 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
> } GDBRegisterState;
>
> typedef struct GDBProcess {
> uint32_t pid;
> bool attached;
> +
> + char target_xml[1024];
> } GDBProcess;
>
> enum RSState {
> RS_INACTIVE,
> RS_IDLE,
> @@ -804,55 +806,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
> }
>
> return cpu;
> }
>
> -static const char *get_feature_xml(const char *p, const char **newp,
> - CPUClass *cc)
> +static const char *get_feature_xml(const GDBState *s, const char *p,
> + const char **newp, GDBProcess *process)
> {
> size_t len;
> int i;
> const char *name;
> - static char target_xml[1024];
> + CPUState *cpu = get_first_cpu_in_process(s, process);
> + CPUClass *cc = CPU_GET_CLASS(cpu);
>
> len = 0;
> while (p[len] && p[len] != ':')
> len++;
> *newp = p + len;
>
> name = NULL;
> if (strncmp(p, "target.xml", len) == 0) {
> + char *buf = process->target_xml;
> + const size_t buf_sz = sizeof(process->target_xml);
> +
> /* Generate the XML description for this CPU. */
> - if (!target_xml[0]) {
> + if (!buf[0]) {
> GDBRegisterState *r;
> - CPUState *cpu = first_cpu;
>
> - pstrcat(target_xml, sizeof(target_xml),
> + pstrcat(buf, buf_sz,
> "<?xml version=\"1.0\"?>"
> "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> "<target>");
> if (cc->gdb_arch_name) {
> gchar *arch = cc->gdb_arch_name(cpu);
> - pstrcat(target_xml, sizeof(target_xml), "<architecture>");
> - pstrcat(target_xml, sizeof(target_xml), arch);
> - pstrcat(target_xml, sizeof(target_xml), "</architecture>");
> + pstrcat(buf, buf_sz, "<architecture>");
> + pstrcat(buf, buf_sz, arch);
> + pstrcat(buf, buf_sz, "</architecture>");
> g_free(arch);
> }
> - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> - pstrcat(target_xml, sizeof(target_xml), cc->gdb_core_xml_file);
> - pstrcat(target_xml, sizeof(target_xml), "\"/>");
> + pstrcat(buf, buf_sz, "<xi:include href=\"");
> + pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
> + pstrcat(buf, buf_sz, "\"/>");
> for (r = cpu->gdb_regs; r; r = r->next) {
> - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> - pstrcat(target_xml, sizeof(target_xml), r->xml);
> - pstrcat(target_xml, sizeof(target_xml), "\"/>");
> + pstrcat(buf, buf_sz, "<xi:include href=\"");
> + pstrcat(buf, buf_sz, r->xml);
> + pstrcat(buf, buf_sz, "\"/>");
> }
> - pstrcat(target_xml, sizeof(target_xml), "</target>");
> + pstrcat(buf, buf_sz, "</target>");
> }
> - return target_xml;
> + return buf;
> }
> if (cc->gdb_get_dynamic_xml) {
> - CPUState *cpu = first_cpu;
> char *xmlname = g_strndup(p, len);
> const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
>
> g_free(xmlname);
> if (xml) {
> @@ -1258,10 +1262,11 @@ out:
> }
>
> static int gdb_handle_packet(GDBState *s, const char *line_buf)
> {
> CPUState *cpu;
> + GDBProcess *process;
> CPUClass *cc;
> const char *p;
> uint32_t pid, tid;
> int ch, reg_size, type, res;
> uint8_t mem_buf[MAX_PACKET_LENGTH];
> @@ -1639,18 +1644,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> }
> if (strncmp(p, "Xfer:features:read:", 19) == 0) {
> const char *xml;
> target_ulong total_len;
>
> - cc = CPU_GET_CLASS(first_cpu);
> + 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(p, &p, cc);
> + xml = get_feature_xml(s, p, &p, process);
> if (!xml) {
> snprintf(buf, sizeof(buf), "E00");
> put_packet(s, buf);
> break;
> }
> @@ -2323,10 +2329,11 @@ static int find_cpu_clusters(Object *child, void *opaque)
> * runtime, we enforce here that the machine does not use a cluster ID
> * that would lead to PID 0. */
> assert(process->pid != UINT32_MAX);
> process->pid = cluster->cluster_id + 1;
> process->attached = false;
> + process->target_xml[0] = '\0';
>
> return 0;
> }
>
> return object_child_foreach(child, find_cpu_clusters, opaque);
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (7 preceding siblings ...)
2018-11-15 9:41 ` [Qemu-devel] [PATCH v6 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:09 ` Edgar E. Iglesias
2018-11-16 21:56 ` Alistair Francis
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
` (7 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add support for multiprocess extension in gdb_vm_state_change()
function.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 4cbc81ae5b..5df9929f92 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1713,10 +1713,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
static void gdb_vm_state_change(void *opaque, int running, RunState state)
{
GDBState *s = gdbserver_state;
CPUState *cpu = s->c_cpu;
char buf[256];
+ char thread_id[16];
const char *type;
int ret;
if (running || s->state == RS_INACTIVE) {
return;
@@ -1724,10 +1725,18 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
/* Is there a GDB syscall waiting to be sent? */
if (s->current_syscall_cb) {
put_packet(s, s->syscall_buf);
return;
}
+
+ if (cpu == NULL) {
+ /* No process attached */
+ return;
+ }
+
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+
switch (state) {
case RUN_STATE_DEBUG:
if (cpu->watchpoint_hit) {
switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
case BP_MEM_READ:
@@ -1741,12 +1750,12 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
break;
}
trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
(target_ulong)cpu->watchpoint_hit->vaddr);
snprintf(buf, sizeof(buf),
- "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
- GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
+ "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+ GDB_SIGNAL_TRAP, thread_id, type,
(target_ulong)cpu->watchpoint_hit->vaddr);
cpu->watchpoint_hit = NULL;
goto send_packet;
} else {
trace_gdbstub_hit_break();
@@ -1784,11 +1793,11 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
trace_gdbstub_hit_unknown(state);
ret = GDB_SIGNAL_UNKNOWN;
break;
}
gdb_set_stop_cpu(cpu);
- snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
+ snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
send_packet:
put_packet(s, buf);
/* disable single step if it was enabled */
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
@ 2018-11-16 10:09 ` Edgar E. Iglesias
2018-11-16 21:56 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:09 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:00AM +0100, Luc Michel wrote:
> Add support for multiprocess extension in gdb_vm_state_change()
> function.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 4cbc81ae5b..5df9929f92 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1713,10 +1713,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
> static void gdb_vm_state_change(void *opaque, int running, RunState state)
> {
> GDBState *s = gdbserver_state;
> CPUState *cpu = s->c_cpu;
> char buf[256];
> + char thread_id[16];
> const char *type;
> int ret;
>
> if (running || s->state == RS_INACTIVE) {
> return;
> @@ -1724,10 +1725,18 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> /* Is there a GDB syscall waiting to be sent? */
> if (s->current_syscall_cb) {
> put_packet(s, s->syscall_buf);
> return;
> }
> +
> + if (cpu == NULL) {
> + /* No process attached */
> + return;
> + }
> +
> + gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
> +
> switch (state) {
> case RUN_STATE_DEBUG:
> if (cpu->watchpoint_hit) {
> switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
> case BP_MEM_READ:
> @@ -1741,12 +1750,12 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> break;
> }
> trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
> (target_ulong)cpu->watchpoint_hit->vaddr);
> snprintf(buf, sizeof(buf),
> - "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
> - GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
> + "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
> + GDB_SIGNAL_TRAP, thread_id, type,
> (target_ulong)cpu->watchpoint_hit->vaddr);
> cpu->watchpoint_hit = NULL;
> goto send_packet;
> } else {
> trace_gdbstub_hit_break();
> @@ -1784,11 +1793,11 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> trace_gdbstub_hit_unknown(state);
> ret = GDB_SIGNAL_UNKNOWN;
> break;
> }
> gdb_set_stop_cpu(cpu);
> - snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
> + snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
>
> send_packet:
> put_packet(s, buf);
>
> /* disable single step if it was enabled */
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
2018-11-16 10:09 ` Edgar E. Iglesias
@ 2018-11-16 21:56 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 21:56 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:42 am, Luc Michel wrote:
> Add support for multiprocess extension in gdb_vm_state_change()
> function.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 4cbc81ae5b..5df9929f92 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1713,10 +1713,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
> static void gdb_vm_state_change(void *opaque, int running, RunState state)
> {
> GDBState *s = gdbserver_state;
> CPUState *cpu = s->c_cpu;
> char buf[256];
> + char thread_id[16];
> const char *type;
> int ret;
>
> if (running || s->state == RS_INACTIVE) {
> return;
> @@ -1724,10 +1725,18 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> /* Is there a GDB syscall waiting to be sent? */
> if (s->current_syscall_cb) {
> put_packet(s, s->syscall_buf);
> return;
> }
> +
> + if (cpu == NULL) {
> + /* No process attached */
> + return;
> + }
> +
> + gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
> +
> switch (state) {
> case RUN_STATE_DEBUG:
> if (cpu->watchpoint_hit) {
> switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
> case BP_MEM_READ:
> @@ -1741,12 +1750,12 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> break;
> }
> trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
> (target_ulong)cpu->watchpoint_hit->vaddr);
> snprintf(buf, sizeof(buf),
> - "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
> - GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
> + "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
> + GDB_SIGNAL_TRAP, thread_id, type,
> (target_ulong)cpu->watchpoint_hit->vaddr);
> cpu->watchpoint_hit = NULL;
> goto send_packet;
> } else {
> trace_gdbstub_hit_break();
> @@ -1784,11 +1793,11 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
> trace_gdbstub_hit_unknown(state);
> ret = GDB_SIGNAL_UNKNOWN;
> break;
> }
> gdb_set_stop_cpu(cpu);
> - snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
> + snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
>
> send_packet:
> put_packet(s, buf);
>
> /* disable single step if it was enabled */
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (8 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:10 ` Edgar E. Iglesias
2018-11-16 22:00 ` Alistair Francis
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet Luc Michel
` (6 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
'D' packets are used by GDB to detach from a process. In multiprocess
mode, the PID to detach from is sent in the request.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
gdbstub.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 53 insertions(+), 7 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 5df9929f92..eec1cf0d09 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1039,24 +1039,39 @@ static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
default:
return -ENOSYS;
}
}
+static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
+{
+ cpu_breakpoint_remove_all(cpu, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+ cpu_watchpoint_remove_all(cpu, BP_GDB);
+#endif
+}
+
+static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
+{
+ CPUState *cpu = get_first_cpu_in_process(s, p);
+
+ while (cpu) {
+ gdb_cpu_breakpoint_remove_all(cpu);
+ cpu = gdb_next_cpu_in_process(s, cpu);
+ }
+}
+
static void gdb_breakpoint_remove_all(void)
{
CPUState *cpu;
if (kvm_enabled()) {
kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
return;
}
CPU_FOREACH(cpu) {
- cpu_breakpoint_remove_all(cpu, BP_GDB);
-#ifndef CONFIG_USER_ONLY
- cpu_watchpoint_remove_all(cpu, BP_GDB);
-#endif
+ gdb_cpu_breakpoint_remove_all(cpu);
}
}
static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
{
@@ -1331,13 +1346,44 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
/* Kill the target */
error_report("QEMU: Terminated via GDBstub");
exit(0);
case 'D':
/* Detach packet */
- gdb_breakpoint_remove_all();
- gdb_syscall_mode = GDB_SYS_DISABLED;
- gdb_continue(s);
+ 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_cpu(s);
+ }
+
+ if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+ s->g_cpu = gdb_first_cpu(s);
+ }
+
+ if (s->c_cpu == NULL) {
+ /* No more process attached */
+ gdb_syscall_mode = GDB_SYS_DISABLED;
+ gdb_continue(s);
+ }
put_packet(s, "OK");
break;
case 's':
if (*p != '\0') {
addr = strtoull(p, (char **)&p, 16);
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
@ 2018-11-16 10:10 ` Edgar E. Iglesias
2018-11-16 22:00 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:10 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:01AM +0100, Luc Michel wrote:
> 'D' packets are used by GDB to detach from a process. In multiprocess
> mode, the PID to detach from is sent in the request.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 5df9929f92..eec1cf0d09 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1039,24 +1039,39 @@ static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
> default:
> return -ENOSYS;
> }
> }
>
> +static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
> +{
> + cpu_breakpoint_remove_all(cpu, BP_GDB);
> +#ifndef CONFIG_USER_ONLY
> + cpu_watchpoint_remove_all(cpu, BP_GDB);
> +#endif
> +}
> +
> +static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
> +{
> + CPUState *cpu = get_first_cpu_in_process(s, p);
> +
> + while (cpu) {
> + gdb_cpu_breakpoint_remove_all(cpu);
> + cpu = gdb_next_cpu_in_process(s, cpu);
> + }
> +}
> +
> static void gdb_breakpoint_remove_all(void)
> {
> CPUState *cpu;
>
> if (kvm_enabled()) {
> kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
> return;
> }
>
> CPU_FOREACH(cpu) {
> - cpu_breakpoint_remove_all(cpu, BP_GDB);
> -#ifndef CONFIG_USER_ONLY
> - cpu_watchpoint_remove_all(cpu, BP_GDB);
> -#endif
> + gdb_cpu_breakpoint_remove_all(cpu);
> }
> }
>
> static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
> {
> @@ -1331,13 +1346,44 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> /* Kill the target */
> error_report("QEMU: Terminated via GDBstub");
> exit(0);
> case 'D':
> /* Detach packet */
> - gdb_breakpoint_remove_all();
> - gdb_syscall_mode = GDB_SYS_DISABLED;
> - gdb_continue(s);
> + 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_cpu(s);
> + }
> +
> + if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> + s->g_cpu = gdb_first_cpu(s);
> + }
> +
> + if (s->c_cpu == NULL) {
> + /* No more process attached */
> + gdb_syscall_mode = GDB_SYS_DISABLED;
> + gdb_continue(s);
> + }
> put_packet(s, "OK");
> break;
> case 's':
> if (*p != '\0') {
> addr = strtoull(p, (char **)&p, 16);
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
2018-11-16 10:10 ` Edgar E. Iglesias
@ 2018-11-16 22:00 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 22:00 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:42 am, Luc Michel wrote:
> 'D' packets are used by GDB to detach from a process. In multiprocess
> mode, the PID to detach from is sent in the request.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 5df9929f92..eec1cf0d09 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1039,24 +1039,39 @@ static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
> default:
> return -ENOSYS;
> }
> }
>
> +static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
> +{
> + cpu_breakpoint_remove_all(cpu, BP_GDB);
> +#ifndef CONFIG_USER_ONLY
> + cpu_watchpoint_remove_all(cpu, BP_GDB);
> +#endif
> +}
> +
> +static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
> +{
> + CPUState *cpu = get_first_cpu_in_process(s, p);
> +
> + while (cpu) {
> + gdb_cpu_breakpoint_remove_all(cpu);
> + cpu = gdb_next_cpu_in_process(s, cpu);
> + }
> +}
> +
> static void gdb_breakpoint_remove_all(void)
> {
> CPUState *cpu;
>
> if (kvm_enabled()) {
> kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
> return;
> }
>
> CPU_FOREACH(cpu) {
> - cpu_breakpoint_remove_all(cpu, BP_GDB);
> -#ifndef CONFIG_USER_ONLY
> - cpu_watchpoint_remove_all(cpu, BP_GDB);
> -#endif
> + gdb_cpu_breakpoint_remove_all(cpu);
> }
> }
>
> static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
> {
> @@ -1331,13 +1346,44 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> /* Kill the target */
> error_report("QEMU: Terminated via GDBstub");
> exit(0);
> case 'D':
> /* Detach packet */
> - gdb_breakpoint_remove_all();
> - gdb_syscall_mode = GDB_SYS_DISABLED;
> - gdb_continue(s);
> + 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_cpu(s);
> + }
> +
> + if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
> + s->g_cpu = gdb_first_cpu(s);
> + }
> +
> + if (s->c_cpu == NULL) {
> + /* No more process attached */
> + gdb_syscall_mode = GDB_SYS_DISABLED;
> + gdb_continue(s);
> + }
> put_packet(s, "OK");
> break;
> case 's':
> if (*p != '\0') {
> addr = strtoull(p, (char **)&p, 16);
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (9 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:10 ` Edgar E. Iglesias
2018-11-16 22:00 ` Alistair Francis
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets Luc Michel
` (5 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add support for the '!' extended mode packet. This is required for the
multiprocess extension.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
---
gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index eec1cf0d09..ada53ac559 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1294,10 +1294,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
trace_gdbstub_io_command(line_buf);
p = line_buf;
ch = *p++;
switch(ch) {
+ case '!':
+ 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);
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet Luc Michel
@ 2018-11-16 10:10 ` Edgar E. Iglesias
2018-11-16 22:00 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:10 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:02AM +0100, Luc Michel wrote:
> Add support for the '!' extended mode packet. This is required for the
> multiprocess extension.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index eec1cf0d09..ada53ac559 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1294,10 +1294,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> trace_gdbstub_io_command(line_buf);
>
> p = line_buf;
> ch = *p++;
> switch(ch) {
> + case '!':
> + 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);
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet Luc Michel
2018-11-16 10:10 ` Edgar E. Iglesias
@ 2018-11-16 22:00 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 22:00 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:42 am, Luc Michel wrote:
> Add support for the '!' extended mode packet. This is required for the
> multiprocess extension.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index eec1cf0d09..ada53ac559 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1294,10 +1294,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> trace_gdbstub_io_command(line_buf);
>
> p = line_buf;
> ch = *p++;
> switch(ch) {
> + case '!':
> + 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);
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (10 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 11/16] gdbstub: add support for extended mode packet Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:11 ` Edgar E. Iglesias
2018-11-16 22:03 ` Alistair Francis
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 13/16] gdbstub: processes initialization on new peer connection Luc Michel
` (4 subsequent siblings)
16 siblings, 2 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add support for the vAttach packets. In multiprocess mode, GDB sends
them to attach to additional processes.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
---
gdbstub.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index ada53ac559..0427d1c4f0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1340,10 +1340,45 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
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 {
goto unknown_command;
}
case 'k':
/* Kill the target */
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets Luc Michel
@ 2018-11-16 10:11 ` Edgar E. Iglesias
2018-11-16 22:03 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:11 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:03AM +0100, Luc Michel wrote:
> Add support for the vAttach packets. In multiprocess mode, GDB sends
> them to attach to additional processes.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index ada53ac559..0427d1c4f0 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1340,10 +1340,45 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> 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 {
> goto unknown_command;
> }
> case 'k':
> /* Kill the target */
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets Luc Michel
2018-11-16 10:11 ` Edgar E. Iglesias
@ 2018-11-16 22:03 ` Alistair Francis
1 sibling, 0 replies; 39+ messages in thread
From: Alistair Francis @ 2018-11-16 22:03 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé, saipava, edgari, qemu-arm
On 15/11/2018 1:42 am, Luc Michel wrote:
> Add support for the vAttach packets. In multiprocess mode, GDB sends
> them to attach to additional processes.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index ada53ac559..0427d1c4f0 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1340,10 +1340,45 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> 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 {
> goto unknown_command;
> }
> case 'k':
> /* Kill the target */
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 13/16] gdbstub: processes initialization on new peer connection
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (11 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 12/16] gdbstub: add support for vAttach packets Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:13 ` Edgar E. Iglesias
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached Luc Michel
` (3 subsequent siblings)
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
When a new connection is established, we set the first process to be
attached, and the others detached. The first CPU of the first process
is selected as the current CPU.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 0427d1c4f0..59eed3b878 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2245,13 +2245,14 @@ static bool gdb_accept(void)
close(fd);
return false;
}
s = g_malloc0(sizeof(GDBState));
- s->c_cpu = first_cpu;
- s->g_cpu = first_cpu;
create_unique_process(s);
+ s->processes[0].attached = true;
+ s->c_cpu = gdb_first_cpu(s);
+ s->g_cpu = s->c_cpu;
s->fd = fd;
gdb_has_xml = false;
gdbserver_state = s;
return true;
@@ -2333,12 +2334,23 @@ static void gdb_chr_receive(void *opaque, const uint8_t *buf, int size)
}
}
static void gdb_chr_event(void *opaque, int event)
{
+ int i;
+ GDBState *s = (GDBState *) opaque;
+
switch (event) {
case CHR_EVENT_OPENED:
+ /* Start with first process attached, others detached */
+ for (i = 0; i < s->process_num; i++) {
+ s->processes[i].attached = !i;
+ }
+
+ s->c_cpu = gdb_first_cpu(s);
+ s->g_cpu = s->c_cpu;
+
vm_stop(RUN_STATE_PAUSED);
gdb_has_xml = false;
break;
default:
break;
@@ -2524,19 +2536,17 @@ int gdbserver_start(const char *device)
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);
+ gdb_chr_event, NULL, s, NULL, true);
}
s->state = chr ? RS_IDLE : RS_INACTIVE;
s->mon_chr = mon_chr;
s->current_syscall_cb = NULL;
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 13/16] gdbstub: processes initialization on new peer connection
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 13/16] gdbstub: processes initialization on new peer connection Luc Michel
@ 2018-11-16 10:13 ` Edgar E. Iglesias
0 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:13 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:04AM +0100, Luc Michel wrote:
> When a new connection is established, we set the first process to be
> attached, and the others detached. The first CPU of the first process
> is selected as the current CPU.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 0427d1c4f0..59eed3b878 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2245,13 +2245,14 @@ static bool gdb_accept(void)
> close(fd);
> return false;
> }
>
> s = g_malloc0(sizeof(GDBState));
> - s->c_cpu = first_cpu;
> - s->g_cpu = first_cpu;
> create_unique_process(s);
> + s->processes[0].attached = true;
> + s->c_cpu = gdb_first_cpu(s);
> + s->g_cpu = s->c_cpu;
> s->fd = fd;
> gdb_has_xml = false;
>
> gdbserver_state = s;
> return true;
> @@ -2333,12 +2334,23 @@ static void gdb_chr_receive(void *opaque, const uint8_t *buf, int size)
> }
> }
>
> static void gdb_chr_event(void *opaque, int event)
> {
> + int i;
> + GDBState *s = (GDBState *) opaque;
> +
> switch (event) {
> case CHR_EVENT_OPENED:
> + /* Start with first process attached, others detached */
> + for (i = 0; i < s->process_num; i++) {
> + s->processes[i].attached = !i;
> + }
> +
> + s->c_cpu = gdb_first_cpu(s);
> + s->g_cpu = s->c_cpu;
> +
> vm_stop(RUN_STATE_PAUSED);
> gdb_has_xml = false;
> break;
> default:
> break;
> @@ -2524,19 +2536,17 @@ int gdbserver_start(const char *device)
> 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);
> + gdb_chr_event, NULL, s, NULL, true);
> }
> s->state = chr ? RS_IDLE : RS_INACTIVE;
> s->mon_chr = mon_chr;
> s->current_syscall_cb = NULL;
>
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (12 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 13/16] gdbstub: processes initialization on new peer connection Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:14 ` Edgar E. Iglesias
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 15/16] gdbstub: add multiprocess extension support Luc Michel
` (2 subsequent siblings)
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
When gdb_set_stop_cpu() is called with a CPU associated to a process
currently not attached by the GDB client, return without modifying the
stop CPU. Otherwise, GDB gets confused if it receives packets with a
thread-id it does not know about.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index 59eed3b878..89819a6a72 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1787,10 +1787,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
return RS_IDLE;
}
void gdb_set_stop_cpu(CPUState *cpu)
{
+ GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
+
+ if (!p->attached) {
+ /* Having a stop CPU corresponding to a process that is not attached
+ * confuses GDB. So we ignore the request.
+ */
+ return;
+ }
+
gdbserver_state->c_cpu = cpu;
gdbserver_state->g_cpu = cpu;
}
#ifndef CONFIG_USER_ONLY
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached Luc Michel
@ 2018-11-16 10:14 ` Edgar E. Iglesias
0 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:14 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:05AM +0100, Luc Michel wrote:
> When gdb_set_stop_cpu() is called with a CPU associated to a process
> currently not attached by the GDB client, return without modifying the
> stop CPU. Otherwise, GDB gets confused if it receives packets with a
> thread-id it does not know about.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 59eed3b878..89819a6a72 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1787,10 +1787,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> return RS_IDLE;
> }
>
> void gdb_set_stop_cpu(CPUState *cpu)
> {
> + GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
> +
> + if (!p->attached) {
> + /* Having a stop CPU corresponding to a process that is not attached
> + * confuses GDB. So we ignore the request.
> + */
> + return;
> + }
> +
> gdbserver_state->c_cpu = cpu;
> gdbserver_state->g_cpu = cpu;
> }
>
> #ifndef CONFIG_USER_ONLY
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 15/16] gdbstub: add multiprocess extension support
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (13 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:14 ` Edgar E. Iglesias
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters Luc Michel
2018-11-16 10:15 ` [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Edgar E. Iglesias
16 siblings, 1 reply; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Add multiprocess extension support by enabling multiprocess mode when
the peer requests it, and by replying that we actually support it in the
qSupported reply packet.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index 89819a6a72..e0b844c040 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1721,10 +1721,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
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;
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 15/16] gdbstub: add multiprocess extension support
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 15/16] gdbstub: add multiprocess extension support Luc Michel
@ 2018-11-16 10:14 ` Edgar E. Iglesias
0 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:14 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:42:06AM +0100, Luc Michel wrote:
> Add multiprocess extension support by enabling multiprocess mode when
> the peer requests it, and by replying that we actually support it in the
> qSupported reply packet.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 89819a6a72..e0b844c040 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1721,10 +1721,16 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> 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;
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH v6 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (14 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 15/16] gdbstub: add multiprocess extension support Luc Michel
@ 2018-11-15 9:42 ` Luc Michel
2018-11-16 10:15 ` [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Edgar E. Iglesias
16 siblings, 0 replies; 39+ messages in thread
From: Luc Michel @ 2018-11-15 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
Create two separate CPU clusters for APUs and RPUs.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
include/hw/arm/xlnx-zynqmp.h | 3 +++
hw/arm/xlnx-zynqmp.c | 21 +++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 98f925ab84..591515c760 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -29,10 +29,11 @@
#include "hw/dma/xlnx_dpdma.h"
#include "hw/dma/xlnx-zdma.h"
#include "hw/display/xlnx_dp.h"
#include "hw/intc/xlnx-zynqmp-ipi.h"
#include "hw/timer/xlnx-zynqmp-rtc.h"
+#include "hw/cpu/cluster.h"
#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
TYPE_XLNX_ZYNQMP)
@@ -75,10 +76,12 @@
typedef struct XlnxZynqMPState {
/*< private >*/
DeviceState parent_obj;
/*< public >*/
+ CPUClusterState apu_cluster;
+ CPUClusterState rpu_cluster;
ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
GICState gic;
MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c195040350..42a138074c 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -176,16 +176,22 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
{
Error *err = NULL;
int i;
int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
+ object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
+ sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
+ &error_abort, NULL);
+
+ qdev_init_nofail(DEVICE(&s->rpu_cluster));
+
for (i = 0; i < num_rpus; i++) {
char *name;
object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
"cortex-r5f-" TYPE_ARM_CPU);
- object_property_add_child(OBJECT(s), "rpu-cpu[*]",
+ object_property_add_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
OBJECT(&s->rpu_cpu[i]), &error_abort);
name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
if (strcmp(name, boot_cpu)) {
/* Secondary CPUs start in PSCI powered-down state */
@@ -211,14 +217,19 @@ static void xlnx_zynqmp_init(Object *obj)
{
XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
int i;
int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
+ object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
+ sizeof(s->apu_cluster), TYPE_CPU_CLUSTER,
+ &error_abort, NULL);
+
for (i = 0; i < num_apus; i++) {
- object_initialize_child(obj, "apu-cpu[*]", &s->apu_cpu[i],
- sizeof(s->apu_cpu[i]),
- "cortex-a53-" TYPE_ARM_CPU, &error_abort, NULL);
+ object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
+ &s->apu_cpu[i], sizeof(s->apu_cpu[i]),
+ "cortex-a53-" TYPE_ARM_CPU, &error_abort,
+ NULL);
}
sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
gic_class_name());
@@ -331,10 +342,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", s->secure);
qdev_prop_set_bit(DEVICE(&s->gic),
"has-virtualization-extensions", s->virt);
+ qdev_init_nofail(DEVICE(&s->apu_cluster));
+
/* Realize APUs before realizing the GIC. KVM requires this. */
for (i = 0; i < num_apus; i++) {
char *name;
object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
--
2.19.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension
2018-11-15 9:41 [Qemu-devel] [PATCH v6 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (15 preceding siblings ...)
2018-11-15 9:42 ` [Qemu-devel] [PATCH v6 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters Luc Michel
@ 2018-11-16 10:15 ` Edgar E. Iglesias
16 siblings, 0 replies; 39+ messages in thread
From: Edgar E. Iglesias @ 2018-11-16 10:15 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé, mark.burton, Eduardo Habkost
On Thu, Nov 15, 2018 at 10:41:51AM +0100, Luc Michel wrote:
Hi Luc,
I think I've either reviewed all of the patches or commented on all now.
Great work!
Thanks,
Edgar
> changes since v5:
> - patch 1 Rebased on top of master
>
> - patch 2 Cluster ID handling hardening to ensure uint32_t overflow
> won't cause PID 0 to be used [Edgar]
>
> - patch 2 Fix the GDB process ordering function to avoid uint32_t
> overflow when comparing two cluster_ids [Edgar]
>
> changes since v4:
> - patch 1 add cpu_cluster.[ch] files into MAINTAINERS Machine core
> section (thanks Eduardo!) [Philippe, Eduardo]
>
> - patch 1 revert to uint32_t for cluster IDs [Philippe]
>
> - patch 1 fixed a coding style issue [patchew]
>
> changes since v3:
> - patch 1 cpu_cluster.h: remove QEMU_ from the multiple includes
> guard #ifdef/#define [Alistair]
>
> - patch 1 cpu_cluster.c: include osdep.h first [Alistair]
>
> - patch 1 use uint64_t for cluster ID for prosperity :) [Philippe]
>
> - patch 1 auto-assign a cluster ID to newly created clusters [Philippe]
>
> - patch 2 fix mid-code variable declaration [Alistair]
>
> - patch 3 add a comment in gdb_get_cpu_pid() when retrieving CPU
> parent canonical path [Alistair]
>
> - patch 14 fix a typo in the commit message [Alistair]
>
> changes since v2:
> - patch 1 introducing the cpu-cluster type. I didn't opt for an
> Interface, but I can add one if you think it's necessary.
> For now this class inherits from Device and has a
> cluster-id property, used by the GDB stub to compute a
> PID.
>
> - patch 2 removed GDB group related code as it has been replaced
> with CPU clusters
>
> - patch 2/8 moved GDBProcess target_xml field introduction into patch
> 8 [Philippe]
>
> - patch 3 gdb_get_cpu_pid() now search for CPU being a child of a
> cpu-cluster object. Use the cluster-id to compute the PID.
>
> - patch 4 gdb_get_process() does not rely on s->processes array
> indices anymore as PIDs can now be sparse. Instead, iterate
> over the array to find the process.
>
> - patch 3/4 removed Reviewed-by tags because of substantial changes.
>
> - patch 4/7 read_thread_id() hardening [Philippe]
>
> - patch 12 safer vAttach packet parsing [Phillipe]
>
> - patch 16 put APUs and RPUs in different clusters instead of GDB
> groups
>
> changes since v1:
> - rename qemu_get_thread_id() to gdb_fmt_thread_id() [Philippe]
> - check qemu_strtoul() return value for 'D' packets [Philippe]
>
>
> This series adds support for the multiprocess extension of the GDB
> remote protocol in the QEMU GDB stub.
>
> This extension is useful to split QEMU emulated CPUs in different
> processes from the point of view of the GDB client. It adds the
> possibility to debug different kind of processors (e.g. an AArch64 and
> an ARMv7 CPU) at the same time (it is not possible otherwise since GDB
> expects an SMP view at the thread granularity.
>
> CPUs are grouped using specially named QOM containers. CPUs that are
> children of such a container are grouped under the same GDB process.
>
> The last patch groups the CPUs of different model in the zynqmp machines
> into separate processes.
>
> To test this patchset, you can use the following commands:
>
> (Note that this requires a recent enough GDB, I think GDB 7.2 is OK.
> Also, it must be compiled to support both ARM and AArch64 architectures)
>
> Run QEMU: (-smp 6 in xlnx-zcu102 enables both cortex-a53 and cortex-r5
> CPUs)
>
> qemu-system-aarch64 -M xlnx-zcu102 -gdb tcp::1234 -S -smp 6
>
> Run the following commands in GDB:
>
> target extended :1234
> add-inferior
> inferior 2
> attach 2
> info threads
>
> I want to thanks the Xilinx's QEMU team who sponsored this work for
> their collaboration and their prototype implementation.
>
> Luc Michel (16):
> hw/cpu: introduce CPU clusters
> gdbstub: introduce GDB processes
> gdbstub: add multiprocess support to '?' packets
> gdbstub: add multiprocess support to 'H' and 'T' packets
> gdbstub: add multiprocess support to vCont packets
> gdbstub: add multiprocess support to 'sC' packets
> gdbstub: add multiprocess support to (f|s)ThreadInfo and
> ThreadExtraInfo
> gdbstub: add multiprocess support to Xfer:features:read:
> gdbstub: add multiprocess support to gdb_vm_state_change()
> gdbstub: add multiprocess support to 'D' packets
> gdbstub: add support for extended mode packet
> gdbstub: add support for vAttach packets
> gdbstub: processes initialization on new peer connection
> gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
> gdbstub: add multiprocess extension support
> arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters
>
> include/hw/arm/xlnx-zynqmp.h | 3 +
> include/hw/cpu/cluster.h | 38 +++
> gdbstub.c | 643 ++++++++++++++++++++++++++++++-----
> hw/arm/xlnx-zynqmp.c | 21 +-
> hw/cpu/cluster.c | 59 ++++
> MAINTAINERS | 2 +
> hw/cpu/Makefile.objs | 2 +-
> 7 files changed, 687 insertions(+), 81 deletions(-)
> create mode 100644 include/hw/cpu/cluster.h
> create mode 100644 hw/cpu/cluster.c
>
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread