[RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Srikar Dronamraju]

Hi, 

This patch implements an in-kernel gdb stub.
It provides an interface between gdb and Linux Kernel by implementing
the remote serial protocol.  This gdbstub uses utrace infrastructure.
This patch provides register set access, signal mapping, process event
handling, input/output operations.

/proc/<pid>/gdb was chosen as file for gdb to interact with the
process through remote serial protocol. 

Hence users would have to use "target remote /proc/<pid>/gdb" command
on gdb prompt to start using this infrastructure.

For Breakpointing support, gdbstub needs User space breakpointing
layer and uprobes layer which will be posted later.

Here is an illustration of what all this can do.

$ pgrep zsh 
8865
$ gdb /bin/zsh 
GNU gdb (GDB) 7.0.0.20091030
..... <snipped gdb header > ...............
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /bin/zsh...Reading symbols from /usr/lib/debug/bin/zsh.debug...done.
(no debugging symbols found)...done.
(gdb) target remote /proc/8865/gdb
Remote debugging using /proc/8865/gdb
.... <snipped several "Reading symbols" and "Loaded symbols" > ......
Loaded symbols for /usr/lib64/zsh/4.2.6/zsh/zle.so
0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
(gdb) info registers 
rax            0xfffffffffffffe00	-512
rbx            0x6a01c8	6947272
rcx            0xffffffffffffffff	-1
rdx            0x1	1
rsi            0x7fff507ad5ef	140734543615471
rdi            0xa	10
rbp            0x0	0x0
rsp            0x7fff507ad3e8	0x7fff507ad3e8
r8             0xffffffff	4294967295
r9             0xffffffff	4294967295
r10            0x7d	125
r11            0x246	582
r12            0x7f43b87a9398	139928834577304
r13            0x0	0
r14            0x0	0
r15            0xd	13
rip            0x3f1dec5f00	0x3f1dec5f00 <__read_nocancel+7>
eflags         0x246	[ PF ZF IF ]
cs             0x33	51
ss             0x2b	43
ds             0x0	0
es             0x0	0
fs             0x0	0
gs             0x0	0
fctrl          0x0	0
fstat          0x0	0
ftag           0x0	0
fiseg          0x0	0
fioff          0x0	0
foseg          0x0	0
fooff          0x0	0
fop            0x0	0
mxcsr          0x0	[ ]
(gdb) bt
#0  0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
#1  0x00007f43b858a39f in raw_getkey (keytmout=0) at /usr/include/bits/unistd.h:35
#2  getkey (keytmout=0) at zle_main.c:610
#3  0x00007f43b8584bf1 in getkeybuf (km=0x0, funcp=0x7fff507ad718, strp=0x7fff507ad710) at zle_keymap.c:1313
#4  getkeymapcmd (km=0x0, funcp=0x7fff507ad718, strp=0x7fff507ad710) at zle_keymap.c:1275
#5  0x00007f43b8584ef0 in getkeycmd () at zle_keymap.c:1349
#6  0x00007f43b8588281 in zlecore () at zle_main.c:695
#7  0x00007f43b85888f8 in zleread (lp=<value optimized out>, rp=<value optimized out>, flags=<value optimized out>, 
    context=<value optimized out>) at zle_main.c:863
#8  0x00000000004388a0 in inputline () at input.c:278
#9  ingetc () at input.c:214
#10 0x000000000043339d in ihgetc () at hist.c:241
#11 0x000000000044126b in gettok () at lex.c:631
#12 0x0000000000441a88 in yylex () at lex.c:347
#13 0x000000000045cec7 in parse_event () at parse.c:449
#14 0x0000000000437328 in loop (toplevel=1, justonce=0) at init.c:128
#15 0x0000000000438021 in zsh_main (argc=<value optimized out>, argv=0x7fff507ae048) at init.c:1280
#16 0x0000003f1de1d994 in __libc_start_main (main=<value optimized out>, argc=<value optimized out>, 
    ubp_av=<value optimized out>, init=<value optimized out>, fini=<value optimized out>, rtld_fini=<value optimized out>, 
    stack_end=Could not find the frame base for "__libc_start_main".
) at libc-start.c:231
#17 0x000000000040cc09 in _start ()
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
(gdb) bt
#0  0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
#1  0x00007f43b858a39f in raw_getkey (keytmout=0) at /usr/include/bits/unistd.h:35
#2  getkey (keytmout=0) at zle_main.c:610
#3  0x00007f43b8584bf1 in getkeybuf (km=0x0, funcp=0x7fff507ad718, strp=0x7fff507ad710) at zle_keymap.c:1313
#4  getkeymapcmd (km=0x0, funcp=0x7fff507ad718, strp=0x7fff507ad710) at zle_keymap.c:1275
#5  0x00007f43b8584ef0 in getkeycmd () at zle_keymap.c:1349
#6  0x00007f43b8588281 in zlecore () at zle_main.c:695
#7  0x00007f43b85888f8 in zleread (lp=<value optimized out>, rp=<value optimized out>, flags=<value optimized out>, 
    context=<value optimized out>) at zle_main.c:863
#8  0x00000000004388a0 in inputline () at input.c:278
#9  ingetc () at input.c:214
#10 0x000000000043339d in ihgetc () at hist.c:241
#11 0x000000000044126b in gettok () at lex.c:631
#12 0x0000000000441a88 in yylex () at lex.c:347
#13 0x000000000045cec7 in parse_event () at parse.c:449
#14 0x0000000000437328 in loop (toplevel=1, justonce=0) at init.c:128
#15 0x0000000000438021 in zsh_main (argc=<value optimized out>, argv=0x7fff507ae048) at init.c:1280
#16 0x0000003f1de1d994 in __libc_start_main (main=<value optimized out>, argc=<value optimized out>, 
    ubp_av=<value optimized out>, init=<value optimized out>, fini=<value optimized out>, rtld_fini=<value optimized out>, 
    stack_end=Could not find the frame base for "__libc_start_main".
) at libc-start.c:231
#17 0x000000000040cc09 in _start ()
(gdb) b zrefresh
Breakpoint 1 at 0x7f43b858f600: file zle_refresh.c, line 278.
### Note: This is 
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x0000003f1dec5f00 in __read_nocancel () from /lib64/libc.so.6
(gdb)  info b
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <PENDING>          in zrefresh at zle_refresh.c:278
#### HERE breakpoint is still pending because, gdbstub needs more support for
#### breakpointing.
(gdb) 
(gdb) q
A debugging session is active.

	Inferior 1 [Remote target] will be killed.

Quit anyway? (y or n) y
$ pgrep zsh 
8865
$

I request you to please review this and lets us know your comments.

Signed-off-by: "Frank Ch. Eigler" <fche@redhat.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
 fs/proc/base.c         |    4 +
 include/linux/utrace.h |    4 +
 init/Kconfig           |   26 +-
 kernel/Makefile        |    1 +
 kernel/utrace-gdb.c    | 1314 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 1340 insertions(+), 9 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index af643b5..98bf9b0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -77,6 +77,7 @@
 #include <linux/audit.h>
 #include <linux/poll.h>
 #include <linux/nsproxy.h>
+#include <linux/utrace.h>
 #include <linux/oom.h>
 #include <linux/elf.h>
 #include <linux/pid_namespace.h>
@@ -2562,6 +2563,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	INF("io",	S_IRUGO, proc_tgid_io_accounting),
 #endif
+#ifdef CONFIG_UTRACE_GDB
+	REG("gdb",	S_IRUSR|S_IWUSR, proc_gdb_operations),
+#endif
 };
 
 static int proc_tgid_base_readdir(struct file * filp,
diff --git a/include/linux/utrace.h b/include/linux/utrace.h
index 23d934d..c78a365 100644
--- a/include/linux/utrace.h
+++ b/include/linux/utrace.h
@@ -726,4 +726,8 @@ static inline __must_check int utrace_barrier_pid(struct pid *pid,
 
 #endif	/* CONFIG_UTRACE */
 
+#ifdef CONFIG_UTRACE_GDB
+extern const struct file_operations proc_gdb_operations;
+#endif
+
 #endif	/* linux/utrace.h */
diff --git a/init/Kconfig b/init/Kconfig
index 10cbcf4..dd04f63 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -295,15 +295,6 @@ config AUDIT
 	  logging of avc messages output).  Does not do system-call
 	  auditing without CONFIG_AUDITSYSCALL.
 
-config UTRACE
-	bool "Infrastructure for tracing and debugging user processes"
-	depends on EXPERIMENTAL
-	depends on HAVE_ARCH_TRACEHOOK
-	help
-	  Enable the utrace process tracing interface.  This is an internal
-	  kernel interface exported to kernel modules, to track events in
-	  user threads, extract and change user thread state.
-
 config AUDITSYSCALL
 	bool "Enable system-call auditing support"
 	depends on AUDIT && (X86 || PPC || S390 || IA64 || UML || SPARC64 || SUPERH)
@@ -1214,6 +1205,23 @@ config STOP_MACHINE
 	help
 	  Need stop_machine() primitive.
 
+menuconfig UTRACE
+	bool "Infrastructure for tracing and debugging user processes"
+	depends on EXPERIMENTAL
+	depends on HAVE_ARCH_TRACEHOOK
+	help
+	  Enable the utrace process tracing interface.  This is an internal
+	  kernel interface exported to kernel modules, to track events in
+	  user threads, extract and change user thread state.
+
+config UTRACE_GDB
+	bool "/proc/<pid>/gdb file for gdb remote connection"
+	select UTRACE
+        default y
+	help
+	  Enable the utrace-based /proc/<pid>/gdb process debugging
+	  interface, for connection using the gdb remote protocol.
+
 source "block/Kconfig"
 
 config PREEMPT_NOTIFIERS
diff --git a/kernel/Makefile b/kernel/Makefile
index 8f41620..eb3679c 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_UTRACE) += utrace.o
+obj-$(CONFIG_UTRACE_GDB) += utrace-gdb.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o audit_watch.o
 obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
 obj-$(CONFIG_GCOV_KERNEL) += gcov/
diff --git a/kernel/utrace-gdb.c b/kernel/utrace-gdb.c
new file mode 100644
index 0000000..e721ae4
--- /dev/null
+++ b/kernel/utrace-gdb.c
@@ -0,0 +1,1314 @@
+/*
+ * utrace-based gdb remote protocol server for user processes
+ *
+ * Copyright (C) 2009 Red Hat, Inc.  All rights reserved.
+ * Copyright (C) IBM Corporation, 2009
+ *
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License v.2.
+ *
+ * Red Hat Author: Frank Ch. Eigler
+ */
+
+/*
+ * TODO list:
+ *
+ * floating-point register bank support for g/G/P/etc. packets
+ * multithreaded process support
+ * concurrent debugger support for same process
+ */
+#define DEBUG 1
+
+#include <asm/syscall.h>
+#include <linux/signal.h>
+#include <linux/ptrace.h>
+#include <linux/err.h>
+#include <linux/pid.h>
+#include <linux/sched.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/proc_fs.h>
+#include <linux/ctype.h>
+#include <linux/regset.h>
+#include <linux/utrace.h>
+#include <linux/tracehook.h>
+
+/*
+ * struct gdb_connection - Tracks one active gdb-process session.
+ */
+
+#define GDB_BUFMAX 4096
+#define MAX_REG_WIDTH 16	/* Maximum Register Width */
+
+struct gdb_connection {
+	pid_t target;
+	struct utrace_engine *engine;
+
+	/* changed under output_mutex */
+	int at_quiesce_do;
+
+	/* set <=> at_quiesce_do = UTRACE_STOP */
+	unsigned char stopcode[GDB_BUFMAX];
+	int pass_signals;
+	int stop_signals;
+	/* XXX: per-thread later */
+
+	char output_buf[GDB_BUFMAX];
+	size_t output_buf_size;
+	loff_t output_buf_read;
+
+	/* Protects output_buf, at_quiesce_do */
+	struct mutex output_mutex;
+	wait_queue_head_t output_wait;
+
+	char input_buf[GDB_BUFMAX];
+	size_t input_buf_size;
+
+	/* Protects input_buf. */
+	struct mutex input_mutex;
+	wait_queue_head_t input_wait;
+
+	struct list_head link;
+};
+
+static LIST_HEAD(gdb_connections);
+static DEFINE_MUTEX(gdb_connections_mutex);
+static const struct utrace_engine_ops gdb_utrace_ops;
+
+/* ------------------------------------------------------------------------ */
+
+/*
+ * Map from kernel-side signal numbers (include/asm/signal.h) to gdb
+ * remote protocol signal numbers (include/gdb/signals.h).
+ */
+unsigned gdb_signal_map[] = {
+	[SIGHUP] = 1,
+	[SIGINT] = 2,
+	[SIGQUIT] = 3,
+	[SIGILL] = 4,
+	[SIGTRAP] = 5,
+	[SIGABRT] = 6,
+	[SIGBUS] = 10,
+	[SIGFPE] = 8,
+	[SIGKILL] = 9,
+	[SIGUSR1] = 30,
+	[SIGSEGV] = 11,
+	[SIGUSR2] = 31,
+	[SIGPIPE] = 13,
+	[SIGALRM] = 14,
+	[SIGTERM] = 15,
+	[SIGCHLD] = 20,
+	[SIGCONT] = 19,
+	[SIGSTOP] = 17,
+	[SIGTSTP] = 18,
+	[SIGTTIN] = 21,
+	[SIGTTOU] = 22,
+	[SIGURG] = 16,
+	[SIGXCPU] = 24,
+	[SIGXFSZ] = 25,
+	[SIGVTALRM] = 26,
+	[SIGPROF] = 27,
+	[SIGWINCH] = 28,
+	[SIGIO] = 23,
+	[SIGPWR] = 32,
+	[SIGSYS] = 12,
+};
+
+/*
+ * Map the incoming signo to TARGET_SIGNAL_foo, if possible.  If not,
+ * pass it through.
+ */
+static unsigned map_signal_kern2gdb(unsigned kernel_signo)
+{
+	if (kernel_signo < sizeof(gdb_signal_map) / sizeof(unsigned))
+		return gdb_signal_map[kernel_signo] ? : kernel_signo;
+	return kernel_signo;
+}
+
+static unsigned map_signal_gdb2kern(unsigned gdb_signo)
+{
+	unsigned i;
+	for (i = 0; i < sizeof(gdb_signal_map) / sizeof(unsigned); i++)
+		if (gdb_signal_map[i] == gdb_signo)
+			return i;
+	return gdb_signo;
+}
+
+/* ------------------------------------------------------------------------ */
+
+static unsigned byteme(unsigned char hex1, unsigned char hex2)
+{
+	return (isdigit(hex1) ? hex1 - '0' : tolower(hex1) - 'a' + 10) * 16 +
+		(isdigit(hex2) ? hex2 - '0' : tolower(hex2) - 'a' + 10);
+}
+
+/* ------------------------------------------------------------------------ */
+
+/*
+ * Begin a new packet.  Add the $, and remember where we put it.  Return
+ * the offset for later checksum addition via push_output_packet_end.
+ */
+static size_t push_output_packet_start(struct gdb_connection *p)
+{
+	size_t start = p->output_buf_size;
+
+	BUG_ON(p->output_buf_size + 1 >= GDB_BUFMAX);
+	p->output_buf[p->output_buf_size++] = '$';
+	return start;
+}
+
+/* Add a character to the output queue.  Assumes output_mutex held. */
+static void push_output(struct gdb_connection *p, unsigned char c)
+{
+	/*
+	 * We know some space must exist; we check for this in
+	 * proc_gdb_write() for example.
+	 */
+	BUG_ON(p->output_buf_size >= GDB_BUFMAX);
+	p->output_buf[p->output_buf_size++] = c;
+}
+
+/* Lower case use due to http://sourceware.org/bugzilla/show_bug.cgi?id=9665 */
+static char hex[] = "0123456789abcdef";
+
+/* Add a byte (hexified) to the output queue.  Assumes output_mutex held. */
+static void push_output_hex(struct gdb_connection *p, unsigned char c)
+{
+	/*
+	 * We know some space must exist; we check for this in
+	 * proc_gdb_write() for example.
+	 */
+	BUG_ON(p->output_buf_size >= GDB_BUFMAX);
+	p->output_buf[p->output_buf_size++] = hex[(c & 0xf0) >> 4];
+	p->output_buf[p->output_buf_size++] = hex[(c & 0x0f) >> 0];
+}
+
+/*
+ * Finish the last packet.  Starting after the given '$' offset, compute
+ * the checksum and append it.
+ */
+static void push_output_packet_end(struct gdb_connection *p, size_t start)
+{
+	unsigned char checksum = 0;
+	int i;
+
+	BUG_ON(p->output_buf_size + 3 >= GDB_BUFMAX);
+	BUG_ON(p->output_buf[start] != '$');
+
+	for (i = start + 1; i < p->output_buf_size; i++)
+		checksum += p->output_buf[i];
+
+	p->output_buf[p->output_buf_size++] = '#';
+	p->output_buf[p->output_buf_size++] = hex[(checksum & 0xf0) >> 4];
+	p->output_buf[p->output_buf_size++] = hex[(checksum & 0x0f) >> 0];
+}
+
+/* Add a complete packet payload to the output queue.  */
+static void push_output_packet(struct gdb_connection *p, const char *s)
+{
+	size_t ss = strlen(s);
+	size_t start;
+	int i;
+
+	start = push_output_packet_start(p);
+	for (i = 0; i < ss; i++)
+		push_output(p, s[i]);
+	push_output_packet_end(p, start);
+}
+
+static inline struct gdb_connection *find_gdb_connection(
+						struct task_struct *task)
+{
+	struct gdb_connection *gc = NULL;
+
+	mutex_lock(&gdb_connections_mutex);
+	list_for_each_entry(gc, &gdb_connections, link) {
+		if (gc->target == task_tgid_nr(task))
+			goto out_connection;
+	}
+
+out_connection:
+	mutex_unlock(&gdb_connections_mutex);
+	return gc;
+}
+
+/* utrace callbacks */
+
+u32 gdb_utrace_report_quiesce(enum utrace_resume_action action,
+				struct utrace_engine *engine,
+				struct task_struct *task, unsigned long event)
+{
+	struct gdb_connection *p = engine->data;
+
+	pr_debug("report_quiesce %d event 0x%lx 0x%x->0x%x\n", task->pid,
+				event, action, p->at_quiesce_do);
+
+	return p->at_quiesce_do;
+}
+
+u32 gdb_utrace_report_clone(enum utrace_resume_action action,
+				struct utrace_engine *engine,
+				struct task_struct *parent,
+				unsigned long clone_flags,
+				struct task_struct *child)
+{
+	pr_debug("report_clone %d->%d\n", parent->pid, child->pid);
+
+	if (clone_flags & CLONE_THREAD) {
+		printk(KERN_WARNING
+			"unsupported multithreading on /proc/%d/gdb.\n",
+			task_pid_nr(parent));
+	}
+	/* XXX: is there anything else to do here? */
+	return UTRACE_RESUME;
+}
+
+u32 gdb_utrace_report_exec(enum utrace_resume_action action,
+					struct utrace_engine *engine,
+					struct task_struct *task,
+					const struct linux_binfmt *fmt,
+					const struct linux_binprm *bprm,
+					struct pt_regs *regs)
+{
+	/* XXX: Model an exec as if it were an exit. */
+	struct gdb_connection *p = engine->data;
+
+	pr_debug("report_exec %d->%s\n", task->pid, task->comm);
+
+	mutex_lock(&p->output_mutex);
+
+	p->at_quiesce_do = UTRACE_STOP;
+	snprintf(p->stopcode, GDB_BUFMAX, "W%02x", 0);
+	push_output_packet(p, p->stopcode);
+
+	mutex_unlock(&p->output_mutex);
+	wake_up(&p->output_wait);
+
+	/*
+	 * Suspend the exec operation, to ensure that the connected gdb
+	 * receives the notification packet, and lets us go.
+	 */
+	return UTRACE_STOP;
+}
+
+u32 gdb_utrace_report_signal(u32 action,
+				struct utrace_engine *engine,
+				struct task_struct *task,
+				struct pt_regs *regs,
+				siginfo_t *info,
+				const struct k_sigaction *orig_ka,
+				struct k_sigaction *return_ka)
+{
+	struct gdb_connection *p = engine->data;
+	u32 ret = action;
+
+	mutex_lock(&p->output_mutex);
+
+	pr_debug("report_signal %d (0x%x) skip %d stop %d\n",
+		 task->pid, action, p->pass_signals, p->stop_signals);
+
+	/*
+	 * The target is about to receive a signal.  There are several
+	 * cases:
+	 *
+	 * 1) This is an ordinary signal.  We UTRACE_STOP to notify gdb.
+	 *
+	 * 2a) This is a SIGTRAP arising from a singlestep arising from
+	 *     another tracing entity.  Or a breakpoint is disarmed.
+	 *     We UTRACE_RESUME.
+	 *
+	 * 2b) This is a SIGTRAP but not from a singlestep.
+	 *     We UTRACE_STOP to pass it to gdb.
+	 *
+	 *  Both case 2a, 2b are yet to be implemented.
+	 *
+	 * 3) This is a UTRACE_SIGNAL_REPORT our code injected to stop the
+	 *     process, as per UTRACE_INTERRUPT.
+	 *     We UTRACE_STOP | UTRACE_SIGNAL_IGN.
+	 *
+	 * 4) This is a signal our code injected on behalf of gdb via the
+	 *    C/S/I packets. We recognize this from p->pass_signals.
+	 *    We UTRACE_RESUME.
+	 *
+	 * 5) This is a UTRACE_SIGNAL_HANDLER event.  UTRACE_RESUME.
+	 */
+
+	switch (utrace_signal_action(action)) {
+	case UTRACE_SIGNAL_HANDLER:	/* case 5 */
+		p->at_quiesce_do = UTRACE_RESUME;
+		ret = UTRACE_RESUME | utrace_signal_action(action);
+		break;
+
+	case UTRACE_SIGNAL_REPORT:
+	case UTRACE_SIGNAL_DELIVER:
+	case UTRACE_SIGNAL_IGN:	/* XXX: bother notify? */
+	case UTRACE_SIGNAL_TERM:
+	case UTRACE_SIGNAL_CORE:
+	case UTRACE_SIGNAL_STOP:
+	case UTRACE_SIGNAL_TSTP:
+		if (!orig_ka) {	/* case 3 */
+			/* This should be UTRACE_SIGNAL_REPORT */
+			if (p->stop_signals > 0) {
+				p->stop_signals = 0;
+				/* Indicate as if thread received a SIGINT */
+				snprintf(p->stopcode, GDB_BUFMAX, "S%02x", 2);
+				push_output_packet(p, p->stopcode);
+				p->at_quiesce_do = UTRACE_STOP;
+				ret = UTRACE_STOP | UTRACE_SIGNAL_IGN;
+			} else
+				ret = p->at_quiesce_do |
+						utrace_signal_action(action);
+
+			break;
+		}
+		if (p->pass_signals > 0) {	/* case 4 */
+			p->pass_signals--;
+			p->at_quiesce_do = UTRACE_RESUME;
+			ret = UTRACE_RESUME | utrace_signal_action(action);
+		} else {	/* case 1 */
+			snprintf(p->stopcode, GDB_BUFMAX, "S%02x",
+				 map_signal_kern2gdb(info->si_signo));
+			push_output_packet(p, p->stopcode);
+			p->at_quiesce_do = UTRACE_STOP;
+			ret = UTRACE_STOP | utrace_signal_action(action);
+		}
+		break;
+	}
+
+	pr_debug("action 0x%x\n", ret);
+
+	mutex_unlock(&p->output_mutex);
+	wake_up(&p->output_wait);
+
+	return ret;
+}
+
+u32 gdb_utrace_report_exit(enum utrace_resume_action action,
+					struct utrace_engine *engine,
+					struct task_struct *task,
+					long orig_code, long *code)
+{
+	struct gdb_connection *p = engine->data;
+
+	pr_debug("report_exit %d (%lx)\n", task->pid, orig_code);
+
+	mutex_lock(&p->output_mutex);
+
+	p->at_quiesce_do = UTRACE_STOP;
+	snprintf(p->stopcode, GDB_BUFMAX,
+		 "W%02x", (unsigned)(orig_code & 0xFF));
+	push_output_packet(p, p->stopcode);
+
+	mutex_unlock(&p->output_mutex);
+	wake_up(&p->output_wait);
+
+	/*
+	 * Suspend the exit operation, to ensure that the connected gdb
+	 * receives the notification packet, and lets us go.
+	 */
+	return UTRACE_STOP;
+}
+
+u32 gdb_utrace_report_death(struct utrace_engine *engine,
+			struct task_struct *task, bool group_dead, int signal)
+{
+	struct gdb_connection *p = engine->data;
+
+	pr_debug("report_death %d (%d)\n", task->pid, signal);
+
+	mutex_lock(&p->output_mutex);
+
+	p->at_quiesce_do = UTRACE_DETACH;
+	snprintf(p->stopcode, GDB_BUFMAX, "X%2x", (unsigned)(signal & 0xFF));
+	push_output_packet(p, p->stopcode);
+
+	mutex_unlock(&p->output_mutex);
+	wake_up(&p->output_wait);
+	return UTRACE_DETACH;
+}
+
+static const struct utrace_engine_ops gdb_utrace_ops = {
+	.report_quiesce = gdb_utrace_report_quiesce,
+	.report_signal = gdb_utrace_report_signal,
+	.report_death = gdb_utrace_report_death,
+	.report_exit = gdb_utrace_report_exit,
+	.report_exec = gdb_utrace_report_exec,
+	.report_clone = gdb_utrace_report_clone,
+	/* XXX: syscall trapping is also possible. */
+};
+
+/*
+ * XXX: arch-dependent lookup of gdb remote protocol register
+ * numbering.  The register numbers (user-side) & expected sizes come
+ * from gdb's regformats/FOO-linux.dat.  The regset (kernel-side)
+ * numbers could come from offsetof/sizeof constructs based upon each
+ * arch's asm/user*.h.
+ */
+
+struct gdb_map_regset {
+	unsigned pos;		/* regset offset */
+	unsigned count;		/* regset byte count */
+	unsigned rsn;		/* regset number */
+	unsigned bytes;		/* gdb's view of register width; <= count */
+};
+
+struct gdb_map_regset arch_i386_map_regset[] = {
+	[0] = { /* eax */ 6 * 4, 4, NT_PRSTATUS, 4,},
+	[1] = { /* ecx */ 1 * 4, 4, NT_PRSTATUS, 4,},
+	[2] = { /* edx */ 2 * 4, 4, NT_PRSTATUS, 4,},
+	[3] = { /* ebx */ 0 * 4, 4, NT_PRSTATUS, 4,},
+	[4] = { /* esp */ 15 * 4, 4, NT_PRSTATUS, 4,},
+	[5] = { /* ebp */ 5 * 4, 4, NT_PRSTATUS, 4,},
+	[6] = { /* esi */ 3 * 4, 4, NT_PRSTATUS, 4,},
+	[7] = { /* edi */ 4 * 4, 4, NT_PRSTATUS, 4,},
+	[8] = { /* eip */ 12 * 4, 4, NT_PRSTATUS, 4,},
+	[9] = { /* eflags */ 14 * 4, 4, NT_PRSTATUS, 4,},
+	[10] = { /* cs */ 13 * 4, 4, NT_PRSTATUS, 4,},
+	[11] = { /* ss */ 16 * 4, 4, NT_PRSTATUS, 4,},
+	[12] = { /* ds */ 7 * 4, 4, NT_PRSTATUS, 4,},
+	[13] = { /* es */ 8 * 4, 4, NT_PRSTATUS, 4,},
+	[14] = { /* fs */ 9 * 4, 4, NT_PRSTATUS, 4,},
+	[15] = { /* gs */ 10 * 4, 4, NT_PRSTATUS, 4,},
+	[16] = { /* st0 */ 0, 0, NT_PRFPREG, 10,},
+	[17] = { /* st1 */ 0, 0, NT_PRFPREG, 10,},
+	[18] = { /* st2 */ 0, 0, NT_PRFPREG, 10,},
+	[19] = { /* st3 */ 0, 0, NT_PRFPREG, 10,},
+	[20] = { /* st4 */ 0, 0, NT_PRFPREG, 10,},
+	[21] = { /* st5 */ 0, 0, NT_PRFPREG, 10,},
+	[22] = { /* st6 */ 0, 0, NT_PRFPREG, 10,},
+	[23] = { /* st7 */ 0, 0, NT_PRFPREG, 10,},
+	[24] = { /* fctrl */ 0, 0, NT_PRFPREG, 4,},
+	[25] = { /* fstat */ 0, 0, NT_PRFPREG, 4,},
+	[26] = { /* ftag */ 0, 0, NT_PRFPREG, 4,},
+	[27] = { /* fiseg */ 0, 0, NT_PRFPREG, 4,},
+	[28] = { /* fioff */ 0, 0, NT_PRFPREG, 4,},
+	[29] = { /* foseg */ 0, 0, NT_PRFPREG, 4,},
+	[30] = { /* fooff */ 0, 0, NT_PRFPREG, 4,},
+	[31] = { /* fop */ 0, 0, NT_PRFPREG, 4,},
+	[32] = { /* xmm0 */ 0, 0, NT_PRFPREG, 16,},
+	[33] = { /* xmm1 */ 0, 0, NT_PRFPREG, 16,},
+	[34] = { /* xmm2 */ 0, 0, NT_PRFPREG, 16,},
+	[35] = { /* xmm3 */ 0, 0, NT_PRFPREG, 16,},
+	[36] = { /* xmm4 */ 0, 0, NT_PRFPREG, 16,},
+	[37] = { /* xmm5 */ 0, 0, NT_PRFPREG, 16,},
+	[38] = { /* xmm6 */ 0, 0, NT_PRFPREG, 16,},
+	[39] = { /* xmm7 */ 0, 0, NT_PRFPREG, 16,},
+	[40] = { /* mxcsr */ 0, 0, NT_PRFPREG, 4,},
+	[41] = { /* orig_eax */ 0, 0, NT_PRSTATUS, 4,},
+};
+
+struct gdb_map_regset arch_x86_64_map_regset[] = {
+	[0] = { /* rax */ 10 * 8, 8, NT_PRSTATUS, 8,},
+	[1] = { /* rbx */ 5 * 8, 8, NT_PRSTATUS, 8,},
+	[2] = { /* rcx */ 11 * 8, 8, NT_PRSTATUS, 8,},
+	[3] = { /* rdx */ 12 * 8, 8, NT_PRSTATUS, 8,},
+	[4] = { /* rsi */ 13 * 8, 8, NT_PRSTATUS, 8,},
+	[5] = { /* rdi */ 14 * 8, 8, NT_PRSTATUS, 8,},
+	[6] = { /* rbp */ 4 * 8, 8, NT_PRSTATUS, 8,},
+	[7] = { /* rsp */ 19 * 8, 8, NT_PRSTATUS, 8,},
+	[8] = { /* r8 */ 9 * 8, 8, NT_PRSTATUS, 8,},
+	[9] = { /* r9 */ 8 * 8, 8, NT_PRSTATUS, 8,},
+	[10] = { /* r10 */ 7 * 8, 8, NT_PRSTATUS, 8,},
+	[11] = { /* r11 */ 6 * 8, 8, NT_PRSTATUS, 8,},
+	[12] = { /* r12 */ 3 * 8, 8, NT_PRSTATUS, 8,},
+	[13] = { /* r13 */ 2 * 8, 8, NT_PRSTATUS, 8,},
+	[14] = { /* r14 */ 1 * 8, 8, NT_PRSTATUS, 8,},
+	[15] = { /* r15 */ 0 * 8, 8, NT_PRSTATUS, 8,},
+	[16] = { /* rip */ 16 * 8, 8, NT_PRSTATUS, 8,},
+	[17] = { /* flags */ 18 * 8, 8, NT_PRSTATUS, 4,},
+	[18] = { /* cs */ 17 * 8, 8, NT_PRSTATUS, 4,},
+	[19] = { /* ss */ 20 * 8, 8, NT_PRSTATUS, 4,},
+	[20] = { /* ds */ 23 * 8, 8, NT_PRSTATUS, 4,},
+	[21] = { /* es */ 24 * 8, 8, NT_PRSTATUS, 4,},
+	[22] = { /* fs */ 25 * 8, 8, NT_PRSTATUS, 4,},
+	[23] = { /* gs */ 26 * 8, 8, NT_PRSTATUS, 4,},
+	[24] = { /* st0 */ 0, 0, NT_PRFPREG, 10,},
+	[25] = { /* st1 */ 0, 0, NT_PRFPREG, 10,},
+	[26] = { /* st2 */ 0, 0, NT_PRFPREG, 10,},
+	[27] = { /* st3 */ 0, 0, NT_PRFPREG, 10,},
+	[28] = { /* st4 */ 0, 0, NT_PRFPREG, 10,},
+	[29] = { /* st5 */ 0, 0, NT_PRFPREG, 10,},
+	[30] = { /* st6 */ 0, 0, NT_PRFPREG, 10,},
+	[31] = { /* st7 */ 0, 0, NT_PRFPREG, 10,},
+	[32] = { /* fctrl */ 0, 0, NT_PRFPREG, 4,},
+	[33] = { /* fstat */ 0, 0, NT_PRFPREG, 4,},
+	[34] = { /* ftag */ 0, 0, NT_PRFPREG, 4,},
+	[35] = { /* fiseg */ 0, 0, NT_PRFPREG, 4,},
+	[36] = { /* fioff */ 0, 0, NT_PRFPREG, 4,},
+	[37] = { /* foseg */ 0, 0, NT_PRFPREG, 4,},
+	[38] = { /* fooff */ 0, 0, NT_PRFPREG, 4,},
+	[39] = { /* fop */ 0, 0, NT_PRFPREG, 4,},
+	[40] = { /* xmm0 */ 0, 0, NT_PRFPREG, 16,},
+	[41] = { /* xmm1 */ 0, 0, NT_PRFPREG, 16,},
+	[42] = { /* xmm2 */ 0, 0, NT_PRFPREG, 16,},
+	[43] = { /* xmm3 */ 0, 0, NT_PRFPREG, 16,},
+	[44] = { /* xmm4 */ 0, 0, NT_PRFPREG, 16,},
+	[45] = { /* xmm5 */ 0, 0, NT_PRFPREG, 16,},
+	[46] = { /* xmm6 */ 0, 0, NT_PRFPREG, 16,},
+	[47] = { /* xmm7 */ 0, 0, NT_PRFPREG, 16,},
+	[48] = { /* xmm8 */ 0, 0, NT_PRFPREG, 16,},
+	[49] = { /* xmm9 */ 0, 0, NT_PRFPREG, 16,},
+	[50] = { /* xmm10 */ 0, 0, NT_PRFPREG, 16,},
+	[51] = { /* xmm11 */ 0, 0, NT_PRFPREG, 16,},
+	[52] = { /* xmm12 */ 0, 0, NT_PRFPREG, 16,},
+	[53] = { /* xmm13 */ 0, 0, NT_PRFPREG, 16,},
+	[54] = { /* xmm14 */ 0, 0, NT_PRFPREG, 16,},
+	[55] = { /* xmm15 */ 0, 0, NT_PRFPREG, 16,},
+	[56] = { /* mxcsr */ 0, 0, NT_PRFPREG, 4,},
+	[57] = { /* orig_rax */ 15 * 8, 8, NT_PRSTATUS, 8,},
+};
+
+static int gdb_remote_register_info(struct gdb_connection *p,
+					struct task_struct *task,
+					unsigned number,
+					unsigned *pos, unsigned *count,
+					unsigned *bytes)
+{
+	const struct user_regset_view *rs = task_user_regset_view(task);
+	int rsn = -1;
+
+	if (rs == 0)
+		return -ENOENT;
+
+#define GMRSIZE (sizeof(struct gdb_map_regset))
+
+	if (rs->e_machine == EM_386) {
+		if (number < sizeof(arch_i386_map_regset) / GMRSIZE) {
+			*pos = arch_i386_map_regset[number].pos;
+			*count = arch_i386_map_regset[number].count;
+			*bytes = arch_i386_map_regset[number].bytes;
+			rsn = arch_i386_map_regset[number].rsn;
+		}
+	} else if (rs->e_machine == EM_X86_64) {
+		if (number < sizeof(arch_x86_64_map_regset) / GMRSIZE) {
+			*pos = arch_x86_64_map_regset[number].pos;
+			*count = arch_x86_64_map_regset[number].count;
+			*bytes = arch_x86_64_map_regset[number].bytes;
+			rsn = arch_x86_64_map_regset[number].rsn;
+		}
+	}
+	/* else ... rsn stays -1. */
+#undef GMRSIZE
+
+	/*
+	 * Now map to the per-architecture regset index, based on the
+	 * elf core_note_type we found.
+	 */
+	if (rsn >= 0) {
+		unsigned j;
+		for (j = 0; j < rs->n; j++) {
+			if (rs->regsets[j].core_note_type == rsn)
+				return j;
+		}
+	}
+
+	/*
+	 * Invalid machines, register numbers, rsns, or unset rsns all
+	 * fall through here.
+	 */
+	return -ENOENT;
+}
+
+static inline int fetch_remote_register_info(struct gdb_connection *p,
+					struct task_struct *task,
+					int index,
+					unsigned *pos,
+					unsigned *count,
+					unsigned *bytes)
+{
+	int rsn;
+
+	rsn = gdb_remote_register_info(p, task, index, pos, count, bytes);
+	pr_debug("gdb register %u => rsn %d p%u c%u b%u\n", index, rsn,
+							*pos, *count, *bytes);
+	if (rsn > 0) {
+		/*
+		 * If we want to extract register data, make sure we're
+		 * fetching at least that much.
+		 */
+		BUG_ON(*count > 0 && *count < *bytes);
+		/* Assert reg_contents size is right. */
+		BUG_ON(MAX_REG_WIDTH < *bytes || MAX_REG_WIDTH < *count);
+	}
+	return rsn;
+}
+
+static inline int get_set_regset_view(struct task_struct *task,
+			unsigned pos, unsigned count,
+			unsigned char *reg_contents,
+			int rsn, bool set)
+{
+	int rc;
+	const struct user_regset_view *rsv;
+	const struct user_regset *rs;
+
+	if (!count) {
+		if (!set)
+			memset(reg_contents, 0, MAX_REG_WIDTH);
+		return 0;
+	}
+
+	/* real register */
+	if (set)
+		BUG_ON(count > MAX_REG_WIDTH);
+
+	rsv = task_user_regset_view(task);
+	BUG_ON(rsn >= rsv->n);
+	rs = &rsv->regsets[rsn];
+
+	/* Extract the register values into reg_contents[]. */
+	if (set)
+		rc = (rs->set)(task, rs, pos, count, reg_contents, NULL);
+	else
+		rc = (rs->get)(task, rs, pos, count, reg_contents, NULL);
+
+	return rc;
+}
+
+static inline void prepare_regset_set(struct gdb_connection *p,
+				unsigned char *reg_contents,
+				size_t *start, unsigned bytes)
+{
+	int j;
+
+	/*
+	 * 0-fill the register copy.
+	 * XXX initialize it from rs->get() instead?
+	 */
+	memset(reg_contents, 0, MAX_REG_WIDTH);
+
+	/*
+	 * Hex-unconvert all the bytes.
+	 * XXX: endianness adjust for count != bytes
+	 */
+	for (j = 0; j < bytes; j++)
+		reg_contents[j] = byteme(p->input_buf[*start + 2 * j],
+					   p->input_buf[*start + 2 * j + 1]);
+	*start += 2 * bytes;
+}
+
+/*
+ * Process an entire, checksum-confirmed $command# at the front of
+ * p->input_buf[].  The input and output mutexes are being held.
+ */
+static void handle_gdb_command_packet(struct gdb_connection *p,
+						struct task_struct *task)
+{
+	unsigned long arg1, arg2, arg3;
+	size_t op_start;
+	int rc = 0;
+	int i, j;
+
+	pr_debug("gdb packet code %c\n", p->input_buf[1]);
+
+	switch (p->input_buf[1]) {
+	case '?':
+		if (p->at_quiesce_do != UTRACE_STOP) {
+			/* shouldn't happen */
+			p->stop_signals++;
+			rc = utrace_control(task, p->engine, UTRACE_INTERRUPT);
+			if (rc == -EINPROGRESS)
+				rc = utrace_barrier(task, p->engine);
+			/*
+			 * Note that we don't enqueue a reply packet here,
+			 * but make gdb wait for a response from the
+			 * utrace report_FOO callbacks.
+			 */
+		} else {
+			push_output_packet(p, p->stopcode);
+		}
+		break;
+
+	case 'i':		/* [ADDR[,NNN]] */
+	case 's':		/* [ADDR] */
+		/* XXX: if !arch_has_single_step() ... then what? */
+	case 'c':		/* [ADDR] */
+		rc = sscanf(&p->input_buf[2], "%lx,%lx", &arg1, &arg2);
+		/* XXX: args ignored */
+		p->stopcode[0] = '\0';
+		p->at_quiesce_do =
+		    ((p->input_buf[1] == 'c' || !arch_has_single_step())
+		     ? UTRACE_RESUME : UTRACE_SINGLESTEP);
+		if (p->at_quiesce_do == UTRACE_SINGLESTEP)
+			p->stop_signals++;
+		rc = utrace_control(task, p->engine, p->at_quiesce_do);
+		if (rc == -EINPROGRESS)
+			rc = utrace_barrier(task, p->engine);
+		break;
+	case 'C':		/* SIG[;ADDR] */
+	case 'S':		/* SIG[;ADDR] */
+		/* XXX: if !arch_has_single_step() ... then what? */
+	case 'I':		/* SIG[;ADDR[,NNN?]] */
+		rc = sscanf(&p->input_buf[2], "%lx;%lx,%lx", &arg1, &arg2,
+			    &arg3);
+		if (rc >= 1)	/* SIG present */
+			send_sig(map_signal_gdb2kern(arg1), task, 1);
+
+		p->pass_signals++;
+		p->stopcode[0] = '\0';
+		p->at_quiesce_do =
+		    ((p->input_buf[1] == 'C' || !arch_has_single_step())
+		     ? UTRACE_RESUME : UTRACE_SINGLESTEP);
+		if (p->at_quiesce_do == UTRACE_SINGLESTEP)
+			p->stop_signals++;
+
+		rc = utrace_control(task, p->engine, p->at_quiesce_do);
+		if (rc == -EINPROGRESS)
+			rc = utrace_barrier(task, p->engine);
+
+		/* Response will come at next report_signal. */
+		break;
+	case 'D':
+		push_output_packet(p, "OK");
+		/* NB: proc_gdb_release() performs actual utrace detach. */
+		break;
+	case 'g':
+		op_start = push_output_packet_start(p);
+
+		/*
+		 * GDB_BUFMAX stands for some random large number,
+		 * known to be larger than the number of gdb indexed
+		 * registers.
+		 */
+		for (i = 0; i < GDB_BUFMAX; i++) {
+			unsigned rs_count;
+			unsigned rs_pos;
+			unsigned bytes;
+			int rsn;
+			unsigned char reg_contents[MAX_REG_WIDTH];
+
+			rsn = fetch_remote_register_info(p, task, i,
+					   &rs_pos, &rs_count, &bytes);
+			if (rsn < 0)
+				break;
+
+			rc = get_set_regset_view(task, rs_pos, rs_count,
+						reg_contents, rsn, false);
+			if (rc)
+				break;
+
+			/*
+			 * Hex-dump it.
+			 * XXX: endianness adjust for count != bytes
+			 */
+			for (j = 0; j < bytes; j++) {
+				/* pr_debug("%02x", reg_contents[j]); */
+				push_output_hex(p, reg_contents[j]);
+			}
+			/* pr_debug(")\n"); */
+
+		}
+		push_output_packet_end(p, op_start);
+		break;
+	case 'G':
+		i = 0;
+		op_start = 2;	/* use as input pointer, past $G in command */
+		while (p->input_buf[op_start] != '#' &&
+				       op_start < p->input_buf_size) {
+			unsigned rs_count;
+			unsigned rs_pos;
+			unsigned bytes;
+			int rsn;
+			unsigned char reg_contents[MAX_REG_WIDTH];
+
+			rsn = fetch_remote_register_info(p, task, i,
+					   &rs_pos, &rs_count, &bytes);
+			if (rsn < 0)
+				break;
+
+			/* Remaining packet too short? */
+			if ((op_start + 2 * bytes + 3) < p->input_buf_size)
+				break;
+
+			prepare_regset_set(p, reg_contents, &op_start, bytes);
+			rc = get_set_regset_view(task, rs_pos, rs_count,
+						reg_contents, rsn, true);
+			if (rc)
+				break;
+		}
+		if (p->input_buf[op_start] == '#' && rc == 0)
+			push_output_packet(p, "OK");
+		else
+			push_output_packet(p, "E01");
+
+		break;
+	case 'p':		/* REG */
+		push_output_packet(p, "E01");
+		break;
+	case 'P':		/* REG=VAL */
+		/* `i' will index p->input_buf to consume VAL hex bytes. */
+		rc = sscanf(&p->input_buf[2], "%lx=%n", &arg1, &i);
+		op_start = i + 2;	/* Skip the leading $P also */
+		if (rc != 1) {
+			push_output_packet(p, "E01");
+		} else {
+			unsigned rs_count;
+			unsigned rs_pos;
+			unsigned bytes;
+			int rsn;
+			unsigned char reg_contents[MAX_REG_WIDTH];
+
+			rsn = fetch_remote_register_info(p, task, arg1,
+					   &rs_pos, &rs_count, &bytes);
+			if (rsn < 0)
+				break;
+
+			/* Remaining packet too short? */
+			if ((op_start + 2 * bytes + 3) < p->input_buf_size)
+				break;
+
+			prepare_regset_set(p, reg_contents, &op_start, bytes);
+			rc = get_set_regset_view(task, rs_pos, rs_count,
+						reg_contents, rsn, true);
+			WARN_ON(rc);
+			if (p->input_buf[op_start] == '#' && rc == 0)
+				push_output_packet(p, "OK");
+			else
+				push_output_packet(p, "E01");
+		}
+		break;
+	case 'm':		/* ADDR,LENGTH */
+		rc = sscanf(&p->input_buf[2], "%lx,%lx", &arg1, &arg2);
+		if (rc != 2)
+			push_output_packet(p, "E01");
+		else {
+			size_t o = push_output_packet_start(p);
+			while (arg2 > 0) {
+				unsigned char value;
+
+				/*
+				 * Simply stop looping if requested length
+				 * was too large.  gdb will probably retry
+				 * from this point on.
+				 */
+				if (p->output_buf_size + 5 > GDB_BUFMAX)
+					break;
+
+				rc = access_process_vm(task, arg1, &value, 1,
+						       0);
+				if (rc != 1)
+					break;	/* EFAULT */
+				else
+					push_output_hex(p, value);
+
+				arg1++;
+				arg2--;
+			}
+			push_output_packet_end(p, o);
+		}
+		break;
+	case 'M':		/* ADDR,LENGTH:XX */
+		/* `i' will index p->input_buf to consume XX hex bytes. */
+		rc = sscanf(&p->input_buf[2], "%lx,%lx:%n", &arg1, &arg2, &i);
+		op_start = i + 2;	/* Skip the leading $M also. */
+		if (rc < 2) {
+			push_output_packet(p, "E01");
+			break;
+		}
+		while (arg2 > 0) {
+			unsigned char value;
+
+			/*
+			 * Check that enough input bytes left for
+			 * these two hex chars, plus the #XX checksum.
+			 */
+			if (i + 4 >= p->input_buf_size)
+				break;
+
+			value = byteme(p->input_buf[i], p->input_buf[i + 1]);
+			rc = access_process_vm(task, arg1, &value, 1, 1);
+			if (rc != 1)
+				break;	/* EFAULT */
+
+			i += 2;
+			arg1++;
+			arg2--;
+		}
+		if (arg2 != 0)
+			push_output_packet(p, "E02");
+		else
+			push_output_packet(p, "OK");
+
+		break;
+	case 'Z':               /* TYPE,ADDR,LENGTH */
+		/* Not yet implemented */
+		push_output_packet(p, "E03");
+		break;
+	case 'z':               /* TYPE,ADDR,LENGTH */
+		/* Not yet implemented */
+		push_output_packet(p, "E03");
+		break;
+
+	default:
+		push_output_packet(p, "");
+	}
+}
+
+/* ------------------------------------------------------------------------ */
+
+/* gdb control callbacks */
+
+#define get_proc_task(inode) get_pid_task(PROC_I((inode))->pid, PIDTYPE_PID)
+
+static int proc_gdb_open(struct inode *inode, struct file *filp)
+{
+	struct task_struct *task = get_proc_task(inode);
+	int ret = -EBUSY;
+	struct gdb_connection *p;
+
+	pr_debug("opened /proc/%d/gdb\n", task->pid);
+
+	/* Reject kernel threads. */
+	if (task->flags & PF_KTHREAD) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Reject if connection is for other than tg-leader thread. */
+	if (task_pid_nr(task) != task_tgid_nr(task)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	mutex_lock(&gdb_connections_mutex);
+
+	/*
+	 * Reject if a connection exists for the thread group
+	 * leader.
+	 */
+	list_for_each_entry(p, &gdb_connections, link) {
+		if (p->target == task_tgid_nr(task)) {
+			ret = -EBUSY;
+			goto out_mutex;
+		}
+	}
+	/* (Don't unlock yet, to defeat a race of two concurrent opens.) */
+
+	p = kzalloc(sizeof(struct gdb_connection), GFP_KERNEL);
+	if (!p) {
+		ret = -ENOMEM;
+		goto out_mutex;
+	}
+
+	/* Send initial ping to gdb. */
+	push_output_packet(p, "");
+
+	mutex_init(&p->output_mutex);
+	init_waitqueue_head(&p->output_wait);
+
+	mutex_init(&p->input_mutex);
+	init_waitqueue_head(&p->input_wait);
+
+	/*
+	 * NB: During attach, we don't want to bother the target.
+	 * Soon though a send_sig will interrupt it.
+	 */
+	p->at_quiesce_do = UTRACE_RESUME;
+	p->target = task->tgid;
+
+	p->engine = utrace_attach_task(task,
+					UTRACE_ATTACH_CREATE |
+					UTRACE_ATTACH_EXCLUSIVE |
+					UTRACE_ATTACH_MATCH_OPS,
+					&gdb_utrace_ops, p);
+	if (IS_ERR(p->engine) || p->engine == NULL) {
+		ret = -EBUSY;
+		goto out_free;
+	}
+
+	ret = utrace_set_events(task, p->engine,
+				UTRACE_EVENT_SIGNAL_ALL |
+				UTRACE_EVENT(QUIESCE) |
+				UTRACE_EVENT(DEATH) |
+				UTRACE_EVENT(EXIT) |
+				UTRACE_EVENT(EXEC) | UTRACE_EVENT(CLONE));
+	pr_debug("utrace_set_events sent, ret=%d\n", ret);
+	utrace_engine_put(p->engine);
+	filp->private_data = p;
+
+	INIT_LIST_HEAD(&p->link);
+	list_add_tail(&p->link, &gdb_connections);
+
+	p->stop_signals++;
+	ret = utrace_control(task, p->engine, UTRACE_INTERRUPT);
+	if (ret == -EINPROGRESS)
+		ret = utrace_barrier(task, p->engine);
+
+	goto out_mutex;
+
+out_free:
+	kfree(p);
+out_mutex:
+	mutex_unlock(&gdb_connections_mutex);
+out:
+	return ret;
+}
+
+static int proc_gdb_release(struct inode *inode, struct file *filp)
+{
+	struct task_struct *task = get_proc_task(inode);
+	struct gdb_connection *p = filp->private_data;
+	int ret = 0;
+
+	mutex_lock(&gdb_connections_mutex);
+	if (task == NULL) {
+		/* Thread is already gone; report_death was already called. */
+		pr_debug("gdb %d releasing old\n", p->target);
+	} else if (p->at_quiesce_do != UTRACE_DETACH) {
+		pr_debug("gdb %d releasing current\n", p->target);
+
+		ret = utrace_set_events(task, p->engine, 0);
+		if (ret == -EINPROGRESS)
+			ret = utrace_barrier(task, p->engine);
+
+		/*
+		 * No more callbacks will be received!
+		 * Detach implies Resume
+		 */
+		ret = utrace_control(task, p->engine, UTRACE_DETACH);
+		if (ret == -EINPROGRESS)
+			ret = utrace_barrier(task, p->engine);
+	}
+
+	list_del(&p->link);
+	mutex_unlock(&gdb_connections_mutex);
+
+	kfree(p);
+	return ret;
+}
+
+static int proc_gdb_ioctl(struct inode *inode, struct file *file,
+				unsigned int cmd, unsigned long arg)
+{
+	/*
+	 * XXX: GDB usually thinks that a file name for "target
+	 * remote" implies a serial port with tty-ish ioctl's
+	 * available.  We pretend to accept them all.
+	 */
+	return 0;
+}
+
+static ssize_t proc_gdb_read(struct file *filp, char __user * buf,
+					size_t count, loff_t *ppos)
+{
+	struct gdb_connection *p = filp->private_data;
+	struct task_struct *task;
+	int rc = 0;
+	size_t len;
+
+	task = find_task_by_vpid(p->target);
+	if (!task)
+		return -EINVAL;
+
+	if ((p->output_buf_size <= p->output_buf_read) &&
+	    filp->f_flags & O_NONBLOCK)
+		return -EAGAIN;
+
+again:
+	rc = wait_event_interruptible(p->output_wait,
+				      (p->output_buf_size >
+				       p->output_buf_read));
+	if (rc)
+		goto out;
+
+	mutex_lock(&p->output_mutex);
+
+	if (p->output_buf_size <= p->output_buf_read) {
+		mutex_unlock(&p->output_mutex);
+		goto again;
+	}
+	len = min(count, (size_t) (p->output_buf_size - p->output_buf_read));
+	if (copy_to_user(buf, &p->output_buf[p->output_buf_read], len)) {
+		rc = -EFAULT;
+		goto out_unlock;
+	}
+
+	pr_debug("sent %u bytes (%ld left) data (%.*s)\n",
+		(unsigned)len,
+		((long)p->output_buf_size - (long)p->output_buf_read) - len,
+		(int)len, &p->output_buf[p->output_buf_read]);
+
+	p->output_buf_read += len;
+	rc = len;
+
+	/* If whole packet is consumed, reset for next one. */
+	BUG_ON(p->output_buf_read > p->output_buf_size);
+	if (p->output_buf_read == p->output_buf_size) {
+		p->output_buf_read = 0;
+		p->output_buf_size = 0;
+	}
+
+out_unlock:
+	mutex_unlock(&p->output_mutex);
+
+out:
+	return rc;
+}
+
+static ssize_t proc_gdb_write(struct file *filp, const char __user * buf,
+					size_t count, loff_t *ppos)
+{
+	struct gdb_connection *p = filp->private_data;
+	size_t last_input_buf_size;
+	struct task_struct *task;
+	size_t len;
+	int ret = 0;
+
+	task = find_task_by_vpid(p->target);
+	if (!task)
+		return -EINVAL;
+
+again:
+	ret = wait_event_interruptible(p->input_wait,
+				       (p->input_buf_size < GDB_BUFMAX));
+	if (ret)
+		goto out;
+
+	mutex_lock(&p->input_mutex);
+	if (p->input_buf_size == GDB_BUFMAX) {
+		mutex_unlock(&p->input_mutex);
+		goto again;
+	}
+	mutex_lock(&p->output_mutex);
+
+	/* We now know there is some room in the input buffer.  Upon
+	   entry, the input_buf will either be empty, or contain a
+	   partial gdb request packet. */
+
+	/* Copy the data. */
+	len = min(count, (size_t) (GDB_BUFMAX - p->input_buf_size));
+	if (copy_from_user(&p->input_buf[p->input_buf_size], buf, len)) {
+		ret = -EFAULT;
+		goto out_unlock;
+	}
+
+	/*
+	 * pr_debug("received data %.*s\n", (int)len,
+	 *				&p->input_buf[p->input_buf_size]);
+	 */
+
+	p->input_buf_size += len;
+	ret = len;
+
+	/*
+	 * Process any packets in the buffer to restore the incoming
+	 * invariant.  (Normal GDB will not send more than one packet before
+	 * waiting for a response.)
+	 */
+
+	/*
+	 * We iterate until we can no longer shrink the input buffer.
+	 * Usually we will not iterate more than once, since there may be
+	 * one +/- ack byte and/or one gdb packet.
+	 */
+	last_input_buf_size = 0;
+	while (p->input_buf_size && p->input_buf_size != last_input_buf_size) {
+		last_input_buf_size = p->input_buf_size;
+
+		if (p->input_buf[0] == '+') {
+			/*
+			 * This must have been an ack to our
+			 * previously output packet.
+			 * Consume the input.
+			 */
+			memmove(&p->input_buf[0], &p->input_buf[1],
+				--p->input_buf_size);
+		} else if (p->input_buf[0] == '-') {
+			/*
+			 * Whoops, a nak.  Unfortunately, we don't
+			 * handle transmission errors by
+			 * retransmitting the last output_buf; it's
+			 * already gone.  OTOH we should not encounter
+			 * transmission errors on a reliable channel
+			 * such as a read syscall.
+			 * Consume the input.
+			 */
+			printk(KERN_WARNING "Unexpected NAK received"
+			       "on /proc/%d/gdb connection.\n",
+			       task_pid_nr(task));
+			memmove(&p->input_buf[0], &p->input_buf[1],
+				--p->input_buf_size);
+		} else if (p->input_buf[0] == 3) {	/* ^C == INTR */
+			int rc;	/* NB: don't overwrite 'ret'. */
+			pr_debug("received gdb interrupt\n");
+			p->stop_signals++;
+			rc = utrace_control(task, p->engine, UTRACE_INTERRUPT);
+			if (rc == -EINPROGRESS)
+				rc = utrace_barrier(task, p->engine);
+
+			/*
+			 * p->at_quiesce_do will be set in
+			 * report_signal(SIGNAL_REPORT).  NB: this packet
+			 * does not generate an +/- ack. Consume the input.
+			 */
+			memmove(&p->input_buf[0], &p->input_buf[1],
+				--p->input_buf_size);
+		} else if (p->input_buf[0] == '$') {	/* command packet */
+			int j;
+			unsigned char checksum = 0;
+			unsigned char checksum2;
+			for (j = 1; j < p->input_buf_size - 2; j++) {
+				if (p->input_buf[j] != '#') {
+					checksum += p->input_buf[j];
+					continue;
+				}
+				checksum2 = byteme(p->input_buf[j + 1],
+						   p->input_buf[j + 2]);
+				pr_debug("received gdb packet %.*s\n",
+						 j + 3, &p->input_buf[0]);
+				if (checksum == checksum2) {
+					push_output(p, '+');
+					handle_gdb_command_packet(p, task);
+				} else
+					push_output(p, '-');
+
+				/* Consume the whole packet. */
+				p->input_buf_size -= (j + 3);
+				memmove(&p->input_buf[0], &p->input_buf[j + 3],
+							p->input_buf_size);
+				break;
+			}	/* End searching for end of packet */
+
+			/*
+			 * We may not have found the #<hex><hex>
+			 * checksum.  If so, leave the partial packet
+			 * in input_buf.  Since input_buf_size will
+			 * not have decreased, the while() loop above
+			 * will detect a fixpoint and exit.
+			 *
+			 * Alternately, there could be another gdb packet
+			 * just behind the one we just consumed.  In this
+			 * we'll iterate one more time in this loop.
+			 */
+		} else {	/* junk character */
+			printk(KERN_WARNING "Unexpected character (%x) "
+			       "received on /proc/%d/gdb connection.\n",
+			       (int)p->input_buf[0], task_pid_nr(task));
+			/* Consume the input. */
+			memmove(&p->input_buf[0], &p->input_buf[1],
+				--p->input_buf_size);
+		}
+	}
+
+out_unlock:
+	/* Probably have more room in input_buf. */
+	wake_up(&p->input_wait);
+	/* Probably have data in output_buf. */
+	wake_up(&p->output_wait);
+
+	mutex_unlock(&p->output_mutex);
+	mutex_unlock(&p->input_mutex);
+out:
+	return ret;
+}
+
+const struct file_operations proc_gdb_operations = {
+	.open = proc_gdb_open,
+	.read = proc_gdb_read,
+	.write = proc_gdb_write,
+	.release = proc_gdb_release,
+	.ioctl = proc_gdb_ioctl,
+};

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Peter Zijlstra]

On Mon, 2009-11-30 at 17:33 +0530, Srikar Dronamraju wrote:
> This patch implements an in-kernel gdb stub.
> It provides an interface between gdb and Linux Kernel by implementing
> the remote serial protocol.  This gdbstub uses utrace infrastructure.
> This patch provides register set access, signal mapping, process event
> handling, input/output operations.
> 
> /proc/<pid>/gdb was chosen as file for gdb to interact with the
> process through remote serial protocol. 
> 
> Hence users would have to use "target remote /proc/<pid>/gdb" command
> on gdb prompt to start using this infrastructure.
> 
> For Breakpointing support, gdbstub needs User space breakpointing
> layer and uprobes layer which will be posted later.

How does this compare to kgdb and related efforts?

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Srikar Dronamraju]

* Peter Zijlstra <peterz@infradead.org> [2009-11-30 13:09:12]:

> On Mon, 2009-11-30 at 17:33 +0530, Srikar Dronamraju wrote:
> > This patch implements an in-kernel gdb stub.
> > It provides an interface between gdb and Linux Kernel by implementing
> > the remote serial protocol.  This gdbstub uses utrace infrastructure.
> > This patch provides register set access, signal mapping, process event
> > handling, input/output operations.
> > 
> > /proc/<pid>/gdb was chosen as file for gdb to interact with the
> > process through remote serial protocol. 
> > 
> > Hence users would have to use "target remote /proc/<pid>/gdb" command
> > on gdb prompt to start using this infrastructure.
> > 
> > For Breakpointing support, gdbstub needs User space breakpointing
> > layer and uprobes layer which will be posted later.
> 
> How does this compare to kgdb and related efforts?

This is a In-kernel gdbstub to debug user space programs. 
This stub doesnt help in debugging kernel.

Hence I am not sure how to compare kgdb gdbstub with this gdbstub.
Can you please provide more pointers on what you were referring to?

--
Thanks and Regards
Srikar

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Peter Zijlstra]

On Mon, 2009-11-30 at 18:02 +0530, Srikar Dronamraju wrote:
> * Peter Zijlstra <peterz@infradead.org> [2009-11-30 13:09:12]:
> 
> > On Mon, 2009-11-30 at 17:33 +0530, Srikar Dronamraju wrote:
> > > This patch implements an in-kernel gdb stub.
> > > It provides an interface between gdb and Linux Kernel by implementing
> > > the remote serial protocol.  This gdbstub uses utrace infrastructure.
> > > This patch provides register set access, signal mapping, process event
> > > handling, input/output operations.
> > > 
> > > /proc/<pid>/gdb was chosen as file for gdb to interact with the
> > > process through remote serial protocol. 
> > > 
> > > Hence users would have to use "target remote /proc/<pid>/gdb" command
> > > on gdb prompt to start using this infrastructure.
> > > 
> > > For Breakpointing support, gdbstub needs User space breakpointing
> > > layer and uprobes layer which will be posted later.
> > 
> > How does this compare to kgdb and related efforts?
> 
> This is a In-kernel gdbstub to debug user space programs. 
> This stub doesnt help in debugging kernel.
> 
> Hence I am not sure how to compare kgdb gdbstub with this gdbstub.
> Can you please provide more pointers on what you were referring to?

Well, not even that much was clear from your changelog, so I wasn't
really sure wtf I was looking at. All it says was an in-kernel gdb stub,
what other than to debug the kernel would you need in-kernel stubs for?

So now my question is, what do you need an in-kernel stub to debug
userspace for?

In general, tell me about this patch thing, what does it do, why does it
do it, and how does it improve on the current situation.

Your changelog doesn't address any of those things, so wth are we
supposed to think?

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Srikar Dronamraju]

* Peter Zijlstra <peterz@infradead.org> [2009-11-30 13:41:47]:

> > 
> > This is a In-kernel gdbstub to debug user space programs. 
> > This stub doesnt help in debugging kernel.
> > 
> > Hence I am not sure how to compare kgdb gdbstub with this gdbstub.
> > Can you please provide more pointers on what you were referring to?
> 
> Well, not even that much was clear from your changelog, so I wasn't
> really sure wtf I was looking at. All it says was an in-kernel gdb stub,
> what other than to debug the kernel would you need in-kernel stubs for?
> 
> So now my question is, what do you need an in-kernel stub to debug
> userspace for?
> 

This stub would allow users use features provided by utrace but through
a gdb interface.  This idea was brought up in this year's Tracing
roundtable at the Linux Foundation Collaboration summit, April 8-10 in
San Francisco.  Here is the link to the minutes of the
meeting sent by Christoph Hellwig.
http://www.mail-archive.com/utrace-devel@redhat.com/msg00830.html

> In general, tell me about this patch thing, what does it do, why does it
> do it, and how does it improve on the current situation.

This is suppose to be one of the interfaces to use utrace, i.e Allow
gdb to use utrace features without having to change gdb itself.
Though there are not enough features in this patch, intentions include
support multi-threaded debugging, concurrent debugger support for same
process, syscall tracing.

For Breakpoint support(not yet submitted to LKML), it would use
execution out of line rather than the conventional inline-single
stepping.

I guess Christoph, Roland and Frank would be able to explain in a better
fashion the rational and advantages of this stub over convential gdb.

> 
> Your changelog doesn't address any of those things, so wth are we
> supposed to think?

Thanks for pointing out. 
--
Thanks and Regards
Srikar

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Peter Zijlstra]

On Mon, 2009-11-30 at 18:49 +0530, Srikar Dronamraju wrote:

> This is suppose to be one of the interfaces to use utrace, i.e Allow
> gdb to use utrace features without having to change gdb itself.
> Though there are not enough features in this patch, intentions include
> support multi-threaded debugging, concurrent debugger support for same
> process, syscall tracing.
> 
> For Breakpoint support(not yet submitted to LKML), it would use
> execution out of line rather than the conventional inline-single
> stepping.
> 
> I guess Christoph, Roland and Frank would be able to explain in a better
> fashion the rational and advantages of this stub over convential gdb.

Hmm,. wouldn't it make much more sense to extend the current kgdb stub
to include userspace debugging, providing an all-in-one solution?

I think it would be much more powerful to be able to observe the full
software stack and not be limited by this user<->kernel barrier.

(Provided the user has sufficient privileges of course).

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Srikar Dronamraju]

> > 
> > I guess Christoph, Roland and Frank would be able to explain in a better
> > fashion the rational and advantages of this stub over convential gdb.
> 
> Hmm,. wouldn't it make much more sense to extend the current kgdb stub
> to include userspace debugging, providing an all-in-one solution?

I see two limitations but I guess there could be ways to get over it.
1. gdb requiring file that needs to be debugged. I always
thought it can either be a user program or a vmlinux file. gdb makes
most of the information(i.e registers) from the remote protocol to
display the backtrace, variable values and the like by reading this
file. 

2.  Also I am not sure if gdb has a way to tell the remote to switch the
context and provide information(registers) pertaining to the user mode/
kernel mode.

There could be other limitations too that I may not be aware.
> 
> I think it would be much more powerful to be able to observe the full
> software stack and not be limited by this user<->kernel barrier.
> 
> (Provided the user has sufficient privileges of course).

In this implementation, the current user can debug his/her own
processes. May be if we can debug both the context from same gdb then we
might have to place restrictions.. 

--
Thanks and Regards
Srikar
> 

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

peterz wrote: 

> Hmm,. wouldn't it make much more sense to extend the current kgdb stub
> to include userspace debugging, providing an all-in-one solution?
> I think it would be much more powerful to be able to observe the full
> software stack and not be limited by this user<->kernel barrier.

There exist other tools for this broad a scope (systemtap being one),
and present gdb is not well suited for this.  That makes this idea an
exciting potential for the future, but not a practical short-term
goal.

- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Peter Zijlstra]

On Mon, 2009-11-30 at 10:03 -0500, Frank Ch. Eigler wrote:
> peterz wrote: 
> 
> > Hmm,. wouldn't it make much more sense to extend the current kgdb stub
> > to include userspace debugging, providing an all-in-one solution?
> > I think it would be much more powerful to be able to observe the full
> > software stack and not be limited by this user<->kernel barrier.
> 
> There exist other tools for this broad a scope (systemtap being one),
> and present gdb is not well suited for this.  That makes this idea an
> exciting potential for the future, but not a practical short-term
> goal.

Who's talking short-term? And if gdb is unsuited, it'll simply have to
get fixed :-)

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> peterz wrote: 
> 
> > Hmm,. wouldn't it make much more sense to extend the current kgdb stub
> > to include userspace debugging, providing an all-in-one solution?
> > I think it would be much more powerful to be able to observe the full
> > software stack and not be limited by this user<->kernel barrier.
> 
> There exist other tools for this broad a scope (systemtap being one), 
> and present gdb is not well suited for this.  That makes this idea an 
> exciting potential for the future, but not a practical short-term 
> goal.

Well, but Peter's suggestion is the obvious next step - or even a 
necessary first step in my view.

kgdb exists here and today in the kernel and you cannot just build a 
facility that doesnt replace it and doesnt integrate well with it.

So if a unified user/kernel model for debugging is a 'long term' feature 
in your view then perhaps this framework (which introduces _extensive_ 
hooks all around the kernel) is not designed/approached in the right way 
and should not be merged in this form.

Concentrating on 'other tools' just generates extensive dependencies on 
something that is lacking - making it even harder to implement unified 
debugging down the line.

Thanks,

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

On Mon, Nov 30, 2009 at 04:16:50PM +0100, Ingo Molnar wrote:
> [...]
> kgdb exists here and today in the kernel and you cannot just build a 
> facility that doesnt replace it and doesnt integrate well with it.

Surely you don't mean that: every non-kgdb facility in the kernel
meets that definition, even all debugging-related facilities such as
perf and ftrace.


> So if a unified user/kernel model for debugging is a 'long term'
> feature in your view then perhaps this framework (which introduces
> _extensive_ hooks all around the kernel) is not designed/approached
> in the right way and should not be merged in this form.

Which "this framework" are you talking about?  Please clarify what
exactly you're trying to say.


- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> On Mon, Nov 30, 2009 at 04:16:50PM +0100, Ingo Molnar wrote:
> > [...]
> > kgdb exists here and today in the kernel and you cannot just build a 
> > facility that doesnt replace it and doesnt integrate well with it.
> 
> Surely you don't mean that: every non-kgdb facility in the kernel 
> meets that definition, even all debugging-related facilities such as 
> perf and ftrace.

Those facilities are not overlapping with kgdb though so my point doesnt 
apply to them. An in-kernel gdb server sure overlaps/extends kgdb 
though.

Btw., perf does meet that definition: it functionally replaces all 
facilities that it overlaps/extends - such as Oprofile. 'What about 
Oprofile?' was one of the first questions asked when we posted perf. 
'What about kgdb?' is the first question to the in-kernel gdb stub 
submission here, for similar reasons.

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

Hi -

On Tue, Dec 01, 2009 at 05:11:32PM +0100, Ingo Molnar wrote:
> Those facilities are not overlapping with kgdb though so my point doesnt 
> apply to them. An in-kernel gdb server sure overlaps/extends kgdb 
> though.

Only in name.  One is highly invasive, for debugging the kernel across
serial consoles.  The other is highly noninvasive, for debugging user
processes across normal userspace channels.  They both happen to talk
to gdb, but that's the end of the natural "overlap".

Even if kgdb was extended to be able to manage userspace, and if gdb
itself was extended to be able to use that same single channel, this
would still not duplicate the use scenario for an ordinary user
debugging his own processes.

(Plus, in the future where at least gdb is applied toward kernel+user
debugging, it is unlikely to be the case that this would need to be
done *over a single channel*.  A separate channel for kernel and
separate channels for userspace programs are no less likely.)

> Btw., perf does meet that definition: it functionally replaces all 
> facilities that it overlaps/extends - such as Oprofile. [...]

(And they currently separately coexist.)

- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> Hi -
> 
> On Tue, Dec 01, 2009 at 05:11:32PM +0100, Ingo Molnar wrote:
>
> > Those facilities are not overlapping with kgdb though so my point 
> > doesnt apply to them. An in-kernel gdb server sure overlaps/extends 
> > kgdb though.
> 
> Only in name.  One is highly invasive, for debugging the kernel across 
> serial consoles.  The other is highly noninvasive, for debugging user 
> processes across normal userspace channels.  They both happen to talk 
> to gdb, but that's the end of the natural "overlap".
> 
> Even if kgdb was extended to be able to manage userspace, and if gdb 
> itself was extended to be able to use that same single channel, this 
> would still not duplicate the use scenario for an ordinary user 
> debugging his own processes.
> 
> (Plus, in the future where at least gdb is applied toward kernel+user 
> debugging, it is unlikely to be the case that this would need to be 
> done *over a single channel*.  A separate channel for kernel and 
> separate channels for userspace programs are no less likely.)

Well nothing that you mention here changes our obvious suggestion that 
an in-kernel gdb stub should obviously either be a kgdb extension, or a 
replacement of it. We dont want to separate facilities for the same 
conceptual thing: examining application state (be that in user-space and 
kernel-space).

> > Btw., perf does meet that definition: it functionally replaces all 
> > facilities that it overlaps/extends - such as Oprofile. [...]
> 
> (And they currently separately coexist.)

You didnt get my point apparently. Keeping the overlapped facility for 
compatibility (and general user inertia) is fine. Creating a new 
facility that doesnt do everything that the existing facility does, and 
not integrating it either, is not fine.

Which was both Peter's and my point really.

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

Hi -  

> > Only in name.  One is highly invasive, for debugging the kernel across 
> > serial consoles.  The other is highly noninvasive, for debugging user 
> > processes across normal userspace channels.  They both happen to talk 
> > to gdb, but that's the end of the natural "overlap".
> [...]

> Well nothing that you mention here changes our obvious suggestion that 
> an in-kernel gdb stub should obviously either be a kgdb extension, or a 
> replacement of it.

Help me out here: by "kgdb extension" do you imagine "something new
that an unprivileged user can use to debug his own process"?  Or do
you imagine a new userspace facility that single-steps the kernel? 


> We dont want to separate facilities for the same conceptual thing:
> examining application state (be that in user-space and
> kernel-space).

This seems like a shallow sort of consistency.  kgdb was added after
ptrace existed -- why not extend ptrace instead to target the kernel?
After all, it's "examining application state".  The answer is that it
doesn't make a heck of a lot of sense.


> > > Btw., perf does meet that definition: it functionally replaces all 
> > > facilities that it overlaps/extends - such as Oprofile. [...]
> > 
> > (And they currently separately coexist.)
> 
> You didnt get my point apparently. Keeping the overlapped facility for 
> compatibility (and general user inertia) is fine. Creating a new 
> facility that doesnt do everything that the existing facility does, and 
> not integrating it either, is not fine.

oprofile and perfctr are closer in concept than kgdb and ptrace, yet
AFAIK perfctr doesn't "interface" to oprofile, except perhaps to the
extent of resolving contention over the underlying physical resources.
In any case this is not a great analogy.


- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> Hi -  
> 
> > > Only in name.  One is highly invasive, for debugging the kernel across 
> > > serial consoles.  The other is highly noninvasive, for debugging user 
> > > processes across normal userspace channels.  They both happen to talk 
> > > to gdb, but that's the end of the natural "overlap".
> > [...]
> 
> > Well nothing that you mention here changes our obvious suggestion 
> > that an in-kernel gdb stub should obviously either be a kgdb 
> > extension, or a replacement of it.
> 
> Help me out here: by "kgdb extension" do you imagine "something new 
> that an unprivileged user can use to debug his own process"?  Or do 
> you imagine a new userspace facility that single-steps the kernel?

Is this a trick question? Single-stepping the kernel on the same system 
[especially if it's an UP system] would certainly be a challenge ;-)

What i mean is what i said: if you provide a new framework (especially 
if it's user visible - which both kgdb and the gdb stub is) you should 
either fully replace existing functionality or extend it. Overlapping it 
in an incomplete way is not useful to anyone.

Extending kgdb to allow the use of it as if we used gdb locally would 
certainly be interesting - and then you could drop into the kernel 
anytime as well. But i'm not siding with any particular solution - i'm 
just seconding Peter's point that there's very clear overlap and 
inconsistency, and that ought to be resolved one way or another.

> > We dont want to separate facilities for the same conceptual thing:
> > examining application state (be that in user-space and
> > kernel-space).
> 
> This seems like a shallow sort of consistency.  kgdb was added after 
> ptrace existed -- why not extend ptrace instead to target the kernel? 
> After all, it's "examining application state".  The answer is that it 
> doesn't make a heck of a lot of sense.

kgdb simply used gdb's preferred way of remote debugging. That's 
certainly the ugliest bit of it btw - but it's an externality to kgdb.

Had it extended ptrace it wouldnt have gdb compatibility.

So i think this example of yours is inapposite as well.

Having said all that, i certainly subscribe to the view that neither 
kgdb nor ptrace is particularly cleanly done. So i wouldnt mind if 
something new existed that had a modern, flexible, extensible and 
generally pleasant interface and implementation. If you are heading in 
that direction, please let me know.

> > > > Btw., perf does meet that definition: it functionally replaces all 
> > > > facilities that it overlaps/extends - such as Oprofile. [...]
> > > 
> > > (And they currently separately coexist.)
> > 
> > You didnt get my point apparently. Keeping the overlapped facility for 
> > compatibility (and general user inertia) is fine. Creating a new 
> > facility that doesnt do everything that the existing facility does, and 
> > not integrating it either, is not fine.
> 
> oprofile and perfctr are closer in concept than kgdb and ptrace, yet 
> AFAIK perfctr doesn't "interface" to oprofile, except perhaps to the 
> extent of resolving contention over the underlying physical resources. 
> In any case this is not a great analogy.

(FYI, 'perfctr' is a different project that has existed for years, i 
suspect you meant perf events?)

perf replaces oprofile functionally. If the in-kernel gdb stub replaced 
kgdb functionally you'd hear no complaints from me.

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

Hi -


> > Help me out here: by "kgdb extension" do you imagine "something new 
> > that an unprivileged user can use to debug his own process"?  Or do 
> > you imagine a new userspace facility that single-steps the kernel?
> 
> Is this a trick question? Single-stepping the kernel on the same system 
> [especially if it's an UP system] would certainly be a challenge ;-)
> 
> What i mean is what i said: if you provide a new framework (especially 
> if it's user visible - which both kgdb and the gdb stub is) you should 
> either fully replace existing functionality or extend it. Overlapping it 
> in an incomplete way is not useful to anyone.

But there is no "overlap" beyond the name.  The functional scope of
the two interfaces is totally non-overlapping, and are consistent with
the current chasms between kernel- and user-side debugging.

Sure, in the future, it may make sense to teach the kernel-side (kgdb
serial console) interface to manipulate userspace.  But that will
require a gdb extension.  And it would not satisfy an unprivileged
user's need to debug pure userspace (in a better way than current
ptrace can).

This is why I keep asking for specificity as to this "new framework"
you imagine.  Just sharing definitions such as kgdb_arch/kgdb_io but
otherwise completely disconnected (separate channels)?


> Extending kgdb to allow the use of it as if we used gdb locally would 
> certainly be interesting - and then you could drop into the kernel 
> anytime as well.

(Is this a restatement of the "trick question" idea?)


> > > We dont want to separate facilities for the same conceptual thing:
> > > examining application state (be that in user-space and
> > > kernel-space).

> > This seems like a shallow sort of consistency.  kgdb was added after 
> > ptrace existed -- why not extend ptrace instead to target the kernel? 
> > After all, it's "examining application state".  The answer is that it 
> > doesn't make a heck of a lot of sense.
> 
> kgdb simply used gdb's preferred way of remote debugging. That's 
> certainly the ugliest bit of it btw - but it's an externality to kgdb.
> Had it extended ptrace it wouldnt have gdb compatibility.

So, because of a constraint for gdb compatibility, you built a
separate interface for kgdb vs.  ptrace.  Fine.  Do you accept that,
even if a hypothetical single channel existed for which kernel- and
user-space debugging could occur, current gdb is not compatible with
this?  So by your own reasoning, such a facility should not be
mandated as a "necessary first step".


> [...]  perf replaces oprofile functionally.

(I'm told that it's not a strict superset from a functional point of
view, FWIW, something about a larger selection of low level hardware
counters.)

> If the in-kernel gdb stub replaced kgdb functionally you'd hear no
> complaints from me.

Let's leave it as an idea for the future.


- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> > [...]
> >
> > If the in-kernel gdb stub replaced kgdb functionally you'd hear no 
> > complaints from me.
> 
> Let's leave it as an idea for the future.

We came a full circle - that's the argument. We say overlap, duplication 
and incomplete implementation in this area is a problem. Since the speed 
of development in this area is truly glacial at the moment and the 
practical advantages that i can experience personally (directly as a 
Linux user and indirectly as a maintainer) are miniscule so far, caution 
is warranted IMO. You say there is no problem. We'll have to agree to 
disagree i guess.

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

Ingo Molnar <mingo@elte.hu> writes:

> [...]  Since the speed of development in this area is truly glacial
> at the moment and the practical advantages that i can experience
> personally (directly as a Linux user and indirectly as a maintainer)
> are miniscule so far, caution is warranted IMO. [...]

If the "caution" you suggest is operationally equivalent to
discouraging even miniscule improvements, is it any wonder that
progress is glacial?

The gdbstub prototype was constructed for two reasons: to demonstrate
utrace usage now, and in the future to be incrementally useful (over
ptrace, by moving into fast kernel-space operations like
multithreading control, gdb-tracepoint support, other stuff).  #1 is
about done.  With respect to #2, we can certainly commit to ongoing
work on improvements, provided the community shows interest and
goodwill.

- FChE

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Ingo Molnar]

* Frank Ch. Eigler <fche@redhat.com> wrote:

> 
> Ingo Molnar <mingo@elte.hu> writes:
> 
> > [...]  Since the speed of development in this area is truly glacial
> > at the moment and the practical advantages that i can experience
> > personally (directly as a Linux user and indirectly as a maintainer)
> > are miniscule so far, caution is warranted IMO. [...]
> 
> If the "caution" you suggest is operationally equivalent to 
> discouraging even miniscule improvements, is it any wonder that 
> progress is glacial?

I think you might be mixing up cause and causation ;-)

> The gdbstub prototype was constructed for two reasons: to demonstrate 
> utrace usage now, and in the future to be incrementally useful (over 
> ptrace, by moving into fast kernel-space operations like 
> multithreading control, gdb-tracepoint support, other stuff).  #1 is 
> about done.  With respect to #2, we can certainly commit to ongoing 
> work on improvements, provided the community shows interest and 
> goodwill.

What i'd like to see is measurable benefits to users, developers and 
maintainers. I'd like to see the same for SystemTap too btw.

	Ingo

Re: [RFC] [PATCH] In-kernel gdbstub based on utrace Infrastructure.

[Frank Ch. Eigler]

Hi -

On Thu, Dec 10, 2009 at 07:16:38PM +0100, Ingo Molnar wrote:
> [...]
> > The gdbstub prototype was constructed for two reasons: to demonstrate 
> > utrace usage now, and in the future to be incrementally useful (over 
> > ptrace, by moving into fast kernel-space operations like 
> > multithreading control, gdb-tracepoint support, other stuff).  [...]
> 
> What i'd like to see is measurable benefits to users, developers and 
> maintainers. 

OK, it's clear that in the gdb-stub's case, some positive motivation
beyond it being an api-educational example would be appropriate before
merging.  Note that it was only an RFC at the time.


> I'd like to see the same for SystemTap too btw.

systemtap's benefits are beyond question to its users.  (Others are
worried about systemtap problems, but that wasn't your question.)  In
what form do you expect to see such measurements?  It would help if
you could point out prior examples of such "measurable benefit"
analyses that perchance accompanied other then-new kernel features,
say in the tracing/debugging area.


- FChE