Make 'machine ddbcpu' work, and use CPU device number instead of APIC id

KNF lesson and ok from niklas@

4 files changed, 161 insertions, 188 deletions

diff --git a/sys/arch/i386/i386/db_interface.c b/sys/arch/i386/i386/db_interface.c
index 44e09f1c914..5ee659efcbb 100644
--- a/sys/arch/i386/i386/db_interface.c
+++ b/sys/arch/i386/i386/db_interface.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_interface.c,v 1.13 2004/06/13 21:49:15 niklas Exp $	*/
+/*	$OpenBSD: db_interface.c,v 1.14 2004/06/21 22:41:11 andreas Exp $	*/
 /*	$NetBSD: db_interface.c,v 1.22 1996/05/03 19:42:00 christos Exp $	*/
 
 /* 
@@ -56,9 +56,9 @@ extern int trap_types;
 extern boolean_t db_cmd_loop_done;
 
 #ifdef MULTIPROCESSOR
-extern boolean_t	 db_switch_cpu;
-extern long		 db_switch_to_cpu;
-
+extern volatile int ddb_state;
+boolean_t	 db_switch_cpu;
+long		 db_switch_to_cpu;
 #endif
 
 int	db_active = 0;
@@ -70,6 +70,7 @@ void db_cpuinfo_cmd(db_expr_t, int, db_expr_t, char *);
 void db_startproc_cmd(db_expr_t, int, db_expr_t, char *);
 void db_stopproc_cmd(db_expr_t, int, db_expr_t, char *);
 void db_ddbproc_cmd(db_expr_t, int, db_expr_t, char *);
+int db_cpuid2apic(int);
 #endif /* MULTIPROCESSOR */
 
 /*
@@ -114,6 +115,16 @@ kdb_trap(type, code, regs)
 		}
 	}
 
+#ifdef MULTIPROCESSOR
+	s = splhigh();
+	SIMPLE_LOCK(&ddb_mp_slock);
+	if (ddb_state == DDB_STATE_EXITING)
+		ddb_state = DDB_STATE_NOT_RUNNING;
+	SIMPLE_UNLOCK(&ddb_mp_slock);
+	splx(s);
+	while (db_enter_ddb()) {
+#endif /* MULTIPROCESSOR */
+
 	/* XXX Should switch to kdb`s own stack here. */
 
 	ddb_regs = *regs;
@@ -151,6 +162,12 @@ kdb_trap(type, code, regs)
 		regs->tf_ss     = ddb_regs.tf_ss & 0xffff;
 	}
 
+
+#ifdef MULTIPROCESSOR
+		if (!db_switch_cpu)
+			ddb_state = DDB_STATE_EXITING;
+	}
+#endif /* MULTIPROCESSOR */
 	return (1);
 }
 
@@ -193,18 +210,28 @@ db_sysregs_cmd(addr, have_addr, count, modif)
 }
 
 #ifdef MULTIPROCESSOR
-void db_cpuinfo_cmd(addr, have_addr, count, modif)
-	db_expr_t addr;
-	int have_addr;
-	db_expr_t count;
-	char *modif;
+int
+db_cpuid2apic(int id)
+{
+	int apic;
+
+	for (apic = 0; apic < I386_MAXPROCS; apic++) {
+		if (cpu_info[apic] != NULL &&
+		    cpu_info[apic]->ci_dev.dv_unit == id)
+			return (apic);
+	}
+	return (-1);
+}
+
+void
+db_cpuinfo_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
 	int i;
 
 	for (i = 0; i < I386_MAXPROCS; i++) {
 		if (cpu_info[i] != NULL) {
 			db_printf("%c%4d: ", (i == cpu_number()) ? '*' : ' ',
-			    i);
+			    cpu_info[i]->ci_dev.dv_unit);
 			switch(cpu_info[i]->ci_ddb_paused) {
 			case CI_DDB_RUNNING:
 				db_printf("running\n");
@@ -230,62 +257,59 @@ void db_cpuinfo_cmd(addr, have_addr, count, modif)
 	}
 }
 
-void db_startproc_cmd(addr, have_addr, count, modif)
-	db_expr_t addr;
-	int have_addr;
-	db_expr_t count;
-	char *modif;
+void
+db_startproc_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
-	int i;
+	int apic;
 
 	if (have_addr) {
-		if (addr >= 0 && addr < I386_MAXPROCS
-		    && cpu_info[addr] != NULL && addr != cpu_number())
-			db_startcpu(addr);
+		apic = db_cpuid2apic(addr);
+		if (apic >= 0 && apic < I386_MAXPROCS &&
+		    cpu_info[apic] != NULL && apic != cpu_number())
+			db_startcpu(apic);
 		else
 			db_printf("Invalid cpu %d\n", (int)addr);
 	} else {
-		for (i = 0; i < I386_MAXPROCS; i++) {
-			if (cpu_info[i] != NULL && i != cpu_number()) {
-				db_startcpu(i);
+		for (apic = 0; apic < I386_MAXPROCS; apic++) {
+			if (cpu_info[apic] != NULL && apic != cpu_number()) {
+				db_startcpu(apic);
 			}
 		}
 	}
 }
 
-void db_stopproc_cmd(addr, have_addr, count, modif)
-	db_expr_t addr;
-	int have_addr;
-	db_expr_t count;
-	char *modif;
+void
+db_stopproc_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
-	int i;
+	int apic;
 
 	if (have_addr) {
-		if (addr >= 0 && addr < I386_MAXPROCS
-		    && cpu_info[addr] != NULL && addr != cpu_number())
-			db_stopcpu(addr);
+		apic = db_cpuid2apic(addr);
+		if (apic >= 0 && apic < I386_MAXPROCS &&
+		    cpu_info[apic] != NULL && apic != cpu_number())
+			db_stopcpu(apic);
 		else
 			db_printf("Invalid cpu %d\n", (int)addr);
 	} else {
-		for (i = 0; i < I386_MAXPROCS; i++) {
-			if (cpu_info[i] != NULL && i != cpu_number()) {
-				db_stopcpu(i);
+		for (apic = 0; apic < I386_MAXPROCS; apic++) {
+			if (cpu_info[apic] != NULL && apic != cpu_number()) {
+				db_stopcpu(apic);
 			}
 		}
 	}
 }
 
-void db_ddbproc_cmd(addr, have_addr, count, modif)
-	db_expr_t addr;
-	int have_addr;
-	db_expr_t count;
-	char *modif;
+void
+db_ddbproc_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
+	int apic;
+
 	if (have_addr) {
-		if (addr >= 0 && addr < I386_MAXPROCS
-		    && cpu_info[addr] != NULL && addr != cpu_number()) {
-			db_switch_to_cpu = addr;
+		apic = db_cpuid2apic(addr);
+		if (apic >= 0 && apic < I386_MAXPROCS &&
+		    cpu_info[apic] != NULL && apic != cpu_number()) {
+			db_stopcpu(apic);
+			db_switch_to_cpu = apic;
 			db_switch_cpu = 1;
 			db_cmd_loop_done = 1;
 		} else {
diff --git a/sys/arch/i386/i386/db_mp.c b/sys/arch/i386/i386/db_mp.c
index 0a859f08360..3c1b184a927 100644
--- a/sys/arch/i386/i386/db_mp.c
+++ b/sys/arch/i386/i386/db_mp.c
@@ -1,32 +1,19 @@
-/*	$OpenBSD: db_mp.c,v 1.2 2004/06/13 21:49:15 niklas Exp $	*/
+/*	$OpenBSD: db_mp.c,v 1.3 2004/06/21 22:41:11 andreas Exp $	*/
 
 /*
- * Copyright (c) 2003 Andreas Gunnarsson
- * All rights reserved.
+ * Copyright (c) 2003, 2004 Andreas Gunnarsson <andreas@openbsd.org>
  *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
  *
- *    - Redistributions of source code must retain the above copyright
- *      notice, this list of conditions and the following disclaimer.
- *    - Redistributions in binary form must reproduce the above
- *      copyright notice, this list of conditions and the following
- *      disclaimer in the documentation and/or other materials provided
- *      with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
- * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
 #include <sys/types.h>
@@ -36,19 +23,24 @@
 
 #include <ddb/db_output.h>
 
-#define DDB_STATE_NOT_RUNNING	0
-#define DDB_STATE_RUNNING	1
-
 struct SIMPLELOCK ddb_mp_slock;
 
 volatile int ddb_state = DDB_STATE_NOT_RUNNING;	/* protected by ddb_mp_slock */
 volatile cpuid_t ddb_active_cpu;		/* protected by ddb_mp_slock */
 
+extern volatile boolean_t	db_switch_cpu;
+extern volatile long		db_switch_to_cpu;
+
 /*
- * ddb_enter_ddb() is called when ddb is entered to stop the other
- * CPUs. If another cpu is already in ddb we'll wait until it's finished.
+ * All processors wait in db_enter_ddb() (unless explicitly started from
+ * ddb) but only one owns ddb.  If the current processor should own ddb,
+ * db_enter_ddb() returns 1.  If the current processor should keep
+ * executing as usual (if ddb is exited or the processor is explicitly
+ * started), db_enter_ddb returns 0.
+ * If this is the first CPU entering ddb, db_enter_ddb() will stop all
+ * other CPUs by sending IPIs.
  */
-void
+int
 db_enter_ddb()
 {
 	int s, i;
@@ -56,57 +48,72 @@ db_enter_ddb()
 	s = splhigh();
 	SIMPLE_LOCK(&ddb_mp_slock);
 
-	while (ddb_state == DDB_STATE_RUNNING
-	    && ddb_active_cpu != cpu_number()) {
-		db_printf("CPU %d waiting to enter ddb\n", cpu_number());
+	/* If we are first in, grab ddb and stop all other CPUs */
+	if (ddb_state == DDB_STATE_NOT_RUNNING) {
+		ddb_active_cpu = cpu_number();
+		ddb_state = DDB_STATE_RUNNING;
+		curcpu()->ci_ddb_paused = CI_DDB_INDDB;
+		SIMPLE_UNLOCK(&ddb_mp_slock);
+		for (i = 0; i < I386_MAXPROCS; i++) {
+			if (cpu_info[i] != NULL && i != cpu_number() &&
+			    cpu_info[i]->ci_ddb_paused != CI_DDB_STOPPED) {
+				cpu_info[i]->ci_ddb_paused = CI_DDB_SHOULDSTOP;
+				i386_send_ipi(cpu_info[i], I386_IPI_DDB);
+			}
+		}
+		splx(s);
+		return (1);
+	}
+
+	/* Leaving ddb completely.  Start all other CPUs and return 0 */
+	if (ddb_active_cpu == cpu_number() && ddb_state == DDB_STATE_EXITING) {
+		for (i = 0; i < I386_MAXPROCS; i++) {
+			if (cpu_info[i] != NULL) {
+				cpu_info[i]->ci_ddb_paused = CI_DDB_RUNNING;
+			}
+		}
+		SIMPLE_UNLOCK(&ddb_mp_slock);
+		splx(s);
+		return (0);
+	}
+
+	/* We're switching to another CPU.  db_ddbproc_cmd() has made sure
+	 * it is waiting for ddb, we just have to set ddb_active_cpu. */
+	if (ddb_active_cpu == cpu_number() && db_switch_cpu) {
+		curcpu()->ci_ddb_paused = CI_DDB_SHOULDSTOP;
+		db_switch_cpu = 0;
+		ddb_active_cpu = db_switch_to_cpu;
+		cpu_info[db_switch_to_cpu]->ci_ddb_paused = CI_DDB_ENTERDDB;
+	}
+
+	/* Wait until we should enter ddb or resume */
+	while (ddb_active_cpu != cpu_number() &&
+	    curcpu()->ci_ddb_paused != CI_DDB_RUNNING) {
+		if (curcpu()->ci_ddb_paused == CI_DDB_SHOULDSTOP)
+			curcpu()->ci_ddb_paused = CI_DDB_STOPPED;
 		SIMPLE_UNLOCK(&ddb_mp_slock);
 		splx(s);
 
 		/* Busy wait without locking, we'll confirm with lock later */
-		while (ddb_state == DDB_STATE_RUNNING
-		   && ddb_active_cpu != cpu_number())
+		while (ddb_active_cpu != cpu_number() &&
+		    curcpu()->ci_ddb_paused != CI_DDB_RUNNING)
 			;	/* Do nothing */
 
 		s = splhigh();
 		SIMPLE_LOCK(&ddb_mp_slock);
 	}
 
-	ddb_state = DDB_STATE_RUNNING;
-	ddb_active_cpu = cpu_number();
-
-	for (i = 0; i < I386_MAXPROCS; i++) {
-		if (cpu_info[i] != NULL) {
-			if (i == cpu_number())
-				cpu_info[i]->ci_ddb_paused = CI_DDB_INDDB;
-			else if (cpu_info[i]->ci_ddb_paused
-			    != CI_DDB_STOPPED) {
-				cpu_info[i]->ci_ddb_paused = CI_DDB_SHOULDSTOP;
-				db_printf("Sending IPI to cpu %d\n", i);
-				i386_send_ipi(cpu_info[i], I386_IPI_DDB);
-			}
-		}
-	}
-	db_printf("CPU %d entering ddb\n", cpu_number());
-	SIMPLE_UNLOCK(&ddb_mp_slock);
-	splx(s);
-}
-
-void
-db_leave_ddb()
-{
-	int s, i;
-
-	s = splhigh();
-	SIMPLE_LOCK(&ddb_mp_slock);
-	db_printf("CPU %d leaving ddb\n", cpu_number());
-	for (i = 0; i < I386_MAXPROCS; i++) {
-		if (cpu_info[i] != NULL) {
-			cpu_info[i]->ci_ddb_paused = CI_DDB_RUNNING;
-		}
+	/* Either enter ddb or exit */
+	if (ddb_active_cpu == cpu_number() && ddb_state == DDB_STATE_RUNNING) {
+		curcpu()->ci_ddb_paused = CI_DDB_INDDB;
+		SIMPLE_UNLOCK(&ddb_mp_slock);
+		splx(s);
+		return (1);
+	} else {
+		SIMPLE_UNLOCK(&ddb_mp_slock);
+		splx(s);
+		return (0);
 	}
-	ddb_state = DDB_STATE_NOT_RUNNING;
-	SIMPLE_UNLOCK(&ddb_mp_slock);
-	splx(s);
 }
 
 void
@@ -128,60 +135,22 @@ db_stopcpu(int cpu)
 {
 	int s;
 
-	if (cpu != cpu_number() && cpu_info[cpu] != NULL) {
-		s = splhigh();
-		SIMPLE_LOCK(&ddb_mp_slock);
+	s = splhigh();
+	SIMPLE_LOCK(&ddb_mp_slock);
+	if (cpu != cpu_number() && cpu_info[cpu] != NULL &&
+	    cpu_info[cpu]->ci_ddb_paused != CI_DDB_STOPPED) {
 		cpu_info[cpu]->ci_ddb_paused = CI_DDB_SHOULDSTOP;
-		db_printf("Sending IPI to cpu %d\n", cpu);
 		SIMPLE_UNLOCK(&ddb_mp_slock);
 		splx(s);
 		i386_send_ipi(cpu_info[cpu], I386_IPI_DDB);
+	} else {
+		SIMPLE_UNLOCK(&ddb_mp_slock);
+		splx(s);
 	}
 }
 
 void
-db_movetocpu(int cpu)
-{
-	int s;
-
-	s = splhigh();
-	SIMPLE_LOCK(&ddb_mp_slock);
-	cpu_info[cpu]->ci_ddb_paused = CI_DDB_ENTERDDB;
-	db_printf("Sending IPI to cpu %d\n", cpu);
-	SIMPLE_UNLOCK(&ddb_mp_slock);
-	splx(s);
-	/* XXX If other CPU was running and IPI is lost, we lose. */
-	i386_send_ipi(cpu_info[cpu], I386_IPI_DDB);
-}
-
-void
 i386_ipi_db(struct cpu_info *ci)
 {
-	int s;
-
-	s = splhigh();
-	SIMPLE_LOCK(&ddb_mp_slock);
-	db_printf("CPU %d received ddb IPI\n", cpu_number());
-	while (ci->ci_ddb_paused == CI_DDB_SHOULDSTOP
-	    || ci->ci_ddb_paused == CI_DDB_STOPPED) {
-		if (ci->ci_ddb_paused == CI_DDB_SHOULDSTOP)
-			ci->ci_ddb_paused = CI_DDB_STOPPED;
-		SIMPLE_UNLOCK(&ddb_mp_slock);
-		while (ci->ci_ddb_paused == CI_DDB_STOPPED)
-			;	/* Do nothing */
-		SIMPLE_LOCK(&ddb_mp_slock);
-	}
-	if (ci->ci_ddb_paused == CI_DDB_ENTERDDB) {
-		ddb_state = DDB_STATE_RUNNING;
-		ddb_active_cpu = cpu_number();
-		ci->ci_ddb_paused = CI_DDB_INDDB;
-		db_printf("CPU %d grabbing ddb\n", cpu_number());
-		SIMPLE_UNLOCK(&ddb_mp_slock);
-		Debugger();
-		SIMPLE_LOCK(&ddb_mp_slock);
-		ci->ci_ddb_paused = CI_DDB_RUNNING;
-	}
-	db_printf("CPU %d leaving ddb IPI handler\n", cpu_number());
-	SIMPLE_UNLOCK(&ddb_mp_slock);
-	splx(s);
+	Debugger();
 }
diff --git a/sys/arch/i386/include/db_machdep.h b/sys/arch/i386/include/db_machdep.h
index c1f5b9b6dfe..4d4be44f9a7 100644
--- a/sys/arch/i386/include/db_machdep.h
+++ b/sys/arch/i386/include/db_machdep.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_machdep.h,v 1.10 2004/06/13 21:49:16 niklas Exp $	*/
+/*	$OpenBSD: db_machdep.h,v 1.11 2004/06/21 22:41:11 andreas Exp $	*/
 /*	$NetBSD: db_machdep.h,v 1.9 1996/05/03 19:23:59 christos Exp $	*/
 
 /* 
@@ -120,13 +120,16 @@ void		db_task_name(/* task_t */);
 int kdb_trap(int, int, db_regs_t *);
 
 void db_machine_init(void);
-void db_enter_ddb(void);
-void db_leave_ddb(void);
+int db_enter_ddb(void);
 void db_startcpu(int cpu);
 void db_stopcpu(int cpu);
-void db_movetocpu(int cpu);
 void i386_ipi_db(struct cpu_info *);
 
 extern struct SIMPLELOCK ddb_mp_slock;
 
+/* For ddb_state */
+#define DDB_STATE_NOT_RUNNING	0
+#define DDB_STATE_RUNNING	1
+#define DDB_STATE_EXITING	2
+
 #endif	/* _I386_DB_MACHDEP_H_ */
diff --git a/sys/ddb/db_command.c b/sys/ddb/db_command.c
index 12477e0b084..b47ae3b9ddf 100644
--- a/sys/ddb/db_command.c
+++ b/sys/ddb/db_command.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_command.c,v 1.36 2004/06/13 21:49:23 niklas Exp $	*/
+/*	$OpenBSD: db_command.c,v 1.37 2004/06/21 22:41:12 andreas Exp $	*/
 /*	$NetBSD: db_command.c,v 1.20 1996/03/30 22:30:05 christos Exp $	*/
 
 /* 
@@ -60,11 +60,6 @@
 boolean_t	db_cmd_loop_done;
 label_t		*db_recover;
 
-#ifdef MULTIPROCESSOR
-boolean_t	db_switch_cpu;
-long		db_switch_to_cpu;
-#endif
-
 /*
  * if 'ed' style: 'dot' is set at start of last item printed,
  * and '+' points to next line.
@@ -509,11 +504,6 @@ db_command_loop()
 	label_t		*savejmp;
 	extern int	db_output_line;
 
-#ifdef MULTIPROCESSOR
-	db_switch_cpu = 0;
-	db_enter_ddb();
-#endif /* MULTIPROCESSOR */
-
 	/*
 	 * Initialize 'prev' and 'next' to dot.
 	 */
@@ -533,7 +523,7 @@ db_command_loop()
 		db_output_line = 0;
 
 #ifdef MULTIPROCESSOR
-		db_printf("ddb{%ld}> ", (long)cpu_number());
+		db_printf("ddb{%d}> ", curcpu()->ci_dev.dv_unit);
 #else
 		db_printf("ddb> ");
 #endif
@@ -543,19 +533,6 @@ db_command_loop()
 	}
 
 	db_recover = savejmp;
-
-#ifdef MULTIPROCESSOR
-	if (db_switch_cpu) {
-		db_printf("Moving ddb to cpu %d\n", db_switch_to_cpu);
-		curcpu()->ci_ddb_paused = CI_DDB_STOPPED;
-		db_movetocpu(db_switch_to_cpu);
-		while (curcpu()->ci_ddb_paused == CI_DDB_SHOULDSTOP
-		    || curcpu()->ci_ddb_paused == CI_DDB_STOPPED)
-			;	/* Do nothing */
-	} else {
-		db_leave_ddb();
-	}
-#endif /* MULTIPROCESSOR */
 }
 
 void