linux-kernel - Re: [RFC patch 0/4] TSC calibration improvements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080904205236.GA3864@elte.hu>
Date:	Thu, 4 Sep 2008 22:52:36 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	Alok Kataria <akataria@...are.com>,
	Arjan van de Veen <arjan@...radead.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [RFC patch 0/4] TSC calibration improvements


> +static unsigned long quick_pit_calibrate(void)
> +{
[...]
> +	if (pit_expect_msb(0xff)) {
> +		int i;
> +		u64 t1, t2, delta;
> +		unsigned char expect = 0xfe;
> +
> +		t1 = get_cycles();
> +		for (i = 0; i < QUICK_PIT_ITERATIONS; i++, expect--) {
> +			if (!pit_expect_msb(expect))
> +				goto failed;
> +		}
> +		t2 = get_cycles();

hm, unless i'm missing something i think here we still have a small 
window for an SMI or some virtualization delay to slip in and cause 
massive inaccuracy: if the delay happens _after_ the last 
pit_expect_msb() and _before_ the external get_cycles() call. Right?

i fixed that by adding one more pit_expect_msb() call.

plus i think QUICK_PIT_ITERATIONS is quite close to overflowing 255 
which is built into the u32 'expect' variable (the MSB will only 
overflow to 10 bits or so) - so i've added a BUILD_BUG_ON() to make sure 
anyone tuning QUICK_PIT_MS above 60msec or so would get a build error 
instead of some hard(er) to track down calibration error.

but it's getting late here so please double-check me ... The commit is 
below.

	Ingo

------------>
>From 40d2650256289d3ba59c4fd146b86b972db6ec40 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...e.hu>
Date: Thu, 4 Sep 2008 22:47:47 +0200
Subject: [PATCH] x86: quick TSC calibration, improve

- make sure the final TSC timestamp is reliable too

- make sure nobody increases QUICK_PIT_MS so that
  QUICK_PIT_ITERATIONS can get larger than 0xff, breaking the iteration.
  (It would take about 60 msecs to reach that limit.)

Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 arch/x86/kernel/tsc.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 839070b..4832a40 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -304,6 +304,11 @@ static unsigned long quick_pit_calibrate(void)
 	outb(0xff, 0x42);
 	outb(0xff, 0x42);
 
+	/*
+	 * The iteration assumes that expect never goes below zero:
+	 */
+	BUILD_BUG_ON(QUICK_PIT_ITERATIONS >= 0xff);
+
 	if (pit_expect_msb(0xff)) {
 		int i;
 		u64 t1, t2, delta;
@@ -317,6 +322,12 @@ static unsigned long quick_pit_calibrate(void)
 		t2 = get_cycles();
 
 		/*
+		 * Make sure we can rely on the second TSC timestamp:
+		 */
+		if (!pit_expect_msb(--expect))
+			goto failed;
+
+		/*
 		 * Ok, if we get here, then we've seen the
 		 * MSB of the PIT decrement QUICK_PIT_ITERATIONS
 		 * times, and each MSB had many hits, so we never
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/