* [Qemu-devel] [PATCH v8 01/16] hw/cpu: introduce CPU clusters
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 02/16] gdbstub: introduce GDB processes Luc Michel
` (16 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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.
Documentation in cluster.h written with the help of Peter Maydell.
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 | 58 ++++++++++++++++++++++++++++++++++++++++
hw/cpu/cluster.c | 50 ++++++++++++++++++++++++++++++++++
MAINTAINERS | 2 ++
hw/cpu/Makefile.objs | 2 +-
4 files changed, 111 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..7381823243
--- /dev/null
+++ b/include/hw/cpu/cluster.h
@@ -0,0 +1,58 @@
+/*
+ * 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"
+
+/*
+ * CPU Cluster type
+ *
+ * A cluster is a group of CPUs which are all identical and have the same view
+ * of the rest of the system. It is mainly an internal QEMU representation and
+ * does not necessarily match with the notion of clusters on the real hardware.
+ *
+ * If CPUs are not identical (for example, Cortex-A53 and Cortex-A57 CPUs in an
+ * Arm big.LITTLE system) they should be in different clusters. If the CPUs do
+ * not have the same view of memory (for example the main CPU and a management
+ * controller processor) they should be in different clusters.
+ */
+
+#define TYPE_CPU_CLUSTER "cpu-cluster"
+#define CPU_CLUSTER(obj) \
+ OBJECT_CHECK(CPUClusterState, (obj), TYPE_CPU_CLUSTER)
+
+/**
+ * CPUClusterState:
+ * @cluster_id: The cluster ID. This value is for internal use only and should
+ * not be exposed directly to the user or to the guest.
+ *
+ * State of a 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..9d50a235d5
--- /dev/null
+++ b/hw/cpu/cluster.c
@@ -0,0 +1,50 @@
+/*
+ * 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 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),
+ .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 63effdc473..3a6d22fb07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1150,11 +1150,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 02/16] gdbstub: introduce GDB processes
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 01/16] hw/cpu: introduce CPU clusters Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 03/16] gdbstub: add multiprocess support to '?' packets Luc Michel
` (15 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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 represents 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.
A default process is created to handle CPUs that are not in a cluster.
This process gets the PID of the last process PID + 1.
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index c4e4f9f082..2a3aa0f07e 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,34 @@ void gdb_exit(CPUArchState *env, int code)
#ifndef CONFIG_USER_ONLY
qemu_chr_fe_deinit(&s->chr, true);
#endif
}
+/*
+ * Create the process that will contain all the "orphan" CPUs (that are not
+ * part of a CPU cluster). Note that if this process contains no CPUs, it won't
+ * be attachable and thus will be invisible to the user.
+ */
+static void create_default_process(GDBState *s)
+{
+ GDBProcess *process;
+ int max_pid = 0;
+
+ if (s->process_num) {
+ max_pid = s->processes[s->process_num - 1].pid;
+ }
+
+ s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
+ process = &s->processes[s->process_num - 1];
+
+ /* We need an available PID slot for this process */
+ assert(max_pid < UINT32_MAX);
+
+ process->pid = max_pid + 1;
+ process->attached = false;
+}
+
#ifdef CONFIG_USER_ONLY
int
gdb_handlesig(CPUState *cpu, int sig)
{
GDBState *s;
@@ -1846,10 +1879,11 @@ static bool gdb_accept(void)
}
s = g_malloc0(sizeof(GDBState));
s->c_cpu = first_cpu;
s->g_cpu = first_cpu;
+ create_default_process(s);
s->fd = fd;
gdb_has_xml = false;
gdbserver_state = s;
return true;
@@ -2002,10 +2036,68 @@ 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(cluster->cluster_id != 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) {
+ /* Sort by PID */
+ qsort(s->processes, s->process_num, sizeof(s->processes[0]), pid_order);
+ }
+
+ create_default_process(s);
+}
+
+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 +2150,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 03/16] gdbstub: add multiprocess support to '?' packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 01/16] hw/cpu: introduce CPU clusters Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 02/16] gdbstub: introduce GDB processes Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
` (14 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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 default 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 | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 2a3aa0f07e..07508c2e6b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -638,10 +638,57 @@ 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 = NULL;
+ Object *obj;
+ CPUClusterState *cluster;
+ uint32_t ret;
+
+ path = object_get_canonical_path(OBJECT(cpu));
+
+ if (path == NULL) {
+ /* Return the default process' PID */
+ ret = s->processes[s->process_num - 1].pid;
+ goto out;
+ }
+
+ name = object_get_canonical_path_component(OBJECT(cpu));
+ assert(name != NULL);
+
+ /*
+ * 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) {
+ /* Return the default process' PID */
+ ret = s->processes[s->process_num - 1].pid;
+ goto out;
+ }
+
+ cluster = CPU_CLUSTER(obj);
+ ret = cluster->cluster_id + 1;
+
+out:
+ g_free(name);
+ g_free(path);
+
+ return ret;
+
+#else
+ /* TODO: In user mode, we should use the task state PID */
+ return s->processes[s->process_num - 1].pid;
+#endif
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
{
size_t len;
int i;
@@ -907,10 +954,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 +1078,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (2 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 03/16] gdbstub: add multiprocess support to '?' packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-08 0:55 ` Alistair Francis
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
` (13 subsequent siblings)
17 siblings, 1 reply; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
gdbstub.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 136 insertions(+), 18 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 07508c2e6b..911faa225a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -685,10 +685,75 @@ out:
/* TODO: In user mode, we should use the task state PID */
return s->processes[s->process_num - 1].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;
+
+ if (!tid) {
+ /* 0 means any thread, we take the first one */
+ tid = 1;
+ }
+
+ cpu = find_cpu(tid);
+
+ 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;
@@ -941,23 +1006,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",
@@ -967,10 +1019,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 &&
@@ -1075,16 +1181,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++;
@@ -1288,16 +1396,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) {
@@ -1313,12 +1427,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
@ 2018-12-08 0:55 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2018-12-08 0:55 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel@nongnu.org Developers, Peter Maydell, Eduardo Habkost,
Alistair Francis, Mark Burton, Philippe Mathieu-Daudé,
Sai Pavan Boddu, Edgar Iglesias, qemu-arm
On Fri, Dec 7, 2018 at 1:02 AM Luc Michel <luc.michel@greensocs.com> 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>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 136 insertions(+), 18 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 07508c2e6b..911faa225a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -685,10 +685,75 @@ out:
> /* TODO: In user mode, we should use the task state PID */
> return s->processes[s->process_num - 1].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;
> +
> + if (!tid) {
> + /* 0 means any thread, we take the first one */
> + tid = 1;
> + }
> +
> + cpu = find_cpu(tid);
> +
> + 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;
> @@ -941,23 +1006,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",
> @@ -967,10 +1019,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 &&
> @@ -1075,16 +1181,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++;
> @@ -1288,16 +1396,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) {
> @@ -1313,12 +1427,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.2
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (3 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 06/16] gdbstub: add multiprocess support to 'sC' packets Luc Michel
` (12 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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_attached_cpu() and gdb_next_attached_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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 102 insertions(+), 15 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 911faa225a..77b3dbb2c8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -721,10 +721,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;
@@ -750,10 +780,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_attached_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_attached_cpu(const GDBState *s)
+{
+ CPUState *cpu = first_cpu;
+ GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+ if (!process->attached) {
+ return gdb_next_attached_cpu(s, cpu);
+ }
+
+ return cpu;
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
{
size_t len;
int i;
@@ -1088,14 +1149,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) {
@@ -1134,29 +1197,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_attached_cpu(s);
+ while (cpu) {
+ if (newstates[cpu->cpu_index] == 1) {
+ newstates[cpu->cpu_index] = cur_action;
}
+
+ cpu = gdb_next_attached_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;
@@ -1164,10 +1250,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 06/16] gdbstub: add multiprocess support to 'sC' packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (4 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
` (11 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
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 77b3dbb2c8..bea0215f30 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1553,13 +1553,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (5 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 06/16] gdbstub: add multiprocess support to 'sC' packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-12 17:35 ` Alistair Francis
2019-01-29 4:56 ` Max Filippov
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
` (10 subsequent siblings)
17 siblings, 2 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
gdbstub.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index bea0215f30..770915446a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1267,11 +1267,10 @@ out:
static int gdb_handle_packet(GDBState *s, const char *line_buf)
{
CPUState *cpu;
CPUClass *cc;
const char *p;
- uint32_t thread;
uint32_t pid, tid;
int ch, reg_size, type, res;
uint8_t mem_buf[MAX_PACKET_LENGTH];
char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
char thread_id[16];
@@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
snprintf(buf, sizeof(buf), "QC%s",
gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
put_packet(s, buf);
break;
} else if (strcmp(p,"fThreadInfo") == 0) {
- s->query_cpu = first_cpu;
+ s->query_cpu = gdb_first_attached_cpu(s);
goto report_cpuinfo;
} else if (strcmp(p,"sThreadInfo") == 0) {
report_cpuinfo:
if (s->query_cpu) {
- snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
+ snprintf(buf, sizeof(buf), "m%s",
+ gdb_fmt_thread_id(s, s->query_cpu,
+ thread_id, sizeof(thread_id)));
put_packet(s, buf);
- s->query_cpu = CPU_NEXT(s->query_cpu);
+ s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
} else
put_packet(s, "l");
break;
} else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
- thread = strtoull(p+16, (char **)&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 and name in multiprocess mode */
+ ObjectClass *oc = object_get_class(OBJECT(cpu));
+ const char *cpu_model = object_class_get_name(oc);
+ char *cpu_name =
+ object_get_canonical_path_component(OBJECT(cpu));
+ len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+ "%s %s [%s]", cpu_model, cpu_name,
+ cpu->halted ? "halted " : "running");
+ g_free(cpu_name);
+ } else {
+ /* memtohex() doubles the required space */
+ len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+ "CPU#%d [%s]", cpu->cpu_index,
+ cpu->halted ? "halted " : "running");
+ }
trace_gdbstub_op_extra_info((char *)mem_buf);
memtohex(buf, mem_buf, len);
put_packet(s, buf);
}
break;
--
2.19.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
@ 2018-12-12 17:35 ` Alistair Francis
2019-01-29 4:56 ` Max Filippov
1 sibling, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2018-12-12 17:35 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Eduardo Habkost, alistair, mark.burton,
Philippe Mathieu-Daudé,
saipava, edgari, qemu-arm
On 07/12/2018 1:01 am, Luc Michel wrote:
> Change the thread info related packets handling to support multiprocess
> extension.
>
> Add the CPUs class name in the extra info to help differentiate
> them in multiprocess mode.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> gdbstub.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index bea0215f30..770915446a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1267,11 +1267,10 @@ out:
> static int gdb_handle_packet(GDBState *s, const char *line_buf)
> {
> CPUState *cpu;
> CPUClass *cc;
> const char *p;
> - uint32_t thread;
> uint32_t pid, tid;
> int ch, reg_size, type, res;
> uint8_t mem_buf[MAX_PACKET_LENGTH];
> char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
> char thread_id[16];
> @@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> snprintf(buf, sizeof(buf), "QC%s",
> gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
> put_packet(s, buf);
> break;
> } else if (strcmp(p,"fThreadInfo") == 0) {
> - s->query_cpu = first_cpu;
> + s->query_cpu = gdb_first_attached_cpu(s);
> goto report_cpuinfo;
> } else if (strcmp(p,"sThreadInfo") == 0) {
> report_cpuinfo:
> if (s->query_cpu) {
> - snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
> + snprintf(buf, sizeof(buf), "m%s",
> + gdb_fmt_thread_id(s, s->query_cpu,
> + thread_id, sizeof(thread_id)));
> put_packet(s, buf);
> - s->query_cpu = CPU_NEXT(s->query_cpu);
> + s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
> } else
> put_packet(s, "l");
> break;
> } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> - thread = strtoull(p+16, (char **)&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 and name in multiprocess mode */
> + ObjectClass *oc = object_get_class(OBJECT(cpu));
> + const char *cpu_model = object_class_get_name(oc);
> + char *cpu_name =
> + object_get_canonical_path_component(OBJECT(cpu));
> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> + "%s %s [%s]", cpu_model, cpu_name,
> + cpu->halted ? "halted " : "running");
> + g_free(cpu_name);
> + } else {
> + /* memtohex() doubles the required space */
> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> + "CPU#%d [%s]", cpu->cpu_index,
> + cpu->halted ? "halted " : "running");
> + }
> trace_gdbstub_op_extra_info((char *)mem_buf);
> memtohex(buf, mem_buf, len);
> put_packet(s, buf);
> }
> break;
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
2018-12-12 17:35 ` Alistair Francis
@ 2019-01-29 4:56 ` Max Filippov
2019-01-29 10:05 ` Peter Maydell
1 sibling, 1 reply; 26+ messages in thread
From: Max Filippov @ 2019-01-29 4:56 UTC (permalink / raw)
To: Luc Michel
Cc: qemu-devel, Peter Maydell, Eduardo Habkost, alistair,
mark.burton, Philippe Mathieu-Daudé,
saipava, Edgar Iglesias, qemu-arm
Hello,
On Fri, Dec 7, 2018 at 1:04 AM Luc Michel <luc.michel@greensocs.com> 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>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> gdbstub.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
Starting with this commit it is no longer possible to kill QEMU
with the 'kill' command from the gdb. This was a nice feature,
was this removal intentional, or is it just an implementation
bug?
> diff --git a/gdbstub.c b/gdbstub.c
> index bea0215f30..770915446a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1267,11 +1267,10 @@ out:
> static int gdb_handle_packet(GDBState *s, const char *line_buf)
> {
> CPUState *cpu;
> CPUClass *cc;
> const char *p;
> - uint32_t thread;
> uint32_t pid, tid;
> int ch, reg_size, type, res;
> uint8_t mem_buf[MAX_PACKET_LENGTH];
> char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
> char thread_id[16];
> @@ -1563,30 +1562,48 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> snprintf(buf, sizeof(buf), "QC%s",
> gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
> put_packet(s, buf);
> break;
> } else if (strcmp(p,"fThreadInfo") == 0) {
> - s->query_cpu = first_cpu;
> + s->query_cpu = gdb_first_attached_cpu(s);
> goto report_cpuinfo;
> } else if (strcmp(p,"sThreadInfo") == 0) {
> report_cpuinfo:
> if (s->query_cpu) {
> - snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
> + snprintf(buf, sizeof(buf), "m%s",
> + gdb_fmt_thread_id(s, s->query_cpu,
> + thread_id, sizeof(thread_id)));
> put_packet(s, buf);
> - s->query_cpu = CPU_NEXT(s->query_cpu);
> + s->query_cpu = gdb_next_attached_cpu(s, s->query_cpu);
> } else
> put_packet(s, "l");
> break;
> } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
> - thread = strtoull(p+16, (char **)&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 and name in multiprocess mode */
> + ObjectClass *oc = object_get_class(OBJECT(cpu));
> + const char *cpu_model = object_class_get_name(oc);
> + char *cpu_name =
> + object_get_canonical_path_component(OBJECT(cpu));
> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> + "%s %s [%s]", cpu_model, cpu_name,
> + cpu->halted ? "halted " : "running");
> + g_free(cpu_name);
> + } else {
> + /* memtohex() doubles the required space */
> + len = snprintf((char *)mem_buf, sizeof(buf) / 2,
> + "CPU#%d [%s]", cpu->cpu_index,
> + cpu->halted ? "halted " : "running");
> + }
> trace_gdbstub_op_extra_info((char *)mem_buf);
> memtohex(buf, mem_buf, len);
> put_packet(s, buf);
> }
> break;
> --
> 2.19.2
>
>
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2019-01-29 4:56 ` Max Filippov
@ 2019-01-29 10:05 ` Peter Maydell
2019-01-29 20:25 ` Max Filippov
0 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2019-01-29 10:05 UTC (permalink / raw)
To: Max Filippov
Cc: Luc Michel, qemu-devel, Eduardo Habkost, Alistair Francis,
Mark Burton, Philippe Mathieu-Daudé,
Sai Pavan Boddu, Edgar Iglesias, qemu-arm
On Tue, 29 Jan 2019 at 04:56, Max Filippov <jcmvbkbc@gmail.com> wrote:
>
> Hello,
>
> On Fri, Dec 7, 2018 at 1:04 AM Luc Michel <luc.michel@greensocs.com> 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>
> > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> > gdbstub.c | 37 +++++++++++++++++++++++++++----------
> > 1 file changed, 27 insertions(+), 10 deletions(-)
>
> Starting with this commit it is no longer possible to kill QEMU
> with the 'kill' command from the gdb. This was a nice feature,
> was this removal intentional, or is it just an implementation
> bug?
That sounds like a bug. I think with the multiprocess extensions
available gdb may switch from killing using the 'k' packet to using
the 'vKill;pid' packet, which we don't implement? Looking at
what gdb is sending ('set debug remote 1' turns on logging in
gdb of remote protocol packets) would let us check that theory.
It's not clear how our implementation should deal with being asked
to kill just one process if we have more than one, though...
thanks
-- PMM
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo
2019-01-29 10:05 ` Peter Maydell
@ 2019-01-29 20:25 ` Max Filippov
0 siblings, 0 replies; 26+ messages in thread
From: Max Filippov @ 2019-01-29 20:25 UTC (permalink / raw)
To: Peter Maydell
Cc: Luc Michel, qemu-devel, Eduardo Habkost, Alistair Francis,
Mark Burton, Philippe Mathieu-Daudé,
Sai Pavan Boddu, Edgar Iglesias, qemu-arm
On Tue, Jan 29, 2019 at 2:05 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > Starting with this commit it is no longer possible to kill QEMU
> > with the 'kill' command from the gdb. This was a nice feature,
> > was this removal intentional, or is it just an implementation
> > bug?
>
> That sounds like a bug. I think with the multiprocess extensions
> available gdb may switch from killing using the 'k' packet to using
> the 'vKill;pid' packet, which we don't implement? Looking at
> what gdb is sending ('set debug remote 1' turns on logging in
> gdb of remote protocol packets) would let us check that theory.
That's correct:
(gdb) kill
Kill the program being debugged? (y or n) y
Sending packet: $vKill;1#6e...Ack
Packet received:
Packet vKill (kill) is NOT supported
Can't kill process
> It's not clear how our implementation should deal with being asked
> to kill just one process if we have more than one, though...
I'll send a fix that restores previous behavior in case of a single
inferior process.
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v8 08/16] gdbstub: add multiprocess support to Xfer:features:read:
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (6 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
` (9 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 770915446a..c66bf37b49 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,
@@ -811,55 +813,57 @@ static CPUState *gdb_first_attached_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) {
@@ -1265,10 +1269,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];
@@ -1648,18 +1653,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;
}
@@ -2068,10 +2074,11 @@ static void create_default_process(GDBState *s)
/* We need an available PID slot for this process */
assert(max_pid < UINT32_MAX);
process->pid = max_pid + 1;
process->attached = false;
+ process->target_xml[0] = '\0';
}
#ifdef CONFIG_USER_ONLY
int
gdb_handlesig(CPUState *cpu, int sig)
@@ -2342,10 +2349,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(cluster->cluster_id != 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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (7 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 08/16] gdbstub: add multiprocess support to Xfer:features:read: Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
` (8 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index c66bf37b49..4645d59d8c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1722,10 +1722,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;
@@ -1733,10 +1734,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:
@@ -1750,12 +1759,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();
@@ -1793,11 +1802,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 10/16] gdbstub: add multiprocess support to 'D' packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (8 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 09/16] gdbstub: add multiprocess support to gdb_vm_state_change() Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 11/16] gdbstub: add support for extended mode packet Luc Michel
` (7 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 53 insertions(+), 7 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 4645d59d8c..48c2571ebe 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1046,24 +1046,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)
{
@@ -1338,13 +1353,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_attached_cpu(s);
+ }
+
+ if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+ s->g_cpu = gdb_first_attached_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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 11/16] gdbstub: add support for extended mode packet
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (9 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 10/16] gdbstub: add multiprocess support to 'D' packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 12/16] gdbstub: add support for vAttach packets Luc Michel
` (6 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index 48c2571ebe..1cf1bea428 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1301,10 +1301,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 12/16] gdbstub: add support for vAttach packets
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (10 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 11/16] gdbstub: add support for extended mode packet Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 13/16] gdbstub: processes initialization on new peer connection Luc Michel
` (5 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index 1cf1bea428..8601be533b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1347,10 +1347,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 13/16] gdbstub: processes initialization on new peer connection
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (11 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 12/16] gdbstub: add support for vAttach packets Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached Luc Michel
` (4 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
gdbstub.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 8601be533b..81e7742847 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2265,13 +2265,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_default_process(s);
+ s->processes[0].attached = true;
+ s->c_cpu = gdb_first_attached_cpu(s);
+ s->g_cpu = s->c_cpu;
s->fd = fd;
gdb_has_xml = false;
gdbserver_state = s;
return true;
@@ -2353,12 +2354,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_attached_cpu(s);
+ s->g_cpu = s->c_cpu;
+
vm_stop(RUN_STATE_PAUSED);
gdb_has_xml = false;
break;
default:
break;
@@ -2543,19 +2555,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (12 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 13/16] gdbstub: processes initialization on new peer connection Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 15/16] gdbstub: add multiprocess extension support Luc Michel
` (3 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
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 81e7742847..48b49195f7 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1796,10 +1796,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 15/16] gdbstub: add multiprocess extension support
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (13 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters Luc Michel
` (2 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
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 48b49195f7..bb82d97c5c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1730,10 +1730,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v8 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (14 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 15/16] gdbstub: add multiprocess extension support Luc Michel
@ 2018-12-07 9:01 ` Luc Michel
2018-12-17 8:23 ` [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
2019-01-04 15:12 ` Peter Maydell
17 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2018-12-07 9:01 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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
include/hw/arm/xlnx-zynqmp.h | 3 +++
hw/arm/xlnx-zynqmp.c | 23 +++++++++++++++++++----
2 files changed, 22 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..c67ac2e64a 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -176,16 +176,23 @@ 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_prop_set_uint32(DEVICE(&s->rpu_cluster), "cluster-id", 1);
+
+ 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 +218,20 @@ 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);
+ qdev_prop_set_uint32(DEVICE(&s->apu_cluster), "cluster-id", 0);
+
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 +344,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.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (15 preceding siblings ...)
2018-12-07 9:01 ` [Qemu-devel] [PATCH v8 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters Luc Michel
@ 2018-12-17 8:23 ` Luc Michel
2018-12-17 12:43 ` Peter Maydell
2019-01-04 15:12 ` Peter Maydell
17 siblings, 1 reply; 26+ messages in thread
From: Luc Michel @ 2018-12-17 8:23 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-arm, Peter Maydell, saipava, edgari, alistair,
Philippe Mathieu-Daudé,
mark.burton, Eduardo Habkost
Hi all,
What do you think of this re-roll? Is it in good shape for upstreaming?
Thanks!
--
Luc
On 12/7/18 10:01 AM, Luc Michel wrote:
> changes since v7:
> - patch 1 Add documentation about cpu-cluster [Eduardo, Peter]
>
> - patch 1 Remove the cluster-id auto-assign mechanism [Eduardo]
>
> - patch 2 Replace create_unique_process() with
> create_default_process() that is now called even if some
> clusters are found. This default process is used for
> CPUs that are not in a cluster [Eduardo, Peter]
>
> - patch 3 Refactor and harden gdb_get_cpu_pid() [Philippe]
>
> - patch 16 Go back to cluster-id manual assignment [Eduardo]
>
> changes since v6:
> - patch 4 Fix a refactoring issue in gdb_get_cpu [Edgar]
>
> - patch 5 Renamed gdb_first_cpu/gdb_next_cpu to
> gdb_first_attached_cpu/gdb_next_attached_cpu [Edgar]
>
> - patch 7 Added the CPU name and removed the CPU index in the
> ThreadInfo packet in multiprocess mode [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 | 58 +++
> gdbstub.c | 662 +++++++++++++++++++++++++++++++----
> hw/arm/xlnx-zynqmp.c | 23 +-
> hw/cpu/cluster.c | 50 +++
> MAINTAINERS | 2 +
> hw/cpu/Makefile.objs | 2 +-
> 7 files changed, 719 insertions(+), 81 deletions(-)
> create mode 100644 include/hw/cpu/cluster.h
> create mode 100644 hw/cpu/cluster.c
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension
2018-12-17 8:23 ` [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
@ 2018-12-17 12:43 ` Peter Maydell
0 siblings, 0 replies; 26+ messages in thread
From: Peter Maydell @ 2018-12-17 12:43 UTC (permalink / raw)
To: Luc Michel
Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
Alistair Francis, Philippe Mathieu-Daudé,
Mark Burton, Eduardo Habkost
On Mon, 17 Dec 2018 at 08:23, Luc Michel <luc.michel@greensocs.com> wrote:
>
> Hi all,
>
> What do you think of this re-roll? Is it in good shape for upstreaming?
It's on my list to look at still, but I'm off work til
January now, I'm afraid.
thanks
-- PMM
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension
2018-12-07 9:01 [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
` (16 preceding siblings ...)
2018-12-17 8:23 ` [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension Luc Michel
@ 2019-01-04 15:12 ` Peter Maydell
2019-01-07 8:56 ` Luc Michel
17 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2019-01-04 15:12 UTC (permalink / raw)
To: Luc Michel
Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
Alistair Francis, Philippe Mathieu-Daudé,
Mark Burton, Eduardo Habkost
On Fri, 7 Dec 2018 at 09:01, Luc Michel <luc.michel@greensocs.com> wrote:
> 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.
I've applied this patchset to target-arm.next, thanks.
(I fixed up a few checkpatch nits about block comment style.)
> 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
This seems a bit clumsy -- is it just the limitations of
gdb here? I was expecting that just connecting to the
remote would cause gdb to interrogate it and attach to
all the inferior processes that it had.
thanks
-- PMM
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v8 00/16] gdbstub: support for the multiprocess extension
2019-01-04 15:12 ` Peter Maydell
@ 2019-01-07 8:56 ` Luc Michel
0 siblings, 0 replies; 26+ messages in thread
From: Luc Michel @ 2019-01-07 8:56 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
Alistair Francis, Philippe Mathieu-Daudé,
Mark Burton, Eduardo Habkost
On 1/4/19 4:12 PM, Peter Maydell wrote:
> On Fri, 7 Dec 2018 at 09:01, Luc Michel <luc.michel@greensocs.com> wrote:
>> 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.
>
> I've applied this patchset to target-arm.next, thanks.
> (I fixed up a few checkpatch nits about block comment style.)
Thanks!
>
>> 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
>
> This seems a bit clumsy -- is it just the limitations of
> gdb here? I was expecting that just connecting to the
> remote would cause gdb to interrogate it and attach to
> all the inferior processes that it had.
AFAIK this is a "limitation" of GDB. In QEMU context it seems like a
limitation, but when you are connecting to a remote gdbserver that
exposes all the processes currently running on an OS, you probably don't
want to attach them all automatically.
Luc
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 26+ messages in thread