linux-kernel - Re: [PATCH] Fix TSC calibration issues

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: <20080904050929.GA1678@1wt.eu>
Date:	Thu, 4 Sep 2008 07:09:30 +0200
From:	Willy Tarreau <w@....eu>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Arjan van de Ven <arjan@...radead.org>,
	Alok Kataria <akataria@...are.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Larry Finger <Larry.Finger@...inger.net>,
	LKML <linux-kernel@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Michael Buesch <mb@...sch.de>,
	Dan Hecht <dhecht@...are.com>
Subject: Re: [PATCH] Fix TSC calibration issues

On Wed, Sep 03, 2008 at 09:53:35PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 4 Sep 2008, Willy Tarreau wrote:
> >
> > Basically, I would do this :
> > 
> >     pit1 = readpit();
> >     while (readpit() == pit1);
> >     t1 = rdtsc(); // precise beginning of tick 0
> >     while (readpit() != pit1 - 5000);
> >     t2 = rdtsc(); // precise beginning of tick 5000
> 
> There's a few caveats here:
> 
>  - the "readpit()" has to read without actually latching the value
> 
>    latching the PIT value will stop counting.
> 
>  - and all the docs say that you have to be careful about reading the PIT 
>    without latching it because the two 8-bit accesses aren't atomic.

Ah yes you're right, I remember having been doing crappy stuff like re-reading
and checking for difference bigger than 1.

> so the above will work in practice, but there are dangers.
> 
> The best way to fix most of the dangers is probably to only care about the 
> *high* byte, so that it doesn't matter if the low byte doesn't match the 
> high byte.
> 
> So you could probably change your version to wait for 4096 cycles (a 
> change of 16 in the high byte):
> 
> 	static unsigned char read_pit_msb(void)
> 	{
> 		/* Read but throw away the LSB */
> 		inb(0x42);
> 		return inb(0x42);
> 	}
> 
> 	..
> 	/* PIT ch2: square wave, full 16-bit count */
> 	outb(0xb6, 0x43);
> 	outb(0, 0x42);
> 	outb(0, 0x42);
> 	..
> 
> 	unsigned char pit = read_pit_msb();
> 	/* Wait until the MSB changes */
> 	while (read_pit_msb() == pit1);
> 	t1 = rdtsc();
> 	while ((unsigned char) (pit - read_pit_msb()) < 9);
> 	t2 = rdtsc();
> 
> and it might work out ok without explicit latching, and without having to 
> worry about low/high bytes being out of sync.

I like this variation.

> > If someone wants to test this, I'd be interested in the number of
> > ticks required to get a good accuracy, I bet that even with a few
> > hundred ones it's already precise by a few ppm (about the precision
> > of the input clock in fact).
> 
> I actually tested a patch with a counter value of just 1024, and I got the 
> right answer. 
> 
> But if the busy loops aren't busy (due to MSI or virtualization), then all 
> those things fly out the window.

100% agreed, though the problem is already the same with any calibration code,
with more or less sensitivity.

Willy

--
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/