Add comments, including restoring the PIIX4 errata comment, to indicate

what the ACPI-safe workaround is intended to fix.  Requested by phk.

Set the bushandle and tag when attaching the timer, don't do it each time
in read_counter().  Pointed out by bde.

Move test_counter() to the end.  Staticize acpi_timer_reg.

1 files changed, 76 insertions, 45 deletions

diff --git a/sys/dev/acpica/acpi_timer.c b/sys/dev/acpica/acpi_timer.c
index 999c493..9ab5c9e 100644
--- a/sys/dev/acpica/acpi_timer.c
+++ b/sys/dev/acpica/acpi_timer.c
@@ -51,8 +51,10 @@
 #define _COMPONENT	ACPI_TIMER
 ACPI_MODULE_NAME("TIMER")
 
-static device_t	acpi_timer_dev;
-struct resource	*acpi_timer_reg;
+static device_t			acpi_timer_dev;
+static struct resource		*acpi_timer_reg;
+static bus_space_handle_t	timer_bsh;
+static bus_space_tag_t		timer_bst;
 
 static u_int	acpi_timer_frequency = 14318182 / 4;
 
@@ -97,53 +99,13 @@ static struct timecounter acpi_timer_timecounter = {
 static u_int
 read_counter()
 {
-    bus_space_handle_t bsh;
-    bus_space_tag_t bst;
     uint32_t tv;
 
-    bsh = rman_get_bushandle(acpi_timer_reg);
-    bst = rman_get_bustag(acpi_timer_reg);
-    tv = bus_space_read_4(bst, bsh, 0);
-    bus_space_barrier(bst, bsh, 0, 4, BUS_SPACE_BARRIER_READ);
-
+    tv = bus_space_read_4(timer_bst, timer_bsh, 0);
+    bus_space_barrier(timer_bst, timer_bsh, 0, 4, BUS_SPACE_BARRIER_READ);
     return (tv);
 }
 
-#define N 2000
-static int
-test_counter()
-{
-    uint32_t	last, this;
-    int		min, max, n, delta;
-
-    min = 10000000;
-    max = 0;
-    last = read_counter();
-    for (n = 0; n < N; n++) {
-	this = read_counter();
-	delta = acpi_TimerDelta(this, last);
-	if (delta > max)
-	    max = delta;
-	else if (delta < min)
-	    min = delta;
-	last = this;
-    }
-    if (max - min > 2)
-	n = 0;
-    else if (min < 0 || max == 0)
-	n = 0;
-    else
-	n = 1;
-    if (bootverbose) {
-	printf("ACPI timer looks %s min = %d, max = %d, width = %d\n",
-		n ? "GOOD" : "BAD ",
-		min, max, max - min);
-    }
-
-    return (n);
-}
-#undef N
-
 /*
  * Locate the ACPI timer using the FADT, set up and allocate the I/O resources
  * we will be using.
@@ -179,6 +141,8 @@ acpi_timer_identify(driver_t *driver, device_t parent)
 		      rtype == SYS_RES_IOPORT ? "port" : "mem", rstart);
 	return_VOID;
     }
+    timer_bsh = rman_get_bushandle(acpi_timer_reg);
+    timer_bst = rman_get_bustag(acpi_timer_reg);
     if (AcpiGbl_FADT->TmrValExt != 0)
 	acpi_timer_timecounter.tc_counter_mask = 0xffffffff;
     else
@@ -187,6 +151,11 @@ acpi_timer_identify(driver_t *driver, device_t parent)
     if (testenv("debug.acpi.timer_test"))
 	acpi_timer_test();
 
+    /*
+     * If all tests of the counter succeed, use the ACPI-fast method.  If
+     * at least one failed, default to using the safe routine, which reads
+     * the timer multiple times to get a consistent value before returning.
+     */
     j = 0;
     for (i = 0; i < 10; i++)
 	j += test_counter();
@@ -232,7 +201,10 @@ acpi_timer_get_timecount(struct timecounter *tc)
 
 /*
  * Fetch current time value from hardware that may not correctly
- * latch the counter.
+ * latch the counter.  We need to read until we have three monotonic
+ * samples and then use the middle one, otherwise we are not protected
+ * against the fact that the bits can be wrong in two directions.  If
+ * we only cared about monosity, two reads would be enough.
  */
 static u_int
 acpi_timer_get_timecount_safe(struct timecounter *tc)
@@ -275,6 +247,65 @@ SYSCTL_PROC(_machdep, OID_AUTO, acpi_timer_freq, CTLTYPE_INT | CTLFLAG_RW,
 	    0, sizeof(u_int), acpi_timer_sysctl_freq, "I", "");
 
 /*
+ * Some ACPI timers are known or believed to suffer from implementation
+ * problems which can lead to erroneous values being read.  This function
+ * tests for consistent results from the timer and returns 1 if it believes
+ * the timer is consistent, otherwise it returns 0.
+ *
+ * It appears the cause is that the counter is not latched to the PCI bus
+ * clock when read:
+ *
+ * ] 20. ACPI Timer Errata
+ * ]
+ * ]   Problem: The power management timer may return improper result when
+ * ]   read. Although the timer value settles properly after incrementing,
+ * ]   while incrementing there is a 3nS window every 69.8nS where the
+ * ]   timer value is indeterminate (a 4.2% chance that the data will be
+ * ]   incorrect when read). As a result, the ACPI free running count up
+ * ]   timer specification is violated due to erroneous reads.  Implication:
+ * ]   System hangs due to the "inaccuracy" of the timer when used by
+ * ]   software for time critical events and delays.
+ * ]
+ * ] Workaround: Read the register twice and compare.
+ * ] Status: This will not be fixed in the PIIX4 or PIIX4E, it is fixed
+ * ] in the PIIX4M.
+ */
+#define N 2000
+static int
+test_counter()
+{
+    uint32_t	last, this;
+    int		min, max, n, delta;
+
+    min = 10000000;
+    max = 0;
+    last = read_counter();
+    for (n = 0; n < N; n++) {
+	this = read_counter();
+	delta = acpi_TimerDelta(this, last);
+	if (delta > max)
+	    max = delta;
+	else if (delta < min)
+	    min = delta;
+	last = this;
+    }
+    if (max - min > 2)
+	n = 0;
+    else if (min < 0 || max == 0)
+	n = 0;
+    else
+	n = 1;
+    if (bootverbose) {
+	printf("ACPI timer looks %s min = %d, max = %d, width = %d\n",
+		n ? "GOOD" : "BAD ",
+		min, max, max - min);
+    }
+
+    return (n);
+}
+#undef N
+
+/*
  * Test harness for verifying ACPI timer behaviour.
  * Boot with debug.acpi.timer_test set to invoke this.
  */