c78952
From 0b158c30810db86f549cabd6e6021ab15962370d Mon Sep 17 00:00:00 2001
da63b3
From: Peter Jones <pjones@redhat.com>
da63b3
Date: Tue, 7 Nov 2017 17:12:17 -0500
31cddd
Subject: [PATCH] Make pmtimer tsc calibration not take 51 seconds to
da63b3
 fail.
da63b3
da63b3
On my laptop running at 2.4GHz, if I run a VM where tsc calibration
da63b3
using pmtimer will fail presuming a broken pmtimer, it takes ~51 seconds
da63b3
to do so (as measured with the stopwatch on my phone), with a tsc delta
da63b3
of 0x1cd1c85300, or around 125 billion cycles.
da63b3
da63b3
If instead of trying to wait for 5-200ms to show up on the pmtimer, we try
ec4acb
to wait for 5-200us, it decides it's broken in ~0x2626aa0 TSCs, aka ~2.4
da63b3
million cycles, or more or less instantly.
da63b3
da63b3
Additionally, this reading the pmtimer was returning 0xffffffff anyway,
da63b3
and that's obviously an invalid return.  I've added a check for that and
da63b3
0 so we don't bother waiting for the test if what we're seeing is dead
da63b3
pins with no response at all.
da63b3
ec4acb
If "debug" is includes "pmtimer", you will see one of the following
ec4acb
three outcomes.  If pmtimer gives all 0 or all 1 bits, you will see:
ec4acb
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 1
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 2
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 3
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 4
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 5
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 6
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 7
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 8
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 9
ec4acb
kern/i386/tsc_pmtimer.c:77: pmtimer: 0xffffff bad_reads: 10
ec4acb
kern/i386/tsc_pmtimer.c:78: timer is broken; giving up.
ec4acb
ec4acb
This outcome was tested using qemu+kvm with UEFI (OVMF) firmware and
ec4acb
these options: -machine pc-q35-2.10 -cpu Broadwell-noTSX
ec4acb
ec4acb
If pmtimer gives any other bit patterns but is not actually marching
ec4acb
forward fast enough to use for clock calibration, you will see:
ec4acb
ec4acb
kern/i386/tsc_pmtimer.c:121: pmtimer delta is 0x0 (1904 iterations)
ec4acb
kern/i386/tsc_pmtimer.c:124: tsc delta is implausible: 0x2626aa0
ec4acb
ec4acb
This outcome was tested using grub compiled with GRUB_PMTIMER_IGNORE_BAD_READS
ec4acb
defined (so as not to trip the bad read test) using qemu+kvm with UEFI
ec4acb
(OVMF) firmware, and these options: -machine pc-q35-2.10 -cpu Broadwell-noTSX
ec4acb
ec4acb
If pmtimer actually works, you'll see something like:
ec4acb
ec4acb
kern/i386/tsc_pmtimer.c:121: pmtimer delta is 0x0 (1904 iterations)
ec4acb
kern/i386/tsc_pmtimer.c:124: tsc delta is implausible: 0x2626aa0
ec4acb
ec4acb
This outcome was tested using qemu+kvm with UEFI (OVMF) firmware, and
ec4acb
these options: -machine pc-i440fx-2.4 -cpu Broadwell-noTSX
ec4acb
ec4acb
I've also tested this outcome on a real Intel Xeon E3-1275v3 on an Intel
ec4acb
Server Board S1200V3RPS using the SDV.RP.B8 "Release" build here:
ec4acb
https://firmware.intel.com/sites/default/files/UEFIDevKit_S1200RP_vB8.zip
ec4acb
da63b3
Signed-off-by: Peter Jones <pjones@redhat.com>
da63b3
---
ec4acb
 grub-core/kern/i386/tsc_pmtimer.c | 109 +++++++++++++++++++++++++++++++-------
ec4acb
 1 file changed, 89 insertions(+), 20 deletions(-)
da63b3
da63b3
diff --git a/grub-core/kern/i386/tsc_pmtimer.c b/grub-core/kern/i386/tsc_pmtimer.c
ec4acb
index c9c36169978..ca15c3aacd7 100644
da63b3
--- a/grub-core/kern/i386/tsc_pmtimer.c
da63b3
+++ b/grub-core/kern/i386/tsc_pmtimer.c
ec4acb
@@ -28,40 +28,101 @@
ec4acb
 #include <grub/acpi.h>
ec4acb
 #include <grub/cpu/io.h>
ec4acb
 
ec4acb
+/*
ec4acb
+ * Define GRUB_PMTIMER_IGNORE_BAD_READS if you're trying to test a timer that's
ec4acb
+ * present but doesn't keep time well.
ec4acb
+ */
ec4acb
+// #define GRUB_PMTIMER_IGNORE_BAD_READS
ec4acb
+
ec4acb
 grub_uint64_t
ec4acb
 grub_pmtimer_wait_count_tsc (grub_port_t pmtimer,
ec4acb
 			     grub_uint16_t num_pm_ticks)
ec4acb
 {
ec4acb
   grub_uint32_t start;
ec4acb
-  grub_uint32_t last;
ec4acb
-  grub_uint32_t cur, end;
ec4acb
+  grub_uint64_t cur, end;
da63b3
   grub_uint64_t start_tsc;
da63b3
   grub_uint64_t end_tsc;
ec4acb
-  int num_iter = 0;
ec4acb
+  unsigned int num_iter = 0;
ec4acb
+#ifndef GRUB_PMTIMER_IGNORE_BAD_READS
da63b3
+  int bad_reads = 0;
ec4acb
+#endif
da63b3
 
da63b3
-  start = grub_inl (pmtimer) & 0xffffff;
ec4acb
-  last = start;
ec4acb
+  /*
ec4acb
+   * Some timers are 24-bit and some are 32-bit, but it doesn't make much
ec4acb
+   * difference to us.  Caring which one we have isn't really worth it since
ec4acb
+   * the low-order digits will give us enough data to calibrate TSC.  So just
ec4acb
+   * mask the top-order byte off.
ec4acb
+   */
ec4acb
+  cur = start = grub_inl (pmtimer) & 0xffffffUL;
da63b3
   end = start + num_pm_ticks;
da63b3
   start_tsc = grub_get_tsc ();
da63b3
   while (1)
da63b3
     {
da63b3
-      cur = grub_inl (pmtimer) & 0xffffff;
ec4acb
-      if (cur < last)
ec4acb
-	cur |= 0x1000000;
ec4acb
-      num_iter++;
ec4acb
+      cur &= 0xffffffffff000000ULL;
ec4acb
+      cur |= grub_inl (pmtimer) & 0xffffffUL;
ec4acb
+
ec4acb
+      end_tsc = grub_get_tsc();
da63b3
+
ec4acb
+#ifndef GRUB_PMTIMER_IGNORE_BAD_READS
ec4acb
+      /*
ec4acb
+       * If we get 10 reads in a row that are obviously dead pins, there's no
ec4acb
+       * reason to do this thousands of times.
da63b3
+       */
ec4acb
+      if (cur == 0xffffffUL || cur == 0)
da63b3
+	{
da63b3
+	  bad_reads++;
ec4acb
+	  grub_dprintf ("pmtimer",
ec4acb
+			"pmtimer: 0x%"PRIxGRUB_UINT64_T" bad_reads: %d\n",
ec4acb
+			cur, bad_reads);
ec4acb
+	  grub_dprintf ("pmtimer", "timer is broken; giving up.\n");
da63b3
+
da63b3
+	  if (bad_reads == 10)
da63b3
+	    return 0;
da63b3
+	}
ec4acb
+#endif
da63b3
+
ec4acb
+      if (cur < start)
ec4acb
+	cur += 0x1000000;
da63b3
+
da63b3
       if (cur >= end)
da63b3
 	{
ec4acb
-	  end_tsc = grub_get_tsc ();
ec4acb
+	  grub_dprintf ("pmtimer", "pmtimer delta is 0x%"PRIxGRUB_UINT64_T"\n",
ec4acb
+			cur - start);
ec4acb
+	  grub_dprintf ("pmtimer", "tsc delta is 0x%"PRIxGRUB_UINT64_T"\n",
da63b3
+			end_tsc - start_tsc);
da63b3
 	  return end_tsc - start_tsc;
da63b3
 	}
da63b3
-      /* Check for broken PM timer.
da63b3
-	 50000000 TSCs is between 5 ms (10GHz) and 200 ms (250 MHz)
da63b3
-	 if after this time we still don't have 1 ms on pmtimer, then
da63b3
-	 pmtimer is broken.
ec4acb
+
ec4acb
+      /*
ec4acb
+       * Check for broken PM timer.  1ms at 10GHz should be 1E+7 TSCs; at
ec4acb
+       * 250MHz it should be 2.5E6.  So if after 4E+7 TSCs on a 10GHz machine,
ec4acb
+       * we should have seen pmtimer show 4ms of change (i.e. cur =~
ec4acb
+       * start+14320); on a 250MHz machine that should be 16ms (start+57280).
ec4acb
+       * If after this a time we still don't have 1ms on pmtimer, then pmtimer
ec4acb
+       * is broken.
ec4acb
+       *
ec4acb
+       * Likewise, if our code is perfectly efficient and introduces no delays
ec4acb
+       * whatsoever, on a 10GHz system we should see a TSC delta of 3580 in
ec4acb
+       * ~3580 iterations.  On a 250MHz machine that should be ~900 iterations.
ec4acb
+       *
ec4acb
+       * With those factors in mind, there are two limits here.  There's a hard
ec4acb
+       * limit here at 8x our desired pm timer delta, picked as an arbitrarily
ec4acb
+       * large value that's still not a lot of time to humans, because if we
ec4acb
+       * get that far this is either an implausibly fast machine or the pmtimer
ec4acb
+       * is not running.  And there's another limit on 4x our 10GHz tsc delta
ec4acb
+       * without seeing cur converge on our target value.
da63b3
        */
da63b3
-      if ((num_iter & 0xffffff) == 0 && grub_get_tsc () - start_tsc > 5000000) {
da63b3
-	return 0;
da63b3
-      }
ec4acb
+      if ((++num_iter > (grub_uint32_t)num_pm_ticks << 3UL) ||
ec4acb
+	  end_tsc - start_tsc > 40000000)
da63b3
+	{
ec4acb
+	  grub_dprintf ("pmtimer",
ec4acb
+			"pmtimer delta is 0x%"PRIxGRUB_UINT64_T" (%u iterations)\n",
ec4acb
+			cur - start, num_iter);
ec4acb
+	  grub_dprintf ("pmtimer",
ec4acb
+			"tsc delta is implausible: 0x%"PRIxGRUB_UINT64_T"\n",
da63b3
+			end_tsc - start_tsc);
da63b3
+	  return 0;
da63b3
+	}
da63b3
     }
da63b3
 }
da63b3
 
ec4acb
@@ -74,12 +135,20 @@ grub_tsc_calibrate_from_pmtimer (void)
ec4acb
 
ec4acb
   fadt = grub_acpi_find_fadt ();
ec4acb
   if (!fadt)
ec4acb
-    return 0;
ec4acb
+    {
ec4acb
+      grub_dprintf ("pmtimer", "No FADT found; not using pmtimer.\n");
ec4acb
+      return 0;
ec4acb
+    }
ec4acb
   pmtimer = fadt->pmtimer;
ec4acb
   if (!pmtimer)
ec4acb
-    return 0;
ec4acb
+    {
ec4acb
+      grub_dprintf ("pmtimer", "FADT does not specify pmtimer; skipping.\n");
ec4acb
+      return 0;
ec4acb
+    }
ec4acb
 
ec4acb
-  /* It's 3.579545 MHz clock. Wait 1 ms.  */
ec4acb
+  /*
ec4acb
+   * It's 3.579545 MHz clock. Wait 1 ms.
ec4acb
+   */
ec4acb
   tsc_diff = grub_pmtimer_wait_count_tsc (pmtimer, 3580);
ec4acb
   if (tsc_diff == 0)
ec4acb
     return 0;