linux-kernel - Re: [PATCH] x86: c1e_idle: don't mark TSC unstable if CPU has invariant TSC

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] [day] [month] [year] [list]

Message-ID: <20080923054248.GB16896@alberich.amd.com>
Date:	Tue, 23 Sep 2008 07:42:48 +0200
From:	Andreas Herrmann <andreas.herrmann3@....com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Valdis.Kletnieks@...edu, Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86: c1e_idle: don't mark TSC unstable if CPU has
	invariant TSC

On Sat, Sep 20, 2008 at 08:14:57AM +0200, Ingo Molnar wrote:
> 
> * Andreas Herrmann <andreas.herrmann3@....com> wrote:
> 
> > Currently the kernel assumes TSC is stable and there are various 
> > places where Linux might spot when TSC is unstable. c1e_idle is one 
> > such place. But it's wrong to mark TSC unstable for all AMD CPUs in 
> > this function as newer CPU families have TSC's that are P- and C-state 
> > invariant.
> 
> i agree with the purpose of the patch (as it flags the first really sane 
> TSC implementation on x86!!!) - but it would be nice to indicate this in 
> a different CPU feature bit other than X86_FEATURE_CONSTANT_TSC, to 
> reduce confusion. Perhaps introduce a virtual CPU feature bit for that?

Yes, I thought about it as well, but at the moment I don't see a big
benefit. I guess you mean that with a new feature bit we could skip
all those additional checks for good TSCs if the bit is set or exit
mark_tsc_unstable() early?

But I've observed on one test machine with a dual-core CPU that TSC's
were P- and C-state invariant but they also had a constant difference
which was large enough to cause a
  "Measured ... cycles TSC warp between CPUs, turning off TSC clock"
message. It was a family 0x11 CPU which has invariant TSCs and
for it the X86_FEATURE_CONSTANT_TSC bit is set. But obviously both
cores' TSCs were not correctly synced among themselves at start time.

Thus I think the current behaviour of the kernel to check for good TSC
in different places is the right thing to do because it is robust enough
to detect such unexpected behaviour.

Regards,

Andreas

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