[KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Tom Rini]

Hello.  The following patch moves all of the config options into one file,
kernel/Kconfig.kgdb.

diff -zrupN linux-2.6.3+nothing/arch/i386/Kconfig linux-2.6.3+config+serial/arch/i386/Kconfig
--- linux-2.6.3+nothing/arch/i386/Kconfig	2004-02-27 12:16:14.296187607 -0700
+++ linux-2.6.3+config+serial/arch/i386/Kconfig	2004-02-27 12:16:13.707320867 -0700
@@ -1253,36 +1253,7 @@ config DEBUG_SPINLOCK_SLEEP
 	  If you say Y here, various routines which may sleep will become very
 	  noisy if they are called with a spinlock held.	
 
-config KGDB
-	bool "KGDB: kernel debugging with remote gdb"
-	depends on DEBUG_KERNEL
-	select DEBUG_INFO
-	select FRAME_POINTER
-	help
-	  If you say Y here, it will be possible to remotely debug the
-	  kernel using gdb. This enlarges your kernel image disk size by
-	  several megabytes and requires a machine with more than 128 MB
-	  RAM to avoid excessive linking time. 
-	  Documentation of kernel debugger available at
-	  http://kgdb.sourceforge.net
-	  This is only useful for kernel hackers. If unsure, say N.
-
-config KGDB_THREAD
-	bool "KGDB: Thread analysis"
-	depends on KGDB
-	help
-	  With thread analysis enabled, gdb can talk to kgdb stub to list
-	  threads and to get stack trace for a thread. This option also enables
-	  some code which helps gdb get exact status of thread. Thread analysis
-	  adds some overhead to schedule and down functions. You can disable
-	  this option if you do not want to compromise on speed.
-
-config KGDB_CONSOLE
-	bool "KGDB: Console messages through gdb"
-	depends on KGDB
-	help
-	  If you say Y here, console messages will appear through gdb.
-	  Other consoles such as tty or ttyS will continue to work as usual.
+source "kernel/Kconfig.kgdb"
 
 config FRAME_POINTER
 	bool "Compile the kernel with frame pointers"
diff -zrupN linux-2.6.3+nothing/arch/ppc/Kconfig linux-2.6.3+config+serial/arch/ppc/Kconfig
--- linux-2.6.3+nothing/arch/ppc/Kconfig	2004-02-27 12:16:14.403163398 -0700
+++ linux-2.6.3+config+serial/arch/ppc/Kconfig	2004-02-27 12:16:13.771306387 -0700
@@ -1170,52 +1170,7 @@ config DEBUG_SPINLOCK_SLEEP
 	  If you say Y here, various routines which may sleep will become very
 	  noisy if they are called with a spinlock held.
 
-config KGDB
-	bool "Include kgdb kernel debugger"
-	depends on DEBUG_KERNEL
-	select DEBUG_INFO
-	select FRAME_POINTER
-	help
-	  Include in-kernel hooks for kgdb, the Linux kernel source level
-	  debugger.  See <http://kgdb.sourceforge.net/> for more information.
-	  Unless you are intending to debug the kernel, say N here.
-
-choice
-	prompt "Serial Port"
-	depends on KGDB
-	default KGDB_TTYS1
-
-config KGDB_TTYS0
-	bool "ttyS0"
-
-config KGDB_TTYS1
-	bool "ttyS1"
-
-config KGDB_TTYS2
-	bool "ttyS2"
-
-config KGDB_TTYS3
-	bool "ttyS3"
-
-endchoice
-
-config KGDB_THREAD
-	bool "KGDB: Thread analysis"
-	depends on KGDB
-	help
-	  With thread analysis enabled, gdb can talk to kgdb stub to list
-	  threads and to get stack trace for a thread. This option also enables
-	  some code which helps gdb get exact status of thread. Thread analysis
-	  adds some overhead to schedule and down functions. You can disable
-	  this option if you do not want to compromise on speed.
-
-config KGDB_CONSOLE
-	bool "Enable serial console thru kgdb port"
-	depends on KGDB && 8xx || 8260
-	help
-	  If you enable this, all serial console messages will be sent
-	  over the gdb stub.
-	  If unsure, say N.
+source "kernel/Kconfig.kgdb"
 
 config XMON
 	bool "Include xmon kernel debugger"
diff -zrupN linux-2.6.3+nothing/arch/x86_64/Kconfig linux-2.6.3+config+serial/arch/x86_64/Kconfig
--- linux-2.6.3+nothing/arch/x86_64/Kconfig	2004-02-27 12:16:14.350175389 -0700
+++ linux-2.6.3+config+serial/arch/x86_64/Kconfig	2004-02-27 12:16:13.718318378 -0700
@@ -465,37 +465,7 @@ config IOMMU_LEAK
          Add a simple leak tracer to the IOMMU code. This is useful when you
 	 are debugging a buggy device driver that leaks IOMMU mappings.
        
-config KGDB
-	bool "KGDB: kernel debugging with remote gdb"
-	depends on DEBUG_KERNEL
-	select DEBUG_INFO
-	select FRAME_POINTER
-	help
-	  If you say Y here, it will be possible to remotely debug the
-	  kernel using gdb. This enlarges your kernel image disk size by
-	  several megabytes and requires a machine with more than 128 MB
-	  RAM to avoid excessive linking time. 
-	  Documentation of kernel debugger available at
-	  http://kgdb.sourceforge.net
-	  This is only useful for kernel hackers. If unsure, say N.
-
-config KGDB_THREAD
-	bool "KGDB: Thread analysis"
-	depends on KGDB
-	help
-	  With thread analysis enabled, gdb can talk to kgdb stub to list
-	  threads and to get stack trace for a thread. This option also enables
-	  some code which helps gdb get exact status of thread. Thread analysis
-	  adds some overhead to schedule and down functions. You can disable
-	  this option if you do not want to compromise on speed.
-
-config KGDB_CONSOLE
-	bool "KGDB: Console messages through gdb"
-	depends on KGDB
-	help
-	  If you say Y here, console messages will appear through gdb.
-	  Other consoles such as tty or ttyS will continue to work as usual.
-
+source "kernel/Kconfig.kgdb"
 endmenu
 
 source "security/Kconfig"
diff -zrupN linux-2.6.3+nothing/drivers/net/Kconfig linux-2.6.3+config+serial/drivers/net/Kconfig
--- linux-2.6.3+nothing/drivers/net/Kconfig	2004-02-27 12:16:14.521136701 -0700
+++ linux-2.6.3+config+serial/drivers/net/Kconfig	2004-02-27 12:06:22.000000000 -0700
@@ -187,12 +187,6 @@ config NET_ETHERNET
 	  Note that the answer to this question won't directly affect the
 	  kernel: saying N will just cause the configurator to skip all
 	  the questions about Ethernet network cards. If unsure, say N.
-	
-config KGDB_ETH
-	bool "KGDB: On ethernet"
-	depends on KGDB
-	help
-	  Uses ethernet interface for kgdb.
 
 config MII
 	tristate "Generic Media Independent Interface device support"
diff -zrupN linux-2.6.3+nothing/drivers/serial/Kconfig linux-2.6.3+config+serial/drivers/serial/Kconfig
--- linux-2.6.3+nothing/drivers/serial/Kconfig	2004-02-27 12:16:14.545131271 -0700
+++ linux-2.6.3+config+serial/drivers/serial/Kconfig	2004-02-27 12:06:30.000000000 -0700
@@ -6,34 +6,6 @@
 
 menu "Serial drivers"
 
-config KGDB_8250
-	bool "KGDB: On generic serial port (8250)"
-	depends on KGDB
-	help
-	  Uses generic serial port (8250) for kgdb. This is independent of the
-	  option 9250/16550 and compatible serial port.
-
-config KGDB_PORT
-	hex "hex I/O port address of the debug serial port"
-	depends on KGDB_8250
-	default  3f8
-	help
-	  Some systems (x86 family at this writing) allow the port
-	  address to be configured.  The number entered is assumed to be
-	  hex, don't put 0x in front of it.  The standard address are:
-	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
-	  will tell you what you have.  It is good to test the serial
-	  connection with a live system before trying to debug.
-
-config KGDB_IRQ
-	int "IRQ of the debug serial port"
-	depends on KGDB_8250
-	default 4
-	help
-	  This is the irq for the debug port.  If everything is working
-	  correctly and the kernel has interrupts on a control C to the
-	  port should cause a break into the kernel debug stub.
-
 #
 # The new 8250/16550 serial drivers
 config SERIAL_8250
diff -zrupN linux-2.6.3+nothing/kernel/Kconfig.kgdb linux-2.6.3+config+serial/kernel/Kconfig.kgdb
--- linux-2.6.3+nothing/kernel/Kconfig.kgdb	1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.3+config+serial/kernel/Kconfig.kgdb	2004-02-27 12:16:13.000000000 -0700
@@ -0,0 +1,141 @@
+config KGDB
+	bool "KGDB: kernel debugging with remote gdb"
+	depends on DEBUG_KERNEL
+	select DEBUG_INFO
+	select FRAME_POINTER
+	# XXX: Doesn't work w/o this right now
+	select KGDB_THREAD if PPC32
+	help
+	  If you say Y here, it will be possible to remotely debug the
+	  kernel using gdb. This enlarges your kernel image disk size by
+	  several megabytes and requires a machine with more than 128 MB
+	  RAM to avoid excessive linking time. 
+	  Documentation of kernel debugger available at
+	  http://kgdb.sourceforge.net
+	  This is only useful for kernel hackers. If unsure, say N.
+
+choice
+	prompt "Method for KGDB communication"
+	depends on KGDB
+	default PPC_SIMPLE_SERIAL if PPC32 && (8xx || 8260)
+	default KGDB_8250
+	help
+	  There are a number of different ways in which you can communicate
+	  with KGDB.  The oldest is using a serial driver.  A newer method
+	  is to use UDP packets and a special network driver.
+
+config KGDB_8250
+	bool "KGDB: On generic serial port (8250)"
+	help
+	  Uses generic serial port (8250) for kgdb. This is independent of the
+	  option 9250/16550 and compatible serial port.
+
+config KGDB_ETH
+	bool "KGDB: On ethernet"
+	select NETPOLL
+	select NETPOLL_TRAP
+	select NETPOLL_RX
+	help
+	  Uses ethernet interface for kgdb.
+
+config PPC_SIMPLE_SERIAL 
+	bool "KGDB: On any serial port"
+	depends on PPC32
+	help
+	  Use a very simple, and not necessarily feature complete serial
+	  driver.  This is the only serial option currently for MPC8xx or
+	  MPC82xx based ports that do not offer an 8250-style UART.
+
+endchoice
+
+config KGDB_SIMPLE_SERIAL
+	bool "Simple selection of KGDB serial port"
+	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
+	help
+	  If you say Y here, you will only have to pick the baud rate
+	  and serial port (ttyS) that you wish to use for KGDB.  If you
+	  say N, you will have provide the I/O port and IRQ number.  Note
+	  that if your serial ports are iomapped, then you must say Y here.
+	  If in doubt, say Y.
+
+choice
+	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
+    	prompt "Debug serial port BAUD"
+	default KGDB_115200BAUD
+	help
+	  Gdb and the kernel stub need to agree on the baud rate to be
+	  used.  Some systems (x86 family at this writing) allow this to
+	  be configured.
+
+config KGDB_9600BAUD
+	bool "9600"
+
+config KGDB_19200BAUD
+	bool "19200"
+
+config KGDB_38400BAUD
+	bool "38400"
+
+config KGDB_57600BAUD
+	bool "57600"
+
+config KGDB_115200BAUD
+	bool "115200"
+endchoice
+
+choice
+	prompt "Serial port for KGDB"
+	depends on KGDB_SIMPLE_SERIAL
+	default KGDB_TTYS0
+
+config KGDB_TTYS0
+	bool "ttyS0"
+
+config KGDB_TTYS1
+	bool "ttyS1"
+
+config KGDB_TTYS2
+	bool "ttyS2"
+
+config KGDB_TTYS3
+	bool "ttyS3"
+
+endchoice
+
+config KGDB_PORT
+	hex "hex I/O port address of the debug serial port"
+	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
+	default  3f8
+	help
+	  Some systems (x86 family at this writing) allow the port
+	  address to be configured.  The number entered is assumed to be
+	  hex, don't put 0x in front of it.  The standard address are:
+	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
+	  will tell you what you have.  It is good to test the serial
+	  connection with a live system before trying to debug.
+
+config KGDB_IRQ
+	int "IRQ of the debug serial port"
+	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
+	default 4
+	help
+	  This is the irq for the debug port.  If everything is working
+	  correctly and the kernel has interrupts on a control C to the
+	  port should cause a break into the kernel debug stub.
+
+config KGDB_THREAD
+	bool "KGDB: Thread analysis"
+	depends on KGDB
+	help
+	  With thread analysis enabled, gdb can talk to kgdb stub to list
+	  threads and to get stack trace for a thread. This option also enables
+	  some code which helps gdb get exact status of thread. Thread analysis
+	  adds some overhead to schedule and down functions. You can disable
+	  this option if you do not want to compromise on speed.
+
+config KGDB_CONSOLE
+	bool "KGDB: Console messages through gdb"
+	depends on KGDB
+	help
+	  If you say Y here, console messages will appear through gdb.
+	  Other consoles such as tty or ttyS will continue to work as usual.

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][2/7] Serial updates, take 2

[Tom Rini]

The following is my second attempt at making the serial driver more robust.
This has the following changes:
- We don't need kgdb_might_be_resumed or kgdb_killed_or_detached.
- Don't try and look for a connection in put_packet, after we've tried
  to put a packet.  Instead, when we receive a packet, GDB has
  connected.  Also make put_packet look at the char it reads, and handle
  a new packet in the middle (or ^C+packet).
- Remove ok_packet(), excessive, IMHO.
- In kgdb8250_interrupt, only check if this is the right line, and if so,
  call kgdb_schedule_breakpoint().  This ends up killing off a lot of
  (incomplete due to my not testing SMP) locking.
- Remove a duplicate extern of kgdb_connected.

diff -zrupN linux-2.6.3+nothing/arch/ppc/8260_io/uart.c linux-2.6.3+config+serial/arch/ppc/8260_io/uart.c
--- linux-2.6.3+nothing/arch/ppc/8260_io/uart.c	2004-02-27 12:16:14.397164756 -0700
+++ linux-2.6.3+config+serial/arch/ppc/8260_io/uart.c	2004-02-27 12:16:13.765307745 -0700
@@ -396,16 +396,10 @@ static _INLINE_ void receive_chars(ser_i
 #ifdef CONFIG_KGDB
 		if (info->state->smc_scc_num == KGDB_SER_IDX) {
 			while (i-- > 0) {
-				if (*cp == 0x03) {
+				if (*cp == 0x03 || *cp == '$') {
 					breakpoint();
 					return;
 				}
-				if (*cp == '$') {
-					atomic_set(&kgdb_might_be_resumed, 1);
-					breakpoint();
-					atomic_set(&kgdb_might_be_resumed, 0);
-					return;
-				}
 				cp++;
 			}
 			bdp->cbd_sc |= BD_SC_EMPTY;
diff -zrupN linux-2.6.3+nothing/drivers/serial/kgdb_8250.c linux-2.6.3+config+serial/drivers/serial/kgdb_8250.c
--- linux-2.6.3+nothing/drivers/serial/kgdb_8250.c	2004-02-27 12:16:14.557128556 -0700
+++ linux-2.6.3+config+serial/drivers/serial/kgdb_8250.c	2004-02-27 12:16:13.000000000 -0700
@@ -17,7 +17,6 @@
 #include <linux/config.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/spinlock.h>
 #include <linux/kgdb.h>
 #include <linux/interrupt.h>
 #include <linux/tty.h>
@@ -166,32 +165,15 @@ read_data_bfr(void)
 /*
  * Get a char if available, return -1 if nothing available.
  * Empty the receive buffer first, then look at the interface hardware.
- *
- * Locking here is a bit of a problem.	We MUST not lock out communication
- * if we are trying to talk to gdb about a kgdb entry.	ON the other hand
- * we can loose chars in the console pass thru if we don't lock.  It is also
- * possible that we could hold the lock or be waiting for it when kgdb
- * NEEDS to talk.  Since kgdb locks down the world, it does not need locks.
- * We do, of course have possible issues with interrupting a uart operation,
- * but we will just depend on the uart status to help keep that straight.
  */
 
-static spinlock_t uart_interrupt_lock = SPIN_LOCK_UNLOCKED;
-#ifdef CONFIG_SMP
-extern spinlock_t kgdb_spinlock;
-#endif
-
 int
 kgdb_get_debug_char(void)
 {
 	int retchr;
 	unsigned long flags;
 	local_irq_save(flags);
-#ifdef CONFIG_SMP
-	if (!spin_is_locked(&kgdb_spinlock)) {
-		spin_lock(&uart_interrupt_lock);
-	}
-#endif
+
 	/* intr routine has q'd chars */
 	if (atomic_read(&kgdb8250_buf_in_cnt) != 0) {
 		retchr = kgdb8250_buf[kgdb8250_buf_out_inx++];
@@ -205,11 +187,6 @@ kgdb_get_debug_char(void)
 	} while (retchr < 0);
 
 out:
-#ifdef CONFIG_SMP
-	if (!spin_is_locked(&kgdb_spinlock)) {
-		spin_unlock(&uart_interrupt_lock);
-	}
-#endif
 	local_irq_restore(flags);
 
 	return retchr;
@@ -217,72 +194,17 @@ out:
 
 /*
  * This is the receiver interrupt routine for the GDB stub.
- * It will receive a limited number of characters of input
- * from the gdb  host machine and save them up in a buffer.
- *
- * When kgdb_get_debug_char() is called it
- * draws characters out of the buffer until it is empty and
- * then reads directly from the serial port.
- *
- * We do not attempt to write chars from the interrupt routine
- * since the stubs do all of that via kgdb_put_debug_char() which
- * writes one byte after waiting for the interface to become
- * ready.
- *
- * The debug stubs like to run with interrupts disabled since,
- * after all, they run as a consequence of a breakpoint in
- * the kernel.
- *
- * Perhaps someone who knows more about the tty driver than I
- * care to learn can make this work for any low level serial
- * driver.
+ * All that we need to do is verify that the interrupt happened on the
+ * line we're in charge of.  If this is true, schedule a breakpoint and
+ * return.
  */
 static irqreturn_t
 kgdb8250_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
-	int chr, iir;
-	unsigned long flags;
-
 	if (irq != gdb_async_info.line)
 		return IRQ_NONE;
 
-	/* If we get an interrupt, then KGDB is trying to connect. */
-	if (!kgdb_connected) {
-		kgdb_schedule_breakpoint();
-		return IRQ_HANDLED;
-	}
-
-	local_irq_save(flags);
-	spin_lock(&uart_interrupt_lock);
-
-	do {
-		chr = read_data_bfr();
-		iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
-		if (chr < 0)
-			continue;
-
-		/* Check whether gdb sent interrupt */
-		if (chr == 3) {
-			breakpoint();
-			continue;
-		}
-
-		if (atomic_read(&kgdb8250_buf_in_cnt) >= GDB_BUF_SIZE) {
-			/* buffer overflow, clear it */
-			kgdb8250_buf_in_inx = 0;
-			atomic_set(&kgdb8250_buf_in_cnt, 0);
-			kgdb8250_buf_out_inx = 0;
-			break;
-		}
-
-		kgdb8250_buf[kgdb8250_buf_in_inx++] = chr;
-		kgdb8250_buf_in_inx &= (GDB_BUF_SIZE - 1);
-		atomic_inc(&kgdb8250_buf_in_cnt);
-	} while (iir & UART_IIR_RDI);
-
-	spin_unlock(&uart_interrupt_lock);
-	local_irq_restore(flags);
-
+	kgdb_schedule_breakpoint();
 	return IRQ_HANDLED;
 }
 
diff -zrupN linux-2.6.3+nothing/include/linux/kgdb.h linux-2.6.3+config+serial/include/linux/kgdb.h
--- linux-2.6.3+nothing/include/linux/kgdb.h	2004-02-27 12:16:14.000000000 -0700
+++ linux-2.6.3+config+serial/include/linux/kgdb.h	2004-02-27 12:16:13.000000000 -0700
@@ -36,10 +36,6 @@ extern volatile int kgdb_connected;
 #endif
 
 extern atomic_t kgdb_setting_breakpoint;
-extern atomic_t kgdb_killed_or_detached;
-extern atomic_t kgdb_might_be_resumed;
-
-extern volatile int kgdb_connected;
 
 extern struct task_struct *kgdb_usethread, *kgdb_contthread;
 
diff -zrupN linux-2.6.3+nothing/kernel/kgdb.c linux-2.6.3+config+serial/kernel/kgdb.c
--- linux-2.6.3+nothing/kernel/kgdb.c	2004-02-27 12:16:14.000000000 -0700
+++ linux-2.6.3+config+serial/kernel/kgdb.c	2004-02-27 12:16:13.000000000 -0700
@@ -15,6 +15,7 @@
  * Copyright (C) 2002-2004 Timesys Corporation
  * Copyright (C) 2003-2004 Amit S. Kale
  * Copyright (C) 2004 Pavel Machek <pavel@suse.cz>
+ * Copyright (C) 2004 Tom Rini <trini@kernel.crashing.org>
  *
  * Restructured KGDB for 2.6 kernels.
  * thread support, support for multiple processors,support for ia-32(x86) 
@@ -87,8 +88,6 @@ static const char hexchars[] = "01234567
 static spinlock_t slavecpulocks[KGDB_MAX_NO_CPUS];
 static volatile int procindebug[KGDB_MAX_NO_CPUS];
 atomic_t kgdb_setting_breakpoint;
-atomic_t kgdb_killed_or_detached;
-atomic_t kgdb_might_be_resumed;
 struct task_struct *kgdb_usethread, *kgdb_contthread;
 
 int debugger_step;
@@ -212,8 +211,10 @@ static void get_packet(char *buffer)
 	char ch;
 
 	do {
-		/* wait around for the start character, ignore all other characters */
-		while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$') ;
+		/* wait around for the start character, ignore all other
+		 * characters */
+		while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$')
+			;	/* Spin. */
 		kgdb_connected = 1;
 		checksum = 0;
 		xmitcsum = -1;
@@ -249,27 +250,22 @@ static void get_packet(char *buffer)
  * Send the packet in buffer.
  * Check for gdb connection if asked for.
  */
-static void put_packet(char *buffer, int checkconnect)
+static void put_packet(char *buffer)
 {
 	unsigned char checksum;
 	int count;
 	char ch;
-	static char gdbseq[] = "$Hc-1#09";
-	int i;
-	int send_count;
 
 	/*  $<packet info>#<checksum>. */
-	do {
+	while (1) {
 		kgdb_serial->write_char('$');
 		checksum = 0;
 		count = 0;
-		send_count = 0;
 
 		while ((ch = buffer[count])) {
 			kgdb_serial->write_char(ch);
 			checksum += ch;
 			count++;
-			send_count++;
 		}
 
 		kgdb_serial->write_char('#');
@@ -278,30 +274,27 @@ static void put_packet(char *buffer, int
 		if (kgdb_serial->flush)
 			kgdb_serial->flush();
 
-		i = 0;
-		while ((ch = kgdb_serial->read_char()) == gdbseq[i++] &&
-		       checkconnect) {
-			if (!gdbseq[i]) {
-				kgdb_serial->write_char('+');
-				if (kgdb_serial->flush)
-					kgdb_serial->flush();
-				breakpoint();
-
-				/*
-				 * GDB is available now.
-				 * Retransmit this packet.
-				 */
-				break;
-			}
-		}
-		if (checkconnect && ch == 3) {
-			kgdb_serial->write_char('+');
+		/* Now see what we get in reply. */
+		ch = kgdb_serial->read_char();
+
+		if (ch == 3)
+			ch = kgdb_serial->read_char();
+
+		/* If we get an ACK, we are done. */
+		if (ch == '+')
+			return;
+
+		/* If we get the start of another packet, this means
+		 * that GDB is attempting to reconnect.  We will NAK
+		 * the packet being sent, and stop trying to send this
+		 * packet. */
+		if (ch == '$') {
+			kgdb_serial->write_char('-');
 			if (kgdb_serial->flush)
 				kgdb_serial->flush();
-			breakpoint();
+			return;
 		}
-	} while ((ch & 0x7f) != '+');
-
+	}
 }
 
 static int get_char(char *addr, unsigned char *data)
@@ -427,11 +420,6 @@ static inline void error_packet(char *pk
 	pkt[3] = '\0';
 }
 
-static void ok_packet(char *pkt)
-{
-	strcpy(pkt, "OK");
-}
-
 static char *pack_threadid(char *pkt, threadref * id)
 {
 	char *limit;
@@ -502,7 +490,8 @@ void kgdb_wait(struct pt_regs *regs)
 	procindebug[processor] = 1;
 	current->thread.debuggerinfo = regs;
 
-	/* Wait till master processor goes completely into the debugger. FIXME: this looks racy */
+	/* Wait till master processor goes completely into the debugger.
+	 * FIXME: this looks racy */
 	while (!procindebug[atomic_read(&debugger_active) - 1]) {
 		int i = 10;	/* an arbitrary number */
 
@@ -701,17 +690,7 @@ int kgdb_handle_exception(int exVector, 
 	/* Master processor is completely in the debugger */
 	kgdb_post_master_code(linux_regs, exVector, err_code);
 
-	if (atomic_read(&kgdb_killed_or_detached) &&
-	    atomic_read(&kgdb_might_be_resumed)) {
-		get_packet(remcom_in_buffer);
-		if (remcom_in_buffer[0] == 'H' && remcom_in_buffer[1] == 'c') {
-			remove_all_break();
-			atomic_set(&kgdb_killed_or_detached, 0);
-			ok_packet(remcom_out_buffer);
-		} else
-			return 1;
-	} else {
-
+	if (kgdb_connected) {
 		/* reply to host that an exception has occurred */
 		ptr = remcom_out_buffer;
 		*ptr++ = 'T';
@@ -721,11 +700,9 @@ int kgdb_handle_exception(int exVector, 
 		int_to_threadref(&thref, shadow_pid(current->pid));
 		ptr = pack_threadid(ptr, &thref);
 		*ptr++ = ';';
-		*ptr = '\0';
-	}
 
-	put_packet(remcom_out_buffer, 0);
-	kgdb_connected = 1;
+		put_packet(remcom_out_buffer);
+	}
 
 	kgdb_usethread = current;
 	kgdb_usethreadid = shadow_pid(current->pid);
@@ -741,6 +718,11 @@ int kgdb_handle_exception(int exVector, 
 
 		switch (remcom_in_buffer[0]) {
 		case '?':
+			/* We know that this packet is only sent
+			 * during initial connect.  So to be safe,
+			 * we clear out our breakpoints now incase
+			 * GDB is reconnecting. */
+			remove_all_break();
 			remcom_out_buffer[0] = 'S';
 			remcom_out_buffer[1] = hexchars[signo >> 4];
 			remcom_out_buffer[2] = hexchars[signo % 16];
@@ -798,7 +780,7 @@ int kgdb_handle_exception(int exVector, 
 			else {
 				gdb_regs_to_regs(gdb_regs, (struct pt_regs *)
 						 current->thread.debuggerinfo);
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 			}
 
 			break;
@@ -838,10 +820,10 @@ int kgdb_handle_exception(int exVector, 
 			if ((error = remove_all_break()) < 0) {
 				error_packet(remcom_out_buffer, error);
 			} else {
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 				kgdb_connected = 0;
 			}
-			put_packet(remcom_out_buffer, 0);
+			put_packet(remcom_out_buffer);
 			goto default_handle;
 
 		case 'k':
@@ -947,11 +929,10 @@ int kgdb_handle_exception(int exVector, 
 				}
 				kgdb_usethread = thread;
 				kgdb_usethreadid = threadid;
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 				break;
 
 			case 'c':
-				atomic_set(&kgdb_killed_or_detached, 0);
 				ptr = &remcom_in_buffer[2];
 				kgdb_hex2long(&ptr, &threadid);
 				if (!threadid) {
@@ -966,7 +947,7 @@ int kgdb_handle_exception(int exVector, 
 					}
 					kgdb_contthread = thread;
 				}
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 				break;
 			}
 			break;
@@ -977,7 +958,7 @@ int kgdb_handle_exception(int exVector, 
 			kgdb_hex2long(&ptr, &threadid);
 			thread = getthread(linux_regs, threadid);
 			if (thread)
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 			else
 				error_packet(remcom_out_buffer, -EINVAL);
 			break;
@@ -1018,7 +999,7 @@ int kgdb_handle_exception(int exVector, 
 			}
 
 			if (error == 0)
-				ok_packet(remcom_out_buffer);
+				strcpy(remcom_out_buffer, "OK");
 			else
 				error_packet(remcom_out_buffer, error);
 
@@ -1039,7 +1020,7 @@ int kgdb_handle_exception(int exVector, 
 		}		/* switch */
 
 		/* reply to the request */
-		put_packet(remcom_out_buffer, 0);
+		put_packet(remcom_out_buffer);
 	}
 
       kgdb_exit:
@@ -1063,7 +1044,6 @@ int kgdb_handle_exception(int exVector, 
 	}
 
 	/* Free debugger_active */
-	atomic_set(&kgdb_killed_or_detached, 1);
 	atomic_set(&debugger_active, 0);
 	local_irq_restore(flags);
 
@@ -1132,12 +1112,6 @@ void kgdb_entry(void)
 	/* Free debugger_active */
 	atomic_set(&debugger_active, 0);
 
-	/* This flag is used, if gdb has detached and wants to start
-	 * another session
-	 */
-	atomic_set(&kgdb_killed_or_detached, 1);
-	atomic_set(&kgdb_might_be_resumed, 0);
-
 	for (i = 0; i < MAX_BREAKPOINTS; i++)
 		kgdb_break[i].state = bp_disabled;
 
@@ -1222,7 +1196,7 @@ void kgdb_console_write(struct console *
 		*bufptr = '\0';
 		s += wcount;
 
-		put_packet(kgdbconbuf, 1);
+		put_packet(kgdbconbuf);
 
 	}
 	local_irq_restore(flags);

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][3/7] SysRq-G

[Tom Rini]

Hello.  The following adds SysRq-G.  This is always configured in on
CONFIG_KGDB && CONFIG_SYSRQ.

diff -zrupN linux-2.6.3+config+serial/drivers/char/sysrq.c linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c
--- linux-2.6.3+config+serial/drivers/char/sysrq.c	2004-02-27 12:06:22.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c	2004-02-27 12:16:14.000000000 -0700
@@ -31,6 +31,7 @@
 #include <linux/suspend.h>
 #include <linux/writeback.h>
 #include <linux/buffer_head.h>		/* for fsync_bdev() */
+#include <linux/kgdb.h>			/* for breakpoint() */
 
 #include <linux/spinlock.h>
 
@@ -44,6 +45,25 @@ int sysrq_enabled = 1;
 /* Machine specific power off function */
 void (*sysrq_power_off)(void);
 
+/* Make a breakpoint() right now. */
+#ifdef CONFIG_KGDB
+#define  GDB_OP &kgdb_op
+static void kgdb_sysrq(int key, struct pt_regs *pt_regs, struct tty_struct *tty)
+{
+	printk("kgdb sysrq\n");
+	breakpoint();
+}
+
+static struct sysrq_key_op kgdb_op = {
+	.handler	= kgdb_sysrq,
+	.help_msg	= "kGdb|Fgdb",
+	.action_msg	= "Debug breakpoint\n",
+};
+
+#else
+#define  GDB_OP NULL
+#endif
+
 /* Loglevel sysrq handler */
 static void sysrq_handle_loglevel(int key, struct pt_regs *pt_regs,
 				  struct tty_struct *tty) 
@@ -239,7 +259,7 @@ static struct sysrq_key_op *sysrq_key_ta
 /* d */	NULL,
 /* e */	&sysrq_term_op,
 /* f */	NULL,
-/* g */	NULL,
+/* g */	GDB_OP,
 /* h */	NULL,
 /* i */	&sysrq_kill_op,
 /* j */	NULL,

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][4/7] Fix x86_64 hooks

[Tom Rini]

Hello.  The following is more complicated than it looks.  This makes the
x86_64 stub add kgdb to the die_chain, and then remove all of the now
unneeded CHK_DEBUGGER statements, and other whitespace changes.  In the end,
the x86_64 patch looks like:
 arch/x86_64/Kconfig            |    4 
 arch/x86_64/kernel/Makefile    |    1 
 arch/x86_64/kernel/entry.S     |   41 +++
 arch/x86_64/kernel/irq.c       |    2 
 arch/x86_64/kernel/kgdb.c      |  478 +++++++++++++++++++++++++++++++++++++++++
 arch/x86_64/kernel/traps.c     |    3 
 include/asm-x86_64/kgdb.h      |   54 ++++
 include/asm-x86_64/processor.h |    1 
 8 files changed, 577 insertions(+), 7 deletions(-)

As an aside, I now know how to kill off user_schedule bits on i386/x86_64, so
with more thought on how to do it cleanly for everyone, the entry.S changes
can die.

diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/kgdb.c linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/kgdb.c
--- linux-2.6.3+config+serial/arch/x86_64/kernel/kgdb.c	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/kgdb.c	2004-02-27 12:16:14.000000000 -0700
@@ -41,6 +41,7 @@
 #endif
 #include <linux/init.h>
 #include <linux/debugger.h>
+#include <asm/kdebug.h>
 
 /* Put the error code here just in case the user cares.  */
 int gdb_x86_64errcode;
@@ -443,6 +444,33 @@ struct pt_regs *kgdb_shadow_regs(struct 
 	return NULL;
 }
 
+/* Register KGDB with the die_chain so that we hook into all of the right
+ * spots. */
+static int kgdb_notify(struct notifier_block *self, unsigned long cmd,
+		void *ptr)
+{
+        struct die_args *d = ptr;
+
+        if (!kgdb_connected || (cmd == DIE_DEBUG && user_mode(d->regs)))
+                return NOTIFY_DONE;
+        if (cmd == DIE_NMI_IPI) {
+                if (atomic_read(debugger_active))
+                        return NOTIFY_BAD;
+        } else if ((*linux_debug_hook)(d->trapnr, d->signr, d->err, d->regs))
+                return NOTIFY_BAD; /* skip */
+
+        return NOTIFY_DONE;
+}
+
+static struct notifier_block kgdb_notifier = {
+        .notifier_call = kgdb_notify,
+};
+
+int kgdb_arch_init(void)
+{
+	notifier_chain_register(&die_chain, &kgdb_notifier);
+}
+
 struct kgdb_arch arch_kgdb_ops = {
 	.gdb_bpt_instr = {0xcc},
 	.flags = KGDB_HW_BREAKPOINT,
diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/nmi.c linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/nmi.c
--- linux-2.6.3+config+serial/arch/x86_64/kernel/nmi.c	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/nmi.c	2004-02-27 12:11:20.000000000 -0700
@@ -24,7 +24,6 @@
 #include <linux/module.h>
 #include <linux/sysdev.h>
 #include <linux/nmi.h>
-#include <linux/debugger.h>
 
 #include <asm/smp.h>
 #include <asm/mtrr.h>
@@ -318,26 +317,13 @@ void nmi_watchdog_tick (struct pt_regs *
 		return;
 
 	sum = read_pda(apic_timer_irqs);
-	if (atomic_read(&debugger_active)) {
-
-		/*
-		 * The machine is in debugger, hold this cpu if already
-		 * not held.
-		 */
-		debugger_nmihook(cpu, regs);
-		alert_counter[cpu] = 0;
-
-	} else if (last_irq_sums[cpu] == sum) {
-
+	if (last_irq_sums[cpu] == sum) {
 		/*
 		 * Ayiee, looks like this CPU is stuck ...
 		 * wait a few IRQs (5 seconds) before doing the oops ...
 		 */
 		alert_counter[cpu]++;
 		if (alert_counter[cpu] == 5*nmi_hz) {
-
-			CHK_DEBUGGER(2,SIGSEGV,0,regs,)
-
 			if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) == NOTIFY_BAD) { 
 				alert_counter[cpu] = 0; 
 				return;
diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/traps.c linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/traps.c
--- linux-2.6.3+config+serial/arch/x86_64/kernel/traps.c	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/traps.c	2004-02-27 12:16:14.000000000 -0700
@@ -364,7 +364,6 @@ void __die(const char * str, struct pt_r
 
 void die(const char * str, struct pt_regs * regs, long err)
 {
-	CHK_DEBUGGER(1,SIGTRAP,err,regs,)
 	oops_begin();
 	handle_BUG(regs);
 	__die(str, regs, err);
@@ -438,7 +437,6 @@ static void do_trap(int trapnr, int sign
 #define DO_ERROR(trapnr, signr, str, name) \
 asmlinkage void do_##name(struct pt_regs * regs, long error_code) \
 { \
-	CHK_DEBUGGER(1,SIGTRAP,error_code,regs,return) \
 	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) == NOTIFY_BAD) \
 		return; \
 	do_trap(trapnr, signr, str, regs, error_code, NULL); \
@@ -646,7 +644,6 @@ asmlinkage void do_debug(struct pt_regs 
 	force_sig_info(SIGTRAP, &info, tsk);	
 clear_dr7:
 	asm volatile("movq %0,%%db7"::"r"(0UL));
-	CHK_DEBUGGER(1,SIGTRAP,error_code,regs,return)
 	notify_die(DIE_DEBUG, "debug", regs, error_code, 1, SIGTRAP);
 	return;
 
diff -zrupN linux-2.6.3+config+serial/arch/x86_64/mm/fault.c linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/mm/fault.c
--- linux-2.6.3+config+serial/arch/x86_64/mm/fault.c	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/mm/fault.c	2004-02-27 12:10:49.000000000 -0700
@@ -23,7 +23,6 @@
 #include <linux/vt_kern.h>		/* For unblank_screen() */
 #include <linux/compiler.h>
 #include <linux/module.h>
-#include <linux/debugger.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -408,7 +407,6 @@ no_context:
 
  	if (is_prefetch(regs, address))
  		return;
-	CHK_DEBUGGER(14, SIGSEGV,error_code, regs,)
 
 	if (is_errata93(regs, address))
 		return; 
diff -zrupN linux-2.6.3+config+serial/include/asm-x86_64/calling.h linux-2.6.3+config+serial+sysrq+arch_hooks/include/asm-x86_64/calling.h
--- linux-2.6.3+config+serial/include/asm-x86_64/calling.h	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/include/asm-x86_64/calling.h	2004-02-27 12:12:03.000000000 -0700
@@ -12,7 +12,7 @@
 #define RBX 40
 /* arguments: interrupts/non tracing syscalls only save upto here*/
 #define R11 48
-#define R10 56
+#define R10 56	
 #define R9 64
 #define R8 72
 #define RAX 80

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][5/7] Fix ppc32 hooks.

[Tom Rini]

Hello.  The following removes the only PPC32 CHK_DEBUGGER statement,
as we've always been making kgdb take over the various debugger pointers.

diff -zrupN linux-2.6.3+config+serial/arch/ppc/mm/fault.c linux-2.6.3+config+serial+sysrq+arch_hooks/arch/ppc/mm/fault.c
--- linux-2.6.3+config+serial/arch/ppc/mm/fault.c	2004-02-27 12:16:13.000000000 -0700
+++ linux-2.6.3+config+serial+sysrq+arch_hooks/arch/ppc/mm/fault.c	2004-02-27 12:16:14.000000000 -0700
@@ -351,13 +351,11 @@ bad_page_fault(struct pt_regs *regs, uns
 	}
 
 	/* kernel has accessed a bad area */
-#if defined(CONFIG_XMON)
+#if defined(CONFIG_XMON) || defined(CONFIG_KGDB)
 	if (debugger_kernel_faults)
 		debugger(regs);
 #endif
 
-	CHK_DEBUGGER(14, sig,0, regs,)
-
 	die("kernel access of bad area", regs, sig);
 }
 

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][6/7] KGDBOE fixes

[Tom Rini]

Hello.  The following is a couple of small but important cleanups to kgdboe.
- memset out_buf in case we don't have it full when we flush.
- In rx_hook, if netpoll_trap() is set, clear it.  If kgdb_isn't connected,
  schedule a breakpoint, and never call breakpoint() directly.
- In kgdb_serial->hook, set netpoll_trap(1).  kgdb_serial->hook must be
  called prior to using a 'serial' port, so this is where we should stick
  this.
- Backout unneeded changes to other files.

--- linux-2.6.3/drivers/net/kgdb_eth.c.orig	2004-02-27 13:27:53.778658691 -0700
+++ linux-2.6.3/drivers/net/kgdb_eth.c	2004-02-27 13:32:07.920255062 -0700
@@ -18,6 +18,7 @@
  * Refactored for netpoll API by Matt Mackall <mpm@selenic.com>
  *
  * Some cleanups by Pavel Machek <pavel@suse.cz>
+ * Further cleanups by Tom Rini <trini@mvista.com>
  */
 
 #include <linux/module.h>
@@ -60,7 +61,6 @@
 static atomic_t in_count;
 int kgdboe = 0;			/* Default to tty mode */
 
-extern void breakpoint(void);
 static void rx_hook(struct netpoll *np, int port, char *msg, int len);
 
 static struct netpoll np = {
@@ -89,6 +89,7 @@
 {
 	if (out_count && np.dev) {
 		netpoll_send_udp(&np, out_buf, out_count);
+		memset(out_buf, 0, sizeof(out_buf));
 		out_count = 0;
 	}
 }
@@ -107,12 +108,15 @@
 	np->remote_port = port;

+	/* Do we need to clear the trap? */
+	if (netpoll_trap())
+		netpoll_set_trap(0);
 	/* Is this gdb trying to attach? */
-	if (!netpoll_trap() && len == 8 && !strncmp(msg, "$Hc-1#09", 8))
-		breakpoint();
+	if (kgdb_connected) {
+		kgdb_schedule_breakpoint();
 
 	for (i = 0; i < len; i++) {
 		if (msg[i] == 3)
-			breakpoint();
+			kgdb_schedule_breakpoint();
 
 		if (atomic_read(&in_count) >= IN_BUF_SIZE) {
 			/* buffer overflow, clear it */
@@ -138,6 +141,9 @@
 	/* Un-initalized, don't go further. */
 	if (kgdboe != 1)
 		return 1;
+
+	netpoll_set_trap(1);
+
 	return 0;
 }
 
--- linux-2.6.3/net/core/skbuff.c.orig	2004-02-27 13:30:21.107968572 -0700
+++ linux-2.6.3/net/core/skbuff.c	2004-02-27 13:30:44.279825113 -0700
@@ -55,7 +55,6 @@
 #include <linux/rtnetlink.h>
 #include <linux/init.h>
 #include <linux/highmem.h>
-#include <linux/debugger.h>
 
 #include <net/protocol.h>
 #include <net/dst.h>
--- linux-2.6.3/net/core/dev.c.orig	2004-02-27 13:30:27.687508165 -0700
+++ linux-2.6.3/net/core/dev.c	2004-02-27 13:30:44.261829108 -0700
@@ -1547,6 +1547,7 @@
 }
 #endif
 
+
 /**
  *	netif_rx	-	post buffer to the network code
  *	@skb: buffer to post

-- 
Tom Rini
http://gate.crashing.org/~trini/

[KGDB PATCH][7/7] Move debugger_entry()

[Tom Rini]

Hello.  When we use kgdboe, we can't use it until do_basic_setup() is done.
So we have two options, not allow kgdboe to use the initial breakpoint
or move debugger_entry() to be past the point where kgdboe will be usable.
I've opted for the latter, as if an earlier breakpoint is needed you can
still use serial and throw kgdb_schedule_breakpoint/breakpoint where desired.

--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
@@ -581,6 +582,7 @@ static int init(void * unused)
 
 	smp_init();
 	do_basic_setup();
+	debugger_entry();
 
 	prepare_namespace();
 

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[George Anzinger]

Comments below...

Tom Rini wrote:
> Hello.  The following patch moves all of the config options into one file,
> kernel/Kconfig.kgdb.
> 
> diff -zrupN linux-2.6.3+nothing/arch/i386/Kconfig linux-2.6.3+config+serial/arch/i386/Kconfig
> --- linux-2.6.3+nothing/arch/i386/Kconfig	2004-02-27 12:16:14.296187607 -0700
> +++ linux-2.6.3+config+serial/arch/i386/Kconfig	2004-02-27 12:16:13.707320867 -0700
> @@ -1253,36 +1253,7 @@ config DEBUG_SPINLOCK_SLEEP
>  	  If you say Y here, various routines which may sleep will become very
>  	  noisy if they are called with a spinlock held.	
>  
> -config KGDB
> -	bool "KGDB: kernel debugging with remote gdb"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  If you say Y here, it will be possible to remotely debug the
> -	  kernel using gdb. This enlarges your kernel image disk size by
> -	  several megabytes and requires a machine with more than 128 MB
> -	  RAM to avoid excessive linking time. 
> -	  Documentation of kernel debugger available at
> -	  http://kgdb.sourceforge.net
> -	  This is only useful for kernel hackers. If unsure, say N.
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "KGDB: Console messages through gdb"
> -	depends on KGDB
> -	help
> -	  If you say Y here, console messages will appear through gdb.
> -	  Other consoles such as tty or ttyS will continue to work as usual.
> +source "kernel/Kconfig.kgdb"
>  
>  config FRAME_POINTER
>  	bool "Compile the kernel with frame pointers"
> diff -zrupN linux-2.6.3+nothing/arch/ppc/Kconfig linux-2.6.3+config+serial/arch/ppc/Kconfig
> --- linux-2.6.3+nothing/arch/ppc/Kconfig	2004-02-27 12:16:14.403163398 -0700
> +++ linux-2.6.3+config+serial/arch/ppc/Kconfig	2004-02-27 12:16:13.771306387 -0700
> @@ -1170,52 +1170,7 @@ config DEBUG_SPINLOCK_SLEEP
>  	  If you say Y here, various routines which may sleep will become very
>  	  noisy if they are called with a spinlock held.
>  
> -config KGDB
> -	bool "Include kgdb kernel debugger"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  Include in-kernel hooks for kgdb, the Linux kernel source level
> -	  debugger.  See <http://kgdb.sourceforge.net/> for more information.
> -	  Unless you are intending to debug the kernel, say N here.
> -
> -choice
> -	prompt "Serial Port"
> -	depends on KGDB
> -	default KGDB_TTYS1
> -
> -config KGDB_TTYS0
> -	bool "ttyS0"
> -
> -config KGDB_TTYS1
> -	bool "ttyS1"
> -
> -config KGDB_TTYS2
> -	bool "ttyS2"
> -
> -config KGDB_TTYS3
> -	bool "ttyS3"
> -
> -endchoice
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "Enable serial console thru kgdb port"
> -	depends on KGDB && 8xx || 8260
> -	help
> -	  If you enable this, all serial console messages will be sent
> -	  over the gdb stub.
> -	  If unsure, say N.
> +source "kernel/Kconfig.kgdb"
>  
>  config XMON
>  	bool "Include xmon kernel debugger"
> diff -zrupN linux-2.6.3+nothing/arch/x86_64/Kconfig linux-2.6.3+config+serial/arch/x86_64/Kconfig
> --- linux-2.6.3+nothing/arch/x86_64/Kconfig	2004-02-27 12:16:14.350175389 -0700
> +++ linux-2.6.3+config+serial/arch/x86_64/Kconfig	2004-02-27 12:16:13.718318378 -0700
> @@ -465,37 +465,7 @@ config IOMMU_LEAK
>           Add a simple leak tracer to the IOMMU code. This is useful when you
>  	 are debugging a buggy device driver that leaks IOMMU mappings.
>         
> -config KGDB
> -	bool "KGDB: kernel debugging with remote gdb"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  If you say Y here, it will be possible to remotely debug the
> -	  kernel using gdb. This enlarges your kernel image disk size by
> -	  several megabytes and requires a machine with more than 128 MB
> -	  RAM to avoid excessive linking time. 
> -	  Documentation of kernel debugger available at
> -	  http://kgdb.sourceforge.net
> -	  This is only useful for kernel hackers. If unsure, say N.
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "KGDB: Console messages through gdb"
> -	depends on KGDB
> -	help
> -	  If you say Y here, console messages will appear through gdb.
> -	  Other consoles such as tty or ttyS will continue to work as usual.
> -
> +source "kernel/Kconfig.kgdb"
>  endmenu
>  
>  source "security/Kconfig"
> diff -zrupN linux-2.6.3+nothing/drivers/net/Kconfig linux-2.6.3+config+serial/drivers/net/Kconfig
> --- linux-2.6.3+nothing/drivers/net/Kconfig	2004-02-27 12:16:14.521136701 -0700
> +++ linux-2.6.3+config+serial/drivers/net/Kconfig	2004-02-27 12:06:22.000000000 -0700
> @@ -187,12 +187,6 @@ config NET_ETHERNET
>  	  Note that the answer to this question won't directly affect the
>  	  kernel: saying N will just cause the configurator to skip all
>  	  the questions about Ethernet network cards. If unsure, say N.
> -	
> -config KGDB_ETH
> -	bool "KGDB: On ethernet"
> -	depends on KGDB
> -	help
> -	  Uses ethernet interface for kgdb.
>  
>  config MII
>  	tristate "Generic Media Independent Interface device support"
> diff -zrupN linux-2.6.3+nothing/drivers/serial/Kconfig linux-2.6.3+config+serial/drivers/serial/Kconfig
> --- linux-2.6.3+nothing/drivers/serial/Kconfig	2004-02-27 12:16:14.545131271 -0700
> +++ linux-2.6.3+config+serial/drivers/serial/Kconfig	2004-02-27 12:06:30.000000000 -0700
> @@ -6,34 +6,6 @@
>  
>  menu "Serial drivers"
>  
> -config KGDB_8250
> -	bool "KGDB: On generic serial port (8250)"
> -	depends on KGDB
> -	help
> -	  Uses generic serial port (8250) for kgdb. This is independent of the
> -	  option 9250/16550 and compatible serial port.
> -
> -config KGDB_PORT
> -	hex "hex I/O port address of the debug serial port"
> -	depends on KGDB_8250
> -	default  3f8
> -	help
> -	  Some systems (x86 family at this writing) allow the port
> -	  address to be configured.  The number entered is assumed to be
> -	  hex, don't put 0x in front of it.  The standard address are:
> -	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
> -	  will tell you what you have.  It is good to test the serial
> -	  connection with a live system before trying to debug.
> -
> -config KGDB_IRQ
> -	int "IRQ of the debug serial port"
> -	depends on KGDB_8250
> -	default 4
> -	help
> -	  This is the irq for the debug port.  If everything is working
> -	  correctly and the kernel has interrupts on a control C to the
> -	  port should cause a break into the kernel debug stub.
> -
>  #
>  # The new 8250/16550 serial drivers
>  config SERIAL_8250
> diff -zrupN linux-2.6.3+nothing/kernel/Kconfig.kgdb linux-2.6.3+config+serial/kernel/Kconfig.kgdb
> --- linux-2.6.3+nothing/kernel/Kconfig.kgdb	1969-12-31 17:00:00.000000000 -0700
> +++ linux-2.6.3+config+serial/kernel/Kconfig.kgdb	2004-02-27 12:16:13.000000000 -0700
> @@ -0,0 +1,141 @@
> +config KGDB
> +	bool "KGDB: kernel debugging with remote gdb"
> +	depends on DEBUG_KERNEL
> +	select DEBUG_INFO
> +	select FRAME_POINTER
This is not needed and may not be what you want.  It changes the code generated 
(Heisenberg).  With dwarf frames, it is no longer required or even useful.
> +	# XXX: Doesn't work w/o this right now
> +	select KGDB_THREAD if PPC32
> +	help
> +	  If you say Y here, it will be possible to remotely debug the
> +	  kernel using gdb. This enlarges your kernel image disk size by
> +	  several megabytes and requires a machine with more than 128 MB
> +	  RAM to avoid excessive linking time. 
> +	  Documentation of kernel debugger available at
> +	  http://kgdb.sourceforge.net
> +	  This is only useful for kernel hackers. If unsure, say N.
PLEASE, lets not use the word "hackers".  Surly "coders" or some such is more 
complementary.  After all these are US.
> +
> +choice
> +	prompt "Method for KGDB communication"
> +	depends on KGDB
> +	default PPC_SIMPLE_SERIAL if PPC32 && (8xx || 8260)
> +	default KGDB_8250
> +	help
> +	  There are a number of different ways in which you can communicate
> +	  with KGDB.  The oldest is using a serial driver.  A newer method
> +	  is to use UDP packets and a special network driver.
> +
> +config KGDB_8250
> +	bool "KGDB: On generic serial port (8250)"
> +	help
> +	  Uses generic serial port (8250) for kgdb. This is independent of the
> +	  option 9250/16550 and compatible serial port.
> +
> +config KGDB_ETH
> +	bool "KGDB: On ethernet"
> +	select NETPOLL
> +	select NETPOLL_TRAP
> +	select NETPOLL_RX
> +	help
> +	  Uses ethernet interface for kgdb.
> +
> +config PPC_SIMPLE_SERIAL 
> +	bool "KGDB: On any serial port"
> +	depends on PPC32
> +	help
> +	  Use a very simple, and not necessarily feature complete serial
> +	  driver.  This is the only serial option currently for MPC8xx or
> +	  MPC82xx based ports that do not offer an 8250-style UART.
> +
> +endchoice
> +
> +config KGDB_SIMPLE_SERIAL
> +	bool "Simple selection of KGDB serial port"
> +	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
> +	help
> +	  If you say Y here, you will only have to pick the baud rate
> +	  and serial port (ttyS) that you wish to use for KGDB.  If you
> +	  say N, you will have provide the I/O port and IRQ number.  Note
> +	  that if your serial ports are iomapped, then you must say Y here.
> +	  If in doubt, say Y.
> +
> +choice
> +	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
> +    	prompt "Debug serial port BAUD"
> +	default KGDB_115200BAUD
> +	help
> +	  Gdb and the kernel stub need to agree on the baud rate to be
> +	  used.  Some systems (x86 family at this writing) allow this to
> +	  be configured.

Should this depend on the archs that can use it?
> +
> +config KGDB_9600BAUD
> +	bool "9600"
> +
> +config KGDB_19200BAUD
> +	bool "19200"
> +
> +config KGDB_38400BAUD
> +	bool "38400"
> +
> +config KGDB_57600BAUD
> +	bool "57600"
> +
> +config KGDB_115200BAUD
> +	bool "115200"
> +endchoice
> +
> +choice
> +	prompt "Serial port for KGDB"
> +	depends on KGDB_SIMPLE_SERIAL
> +	default KGDB_TTYS0
> +
> +config KGDB_TTYS0
> +	bool "ttyS0"
> +
> +config KGDB_TTYS1
> +	bool "ttyS1"
> +
> +config KGDB_TTYS2
> +	bool "ttyS2"
> +
> +config KGDB_TTYS3
> +	bool "ttyS3"
> +
> +endchoice
> +
> +config KGDB_PORT
> +	hex "hex I/O port address of the debug serial port"
> +	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
> +	default  3f8
> +	help
> +	  Some systems (x86 family at this writing) allow the port
> +	  address to be configured.  The number entered is assumed to be
> +	  hex, don't put 0x in front of it.  The standard address are:
> +	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
> +	  will tell you what you have.  It is good to test the serial
> +	  connection with a live system before trying to debug.
> +
> +config KGDB_IRQ
> +	int "IRQ of the debug serial port"
> +	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
> +	default 4
> +	help
> +	  This is the irq for the debug port.  If everything is working
> +	  correctly and the kernel has interrupts on a control C to the
> +	  port should cause a break into the kernel debug stub.
I think setserial will also spit this out.
> +
> +config KGDB_THREAD
> +	bool "KGDB: Thread analysis"
> +	depends on KGDB
> +	help
> +	  With thread analysis enabled, gdb can talk to kgdb stub to list
> +	  threads and to get stack trace for a thread. This option also enables
> +	  some code which helps gdb get exact status of thread. Thread analysis
> +	  adds some overhead to schedule and down functions. You can disable
> +	  this option if you do not want to compromise on speed.

Lets remove the overhead and eliminate the need for this option in favor of 
always having threads.  Works in the mm kgdb...
> +
> +config KGDB_CONSOLE
> +	bool "KGDB: Console messages through gdb"
> +	depends on KGDB
> +	help
> +	  If you say Y here, console messages will appear through gdb.
> +	  Other consoles such as tty or ttyS will continue to work as usual.
> 
Might want to warn them of the need to connect prior to the first message from 
the kernel....

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Tom Rini]

On Fri, Feb 27, 2004 at 02:30:57PM -0800, George Anzinger wrote:

> Comments below...

I'm going to set aside some of the comments, simply because I'm not
going to change behavior / content right now, we can do that afterwards.

[snip]
> >+choice
> >+	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
> >+    	prompt "Debug serial port BAUD"
> >+	default KGDB_115200BAUD
> >+	help
> >+	  Gdb and the kernel stub need to agree on the baud rate to be
> >+	  used.  Some systems (x86 family at this writing) allow this to
> >+	  be configured.
> 
> Should this depend on the archs that can use it?

The help is wrong, that's all, it's not arch-dependant, it's driver
dependant.

> >+config KGDB_IRQ
> >+	int "IRQ of the debug serial port"
> >+	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
> >+	default 4
> >+	help
> >+	  This is the irq for the debug port.  If everything is working
> >+	  correctly and the kernel has interrupts on a control C to the
> >+	  port should cause a break into the kernel debug stub.
>
> I think setserial will also spit this out.

And of course, ^C isn't needed now too.

> >+config KGDB_THREAD
> >+	bool "KGDB: Thread analysis"
> >+	depends on KGDB
> >+	help
> >+	  With thread analysis enabled, gdb can talk to kgdb stub to list
> >+	  threads and to get stack trace for a thread. This option also 
> >enables
> >+	  some code which helps gdb get exact status of thread. Thread 
> >analysis
> >+	  adds some overhead to schedule and down functions. You can disable
> >+	  this option if you do not want to compromise on speed.
> 
> Lets remove the overhead and eliminate the need for this option in favor of 
> always having threads.  Works in the mm kgdb...

Once it's clean, perhaps.

> >+config KGDB_CONSOLE
> >+	bool "KGDB: Console messages through gdb"
> >+	depends on KGDB
> >+	help
> >+	  If you say Y here, console messages will appear through gdb.
> >+	  Other consoles such as tty or ttyS will continue to work as usual.
>
> Might want to warn them of the need to connect prior to the first message 
> from the kernel....

There is no such need.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[George Anzinger]

Couple of comments below...

Tom Rini wrote:
> The following is my second attempt at making the serial driver more robust.
> This has the following changes:
> - We don't need kgdb_might_be_resumed or kgdb_killed_or_detached.
> - Don't try and look for a connection in put_packet, after we've tried
>   to put a packet.  Instead, when we receive a packet, GDB has
>   connected.  Also make put_packet look at the char it reads, and handle
>   a new packet in the middle (or ^C+packet).
> - Remove ok_packet(), excessive, IMHO.
> - In kgdb8250_interrupt, only check if this is the right line, and if so,
>   call kgdb_schedule_breakpoint().  This ends up killing off a lot of
>   (incomplete due to my not testing SMP) locking.
> - Remove a duplicate extern of kgdb_connected.
> 
> diff -zrupN linux-2.6.3+nothing/arch/ppc/8260_io/uart.c linux-2.6.3+config+serial/arch/ppc/8260_io/uart.c
> --- linux-2.6.3+nothing/arch/ppc/8260_io/uart.c	2004-02-27 12:16:14.397164756 -0700
> +++ linux-2.6.3+config+serial/arch/ppc/8260_io/uart.c	2004-02-27 12:16:13.765307745 -0700
> @@ -396,16 +396,10 @@ static _INLINE_ void receive_chars(ser_i
>  #ifdef CONFIG_KGDB
>  		if (info->state->smc_scc_num == KGDB_SER_IDX) {
>  			while (i-- > 0) {
> -				if (*cp == 0x03) {
> +				if (*cp == 0x03 || *cp == '$') {
>  					breakpoint();
>  					return;
>  				}
> -				if (*cp == '$') {
> -					atomic_set(&kgdb_might_be_resumed, 1);
> -					breakpoint();
> -					atomic_set(&kgdb_might_be_resumed, 0);
> -					return;
> -				}
>  				cp++;
>  			}
>  			bdp->cbd_sc |= BD_SC_EMPTY;
> diff -zrupN linux-2.6.3+nothing/drivers/serial/kgdb_8250.c linux-2.6.3+config+serial/drivers/serial/kgdb_8250.c
> --- linux-2.6.3+nothing/drivers/serial/kgdb_8250.c	2004-02-27 12:16:14.557128556 -0700
> +++ linux-2.6.3+config+serial/drivers/serial/kgdb_8250.c	2004-02-27 12:16:13.000000000 -0700
> @@ -17,7 +17,6 @@
>  #include <linux/config.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> -#include <linux/spinlock.h>
>  #include <linux/kgdb.h>
>  #include <linux/interrupt.h>
>  #include <linux/tty.h>
> @@ -166,32 +165,15 @@ read_data_bfr(void)
>  /*
>   * Get a char if available, return -1 if nothing available.
>   * Empty the receive buffer first, then look at the interface hardware.
> - *
> - * Locking here is a bit of a problem.	We MUST not lock out communication
> - * if we are trying to talk to gdb about a kgdb entry.	ON the other hand
> - * we can loose chars in the console pass thru if we don't lock.  It is also
> - * possible that we could hold the lock or be waiting for it when kgdb
> - * NEEDS to talk.  Since kgdb locks down the world, it does not need locks.
> - * We do, of course have possible issues with interrupting a uart operation,
> - * but we will just depend on the uart status to help keep that straight.
>   */
>  
> -static spinlock_t uart_interrupt_lock = SPIN_LOCK_UNLOCKED;
> -#ifdef CONFIG_SMP
> -extern spinlock_t kgdb_spinlock;
> -#endif
> -
>  int
>  kgdb_get_debug_char(void)
>  {
>  	int retchr;
>  	unsigned long flags;
>  	local_irq_save(flags);
> -#ifdef CONFIG_SMP
> -	if (!spin_is_locked(&kgdb_spinlock)) {
> -		spin_lock(&uart_interrupt_lock);
> -	}
> -#endif
> +
>  	/* intr routine has q'd chars */
>  	if (atomic_read(&kgdb8250_buf_in_cnt) != 0) {
>  		retchr = kgdb8250_buf[kgdb8250_buf_out_inx++];
> @@ -205,11 +187,6 @@ kgdb_get_debug_char(void)
>  	} while (retchr < 0);
>  
>  out:
> -#ifdef CONFIG_SMP
> -	if (!spin_is_locked(&kgdb_spinlock)) {
> -		spin_unlock(&uart_interrupt_lock);
> -	}
> -#endif
>  	local_irq_restore(flags);
>  
>  	return retchr;
> @@ -217,72 +194,17 @@ out:
>  
>  /*
>   * This is the receiver interrupt routine for the GDB stub.
> - * It will receive a limited number of characters of input
> - * from the gdb  host machine and save them up in a buffer.
> - *
> - * When kgdb_get_debug_char() is called it
> - * draws characters out of the buffer until it is empty and
> - * then reads directly from the serial port.
> - *
> - * We do not attempt to write chars from the interrupt routine
> - * since the stubs do all of that via kgdb_put_debug_char() which
> - * writes one byte after waiting for the interface to become
> - * ready.
> - *
> - * The debug stubs like to run with interrupts disabled since,
> - * after all, they run as a consequence of a breakpoint in
> - * the kernel.
> - *
> - * Perhaps someone who knows more about the tty driver than I
> - * care to learn can make this work for any low level serial
> - * driver.
> + * All that we need to do is verify that the interrupt happened on the
> + * line we're in charge of.  If this is true, schedule a breakpoint and
> + * return.
>   */
>  static irqreturn_t
>  kgdb8250_interrupt(int irq, void *dev_id, struct pt_regs *regs)
>  {
> -	int chr, iir;
> -	unsigned long flags;
> -
>  	if (irq != gdb_async_info.line)
>  		return IRQ_NONE;
>  
> -	/* If we get an interrupt, then KGDB is trying to connect. */
> -	if (!kgdb_connected) {
> -		kgdb_schedule_breakpoint();
> -		return IRQ_HANDLED;
> -	}
> -
> -	local_irq_save(flags);
> -	spin_lock(&uart_interrupt_lock);
> -
> -	do {
> -		chr = read_data_bfr();
> -		iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
> -		if (chr < 0)
> -			continue;
> -
> -		/* Check whether gdb sent interrupt */
> -		if (chr == 3) {
> -			breakpoint();
> -			continue;
> -		}
> -
> -		if (atomic_read(&kgdb8250_buf_in_cnt) >= GDB_BUF_SIZE) {
> -			/* buffer overflow, clear it */
> -			kgdb8250_buf_in_inx = 0;
> -			atomic_set(&kgdb8250_buf_in_cnt, 0);
> -			kgdb8250_buf_out_inx = 0;
> -			break;
> -		}
> -
> -		kgdb8250_buf[kgdb8250_buf_in_inx++] = chr;
> -		kgdb8250_buf_in_inx &= (GDB_BUF_SIZE - 1);
> -		atomic_inc(&kgdb8250_buf_in_cnt);
> -	} while (iir & UART_IIR_RDI);
> -
> -	spin_unlock(&uart_interrupt_lock);
> -	local_irq_restore(flags);
> -
I think you need at least this (especially in SMP, but works in all):
	char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
         if(iir & UART_IIR_RDI){
> +		kgdb_schedule_breakpoint();
	}
>  	return IRQ_HANDLED;
>  }
>  
> diff -zrupN linux-2.6.3+nothing/include/linux/kgdb.h linux-2.6.3+config+serial/include/linux/kgdb.h
> --- linux-2.6.3+nothing/include/linux/kgdb.h	2004-02-27 12:16:14.000000000 -0700
> +++ linux-2.6.3+config+serial/include/linux/kgdb.h	2004-02-27 12:16:13.000000000 -0700
> @@ -36,10 +36,6 @@ extern volatile int kgdb_connected;
>  #endif
>  
>  extern atomic_t kgdb_setting_breakpoint;
> -extern atomic_t kgdb_killed_or_detached;
> -extern atomic_t kgdb_might_be_resumed;
> -
> -extern volatile int kgdb_connected;
>  
>  extern struct task_struct *kgdb_usethread, *kgdb_contthread;
>  
> diff -zrupN linux-2.6.3+nothing/kernel/kgdb.c linux-2.6.3+config+serial/kernel/kgdb.c
> --- linux-2.6.3+nothing/kernel/kgdb.c	2004-02-27 12:16:14.000000000 -0700
> +++ linux-2.6.3+config+serial/kernel/kgdb.c	2004-02-27 12:16:13.000000000 -0700
> @@ -15,6 +15,7 @@
>   * Copyright (C) 2002-2004 Timesys Corporation
>   * Copyright (C) 2003-2004 Amit S. Kale
>   * Copyright (C) 2004 Pavel Machek <pavel@suse.cz>
> + * Copyright (C) 2004 Tom Rini <trini@kernel.crashing.org>
>   *
>   * Restructured KGDB for 2.6 kernels.
>   * thread support, support for multiple processors,support for ia-32(x86) 
> @@ -87,8 +88,6 @@ static const char hexchars[] = "01234567
>  static spinlock_t slavecpulocks[KGDB_MAX_NO_CPUS];
>  static volatile int procindebug[KGDB_MAX_NO_CPUS];
>  atomic_t kgdb_setting_breakpoint;
> -atomic_t kgdb_killed_or_detached;
> -atomic_t kgdb_might_be_resumed;
>  struct task_struct *kgdb_usethread, *kgdb_contthread;
>  
>  int debugger_step;
> @@ -212,8 +211,10 @@ static void get_packet(char *buffer)
>  	char ch;
>  
>  	do {
> -		/* wait around for the start character, ignore all other characters */
> -		while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$') ;
> +		/* wait around for the start character, ignore all other
> +		 * characters */
> +		while ((ch = (kgdb_serial->read_char() & 0x7f)) != '$')
> +			;	/* Spin. */
>  		kgdb_connected = 1;
>  		checksum = 0;
>  		xmitcsum = -1;
> @@ -249,27 +250,22 @@ static void get_packet(char *buffer)
>   * Send the packet in buffer.
>   * Check for gdb connection if asked for.
>   */
> -static void put_packet(char *buffer, int checkconnect)
> +static void put_packet(char *buffer)
>  {
>  	unsigned char checksum;
>  	int count;
>  	char ch;
> -	static char gdbseq[] = "$Hc-1#09";
> -	int i;
> -	int send_count;
>  
>  	/*  $<packet info>#<checksum>. */
> -	do {
> +	while (1) {
>  		kgdb_serial->write_char('$');
>  		checksum = 0;
>  		count = 0;
> -		send_count = 0;
>  
>  		while ((ch = buffer[count])) {
>  			kgdb_serial->write_char(ch);
>  			checksum += ch;
>  			count++;
> -			send_count++;
>  		}
>  
>  		kgdb_serial->write_char('#');
> @@ -278,30 +274,27 @@ static void put_packet(char *buffer, int
>  		if (kgdb_serial->flush)
>  			kgdb_serial->flush();
>  
> -		i = 0;
> -		while ((ch = kgdb_serial->read_char()) == gdbseq[i++] &&
> -		       checkconnect) {
> -			if (!gdbseq[i]) {
> -				kgdb_serial->write_char('+');
> -				if (kgdb_serial->flush)
> -					kgdb_serial->flush();
> -				breakpoint();
> -
> -				/*
> -				 * GDB is available now.
> -				 * Retransmit this packet.
> -				 */
> -				break;
> -			}
> -		}
> -		if (checkconnect && ch == 3) {
> -			kgdb_serial->write_char('+');
> +		/* Now see what we get in reply. */
> +		ch = kgdb_serial->read_char();
> +
> +		if (ch == 3)
> +			ch = kgdb_serial->read_char();
> +
> +		/* If we get an ACK, we are done. */
> +		if (ch == '+')
> +			return;
> +
> +		/* If we get the start of another packet, this means
> +		 * that GDB is attempting to reconnect.  We will NAK
> +		 * the packet being sent, and stop trying to send this
> +		 * packet. */
> +		if (ch == '$') {
> +			kgdb_serial->write_char('-');
>  			if (kgdb_serial->flush)
>  				kgdb_serial->flush();
> -			breakpoint();

Flags go up in my mind here about recursion...  What if we are already handling 
a breakpoint???  This may all be cool, but, as I said, alarms are ringing.

> +			return;
>  		}
> -	} while ((ch & 0x7f) != '+');
> -
> +	}
>  }
>  
>  static int get_char(char *addr, unsigned char *data)
> @@ -427,11 +420,6 @@ static inline void error_packet(char *pk
>  	pkt[3] = '\0';
>  }
>  
> -static void ok_packet(char *pkt)
> -{
> -	strcpy(pkt, "OK");
> -}
> -
>  static char *pack_threadid(char *pkt, threadref * id)
>  {
>  	char *limit;
> @@ -502,7 +490,8 @@ void kgdb_wait(struct pt_regs *regs)
>  	procindebug[processor] = 1;
>  	current->thread.debuggerinfo = regs;
>  
> -	/* Wait till master processor goes completely into the debugger. FIXME: this looks racy */
> +	/* Wait till master processor goes completely into the debugger.
> +	 * FIXME: this looks racy */
>  	while (!procindebug[atomic_read(&debugger_active) - 1]) {
>  		int i = 10;	/* an arbitrary number */
>  
> @@ -701,17 +690,7 @@ int kgdb_handle_exception(int exVector, 
>  	/* Master processor is completely in the debugger */
>  	kgdb_post_master_code(linux_regs, exVector, err_code);
>  
> -	if (atomic_read(&kgdb_killed_or_detached) &&
> -	    atomic_read(&kgdb_might_be_resumed)) {
> -		get_packet(remcom_in_buffer);
> -		if (remcom_in_buffer[0] == 'H' && remcom_in_buffer[1] == 'c') {
> -			remove_all_break();
> -			atomic_set(&kgdb_killed_or_detached, 0);
> -			ok_packet(remcom_out_buffer);
> -		} else
> -			return 1;
> -	} else {
> -
> +	if (kgdb_connected) {
>  		/* reply to host that an exception has occurred */
>  		ptr = remcom_out_buffer;
>  		*ptr++ = 'T';
> @@ -721,11 +700,9 @@ int kgdb_handle_exception(int exVector, 
>  		int_to_threadref(&thref, shadow_pid(current->pid));
>  		ptr = pack_threadid(ptr, &thref);
>  		*ptr++ = ';';
> -		*ptr = '\0';
> -	}
>  
> -	put_packet(remcom_out_buffer, 0);
> -	kgdb_connected = 1;
> +		put_packet(remcom_out_buffer);
> +	}
>  
>  	kgdb_usethread = current;
>  	kgdb_usethreadid = shadow_pid(current->pid);
> @@ -741,6 +718,11 @@ int kgdb_handle_exception(int exVector, 
>  
>  		switch (remcom_in_buffer[0]) {
>  		case '?':
> +			/* We know that this packet is only sent
> +			 * during initial connect.  So to be safe,
> +			 * we clear out our breakpoints now incase
> +			 * GDB is reconnecting. */
> +			remove_all_break();
>  			remcom_out_buffer[0] = 'S';
>  			remcom_out_buffer[1] = hexchars[signo >> 4];
>  			remcom_out_buffer[2] = hexchars[signo % 16];
> @@ -798,7 +780,7 @@ int kgdb_handle_exception(int exVector, 
>  			else {
>  				gdb_regs_to_regs(gdb_regs, (struct pt_regs *)
>  						 current->thread.debuggerinfo);
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  			}
>  
>  			break;
> @@ -838,10 +820,10 @@ int kgdb_handle_exception(int exVector, 
>  			if ((error = remove_all_break()) < 0) {
>  				error_packet(remcom_out_buffer, error);
>  			} else {
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  				kgdb_connected = 0;
>  			}
> -			put_packet(remcom_out_buffer, 0);
> +			put_packet(remcom_out_buffer);
>  			goto default_handle;
>  
>  		case 'k':
> @@ -947,11 +929,10 @@ int kgdb_handle_exception(int exVector, 
>  				}
>  				kgdb_usethread = thread;
>  				kgdb_usethreadid = threadid;
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  				break;
>  
>  			case 'c':
> -				atomic_set(&kgdb_killed_or_detached, 0);
>  				ptr = &remcom_in_buffer[2];
>  				kgdb_hex2long(&ptr, &threadid);
>  				if (!threadid) {
> @@ -966,7 +947,7 @@ int kgdb_handle_exception(int exVector, 
>  					}
>  					kgdb_contthread = thread;
>  				}
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  				break;
>  			}
>  			break;
> @@ -977,7 +958,7 @@ int kgdb_handle_exception(int exVector, 
>  			kgdb_hex2long(&ptr, &threadid);
>  			thread = getthread(linux_regs, threadid);
>  			if (thread)
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  			else
>  				error_packet(remcom_out_buffer, -EINVAL);
>  			break;
> @@ -1018,7 +999,7 @@ int kgdb_handle_exception(int exVector, 
>  			}
>  
>  			if (error == 0)
> -				ok_packet(remcom_out_buffer);
> +				strcpy(remcom_out_buffer, "OK");
>  			else
>  				error_packet(remcom_out_buffer, error);
>  
> @@ -1039,7 +1020,7 @@ int kgdb_handle_exception(int exVector, 
>  		}		/* switch */
>  
>  		/* reply to the request */
> -		put_packet(remcom_out_buffer, 0);
> +		put_packet(remcom_out_buffer);
>  	}
>  
>        kgdb_exit:
> @@ -1063,7 +1044,6 @@ int kgdb_handle_exception(int exVector, 
>  	}
>  
>  	/* Free debugger_active */
> -	atomic_set(&kgdb_killed_or_detached, 1);
>  	atomic_set(&debugger_active, 0);
>  	local_irq_restore(flags);
>  
> @@ -1132,12 +1112,6 @@ void kgdb_entry(void)
>  	/* Free debugger_active */
>  	atomic_set(&debugger_active, 0);
>  
> -	/* This flag is used, if gdb has detached and wants to start
> -	 * another session
> -	 */
> -	atomic_set(&kgdb_killed_or_detached, 1);
> -	atomic_set(&kgdb_might_be_resumed, 0);
> -
>  	for (i = 0; i < MAX_BREAKPOINTS; i++)
>  		kgdb_break[i].state = bp_disabled;
>  
> @@ -1222,7 +1196,7 @@ void kgdb_console_write(struct console *
>  		*bufptr = '\0';
>  		s += wcount;
>  
> -		put_packet(kgdbconbuf, 1);
> +		put_packet(kgdbconbuf);
>  
>  	}
>  	local_irq_restore(flags);
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][3/7] SysRq-G

[George Anzinger]

There may be a need to change the keyboard driver for this.  I have had trouble 
with it in the past.  Seems it doesn't like to not get control back for a very 
long time.  I am not sure what the problem is but, be warned.

-g

Tom Rini wrote:
> Hello.  The following adds SysRq-G.  This is always configured in on
> CONFIG_KGDB && CONFIG_SYSRQ.
> 
> diff -zrupN linux-2.6.3+config+serial/drivers/char/sysrq.c linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c
> --- linux-2.6.3+config+serial/drivers/char/sysrq.c	2004-02-27 12:06:22.000000000 -0700
> +++ linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c	2004-02-27 12:16:14.000000000 -0700
> @@ -31,6 +31,7 @@
>  #include <linux/suspend.h>
>  #include <linux/writeback.h>
>  #include <linux/buffer_head.h>		/* for fsync_bdev() */
> +#include <linux/kgdb.h>			/* for breakpoint() */
>  
>  #include <linux/spinlock.h>
>  
> @@ -44,6 +45,25 @@ int sysrq_enabled = 1;
>  /* Machine specific power off function */
>  void (*sysrq_power_off)(void);
>  
> +/* Make a breakpoint() right now. */
> +#ifdef CONFIG_KGDB
> +#define  GDB_OP &kgdb_op
> +static void kgdb_sysrq(int key, struct pt_regs *pt_regs, struct tty_struct *tty)
> +{
> +	printk("kgdb sysrq\n");
> +	breakpoint();
> +}
> +
> +static struct sysrq_key_op kgdb_op = {
> +	.handler	= kgdb_sysrq,
> +	.help_msg	= "kGdb|Fgdb",
> +	.action_msg	= "Debug breakpoint\n",
> +};
> +
> +#else
> +#define  GDB_OP NULL
> +#endif
> +
>  /* Loglevel sysrq handler */
>  static void sysrq_handle_loglevel(int key, struct pt_regs *pt_regs,
>  				  struct tty_struct *tty) 
> @@ -239,7 +259,7 @@ static struct sysrq_key_op *sysrq_key_ta
>  /* d */	NULL,
>  /* e */	&sysrq_term_op,
>  /* f */	NULL,
> -/* g */	NULL,
> +/* g */	GDB_OP,
>  /* h */	NULL,
>  /* i */	&sysrq_kill_op,
>  /* j */	NULL,
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][7/7] Move debugger_entry()

[George Anzinger]

Tom Rini wrote:
> Hello.  When we use kgdboe, we can't use it until do_basic_setup() is done.
> So we have two options, not allow kgdboe to use the initial breakpoint
> or move debugger_entry() to be past the point where kgdboe will be usable.
> I've opted for the latter, as if an earlier breakpoint is needed you can
> still use serial and throw kgdb_schedule_breakpoint/breakpoint where desired.

Please don't do this.  How about configuring it along with the connection 
method.  I really don't want to have to modify the kernel just to get in early.

George
> 
> --- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> +++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
> @@ -581,6 +582,7 @@ static int init(void * unused)
>  
>  	smp_init();
>  	do_basic_setup();
> +	debugger_entry();
>  
>  	prepare_namespace();
>  
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][7/7] Move debugger_entry()

[Tom Rini]

On Fri, Feb 27, 2004 at 02:53:49PM -0800, George Anzinger wrote:
> Tom Rini wrote:
> >Hello.  When we use kgdboe, we can't use it until do_basic_setup() is done.
> >So we have two options, not allow kgdboe to use the initial breakpoint
> >or move debugger_entry() to be past the point where kgdboe will be usable.
> >I've opted for the latter, as if an earlier breakpoint is needed you can
> >still use serial and throw kgdb_schedule_breakpoint/breakpoint where 
> >desired.
> 
> Please don't do this.  How about configuring it along with the connection 
> method.  I really don't want to have to modify the kernel just to get in 
> early.

This is 'early'.  Keep in mind that this version of KGDB already isn't
breaking in nearly as early as the one in -mm (this version is currently
doing it just before do_basic_setup()).  And keep in mind that the goal
of this version right now is to get something clean and small that can
somehow slip into Linus' tree.  Changes to the non-lite
core/i386/whatever to allow for much earlier access is fine.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[Tom Rini]

On Fri, Feb 27, 2004 at 02:44:33PM -0800, George Anzinger wrote:

> Couple of comments below...
> 
> Tom Rini wrote:
[snip]
> >-	spin_unlock(&uart_interrupt_lock);
> >-	local_irq_restore(flags);
> >-
> I think you need at least this (especially in SMP, but works in all):
> 	char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
>         if(iir & UART_IIR_RDI){
> >+		kgdb_schedule_breakpoint();
> 	}
> > 	return IRQ_HANDLED;

Would this be to ensure that we only schedule one breakpoint not 2?

[snip]
> >+		/* If we get the start of another packet, this means
> >+		 * that GDB is attempting to reconnect.  We will NAK
> >+		 * the packet being sent, and stop trying to send this
> >+		 * packet. */
> >+		if (ch == '$') {
> >+			kgdb_serial->write_char('-');
> > 			if (kgdb_serial->flush)
> > 				kgdb_serial->flush();
> >-			breakpoint();
> 
> Flags go up in my mind here about recursion...  What if we are already 
> handling a breakpoint???  This may all be cool, but, as I said, alarms are 
> ringing.

There's two cases here, IMHO.  If GDB is connected, the only time we'll
get a '$' when we're sending a packet is if we're out-of-sync (in
regular gdb/kgdb communication) or we're prempting a console message
(i.e. we're trying to send something while gdb wants to break in).  In
the latter case, it's OK to give up on the console packet (it's not
critical) and trying to send a console message during a breakpoint is
asking for trouble.  If things get out-of-sync in normal gdb/kgdb
communication, what will happen w/o this change is that we get stuck in
a "Packet instead of ACK" loop on gdb, and we get stuck trying to
transmit that same packet on the kgdb side.   I think that if we call
breakpoint() here we can try and start over...

The other case is what the comment describes, and in that case we quite
quickly get to the ponit of acting like gdb is connected again.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Pavel Machek]

Hi!


> >+config KGDB_THREAD
> >+	bool "KGDB: Thread analysis"
> >+	depends on KGDB
> >+	help
> >+	  With thread analysis enabled, gdb can talk to kgdb stub to list
> >+	  threads and to get stack trace for a thread. This option also 
> >enables
> >+	  some code which helps gdb get exact status of thread. Thread 
> >analysis
> >+	  adds some overhead to schedule and down functions. You can disable
> >+	  this option if you do not want to compromise on speed.
> 
> Lets remove the overhead and eliminate the need for this option in favor of 
> always having threads.  Works in the mm kgdb...

No. Thread analysis is unsuitable for the mainline (manipulates
sched.c in ugly way). It may be okay for -mm, but in such case it
should better be separated.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[George Anzinger]

Tom Rini wrote:
> On Fri, Feb 27, 2004 at 02:44:33PM -0800, George Anzinger wrote:
> 
> 
>>Couple of comments below...
>>
>>Tom Rini wrote:
> 
> [snip]
> 
>>>-	spin_unlock(&uart_interrupt_lock);
>>>-	local_irq_restore(flags);
>>>-
>>
>>I think you need at least this (especially in SMP, but works in all):
>>	char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
>>        if(iir & UART_IIR_RDI){
>>
>>>+		kgdb_schedule_breakpoint();
>>
>>	}
>>
>>>	return IRQ_HANDLED;
> 
> 
> Would this be to ensure that we only schedule one breakpoint not 2?
Well, that too, but the notion is to take care of an interrupt on cpu 1 while 
doing console output on cpu 0.  If cpu 0 doesn't grab the '+' fast enough to 
prevent the interrupt cpu 1 may get an interrupt with no characters in the fifo. 
  This code just ignores that.
> 
> [snip]
> 
>>>+		/* If we get the start of another packet, this means
>>>+		 * that GDB is attempting to reconnect.  We will NAK
>>>+		 * the packet being sent, and stop trying to send this
>>>+		 * packet. */
>>>+		if (ch == '$') {
>>>+			kgdb_serial->write_char('-');
>>>			if (kgdb_serial->flush)
>>>				kgdb_serial->flush();
>>>-			breakpoint();
>>
>>Flags go up in my mind here about recursion...  What if we are already 
>>handling a breakpoint???  This may all be cool, but, as I said, alarms are 
>>ringing.
> 
> 
> There's two cases here, IMHO.  If GDB is connected, the only time we'll
> get a '$' when we're sending a packet is if we're out-of-sync (in
> regular gdb/kgdb communication) or we're prempting a console message
> (i.e. we're trying to send something while gdb wants to break in). 


I am a little unclear about this.  Assuming that the user has not been so dumb 
as to put a breakpoint in kgdb's console handling code, I suspect that the entry 
code should allow the console message to complete prior to sending the first 
message to gdb.  I made a rather lame attempt to do this in the -mm kgdb.  What 
I think should happen is that, on entry kgdb should send an nmi to all but self. 
  The kgdb nmi code (at in_kgdb() in the -mm patch) should check to see if the 
CURRENT cpu is in the kgdb console code and, if so, set a flag for the console 
code that a kgdb entry is pending and then return.  The console code should 
check this flag on exit, after clearing the "i am in console" flag and if set, 
do a send nmi self.  This would allow the console code to complete prior to the 
kgdb entry while still rounding up all the other cpus with the nmi.

In
> the latter case, it's OK to give up on the console packet (it's not
> critical) and trying to send a console message during a breakpoint is
> asking for trouble.  

Not only that, it is forbidden by the protocol (dam :(.

If things get out-of-sync in normal gdb/kgdb
> communication, what will happen w/o this change is that we get stuck in
> a "Packet instead of ACK" loop on gdb, and we get stuck trying to
> transmit that same packet on the kgdb side.   I think that if we call
> breakpoint() here we can try and start over...

The problem is that you are now doing a breakpoint from inside kgdb while 
handling a prior breakpoint.  At the very least you would need to consider very 
carefully what happens after the breakpoint.  It is not clear that you can 
recover from this condition... but you may be able to look around.
> 
> The other case is what the comment describes, and in that case we quite
> quickly get to the ponit of acting like gdb is connected again.
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[George Anzinger]

Pavel Machek wrote:
> Hi!
> 
> 
> 
>>>+config KGDB_THREAD
>>>+	bool "KGDB: Thread analysis"
>>>+	depends on KGDB
>>>+	help
>>>+	  With thread analysis enabled, gdb can talk to kgdb stub to list
>>>+	  threads and to get stack trace for a thread. This option also 
>>>enables
>>>+	  some code which helps gdb get exact status of thread. Thread 
>>>analysis
>>>+	  adds some overhead to schedule and down functions. You can disable
>>>+	  this option if you do not want to compromise on speed.
>>
>>Lets remove the overhead and eliminate the need for this option in favor of 
>>always having threads.  Works in the mm kgdb...
> 
> 
> No. Thread analysis is unsuitable for the mainline (manipulates
> sched.c in ugly way). It may be okay for -mm, but in such case it
> should better be separated.

Not in the -mm version.  I agree that sched.c should NEVER be treated this way 
and it is not in the -mm version.  I also think that, most of the time, it is 
useful to have the thread stuff, but that may be just my usage...


-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Amit S. Kale]

On Saturday 28 Feb 2004 6:38 am, George Anzinger wrote:
> Pavel Machek wrote:
> > Hi!
> >
> >>>+config KGDB_THREAD
> >>>+	bool "KGDB: Thread analysis"
> >>>+	depends on KGDB
> >>>+	help
> >>>+	  With thread analysis enabled, gdb can talk to kgdb stub to list
> >>>+	  threads and to get stack trace for a thread. This option also
> >>>enables
> >>>+	  some code which helps gdb get exact status of thread. Thread
> >>>analysis
> >>>+	  adds some overhead to schedule and down functions. You can disable
> >>>+	  this option if you do not want to compromise on speed.
> >>
> >>Lets remove the overhead and eliminate the need for this option in favor
> >> of always having threads.  Works in the mm kgdb...
> >
> > No. Thread analysis is unsuitable for the mainline (manipulates
> > sched.c in ugly way). It may be okay for -mm, but in such case it
> > should better be separated.
>
> Not in the -mm version.  I agree that sched.c should NEVER be treated this
> way and it is not in the -mm version.  I also think that, most of the time,
> it is useful to have the thread stuff, but that may be just my usage...

If threads stuff didn't introduce any unclean code changes, I too would prefer 
to have it on all the time. As things stands, threads stuff is rather 
intrusive.

-Amit

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Amit S. Kale]

I like this. Can you also do the same change for x86_64 before checking in?

We need to review Kconfig help sometime, as George has pointed out in another 
email.

-Amit

On Saturday 28 Feb 2004 2:53 am, Tom Rini wrote:
> Hello.  The following patch moves all of the config options into one file,
> kernel/Kconfig.kgdb.
> <snip>

Re: [Kgdb-bugreport] [KGDB PATCH][3/7] SysRq-G

[Amit S. Kale]

On Saturday 28 Feb 2004 4:19 am, George Anzinger wrote:
> There may be a need to change the keyboard driver for this.  I have had
> trouble with it in the past.  Seems it doesn't like to not get control back
> for a very long time.  I am not sure what the problem is but, be warned.

Let's put it in core instead of core-lite, then,

-Amit

>
> -g
>
> Tom Rini wrote:
> > Hello.  The following adds SysRq-G.  This is always configured in on
> > CONFIG_KGDB && CONFIG_SYSRQ.
> >
> > diff -zrupN linux-2.6.3+config+serial/drivers/char/sysrq.c
> > linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c ---
> > linux-2.6.3+config+serial/drivers/char/sysrq.c	2004-02-27
> > 12:06:22.000000000 -0700 +++
> > linux-2.6.3+config+serial+sysrq+arch_hooks/drivers/char/sysrq.c	2004-02-2
> >7 12:16:14.000000000 -0700 @@ -31,6 +31,7 @@
> >  #include <linux/suspend.h>
> >  #include <linux/writeback.h>
> >  #include <linux/buffer_head.h>		/* for fsync_bdev() */
> > +#include <linux/kgdb.h>			/* for breakpoint() */
> >
> >  #include <linux/spinlock.h>
> >
> > @@ -44,6 +45,25 @@ int sysrq_enabled = 1;
> >  /* Machine specific power off function */
> >  void (*sysrq_power_off)(void);
> >
> > +/* Make a breakpoint() right now. */
> > +#ifdef CONFIG_KGDB
> > +#define  GDB_OP &kgdb_op
> > +static void kgdb_sysrq(int key, struct pt_regs *pt_regs, struct
> > tty_struct *tty) +{
> > +	printk("kgdb sysrq\n");
> > +	breakpoint();
> > +}
> > +
> > +static struct sysrq_key_op kgdb_op = {
> > +	.handler	= kgdb_sysrq,
> > +	.help_msg	= "kGdb|Fgdb",
> > +	.action_msg	= "Debug breakpoint\n",
> > +};
> > +
> > +#else
> > +#define  GDB_OP NULL
> > +#endif
> > +
> >  /* Loglevel sysrq handler */
> >  static void sysrq_handle_loglevel(int key, struct pt_regs *pt_regs,
> >  				  struct tty_struct *tty)
> > @@ -239,7 +259,7 @@ static struct sysrq_key_op *sysrq_key_ta
> >  /* d */	NULL,
> >  /* e */	&sysrq_term_op,
> >  /* f */	NULL,
> > -/* g */	NULL,
> > +/* g */	GDB_OP,
> >  /* h */	NULL,
> >  /* i */	&sysrq_kill_op,
> >  /* j */	NULL,

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Amit S. Kale]

OK to checkin.

-Amit

On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> Hello.  When we use kgdboe, we can't use it until do_basic_setup() is done.
> So we have two options, not allow kgdboe to use the initial breakpoint
> or move debugger_entry() to be past the point where kgdboe will be usable.
> I've opted for the latter, as if an earlier breakpoint is needed you can
> still use serial and throw kgdb_schedule_breakpoint/breakpoint where
> desired.
>
> --- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> +++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
> @@ -581,6 +582,7 @@ static int init(void * unused)
>
>  	smp_init();
>  	do_basic_setup();
> +	debugger_entry();
>
>  	prepare_namespace();

Re: [KGDB PATCH][6/7] KGDBOE fixes

[Amit S. Kale]

ok by me
-Amit

On Saturday 28 Feb 2004 3:22 am, Tom Rini wrote:
> Hello.  The following is a couple of small but important cleanups to
> kgdboe. - memset out_buf in case we don't have it full when we flush.
> - In rx_hook, if netpoll_trap() is set, clear it.  If kgdb_isn't connected,
>   schedule a breakpoint, and never call breakpoint() directly.
> - In kgdb_serial->hook, set netpoll_trap(1).  kgdb_serial->hook must be
>   called prior to using a 'serial' port, so this is where we should stick
>   this.
> - Backout unneeded changes to other files.
>
> --- linux-2.6.3/drivers/net/kgdb_eth.c.orig	2004-02-27 13:27:53.778658691
> -0700 +++ linux-2.6.3/drivers/net/kgdb_eth.c	2004-02-27 13:32:07.920255062
> -0700 @@ -18,6 +18,7 @@
>   * Refactored for netpoll API by Matt Mackall <mpm@selenic.com>
>   *
>   * Some cleanups by Pavel Machek <pavel@suse.cz>
> + * Further cleanups by Tom Rini <trini@mvista.com>
>   */
>
>  #include <linux/module.h>
> @@ -60,7 +61,6 @@
>  static atomic_t in_count;
>  int kgdboe = 0;			/* Default to tty mode */
>
> -extern void breakpoint(void);
>  static void rx_hook(struct netpoll *np, int port, char *msg, int len);
>
>  static struct netpoll np = {
> @@ -89,6 +89,7 @@
>  {
>  	if (out_count && np.dev) {
>  		netpoll_send_udp(&np, out_buf, out_count);
> +		memset(out_buf, 0, sizeof(out_buf));
>  		out_count = 0;
>  	}
>  }
> @@ -107,12 +108,15 @@
>  	np->remote_port = port;
>
> +	/* Do we need to clear the trap? */
> +	if (netpoll_trap())
> +		netpoll_set_trap(0);
>  	/* Is this gdb trying to attach? */
> -	if (!netpoll_trap() && len == 8 && !strncmp(msg, "$Hc-1#09", 8))
> -		breakpoint();
> +	if (kgdb_connected) {
> +		kgdb_schedule_breakpoint();
>
>  	for (i = 0; i < len; i++) {
>  		if (msg[i] == 3)
> -			breakpoint();
> +			kgdb_schedule_breakpoint();
>
>  		if (atomic_read(&in_count) >= IN_BUF_SIZE) {
>  			/* buffer overflow, clear it */
> @@ -138,6 +141,9 @@
>  	/* Un-initalized, don't go further. */
>  	if (kgdboe != 1)
>  		return 1;
> +
> +	netpoll_set_trap(1);
> +
>  	return 0;
>  }
>
> --- linux-2.6.3/net/core/skbuff.c.orig	2004-02-27 13:30:21.107968572 -0700
> +++ linux-2.6.3/net/core/skbuff.c	2004-02-27 13:30:44.279825113 -0700
> @@ -55,7 +55,6 @@
>  #include <linux/rtnetlink.h>
>  #include <linux/init.h>
>  #include <linux/highmem.h>
> -#include <linux/debugger.h>
>
>  #include <net/protocol.h>
>  #include <net/dst.h>
> --- linux-2.6.3/net/core/dev.c.orig	2004-02-27 13:30:27.687508165 -0700
> +++ linux-2.6.3/net/core/dev.c	2004-02-27 13:30:44.261829108 -0700
> @@ -1547,6 +1547,7 @@
>  }
>  #endif
>
> +
>  /**
>   *	netif_rx	-	post buffer to the network code
>   *	@skb: buffer to post

Re: [KGDB PATCH][5/7] Fix ppc32 hooks.

[Amit S. Kale]

Ok to checkin.

-Amit

On Saturday 28 Feb 2004 3:16 am, Tom Rini wrote:
> Hello.  The following removes the only PPC32 CHK_DEBUGGER statement,
> as we've always been making kgdb take over the various debugger pointers.
>
> diff -zrupN linux-2.6.3+config+serial/arch/ppc/mm/fault.c
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/ppc/mm/fault.c ---
> linux-2.6.3+config+serial/arch/ppc/mm/fault.c	2004-02-27 12:16:13.000000000
> -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/ppc/mm/fault.c	2004-02-27
> 12:16:14.000000000 -0700 @@ -351,13 +351,11 @@ bad_page_fault(struct
> pt_regs *regs, uns
>  	}
>
>  	/* kernel has accessed a bad area */
> -#if defined(CONFIG_XMON)
> +#if defined(CONFIG_XMON) || defined(CONFIG_KGDB)
>  	if (debugger_kernel_faults)
>  		debugger(regs);
>  #endif
>
> -	CHK_DEBUGGER(14, sig,0, regs,)
> -
>  	die("kernel access of bad area", regs, sig);
>  }

Re: [KGDB PATCH][4/7] Fix x86_64 hooks

[Amit S. Kale]

I guess you haven't tested this on an x86_64 machine. Please checkin. I'll 
test once you do that. If there is something incorrect (unlikely), I'll fix 
it.

-Amit


On Saturday 28 Feb 2004 3:10 am, Tom Rini wrote:
> Hello.  The following is more complicated than it looks.  This makes the
> x86_64 stub add kgdb to the die_chain, and then remove all of the now
> unneeded CHK_DEBUGGER statements, and other whitespace changes.  In the
> end, the x86_64 patch looks like:
>  arch/x86_64/Kconfig            |    4
>  arch/x86_64/kernel/Makefile    |    1
>  arch/x86_64/kernel/entry.S     |   41 +++
>  arch/x86_64/kernel/irq.c       |    2
>  arch/x86_64/kernel/kgdb.c      |  478
> +++++++++++++++++++++++++++++++++++++++++ arch/x86_64/kernel/traps.c     | 
>   3
>  include/asm-x86_64/kgdb.h      |   54 ++++
>  include/asm-x86_64/processor.h |    1
>  8 files changed, 577 insertions(+), 7 deletions(-)
>
> As an aside, I now know how to kill off user_schedule bits on i386/x86_64,
> so with more thought on how to do it cleanly for everyone, the entry.S
> changes can die.
>
> diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/kgdb.c
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/kgdb.c ---
> linux-2.6.3+config+serial/arch/x86_64/kernel/kgdb.c	2004-02-27
> 12:16:13.000000000 -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/kgdb.c	2004-0
>2-27 12:16:14.000000000 -0700 @@ -41,6 +41,7 @@
>  #endif
>  #include <linux/init.h>
>  #include <linux/debugger.h>
> +#include <asm/kdebug.h>
>
>  /* Put the error code here just in case the user cares.  */
>  int gdb_x86_64errcode;
> @@ -443,6 +444,33 @@ struct pt_regs *kgdb_shadow_regs(struct
>  	return NULL;
>  }
>
> +/* Register KGDB with the die_chain so that we hook into all of the right
> + * spots. */
> +static int kgdb_notify(struct notifier_block *self, unsigned long cmd,
> +		void *ptr)
> +{
> +        struct die_args *d = ptr;
> +
> +        if (!kgdb_connected || (cmd == DIE_DEBUG && user_mode(d->regs)))
> +                return NOTIFY_DONE;
> +        if (cmd == DIE_NMI_IPI) {
> +                if (atomic_read(debugger_active))
> +                        return NOTIFY_BAD;
> +        } else if ((*linux_debug_hook)(d->trapnr, d->signr, d->err,
> d->regs)) +                return NOTIFY_BAD; /* skip */
> +
> +        return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block kgdb_notifier = {
> +        .notifier_call = kgdb_notify,
> +};
> +
> +int kgdb_arch_init(void)
> +{
> +	notifier_chain_register(&die_chain, &kgdb_notifier);
> +}
> +
>  struct kgdb_arch arch_kgdb_ops = {
>  	.gdb_bpt_instr = {0xcc},
>  	.flags = KGDB_HW_BREAKPOINT,
> diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/nmi.c
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/nmi.c ---
> linux-2.6.3+config+serial/arch/x86_64/kernel/nmi.c	2004-02-27
> 12:16:13.000000000 -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/nmi.c	2004-02
>-27 12:11:20.000000000 -0700 @@ -24,7 +24,6 @@
>  #include <linux/module.h>
>  #include <linux/sysdev.h>
>  #include <linux/nmi.h>
> -#include <linux/debugger.h>
>
>  #include <asm/smp.h>
>  #include <asm/mtrr.h>
> @@ -318,26 +317,13 @@ void nmi_watchdog_tick (struct pt_regs *
>  		return;
>
>  	sum = read_pda(apic_timer_irqs);
> -	if (atomic_read(&debugger_active)) {
> -
> -		/*
> -		 * The machine is in debugger, hold this cpu if already
> -		 * not held.
> -		 */
> -		debugger_nmihook(cpu, regs);
> -		alert_counter[cpu] = 0;
> -
> -	} else if (last_irq_sums[cpu] == sum) {
> -
> +	if (last_irq_sums[cpu] == sum) {
>  		/*
>  		 * Ayiee, looks like this CPU is stuck ...
>  		 * wait a few IRQs (5 seconds) before doing the oops ...
>  		 */
>  		alert_counter[cpu]++;
>  		if (alert_counter[cpu] == 5*nmi_hz) {
> -
> -			CHK_DEBUGGER(2,SIGSEGV,0,regs,)
> -
>  			if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) == NOTIFY_BAD)
> { alert_counter[cpu] = 0;
>  				return;
> diff -zrupN linux-2.6.3+config+serial/arch/x86_64/kernel/traps.c
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/traps.c ---
> linux-2.6.3+config+serial/arch/x86_64/kernel/traps.c	2004-02-27
> 12:16:13.000000000 -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/kernel/traps.c	2004-
>02-27 12:16:14.000000000 -0700 @@ -364,7 +364,6 @@ void __die(const char *
> str, struct pt_r
>
>  void die(const char * str, struct pt_regs * regs, long err)
>  {
> -	CHK_DEBUGGER(1,SIGTRAP,err,regs,)
>  	oops_begin();
>  	handle_BUG(regs);
>  	__die(str, regs, err);
> @@ -438,7 +437,6 @@ static void do_trap(int trapnr, int sign
>  #define DO_ERROR(trapnr, signr, str, name) \
>  asmlinkage void do_##name(struct pt_regs * regs, long error_code) \
>  { \
> -	CHK_DEBUGGER(1,SIGTRAP,error_code,regs,return) \
>  	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) ==
> NOTIFY_BAD) \ return; \
>  	do_trap(trapnr, signr, str, regs, error_code, NULL); \
> @@ -646,7 +644,6 @@ asmlinkage void do_debug(struct pt_regs
>  	force_sig_info(SIGTRAP, &info, tsk);
>  clear_dr7:
>  	asm volatile("movq %0,%%db7"::"r"(0UL));
> -	CHK_DEBUGGER(1,SIGTRAP,error_code,regs,return)
>  	notify_die(DIE_DEBUG, "debug", regs, error_code, 1, SIGTRAP);
>  	return;
>
> diff -zrupN linux-2.6.3+config+serial/arch/x86_64/mm/fault.c
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/mm/fault.c ---
> linux-2.6.3+config+serial/arch/x86_64/mm/fault.c	2004-02-27
> 12:16:13.000000000 -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/arch/x86_64/mm/fault.c	2004-02-2
>7 12:10:49.000000000 -0700 @@ -23,7 +23,6 @@
>  #include <linux/vt_kern.h>		/* For unblank_screen() */
>  #include <linux/compiler.h>
>  #include <linux/module.h>
> -#include <linux/debugger.h>
>
>  #include <asm/system.h>
>  #include <asm/uaccess.h>
> @@ -408,7 +407,6 @@ no_context:
>
>   	if (is_prefetch(regs, address))
>   		return;
> -	CHK_DEBUGGER(14, SIGSEGV,error_code, regs,)
>
>  	if (is_errata93(regs, address))
>  		return;
> diff -zrupN linux-2.6.3+config+serial/include/asm-x86_64/calling.h
> linux-2.6.3+config+serial+sysrq+arch_hooks/include/asm-x86_64/calling.h ---
> linux-2.6.3+config+serial/include/asm-x86_64/calling.h	2004-02-27
> 12:16:13.000000000 -0700 +++
> linux-2.6.3+config+serial+sysrq+arch_hooks/include/asm-x86_64/calling.h	200
>4-02-27 12:12:03.000000000 -0700 @@ -12,7 +12,7 @@
>  #define RBX 40
>  /* arguments: interrupts/non tracing syscalls only save upto here*/
>  #define R11 48
> -#define R10 56
> +#define R10 56
>  #define R9 64
>  #define R8 72
>  #define RAX 80

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[Tom Rini]

On Fri, Feb 27, 2004 at 03:53:12PM -0800, George Anzinger wrote:
> Tom Rini wrote:
> >On Fri, Feb 27, 2004 at 02:44:33PM -0800, George Anzinger wrote:
> >
> >
> >>Couple of comments below...
> >>
> >>Tom Rini wrote:
> >
> >[snip]
> >
> >>>-	spin_unlock(&uart_interrupt_lock);
> >>>-	local_irq_restore(flags);
> >>>-
> >>
> >>I think you need at least this (especially in SMP, but works in all):
> >>	char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
> >>       if(iir & UART_IIR_RDI){
> >>
> >>>+		kgdb_schedule_breakpoint();
> >>
> >>	}
> >>
> >>>	return IRQ_HANDLED;
> >
> >
> >Would this be to ensure that we only schedule one breakpoint not 2?
> Well, that too, but the notion is to take care of an interrupt on cpu 1 
> while doing console output on cpu 0.  If cpu 0 doesn't grab the '+' fast 
> enough to prevent the interrupt cpu 1 may get an interrupt with no 
> characters in the fifo. This code just ignores that.

OK, done.

> >[snip]
> >
> >>>+		/* If we get the start of another packet, this means
> >>>+		 * that GDB is attempting to reconnect.  We will NAK
> >>>+		 * the packet being sent, and stop trying to send this
> >>>+		 * packet. */
> >>>+		if (ch == '$') {
> >>>+			kgdb_serial->write_char('-');
> >>>			if (kgdb_serial->flush)
> >>>				kgdb_serial->flush();
> >>>-			breakpoint();
> >>
> >>Flags go up in my mind here about recursion...  What if we are already 
> >>handling a breakpoint???  This may all be cool, but, as I said, alarms 
> >>are ringing.
> >
> >
> >There's two cases here, IMHO.  If GDB is connected, the only time we'll
> >get a '$' when we're sending a packet is if we're out-of-sync (in
> >regular gdb/kgdb communication) or we're prempting a console message
> >(i.e. we're trying to send something while gdb wants to break in). 
> 
> 
> I am a little unclear about this.  Assuming that the user has not been so 
> dumb as to put a breakpoint in kgdb's console handling code, I suspect that 
> the entry code should allow the console message to complete prior to 
> sending the first message to gdb.

I don't think so.  If you don't break out of put_packet(), kgdb will
try to send the packet (console or for a previous, now dead, session)
forever.  And gdb will keep trying to send it's first packet, timing out
and moving on.

> I made a rather lame attempt to do this 
> in the -mm kgdb.  What I think should happen is that, on entry kgdb should 
> send an nmi to all but self. The kgdb nmi code (at in_kgdb() in the -mm 
>  patch) should check to see if the CURRENT cpu is in the kgdb console code 
> and, if so, set a flag for the console code that a kgdb entry is pending 
> and then return.  The console code should check this flag on exit, after 
> clearing the "i am in console" flag and if set, do a send nmi self.  This 
> would allow the console code to complete prior to the kgdb entry while 
> still rounding up all the other cpus with the nmi.

IMHO, that's awfully complex for something we can just not skip out on.

> >If things get out-of-sync in normal gdb/kgdb
> >communication, what will happen w/o this change is that we get stuck in
> >a "Packet instead of ACK" loop on gdb, and we get stuck trying to
> >transmit that same packet on the kgdb side.   I think that if we call
> >breakpoint() here we can try and start over...
> 
> The problem is that you are now doing a breakpoint from inside kgdb while 
> handling a prior breakpoint.

Only in the case where we aren't out-of-sync, but gdb died /
disconnected and we then hit a breakpoint.

> At the very least you would need to consider 
> very carefully what happens after the breakpoint.  It is not clear that you 
> can recover from this condition... but you may be able to look around.

I don't follow you here.  I agree that in the case of hitting a
breakpoint while gdb isn't connected isn't necessarily clean, but since
gdb didn't go away cleanly, and there isn't a way for it to tell us it's
trying to recover, I'm not sure what we can do aside from clear out the
existing breakpoints (just like we could have done, if gdb was still
connected) and keep going.  We should document that this particular
behavior might not be a good thing, but it's just that one corner case
of things (disconnect is fine, detach/reattach is fine, heck even gdb
dying and not hitting a breakpoint is fine).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[Amit S. Kale]

This patch broke Ctrl+C response on my system. Fix as follows
With this fix I can kill or detach gdb with pending console messages and then 
connect a new gdb.

Is following fix ok to checkin? (interdiff)

@@ -412,11 +418,11 @@
 +              /* The user has selected one of ttyS[0-3], which we pull
 +               * from rs_table[].  If this doesn't exist, user error. */
 +              gdb_async_info.port = gdb_async_info.state->port =
-+                  rs_table[KGDB_PORT].port;
++                  rs_table[kgdb8250_ttyS].port;
 +              gdb_async_info.line = gdb_async_info.state->irq =
-+                  rs_table[KGDB_PORT].irq;
-+              gdb_async_info.state->io_type = rs_table[KGDB_PORT].io_type;
-+              reg_shift = rs_table[KGDB_PORT].iomem_reg_shift;
++                  rs_table[kgdb8250_ttyS].irq;
++              gdb_async_info.state->io_type = 
rs_table[kgdb8250_ttyS].io_type;
++              reg_shift = rs_table[kgdb8250_ttyS].iomem_reg_shift;
 +      }
 +
 +      switch (gdb_async_info.state->io_type) {



On Monday 01 Mar 2004 8:58 pm, Tom Rini wrote:
> On Fri, Feb 27, 2004 at 03:53:12PM -0800, George Anzinger wrote:
> > Tom Rini wrote:
> > >On Fri, Feb 27, 2004 at 02:44:33PM -0800, George Anzinger wrote:
> > >>Couple of comments below...
> > >>
> > >>Tom Rini wrote:
> > >
> > >[snip]
> > >
> > >>>-	spin_unlock(&uart_interrupt_lock);
> > >>>-	local_irq_restore(flags);
> > >>>-
> > >>
> > >>I think you need at least this (especially in SMP, but works in all):
> > >>	char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
> > >>       if(iir & UART_IIR_RDI){
> > >>
> > >>>+		kgdb_schedule_breakpoint();
> > >>
> > >>	}
> > >>
> > >>>	return IRQ_HANDLED;
> > >
> > >Would this be to ensure that we only schedule one breakpoint not 2?
> >
> > Well, that too, but the notion is to take care of an interrupt on cpu 1
> > while doing console output on cpu 0.  If cpu 0 doesn't grab the '+' fast
> > enough to prevent the interrupt cpu 1 may get an interrupt with no
> > characters in the fifo. This code just ignores that.
>
> OK, done.
>
> > >[snip]
> > >
> > >>>+		/* If we get the start of another packet, this means
> > >>>+		 * that GDB is attempting to reconnect.  We will NAK
> > >>>+		 * the packet being sent, and stop trying to send this
> > >>>+		 * packet. */
> > >>>+		if (ch == '$') {
> > >>>+			kgdb_serial->write_char('-');
> > >>>			if (kgdb_serial->flush)
> > >>>				kgdb_serial->flush();
> > >>>-			breakpoint();
> > >>
> > >>Flags go up in my mind here about recursion...  What if we are already
> > >>handling a breakpoint???  This may all be cool, but, as I said, alarms
> > >>are ringing.
> > >
> > >There's two cases here, IMHO.  If GDB is connected, the only time we'll
> > >get a '$' when we're sending a packet is if we're out-of-sync (in
> > >regular gdb/kgdb communication) or we're prempting a console message
> > >(i.e. we're trying to send something while gdb wants to break in).
> >
> > I am a little unclear about this.  Assuming that the user has not been so
> > dumb as to put a breakpoint in kgdb's console handling code, I suspect
> > that the entry code should allow the console message to complete prior to
> > sending the first message to gdb.
>
> I don't think so.  If you don't break out of put_packet(), kgdb will
> try to send the packet (console or for a previous, now dead, session)
> forever.  And gdb will keep trying to send it's first packet, timing out
> and moving on.
>
> > I made a rather lame attempt to do this
> > in the -mm kgdb.  What I think should happen is that, on entry kgdb
> > should send an nmi to all but self. The kgdb nmi code (at in_kgdb() in
> > the -mm patch) should check to see if the CURRENT cpu is in the kgdb
> > console code and, if so, set a flag for the console code that a kgdb
> > entry is pending and then return.  The console code should check this
> > flag on exit, after clearing the "i am in console" flag and if set, do a
> > send nmi self.  This would allow the console code to complete prior to
> > the kgdb entry while still rounding up all the other cpus with the nmi.
>
> IMHO, that's awfully complex for something we can just not skip out on.
>
> > >If things get out-of-sync in normal gdb/kgdb
> > >communication, what will happen w/o this change is that we get stuck in
> > >a "Packet instead of ACK" loop on gdb, and we get stuck trying to
> > >transmit that same packet on the kgdb side.   I think that if we call
> > >breakpoint() here we can try and start over...
> >
> > The problem is that you are now doing a breakpoint from inside kgdb while
> > handling a prior breakpoint.
>
> Only in the case where we aren't out-of-sync, but gdb died /
> disconnected and we then hit a breakpoint.
>
> > At the very least you would need to consider
> > very carefully what happens after the breakpoint.  It is not clear that
> > you can recover from this condition... but you may be able to look
> > around.
>
> I don't follow you here.  I agree that in the case of hitting a
> breakpoint while gdb isn't connected isn't necessarily clean, but since
> gdb didn't go away cleanly, and there isn't a way for it to tell us it's
> trying to recover, I'm not sure what we can do aside from clear out the
> existing breakpoints (just like we could have done, if gdb was still
> connected) and keep going.  We should document that this particular
> behavior might not be a good thing, but it's just that one corner case
> of things (disconnect is fine, detach/reattach is fine, heck even gdb
> dying and not hitting a breakpoint is fine).

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Amit S. Kale]

It also makes core.patch dependent on 8250.patch
Any ideas on fixing that?

-Amit

On Saturday 28 Feb 2004 2:53 am, Tom Rini wrote:
> Hello.  The following patch moves all of the config options into one file,
> kernel/Kconfig.kgdb.
>
> diff -zrupN linux-2.6.3+nothing/arch/i386/Kconfig
> linux-2.6.3+config+serial/arch/i386/Kconfig ---
> linux-2.6.3+nothing/arch/i386/Kconfig	2004-02-27 12:16:14.296187607 -0700
> +++ linux-2.6.3+config+serial/arch/i386/Kconfig	2004-02-27
> 12:16:13.707320867 -0700 @@ -1253,36 +1253,7 @@ config DEBUG_SPINLOCK_SLEEP
>  	  If you say Y here, various routines which may sleep will become very
>  	  noisy if they are called with a spinlock held.
>
> -config KGDB
> -	bool "KGDB: kernel debugging with remote gdb"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  If you say Y here, it will be possible to remotely debug the
> -	  kernel using gdb. This enlarges your kernel image disk size by
> -	  several megabytes and requires a machine with more than 128 MB
> -	  RAM to avoid excessive linking time.
> -	  Documentation of kernel debugger available at
> -	  http://kgdb.sourceforge.net
> -	  This is only useful for kernel hackers. If unsure, say N.
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "KGDB: Console messages through gdb"
> -	depends on KGDB
> -	help
> -	  If you say Y here, console messages will appear through gdb.
> -	  Other consoles such as tty or ttyS will continue to work as usual.
> +source "kernel/Kconfig.kgdb"
>
>  config FRAME_POINTER
>  	bool "Compile the kernel with frame pointers"
> diff -zrupN linux-2.6.3+nothing/arch/ppc/Kconfig
> linux-2.6.3+config+serial/arch/ppc/Kconfig ---
> linux-2.6.3+nothing/arch/ppc/Kconfig	2004-02-27 12:16:14.403163398 -0700
> +++ linux-2.6.3+config+serial/arch/ppc/Kconfig	2004-02-27
> 12:16:13.771306387 -0700 @@ -1170,52 +1170,7 @@ config DEBUG_SPINLOCK_SLEEP
>  	  If you say Y here, various routines which may sleep will become very
>  	  noisy if they are called with a spinlock held.
>
> -config KGDB
> -	bool "Include kgdb kernel debugger"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  Include in-kernel hooks for kgdb, the Linux kernel source level
> -	  debugger.  See <http://kgdb.sourceforge.net/> for more information.
> -	  Unless you are intending to debug the kernel, say N here.
> -
> -choice
> -	prompt "Serial Port"
> -	depends on KGDB
> -	default KGDB_TTYS1
> -
> -config KGDB_TTYS0
> -	bool "ttyS0"
> -
> -config KGDB_TTYS1
> -	bool "ttyS1"
> -
> -config KGDB_TTYS2
> -	bool "ttyS2"
> -
> -config KGDB_TTYS3
> -	bool "ttyS3"
> -
> -endchoice
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "Enable serial console thru kgdb port"
> -	depends on KGDB && 8xx || 8260
> -	help
> -	  If you enable this, all serial console messages will be sent
> -	  over the gdb stub.
> -	  If unsure, say N.
> +source "kernel/Kconfig.kgdb"
>
>  config XMON
>  	bool "Include xmon kernel debugger"
> diff -zrupN linux-2.6.3+nothing/arch/x86_64/Kconfig
> linux-2.6.3+config+serial/arch/x86_64/Kconfig ---
> linux-2.6.3+nothing/arch/x86_64/Kconfig	2004-02-27 12:16:14.350175389 -0700
> +++ linux-2.6.3+config+serial/arch/x86_64/Kconfig	2004-02-27
> 12:16:13.718318378 -0700 @@ -465,37 +465,7 @@ config IOMMU_LEAK
>           Add a simple leak tracer to the IOMMU code. This is useful when
> you are debugging a buggy device driver that leaks IOMMU mappings.
>
> -config KGDB
> -	bool "KGDB: kernel debugging with remote gdb"
> -	depends on DEBUG_KERNEL
> -	select DEBUG_INFO
> -	select FRAME_POINTER
> -	help
> -	  If you say Y here, it will be possible to remotely debug the
> -	  kernel using gdb. This enlarges your kernel image disk size by
> -	  several megabytes and requires a machine with more than 128 MB
> -	  RAM to avoid excessive linking time.
> -	  Documentation of kernel debugger available at
> -	  http://kgdb.sourceforge.net
> -	  This is only useful for kernel hackers. If unsure, say N.
> -
> -config KGDB_THREAD
> -	bool "KGDB: Thread analysis"
> -	depends on KGDB
> -	help
> -	  With thread analysis enabled, gdb can talk to kgdb stub to list
> -	  threads and to get stack trace for a thread. This option also enables
> -	  some code which helps gdb get exact status of thread. Thread analysis
> -	  adds some overhead to schedule and down functions. You can disable
> -	  this option if you do not want to compromise on speed.
> -
> -config KGDB_CONSOLE
> -	bool "KGDB: Console messages through gdb"
> -	depends on KGDB
> -	help
> -	  If you say Y here, console messages will appear through gdb.
> -	  Other consoles such as tty or ttyS will continue to work as usual.
> -
> +source "kernel/Kconfig.kgdb"
>  endmenu
>
>  source "security/Kconfig"
> diff -zrupN linux-2.6.3+nothing/drivers/net/Kconfig
> linux-2.6.3+config+serial/drivers/net/Kconfig ---
> linux-2.6.3+nothing/drivers/net/Kconfig	2004-02-27 12:16:14.521136701 -0700
> +++ linux-2.6.3+config+serial/drivers/net/Kconfig	2004-02-27
> 12:06:22.000000000 -0700 @@ -187,12 +187,6 @@ config NET_ETHERNET
>  	  Note that the answer to this question won't directly affect the
>  	  kernel: saying N will just cause the configurator to skip all
>  	  the questions about Ethernet network cards. If unsure, say N.
> -
> -config KGDB_ETH
> -	bool "KGDB: On ethernet"
> -	depends on KGDB
> -	help
> -	  Uses ethernet interface for kgdb.
>
>  config MII
>  	tristate "Generic Media Independent Interface device support"
> diff -zrupN linux-2.6.3+nothing/drivers/serial/Kconfig
> linux-2.6.3+config+serial/drivers/serial/Kconfig ---
> linux-2.6.3+nothing/drivers/serial/Kconfig	2004-02-27 12:16:14.545131271
> -0700 +++ linux-2.6.3+config+serial/drivers/serial/Kconfig	2004-02-27
> 12:06:30.000000000 -0700 @@ -6,34 +6,6 @@
>
>  menu "Serial drivers"
>
> -config KGDB_8250
> -	bool "KGDB: On generic serial port (8250)"
> -	depends on KGDB
> -	help
> -	  Uses generic serial port (8250) for kgdb. This is independent of the
> -	  option 9250/16550 and compatible serial port.
> -
> -config KGDB_PORT
> -	hex "hex I/O port address of the debug serial port"
> -	depends on KGDB_8250
> -	default  3f8
> -	help
> -	  Some systems (x86 family at this writing) allow the port
> -	  address to be configured.  The number entered is assumed to be
> -	  hex, don't put 0x in front of it.  The standard address are:
> -	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
> -	  will tell you what you have.  It is good to test the serial
> -	  connection with a live system before trying to debug.
> -
> -config KGDB_IRQ
> -	int "IRQ of the debug serial port"
> -	depends on KGDB_8250
> -	default 4
> -	help
> -	  This is the irq for the debug port.  If everything is working
> -	  correctly and the kernel has interrupts on a control C to the
> -	  port should cause a break into the kernel debug stub.
> -
>  #
>  # The new 8250/16550 serial drivers
>  config SERIAL_8250
> diff -zrupN linux-2.6.3+nothing/kernel/Kconfig.kgdb
> linux-2.6.3+config+serial/kernel/Kconfig.kgdb ---
> linux-2.6.3+nothing/kernel/Kconfig.kgdb	1969-12-31 17:00:00.000000000 -0700
> +++ linux-2.6.3+config+serial/kernel/Kconfig.kgdb	2004-02-27
> 12:16:13.000000000 -0700 @@ -0,0 +1,141 @@
> +config KGDB
> +	bool "KGDB: kernel debugging with remote gdb"
> +	depends on DEBUG_KERNEL
> +	select DEBUG_INFO
> +	select FRAME_POINTER
> +	# XXX: Doesn't work w/o this right now
> +	select KGDB_THREAD if PPC32
> +	help
> +	  If you say Y here, it will be possible to remotely debug the
> +	  kernel using gdb. This enlarges your kernel image disk size by
> +	  several megabytes and requires a machine with more than 128 MB
> +	  RAM to avoid excessive linking time.
> +	  Documentation of kernel debugger available at
> +	  http://kgdb.sourceforge.net
> +	  This is only useful for kernel hackers. If unsure, say N.
> +
> +choice
> +	prompt "Method for KGDB communication"
> +	depends on KGDB
> +	default PPC_SIMPLE_SERIAL if PPC32 && (8xx || 8260)
> +	default KGDB_8250
> +	help
> +	  There are a number of different ways in which you can communicate
> +	  with KGDB.  The oldest is using a serial driver.  A newer method
> +	  is to use UDP packets and a special network driver.
> +
> +config KGDB_8250
> +	bool "KGDB: On generic serial port (8250)"
> +	help
> +	  Uses generic serial port (8250) for kgdb. This is independent of the
> +	  option 9250/16550 and compatible serial port.
> +
> +config KGDB_ETH
> +	bool "KGDB: On ethernet"
> +	select NETPOLL
> +	select NETPOLL_TRAP
> +	select NETPOLL_RX
> +	help
> +	  Uses ethernet interface for kgdb.
> +
> +config PPC_SIMPLE_SERIAL
> +	bool "KGDB: On any serial port"
> +	depends on PPC32
> +	help
> +	  Use a very simple, and not necessarily feature complete serial
> +	  driver.  This is the only serial option currently for MPC8xx or
> +	  MPC82xx based ports that do not offer an 8250-style UART.
> +
> +endchoice
> +
> +config KGDB_SIMPLE_SERIAL
> +	bool "Simple selection of KGDB serial port"
> +	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
> +	help
> +	  If you say Y here, you will only have to pick the baud rate
> +	  and serial port (ttyS) that you wish to use for KGDB.  If you
> +	  say N, you will have provide the I/O port and IRQ number.  Note
> +	  that if your serial ports are iomapped, then you must say Y here.
> +	  If in doubt, say Y.
> +
> +choice
> +	depends on KGDB_8250 || PPC_SIMPLE_SERIAL
> +    	prompt "Debug serial port BAUD"
> +	default KGDB_115200BAUD
> +	help
> +	  Gdb and the kernel stub need to agree on the baud rate to be
> +	  used.  Some systems (x86 family at this writing) allow this to
> +	  be configured.
> +
> +config KGDB_9600BAUD
> +	bool "9600"
> +
> +config KGDB_19200BAUD
> +	bool "19200"
> +
> +config KGDB_38400BAUD
> +	bool "38400"
> +
> +config KGDB_57600BAUD
> +	bool "57600"
> +
> +config KGDB_115200BAUD
> +	bool "115200"
> +endchoice
> +
> +choice
> +	prompt "Serial port for KGDB"
> +	depends on KGDB_SIMPLE_SERIAL
> +	default KGDB_TTYS0
> +
> +config KGDB_TTYS0
> +	bool "ttyS0"
> +
> +config KGDB_TTYS1
> +	bool "ttyS1"
> +
> +config KGDB_TTYS2
> +	bool "ttyS2"
> +
> +config KGDB_TTYS3
> +	bool "ttyS3"
> +
> +endchoice
> +
> +config KGDB_PORT
> +	hex "hex I/O port address of the debug serial port"
> +	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
> +	default  3f8
> +	help
> +	  Some systems (x86 family at this writing) allow the port
> +	  address to be configured.  The number entered is assumed to be
> +	  hex, don't put 0x in front of it.  The standard address are:
> +	  COM1 3f8 , irq 4 and COM2 2f8 irq 3.  Setserial /dev/ttySx
> +	  will tell you what you have.  It is good to test the serial
> +	  connection with a live system before trying to debug.
> +
> +config KGDB_IRQ
> +	int "IRQ of the debug serial port"
> +	depends on !KGDB_SIMPLE_SERIAL && (KGDB_8250 || PPC_SIMPLE_SERIAL)
> +	default 4
> +	help
> +	  This is the irq for the debug port.  If everything is working
> +	  correctly and the kernel has interrupts on a control C to the
> +	  port should cause a break into the kernel debug stub.
> +
> +config KGDB_THREAD
> +	bool "KGDB: Thread analysis"
> +	depends on KGDB
> +	help
> +	  With thread analysis enabled, gdb can talk to kgdb stub to list
> +	  threads and to get stack trace for a thread. This option also enables
> +	  some code which helps gdb get exact status of thread. Thread analysis
> +	  adds some overhead to schedule and down functions. You can disable
> +	  this option if you do not want to compromise on speed.
> +
> +config KGDB_CONSOLE
> +	bool "KGDB: Console messages through gdb"
> +	depends on KGDB
> +	help
> +	  If you say Y here, console messages will appear through gdb.
> +	  Other consoles such as tty or ttyS will continue to work as usual.

Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

[Tom Rini]

On Tue, Mar 02, 2004 at 05:06:03PM +0530, Amit S. Kale wrote:

> This patch broke Ctrl+C response on my system. Fix as follows
> With this fix I can kill or detach gdb with pending console messages and then 
> connect a new gdb.
> 
> Is following fix ok to checkin? (interdiff)
> 
> @@ -412,11 +418,11 @@
>  +              /* The user has selected one of ttyS[0-3], which we pull
>  +               * from rs_table[].  If this doesn't exist, user error. */
>  +              gdb_async_info.port = gdb_async_info.state->port =
> -+                  rs_table[KGDB_PORT].port;
> ++                  rs_table[kgdb8250_ttyS].port;
>  +              gdb_async_info.line = gdb_async_info.state->irq =
> -+                  rs_table[KGDB_PORT].irq;
> -+              gdb_async_info.state->io_type = rs_table[KGDB_PORT].io_type;
> -+              reg_shift = rs_table[KGDB_PORT].iomem_reg_shift;
> ++                  rs_table[kgdb8250_ttyS].irq;
> ++              gdb_async_info.state->io_type = 
> rs_table[kgdb8250_ttyS].io_type;
> ++              reg_shift = rs_table[kgdb8250_ttyS].iomem_reg_shift;
>  +      }
>  +
>  +      switch (gdb_async_info.state->io_type) {

Hmm.  I take it you passed in a different ttyS than you configured for?
Been a while since I tried that, and the above look correct.  Go ahead.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Tom Rini]

On Tue, Mar 02, 2004 at 05:09:26PM +0530, Amit S. Kale wrote:

> It also makes core.patch dependent on 8250.patch
> Any ideas on fixing that?

No, it's intentional.  eth.patch also depends on 8250.patch.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[George Anzinger]

Amit S. Kale wrote:
> On Saturday 28 Feb 2004 6:38 am, George Anzinger wrote:
> 
>>Pavel Machek wrote:
>>
>>>Hi!
>>>
>>>
>>>>>+config KGDB_THREAD
>>>>>+	bool "KGDB: Thread analysis"
>>>>>+	depends on KGDB
>>>>>+	help
>>>>>+	  With thread analysis enabled, gdb can talk to kgdb stub to list
>>>>>+	  threads and to get stack trace for a thread. This option also
>>>>>enables
>>>>>+	  some code which helps gdb get exact status of thread. Thread
>>>>>analysis
>>>>>+	  adds some overhead to schedule and down functions. You can disable
>>>>>+	  this option if you do not want to compromise on speed.
>>>>
>>>>Lets remove the overhead and eliminate the need for this option in favor
>>>>of always having threads.  Works in the mm kgdb...
>>>
>>>No. Thread analysis is unsuitable for the mainline (manipulates
>>>sched.c in ugly way). It may be okay for -mm, but in such case it
>>>should better be separated.
>>
>>Not in the -mm version.  I agree that sched.c should NEVER be treated this
>>way and it is not in the -mm version.  I also think that, most of the time,
>>it is useful to have the thread stuff, but that may be just my usage...
> 
> 
> If threads stuff didn't introduce any unclean code changes, I too would prefer 
> to have it on all the time. As things stands, threads stuff is rather 
> intrusive.

Lets put the threads stuff in the stub.  The only stuff we need in the kernel is 
the flag that indicateds that the pid hash table has been initialized.

Meanwhile, I would like to make a change to the gdb "info thread" command to do 
a better job of displaying the threads.  Here is what I am proposing:

Gdb would work as it does now if the following set is not done.

A new "set thread_level" command that would take the "bt" level to use on the 
thread display.
A new "set thread_limits command that would take two expressions that would 
reduce to two memory addresses.

Which ever of these is entered last will be active and used by "info thread" as 
follows:

if thread_level is active gdb will do the indicated number of  "up" operations 
and display the result on the info thread line for that thread (note there is 
other info on this line that will not be changed).

if thread_limits is active gdb will do 0 or more "up" commands until the 
resultant PC is NOT between the given limits.

The kernel, at this time, has defined symbols for the thread_limits command (it 
is used in the kernel for its internal display of threads).  I would expect that 
the thread_level version would be the answer for theaded application programs.

Daniel, how does this sound?

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Pavel Machek]

Hi!

> > It also makes core.patch dependent on 8250.patch
> > Any ideas on fixing that?
> 
> No, it's intentional.  eth.patch also depends on 8250.patch.

Perhaps that parts should be moved to core-lite? It should not be
neccessary to have serial support...

Or perhaps we want to decrease number of modules, and merge 8250 into
core-lite, having one less patch to care about?
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Tom Rini]

On Tue, Mar 02, 2004 at 11:23:07PM +0100, Pavel Machek wrote:

> Hi!
> 
> > > It also makes core.patch dependent on 8250.patch
> > > Any ideas on fixing that?
> > 
> > No, it's intentional.  eth.patch also depends on 8250.patch.
> 
> Perhaps that parts should be moved to core-lite? It should not be
> neccessary to have serial support...
> 
> Or perhaps we want to decrease number of modules, and merge 8250 into
> core-lite, having one less patch to care about?

If it's really important to not have patches depend on eachother until
we get to the non-lite core, i386 (and soon ppc, x86_64) patches, then
we should put all of the Kconfig bits in the main patch, so long as we
think all of the related drivers will be able to make it in as well (or
will move those bits out again when it's time).  Or we could just
document dependancies and move on.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Pavel Machek]

Hi!

> > > > It also makes core.patch dependent on 8250.patch
> > > > Any ideas on fixing that?
> > > 
> > > No, it's intentional.  eth.patch also depends on 8250.patch.
> > 
> > Perhaps that parts should be moved to core-lite? It should not be
> > neccessary to have serial support...
> > 
> > Or perhaps we want to decrease number of modules, and merge 8250 into
> > core-lite, having one less patch to care about?
> 
> If it's really important to not have patches depend on eachother until
> we get to the non-lite core, i386 (and soon ppc, x86_64) patches, then
> we should put all of the Kconfig bits in the main patch, so long as we
> think all of the related drivers will be able to make it in as well (or
> will move those bits out again when it's time).  Or we could just
> document dependancies and move on.

I believe "put kconfig into core" is the right thing to do.
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [KGDB PATCH][7/7] Move debugger_entry()

[George Anzinger]

Amit S. Kale wrote:
> OK to checkin.
> 
> -Amit
> 
> On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> 
>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is done.
>>So we have two options, not allow kgdboe to use the initial breakpoint
>>or move debugger_entry() to be past the point where kgdboe will be usable.
>>I've opted for the latter, as if an earlier breakpoint is needed you can
>>still use serial and throw kgdb_schedule_breakpoint/breakpoint where
>>desired.
>>
>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
>>@@ -581,6 +582,7 @@ static int init(void * unused)
>>
>> 	smp_init();
>> 	do_basic_setup();
>>+	debugger_entry();
>>

It would be nice to not need this.  Could it be a side effect of configuring the 
interface or some such so we don't have to patch init/main.c

-g
>> 	prepare_namespace();
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Amit S. Kale]

On Wednesday 03 Mar 2004 3:08 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > On Saturday 28 Feb 2004 6:38 am, George Anzinger wrote:
> >>Pavel Machek wrote:
> >>>Hi!
> >>>
> >>>>>+config KGDB_THREAD
> >>>>>+	bool "KGDB: Thread analysis"
> >>>>>+	depends on KGDB
> >>>>>+	help
> >>>>>+	  With thread analysis enabled, gdb can talk to kgdb stub to list
> >>>>>+	  threads and to get stack trace for a thread. This option also
> >>>>>enables
> >>>>>+	  some code which helps gdb get exact status of thread. Thread
> >>>>>analysis
> >>>>>+	  adds some overhead to schedule and down functions. You can disable
> >>>>>+	  this option if you do not want to compromise on speed.
> >>>>
> >>>>Lets remove the overhead and eliminate the need for this option in
> >>>> favor of always having threads.  Works in the mm kgdb...
> >>>
> >>>No. Thread analysis is unsuitable for the mainline (manipulates
> >>>sched.c in ugly way). It may be okay for -mm, but in such case it
> >>>should better be separated.
> >>
> >>Not in the -mm version.  I agree that sched.c should NEVER be treated
> >> this way and it is not in the -mm version.  I also think that, most of
> >> the time, it is useful to have the thread stuff, but that may be just my
> >> usage...
> >
> > If threads stuff didn't introduce any unclean code changes, I too would
> > prefer to have it on all the time. As things stands, threads stuff is
> > rather intrusive.
>
> Lets put the threads stuff in the stub.  The only stuff we need in the
> kernel is the flag that indicateds that the pid hash table has been
> initialized.
>
> Meanwhile, I would like to make a change to the gdb "info thread" command
> to do a better job of displaying the threads.  Here is what I am proposing:
>
> Gdb would work as it does now if the following set is not done.
>
> A new "set thread_level" command that would take the "bt" level to use on
> the thread display.
> A new "set thread_limits command that would take two expressions that would
> reduce to two memory addresses.
>
> Which ever of these is entered last will be active and used by "info
> thread" as follows:
>
> if thread_level is active gdb will do the indicated number of  "up"
> operations and display the result on the info thread line for that thread
> (note there is other info on this line that will not be changed).

You can already do a backtrace on all threads using gdb command
"thread apply all backtrace".

>
> if thread_limits is active gdb will do 0 or more "up" commands until the
> resultant PC is NOT between the given limits.

How does a user specify PC? There are umpteen number of kernel entry points 
(irqs, exceptions, system calls).

> The kernel, at this time, has defined symbols for the thread_limits command
> (it is used in the kernel for its internal display of threads).  I would
> expect that the thread_level version would be the answer for theaded
> application programs.
>
> Daniel, how does this sound?


The problem with kernel backtraces not stopping at kernel entry points is a 
tough one. gdbmod at kgdb.sourceforge.net attempts to do that. This gdb 
detects if we are debugging a kernel. If we are, a few things kick in like 
scanning of modules instead of .so libraries and stopping backtraces earlier.

GDB uses main as the function where backtraces stop unless overridden. This is 
broken by definition for multithreaded programs becauses non initial threads 
don't start from main. Kernel too doesn't have main and has several entry 
points.

If there is a way for gdb to know entry points from the kernel, it would be 
very easy to maintain. Say a .entrypoint section that lists pc ranges of 
entry points. GDB then stop a backtrace as soon as it enters one of these 
ranges.
-- 
Amit Kale
EmSysSoft (http://www.emsyssoft.com)
KGDB: Linux Kernel Source Level Debugger (http://kgdb.sourceforge.net)

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Amit S. Kale]

On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > OK to checkin.
> >
> > -Amit
> >
> > On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> >>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
> >> done. So we have two options, not allow kgdboe to use the initial
> >> breakpoint or move debugger_entry() to be past the point where kgdboe
> >> will be usable. I've opted for the latter, as if an earlier breakpoint
> >> is needed you can still use serial and throw
> >> kgdb_schedule_breakpoint/breakpoint where desired.
> >>
> >>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> >>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
> >>@@ -581,6 +582,7 @@ static int init(void * unused)
> >>
> >> 	smp_init();
> >> 	do_basic_setup();
> >>+	debugger_entry();
>
> It would be nice to not need this.  Could it be a side effect of
> configuring the interface or some such so we don't have to patch
> init/main.c

I attempted doing this when I was trying to code a netpoll independent 
ethernet interface. I couldn't do without it. I needed one hook to kgdb in 
init to mark completion of smp_init. If an interface was ready, that hook 
called breakpoint. A similar hook was placed in interface initialization 
code, it called breakpoint, if kgdb core was ready on account of smp_init 
completion.

-Amit



> -g
>
> >> 	prepare_namespace();
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

Re: [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[Amit S. Kale]

On Wednesday 03 Mar 2004 4:05 am, Pavel Machek wrote:
> Hi!
>
> > > > > It also makes core.patch dependent on 8250.patch
> > > > > Any ideas on fixing that?
> > > >
> > > > No, it's intentional.  eth.patch also depends on 8250.patch.
> > >
> > > Perhaps that parts should be moved to core-lite? It should not be
> > > neccessary to have serial support...
> > >
> > > Or perhaps we want to decrease number of modules, and merge 8250 into
> > > core-lite, having one less patch to care about?

Let's keep currents splits as they are. (core-lite, i386-lite, core, i386, 
x86_64, ppc, 8250, eth) It's very tempting to start merging, but that makes 
it difficult to modify only a few of them and eventually user won't have the 
choice of selecting only those that are needed.

> > If it's really important to not have patches depend on eachother until
> > we get to the non-lite core, i386 (and soon ppc, x86_64) patches, then
> > we should put all of the Kconfig bits in the main patch, so long as we
> > think all of the related drivers will be able to make it in as well (or
> > will move those bits out again when it's time).  Or we could just
> > document dependancies and move on.
>
> I believe "put kconfig into core" is the right thing to do.

I have a different opinion. Ideally these patches are best kept completely 
independent of each other. We may not be able to achieve that all the time. 
Right now we have following order of patches. [core-lite, i386-lite, 
8250][core, i386, x86_64, eth]. Second set depends on first. Patches in each 
set are independent of each other. We anyway have this dependency because of 
lite patches. So 8250 dependency should be ok.

-Amit

Re: [Kgdb-bugreport] [KGDB PATCH][1/7] Add / use kernel/Kconfig.kgdb

[George Anzinger]

[-- Attachment #1: Type: text/plain, Size: 4879 bytes --]

Amit S. Kale wrote:
> On Wednesday 03 Mar 2004 3:08 am, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>On Saturday 28 Feb 2004 6:38 am, George Anzinger wrote:
>>>
>>>>Pavel Machek wrote:
>>>>
>>>>>Hi!
>>>>>
>>>>>
>>>>>>>+config KGDB_THREAD
>>>>>>>+	bool "KGDB: Thread analysis"
>>>>>>>+	depends on KGDB
>>>>>>>+	help
>>>>>>>+	  With thread analysis enabled, gdb can talk to kgdb stub to list
>>>>>>>+	  threads and to get stack trace for a thread. This option also
>>>>>>>enables
>>>>>>>+	  some code which helps gdb get exact status of thread. Thread
>>>>>>>analysis
>>>>>>>+	  adds some overhead to schedule and down functions. You can disable
>>>>>>>+	  this option if you do not want to compromise on speed.
>>>>>>
>>>>>>Lets remove the overhead and eliminate the need for this option in
>>>>>>favor of always having threads.  Works in the mm kgdb...
>>>>>
>>>>>No. Thread analysis is unsuitable for the mainline (manipulates
>>>>>sched.c in ugly way). It may be okay for -mm, but in such case it
>>>>>should better be separated.
>>>>
>>>>Not in the -mm version.  I agree that sched.c should NEVER be treated
>>>>this way and it is not in the -mm version.  I also think that, most of
>>>>the time, it is useful to have the thread stuff, but that may be just my
>>>>usage...
>>>
>>>If threads stuff didn't introduce any unclean code changes, I too would
>>>prefer to have it on all the time. As things stands, threads stuff is
>>>rather intrusive.
>>
>>Lets put the threads stuff in the stub.  The only stuff we need in the
>>kernel is the flag that indicateds that the pid hash table has been
>>initialized.
>>
>>Meanwhile, I would like to make a change to the gdb "info thread" command
>>to do a better job of displaying the threads.  Here is what I am proposing:
>>
>>Gdb would work as it does now if the following set is not done.
>>
>>A new "set thread_level" command that would take the "bt" level to use on
>>the thread display.
>>A new "set thread_limits command that would take two expressions that would
>>reduce to two memory addresses.
>>
>>Which ever of these is entered last will be active and used by "info
>>thread" as follows:
>>
>>if thread_level is active gdb will do the indicated number of  "up"
>>operations and display the result on the info thread line for that thread
>>(note there is other info on this line that will not be changed).
> 
> 
> You can already do a backtrace on all threads using gdb command
> "thread apply all backtrace".
> 
> 
>>if thread_limits is active gdb will do 0 or more "up" commands until the
>>resultant PC is NOT between the given limits.
> 
> 
> How does a user specify PC? There are umpteen number of kernel entry points 
> (irqs, exceptions, system calls).

We are not interested in that.  There are two entry points in the kernel:

void scheduling_functions_start_here(void);
void scheduling_functions_end_here(void);

that bound the area we don't want to show the task in in the info thread 
command.  $PC is the same as $ESP in the x86, but it is defined in all archs 
regardless of the particular local name for the stack pointer.

> 
> 
>>The kernel, at this time, has defined symbols for the thread_limits command
>>(it is used in the kernel for its internal display of threads).  I would
>>expect that the thread_level version would be the answer for theaded
>>application programs.
>>
>>Daniel, how does this sound?
> 
> 
> 
> The problem with kernel backtraces not stopping at kernel entry points is a 
> tough one. gdbmod at kgdb.sourceforge.net attempts to do that. This gdb 
> detects if we are debugging a kernel. If we are, a few things kick in like 
> scanning of modules instead of .so libraries and stopping backtraces earlier.

Stopping "bt" is a different problem, one which should be addressed by fixing up 
the debug frame records to indicate where the bottom of the stack is.  I have 
code for this for the x86, but it uses CPP macros rather than the gas interface 
for such which is rather restrictive.
> 
> GDB uses main as the function where backtraces stop unless overridden. This is 
> broken by definition for multithreaded programs becauses non initial threads 
> don't start from main. Kernel too doesn't have main and has several entry 
> points.
> 
> If there is a way for gdb to know entry points from the kernel, it would be 
> very easy to maintain. Say a .entrypoint section that lists pc ranges of 
> entry points. GDB then stop a backtrace as soon as it enters one of these 
> ranges.

No, this is not enough.  It needs to also know to stop when the interrupt/ trap 
returns to user space.  You might try the attached patch.  Be warned, however, 
that it requires a CVS gdb (I found broken code).

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

[-- Attachment #2: kgdb-dwarf2-2.6.0-1.0.patch.gz --]
[-- Type: application/x-gzip, Size: 11623 bytes --]

Re: [KGDB PATCH][7/7] Move debugger_entry()

[George Anzinger]

Amit S. Kale wrote:
> On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>OK to checkin.
>>>
>>>-Amit
>>>
>>>On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
>>>
>>>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
>>>>done. So we have two options, not allow kgdboe to use the initial
>>>>breakpoint or move debugger_entry() to be past the point where kgdboe
>>>>will be usable. I've opted for the latter, as if an earlier breakpoint
>>>>is needed you can still use serial and throw
>>>>kgdb_schedule_breakpoint/breakpoint where desired.
>>>>
>>>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
>>>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988 -0700
>>>>@@ -581,6 +582,7 @@ static int init(void * unused)
>>>>
>>>>	smp_init();
>>>>	do_basic_setup();
>>>>+	debugger_entry();
>>
>>It would be nice to not need this.  Could it be a side effect of
>>configuring the interface or some such so we don't have to patch
>>init/main.c
> 
> 
> I attempted doing this when I was trying to code a netpoll independent 
> ethernet interface. I couldn't do without it. I needed one hook to kgdb in 
> init to mark completion of smp_init. If an interface was ready, that hook 
> called breakpoint. A similar hook was placed in interface initialization 
> code, it called breakpoint, if kgdb core was ready on account of smp_init 
> completion.
> 
I guess the real question is why do you need to wait so long?  What is it that 
needs to be done prior to this call?

On the other hand, I have a late call (I use module init) to set up the 
interrupt handler for the UART, which needs to happen after malloc is working. 
This, however, does not cause, of it self, a break.

> -Amit
> 
> 
> 
> 
>>-g
>>
>>
>>>>	prepare_namespace();
>>>
>>>-
>>>To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>>>in the body of a message to majordomo@vger.kernel.org
>>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Tom Rini]

On Thu, Mar 11, 2004 at 01:24:59PM -0800, George Anzinger wrote:
> Amit S. Kale wrote:
> >On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
> >
> >>Amit S. Kale wrote:
> >>
> >>>OK to checkin.
> >>>
> >>>-Amit
> >>>
> >>>On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> >>>
> >>>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
> >>>>done. So we have two options, not allow kgdboe to use the initial
> >>>>breakpoint or move debugger_entry() to be past the point where kgdboe
> >>>>will be usable. I've opted for the latter, as if an earlier breakpoint
> >>>>is needed you can still use serial and throw
> >>>>kgdb_schedule_breakpoint/breakpoint where desired.
> >>>>
> >>>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> >>>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 
> >>>>11:33:51.854388988 -0700
> >>>>@@ -581,6 +582,7 @@ static int init(void * unused)
> >>>>
> >>>>	smp_init();
> >>>>	do_basic_setup();
> >>>>+	debugger_entry();
> >>
> >>It would be nice to not need this.  Could it be a side effect of
> >>configuring the interface or some such so we don't have to patch
> >>init/main.c
> >
> >
> >I attempted doing this when I was trying to code a netpoll independent 
> >ethernet interface. I couldn't do without it. I needed one hook to kgdb in 
> >init to mark completion of smp_init. If an interface was ready, that hook 
> >called breakpoint. A similar hook was placed in interface initialization 
> >code, it called breakpoint, if kgdb core was ready on account of smp_init 
> >completion.
> >
> I guess the real question is why do you need to wait so long?  What is it 
> that needs to be done prior to this call?

For kgdboe, you need to wait for the ethernet driver, networking, etc.
Without going and trying to do some big special case, that's when it has
to be, for kgdboe.

For serial, esp if you make kgdb_arch_init() handle kgdb_serial pointer,
it can happen much earlier.  In fact, in that case you could probably
throw breakpoint() in as first line of C (PPC32) or close to it (some
mappings needed i386, sh, others, generally speaking).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [KGDB PATCH][7/7] Move debugger_entry()

[George Anzinger]

Tom Rini wrote:
> On Thu, Mar 11, 2004 at 01:24:59PM -0800, George Anzinger wrote:
> 
>>Amit S. Kale wrote:
>>
>>>On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
>>>
>>>
>>>>Amit S. Kale wrote:
>>>>
>>>>
>>>>>OK to checkin.
>>>>>
>>>>>-Amit
>>>>>
>>>>>On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
>>>>>
>>>>>
>>>>>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
>>>>>>done. So we have two options, not allow kgdboe to use the initial
>>>>>>breakpoint or move debugger_entry() to be past the point where kgdboe
>>>>>>will be usable. I've opted for the latter, as if an earlier breakpoint
>>>>>>is needed you can still use serial and throw
>>>>>>kgdb_schedule_breakpoint/breakpoint where desired.
>>>>>>
>>>>>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
>>>>>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 
>>>>>>11:33:51.854388988 -0700
>>>>>>@@ -581,6 +582,7 @@ static int init(void * unused)
>>>>>>
>>>>>>	smp_init();
>>>>>>	do_basic_setup();
>>>>>>+	debugger_entry();
>>>>
>>>>It would be nice to not need this.  Could it be a side effect of
>>>>configuring the interface or some such so we don't have to patch
>>>>init/main.c
>>>
>>>
>>>I attempted doing this when I was trying to code a netpoll independent 
>>>ethernet interface. I couldn't do without it. I needed one hook to kgdb in 
>>>init to mark completion of smp_init. If an interface was ready, that hook 
>>>called breakpoint. A similar hook was placed in interface initialization 
>>>code, it called breakpoint, if kgdb core was ready on account of smp_init 
>>>completion.
>>>
>>
>>I guess the real question is why do you need to wait so long?  What is it 
>>that needs to be done prior to this call?
> 
> 
> For kgdboe, you need to wait for the ethernet driver, networking, etc.
> Without going and trying to do some big special case, that's when it has
> to be, for kgdboe.
> 
> For serial, esp if you make kgdb_arch_init() handle kgdb_serial pointer,
> it can happen much earlier.  In fact, in that case you could probably
> throw breakpoint() in as first line of C (PPC32) or close to it (some
> mappings needed i386, sh, others, generally speaking).

So the question is do we cater to the worst or the best?  If I put kgdb or what 
ever on the command line, I would like to see the connection ASAP what ever the 
connection is.  So possibly, the command line parse part of kgdb sets a flag it 
it can not do the break NOW.  The late bloomer code finds this flag when doing 
the kgdboe or what ever set up and enters at that time.  Puts a little more 
smarts in the command line part, but gets things rolling ASAP regardless of the 
interface.  This, of course, assumes that the kgdboe or what ever has an 
independent way of getting entered as soon as it can be set up, i.e. its 
prerequisites are satisfied.
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Tom Rini]

On Thu, Mar 11, 2004 at 02:49:16PM -0800, George Anzinger wrote:

> Tom Rini wrote:
[snip]
> >For kgdboe, you need to wait for the ethernet driver, networking, etc.
> >Without going and trying to do some big special case, that's when it has
> >to be, for kgdboe.
> >
> >For serial, esp if you make kgdb_arch_init() handle kgdb_serial pointer,
> >it can happen much earlier.  In fact, in that case you could probably
> >throw breakpoint() in as first line of C (PPC32) or close to it (some
> >mappings needed i386, sh, others, generally speaking).
> 
> So the question is do we cater to the worst or the best?  If I put kgdb or 
> what ever on the command line, I would like to see the connection ASAP what 
> ever the connection is.  So possibly, the command line parse part of kgdb 
> sets a flag it it can not do the break NOW.  The late bloomer code finds 
> this flag when doing the kgdboe or what ever set up and enters at that 
> time.  Puts a little more smarts in the command line part, but gets things 
> rolling ASAP regardless of the interface.  This, of course, assumes that 
> the kgdboe or what ever has an independent way of getting entered as soon 
> as it can be set up, i.e. its prerequisites are satisfied.

This sound like what I passed along from dwmw2 a week ago I think. :)

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Amit S. Kale]

On Friday 12 Mar 2004 2:54 am, George Anzinger wrote:
> Amit S. Kale wrote:
> > On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
> >>Amit S. Kale wrote:
> >>>OK to checkin.
> >>>
> >>>-Amit
> >>>
> >>>On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> >>>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
> >>>>done. So we have two options, not allow kgdboe to use the initial
> >>>>breakpoint or move debugger_entry() to be past the point where kgdboe
> >>>>will be usable. I've opted for the latter, as if an earlier breakpoint
> >>>>is needed you can still use serial and throw
> >>>>kgdb_schedule_breakpoint/breakpoint where desired.
> >>>>
> >>>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> >>>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988
> >>>> -0700 @@ -581,6 +582,7 @@ static int init(void * unused)
> >>>>
> >>>>	smp_init();
> >>>>	do_basic_setup();
> >>>>+	debugger_entry();
> >>
> >>It would be nice to not need this.  Could it be a side effect of
> >>configuring the interface or some such so we don't have to patch
> >>init/main.c
> >
> > I attempted doing this when I was trying to code a netpoll independent
> > ethernet interface. I couldn't do without it. I needed one hook to kgdb
> > in init to mark completion of smp_init. If an interface was ready, that
> > hook called breakpoint. A similar hook was placed in interface
> > initialization code, it called breakpoint, if kgdb core was ready on
> > account of smp_init completion.
>
> I guess the real question is why do you need to wait so long?  What is it
> that needs to be done prior to this call?

2.4.x kgdb legacy code! That was needed in some early 2.4 kgdbs. Perhaps we 
can move it earlier now. If someone tests the patch on an smp machine and 
confirms that it works, I'll be happy change it.

The earlier kgdb is initialized, the better it is.

-Amit

>
> On the other hand, I have a late call (I use module init) to set up the
> interrupt handler for the UART, which needs to happen after malloc is
> working. This, however, does not cause, of it self, a break.
>
> > -Amit
> >
> >>-g
> >>
> >>>>	prepare_namespace();
> >>>
> >>>-
> >>>To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> >>>in the body of a message to majordomo@vger.kernel.org
> >>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>Please read the FAQ at  http://www.tux.org/lkml/
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

Re: [KGDB PATCH][7/7] Move debugger_entry()

[Tom Rini]

On Fri, Mar 12, 2004 at 10:12:34AM +0530, Amit S. Kale wrote:
> On Friday 12 Mar 2004 2:54 am, George Anzinger wrote:
> > Amit S. Kale wrote:
> > > On Wednesday 03 Mar 2004 6:38 am, George Anzinger wrote:
> > >>Amit S. Kale wrote:
> > >>>OK to checkin.
> > >>>
> > >>>-Amit
> > >>>
> > >>>On Saturday 28 Feb 2004 3:24 am, Tom Rini wrote:
> > >>>>Hello.  When we use kgdboe, we can't use it until do_basic_setup() is
> > >>>>done. So we have two options, not allow kgdboe to use the initial
> > >>>>breakpoint or move debugger_entry() to be past the point where kgdboe
> > >>>>will be usable. I've opted for the latter, as if an earlier breakpoint
> > >>>>is needed you can still use serial and throw
> > >>>>kgdb_schedule_breakpoint/breakpoint where desired.
> > >>>>
> > >>>>--- linux-2.6.3-rc4/init/main.c	2004-02-17 09:51:19.000000000 -0700
> > >>>>+++ linux-2.6.3-rc4-kgdb/init/main.c	2004-02-17 11:33:51.854388988
> > >>>> -0700 @@ -581,6 +582,7 @@ static int init(void * unused)
> > >>>>
> > >>>>	smp_init();
> > >>>>	do_basic_setup();
> > >>>>+	debugger_entry();
> > >>
> > >>It would be nice to not need this.  Could it be a side effect of
> > >>configuring the interface or some such so we don't have to patch
> > >>init/main.c
> > >
> > > I attempted doing this when I was trying to code a netpoll independent
> > > ethernet interface. I couldn't do without it. I needed one hook to kgdb
> > > in init to mark completion of smp_init. If an interface was ready, that
> > > hook called breakpoint. A similar hook was placed in interface
> > > initialization code, it called breakpoint, if kgdb core was ready on
> > > account of smp_init completion.
> >
> > I guess the real question is why do you need to wait so long?  What is it
> > that needs to be done prior to this call?
> 
> 2.4.x kgdb legacy code! That was needed in some early 2.4 kgdbs. Perhaps we 
> can move it earlier now. If someone tests the patch on an smp machine and 
> confirms that it works, I'll be happy change it.

It's only legacy code under the following conditions (with current code):
- You don't care about getting an initial break from kgdboe (fixable...)
- kgdb_arch_init sets up kgdb_serial to an I/O that is suitably used
  early on.

-- 
Tom Rini
http://gate.crashing.org/~trini/