x86_64: use HPET in 64-bit mode to avoid a harware 32-bit wraparound bug
diff mbox series

Message ID 20171208171700.pbridtbt3tkvjbhi@dwarf.suse.cz
State New, archived
Headers show
Series
  • x86_64: use HPET in 64-bit mode to avoid a harware 32-bit wraparound bug
Related show

Commit Message

Jiri Bohac Dec. 8, 2017, 5:17 p.m. UTC
The HPET clockevent configures the HPET Timer 0 in 32-bit mode. With a
wrap-around time of >4 minutes, this is OK for the clockevent purposes.
Unfortunately, some chipsets contain a documented bug, where this setting
also makes the HPET Main Counter wrap to 32 bits. HPET can be mmapped to
userspace, which makes this bug visible to applications.

The original HPET specification 1.0 from 2004 does not mention any
side-effects of setting TN_32MODE_CNF on the individual timers. But e.g.
the ICH9 documentation documents this bug:

    NOTE: When this bit is set to ‘1’, the hardware counter will
    do a 32-bit operation on comparator match and rollovers, thus
    the upper 32-bit of the Timer 0 Comparator Value register is
    ignored. The upper 32-bit of the main counter is not involved
    in any rollover from lower 32-bit of the main counter and
    becomes all zeros.

(see http://www.intel.com/assets/pdf/datasheet/316972.pdf, page
819, section 21.1.5, Bit 8). 
I've seen this behaviour on many other Intel chipsets as well.

This patch changes the HPET clockevent code to use the Timer 0 in 64-bit
mode (if compiled for x86_64; doing this for 32-bit kernels would bring
a performance penalty as a single read would require three 32-bit HPET
accesses).

Signed-off-by: Jiri Bohac <jbohac@suse.cz>

Comments

Thomas Gleixner Dec. 28, 2017, 11:49 a.m. UTC | #1
On Fri, 8 Dec 2017, Jiri Bohac wrote:

> This patch changes the HPET clockevent code to use the Timer 0 in 64-bit
> mode (if compiled for x86_64; doing this for 32-bit kernels would bring
> a performance penalty as a single read would require three 32-bit HPET
> accesses).

We tried that before and 64bit mode did not work on all tested platforms.

Aside of that your patch is broken because it does not check whether HPET
actually supports 64bit mode at all. The COUNT_SIZE_CAP bit is there for a
reason.

Thanks,

	tglx

Patch
diff mbox series

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 8ce4212e2b8d..1a500220d0dc 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -69,6 +69,31 @@  static inline void hpet_writel(unsigned int d, unsigned int a)
 	writel(d, hpet_virt_address + a);
 }
 
+
+static inline unsigned long hpet_read_long(unsigned int a)
+{
+#ifdef CONFIG_64BIT
+	return readq(hpet_virt_address + a);
+#else
+	return readl(hpet_virt_address + a);
+#endif
+}
+
+static inline void hpet_write_long(unsigned long d, unsigned int a)
+{
+#ifdef CONFIG_64BIT
+	writeq(d, hpet_virt_address + a);
+#else
+	writel(d, hpet_virt_address + a);
+#endif
+}
+
+#ifdef CONFIG_64BIT
+#define HPET_TN_WIDTH_FLAG 0
+#else
+#define HPET_TN_WIDTH_FLAG HPET_TN_32BIT
+#endif
+
 #ifdef CONFIG_X86_64
 #include <asm/pgtable.h>
 #endif
@@ -294,19 +319,20 @@  static void hpet_legacy_clockevent_register(void)
 
 static int hpet_set_periodic(struct clock_event_device *evt, int timer)
 {
-	unsigned int cfg, cmp, now;
+	unsigned int cfg;
+	unsigned long cmp, now;
 	uint64_t delta;
 
 	hpet_stop_counter();
 	delta = ((uint64_t)(NSEC_PER_SEC / HZ)) * evt->mult;
 	delta >>= evt->shift;
-	now = hpet_readl(HPET_COUNTER);
-	cmp = now + (unsigned int)delta;
+	now = hpet_read_long(HPET_COUNTER);
+	cmp = now + (unsigned long)delta;
 	cfg = hpet_readl(HPET_Tn_CFG(timer));
 	cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
-	       HPET_TN_32BIT;
+	       HPET_TN_WIDTH_FLAG;
 	hpet_writel(cfg, HPET_Tn_CFG(timer));
-	hpet_writel(cmp, HPET_Tn_CMP(timer));
+	hpet_write_long(cmp, HPET_Tn_CMP(timer));
 	udelay(1);
 	/*
 	 * HPET on AMD 81xx needs a second write (with HPET_TN_SETVAL
@@ -315,7 +341,7 @@  static int hpet_set_periodic(struct clock_event_device *evt, int timer)
 	 * (See AMD-8111 HyperTransport I/O Hub Data Sheet,
 	 * Publication # 24674)
 	 */
-	hpet_writel((unsigned int)delta, HPET_Tn_CMP(timer));
+	hpet_write_long((unsigned long)delta, HPET_Tn_CMP(timer));
 	hpet_start_counter();
 	hpet_print_config();
 
@@ -328,7 +354,7 @@  static int hpet_set_oneshot(struct clock_event_device *evt, int timer)
 
 	cfg = hpet_readl(HPET_Tn_CFG(timer));
 	cfg &= ~HPET_TN_PERIODIC;
-	cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
+	cfg |= HPET_TN_ENABLE | HPET_TN_WIDTH_FLAG;
 	hpet_writel(cfg, HPET_Tn_CFG(timer));
 
 	return 0;
@@ -355,12 +381,12 @@  static int hpet_resume(struct clock_event_device *evt)
 static int hpet_next_event(unsigned long delta,
 			   struct clock_event_device *evt, int timer)
 {
-	u32 cnt;
-	s32 res;
+	unsigned long cnt;
+	long res;
 
-	cnt = hpet_readl(HPET_COUNTER);
-	cnt += (u32) delta;
-	hpet_writel(cnt, HPET_Tn_CMP(timer));
+	cnt = hpet_read_long(HPET_COUNTER);
+	cnt += delta;
+	hpet_write_long(cnt, HPET_Tn_CMP(timer));
 
 	/*
 	 * HPETs are a complete disaster. The compare register is
@@ -384,7 +410,7 @@  static int hpet_next_event(unsigned long delta,
 	 * the event. The minimum programming delta for the generic
 	 * clockevents code is set to 1.5 * HPET_MIN_CYCLES.
 	 */
-	res = (s32)(cnt - hpet_readl(HPET_COUNTER));
+	res = (long)(cnt - hpet_read_long(HPET_COUNTER));
 
 	return res < HPET_MIN_CYCLES ? -ETIME : 0;
 }