From 4df052062401704398629b4a2cb8203b3d6c1ef7 Mon Sep 17 00:00:00 2001
From: Bruce Evans <bde@FreeBSD.org>
Date: Sun, 7 Sep 2003 14:23:08 +0000
Subject: [PATCH] clock.c: Quick fix for calling DELAY() for ddb input in some
 (atkbd-based) console drivers.  ddb must not use any normal locks, but
 DELAY() normally calls getit() which needs clock_lock.  One problem with
 using normal locks in ddb is that deadlock is possible, but deadlock on
 clock_lock is unlikely becaluse clock_lock is bogusly recursive, apparently
 just to hide the problem of ddb using it.  The i8254 clock hardware has
 mostly write-only registers so it is important for it to use a lock that
 gives exclusive access.  (atkbd hardware is also unfriendly to reentrant
 software but that problem is more local and already solved.)  I mostly saw
 the symptoms of the bug caused by unlocking in getit() running cpu_unpend(). 
 cpu_unpend() should not be called while in ddb and Debugger() calls for
 failing assertions about this caused a breakpoint within ddb.

ddb must also not call getit() because ddb may be being used to step
through clock initialization code that has stopped or otherwise mangled
the clock.  If the clock is stopped, then getit() always returns the
same value and DELAY() takes forever if it trusts getit().

The quick fix is implement DELAY(n) as (n * timer_freq / 1000000)
inb(0x84)'s if ddb is active.

machdep.c:
Don't permit recursion on clock_lock.
---
 sys/i386/i386/machdep.c |  2 +-
 sys/i386/isa/clock.c    | 20 ++++++++++++++++++--
 sys/isa/atrtc.c         | 20 ++++++++++++++++++--
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/sys/i386/i386/machdep.c b/sys/i386/i386/machdep.c
index b80ec1067e65..675eb266e31d 100644
--- a/sys/i386/i386/machdep.c
+++ b/sys/i386/i386/machdep.c
@@ -1982,7 +1982,7 @@ init386(first)
 	 *	     under witness.
 	 */
 	mutex_init();
-	mtx_init(&clock_lock, "clk", NULL, MTX_SPIN | MTX_RECURSE);
+	mtx_init(&clock_lock, "clk", NULL, MTX_SPIN);
 	mtx_init(&icu_lock, "icu", NULL, MTX_SPIN | MTX_NOWITNESS);
 
 	/* make ldt memory segments */
diff --git a/sys/i386/isa/clock.c b/sys/i386/isa/clock.c
index 3d7d8c11ae1f..29cfefca4aaf 100644
--- a/sys/i386/isa/clock.c
+++ b/sys/i386/isa/clock.c
@@ -322,8 +322,18 @@ DELAY(int n)
 	 * takes about 1.5 usec for each of the i/o's in getit().  The loop
 	 * takes about 6 usec on a 486/33 and 13 usec on a 386/20.  The
 	 * multiplications and divisions to scale the count take a while).
+	 *
+	 * However, if ddb is active then use a fake counter since reading
+	 * the i8254 counter involves acquiring a lock.  ddb must not go
+	 * locking for many reasons, but it calls here for at least atkbd
+	 * input.
 	 */
-	prev_tick = getit();
+#ifdef DDB
+	if (db_active)
+		prev_tick = 0;
+	else
+#endif
+		prev_tick = getit();
 	n -= 0;			/* XXX actually guess no initial overhead */
 	/*
 	 * Calculate (n * (timer_freq / 1e6)) without using floating point
@@ -350,7 +360,13 @@ DELAY(int n)
 			     / 1000000;
 
 	while (ticks_left > 0) {
-		tick = getit();
+#ifdef DDB
+		if (db_active) {
+			inb(0x84);
+			tick = prev_tick + 1;
+		} else
+#endif
+			tick = getit();
 #ifdef DELAYDEBUG
 		++getit_calls;
 #endif
diff --git a/sys/isa/atrtc.c b/sys/isa/atrtc.c
index 3d7d8c11ae1f..29cfefca4aaf 100644
--- a/sys/isa/atrtc.c
+++ b/sys/isa/atrtc.c
@@ -322,8 +322,18 @@ DELAY(int n)
 	 * takes about 1.5 usec for each of the i/o's in getit().  The loop
 	 * takes about 6 usec on a 486/33 and 13 usec on a 386/20.  The
 	 * multiplications and divisions to scale the count take a while).
+	 *
+	 * However, if ddb is active then use a fake counter since reading
+	 * the i8254 counter involves acquiring a lock.  ddb must not go
+	 * locking for many reasons, but it calls here for at least atkbd
+	 * input.
 	 */
-	prev_tick = getit();
+#ifdef DDB
+	if (db_active)
+		prev_tick = 0;
+	else
+#endif
+		prev_tick = getit();
 	n -= 0;			/* XXX actually guess no initial overhead */
 	/*
 	 * Calculate (n * (timer_freq / 1e6)) without using floating point
@@ -350,7 +360,13 @@ DELAY(int n)
 			     / 1000000;
 
 	while (ticks_left > 0) {
-		tick = getit();
+#ifdef DDB
+		if (db_active) {
+			inb(0x84);
+			tick = prev_tick + 1;
+		} else
+#endif
+			tick = getit();
 #ifdef DELAYDEBUG
 		++getit_calls;
 #endif