dcavalca / rpms / grub2

Forked from rpms/grub2 3 years ago
Clone

Blame SOURCES/0108-Make-pmtimer-tsc-calibration-not-take-51-seconds-to-.patch

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