linux-kernel - Re: [PATCH] make tsc=reliable override boot time stability checks

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: <1250643871.7002.24.camel@localhost.localdomain>
Date:	Tue, 18 Aug 2009 18:04:31 -0700
From:	john stultz <johnstul@...ibm.com>
To:	akataria@...are.com
Cc:	Stephen Hemminger <shemminger@...tta.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] make tsc=reliable override boot time stability checks

On Mon, 2009-08-17 at 18:39 -0700, Alok Kataria wrote:
> Hi John,
> 
> On Mon, 2009-08-17 at 16:40 -0700, john stultz wrote:
> > On Mon, 2009-08-17 at 16:27 -0700, Stephen Hemminger wrote:
> > > On Mon, 17 Aug 2009 16:17:54 -0700
> > > john stultz <johnstul@...ibm.com> wrote:
> > > 
> > > > On Mon, 2009-08-17 at 16:02 -0700, Stephen Hemminger wrote:
> > > > > This adds tsc, but makes it first?  it is reliable, but do I want
> > > > > to make it most important?
> > > > > 
> > > > > $ cat /sys/devices/system/clocksource/clocksource0/available_clocksource
> > > > > tsc acpi_pm
> > > > 
> > > > 
> > > > Well, if you're overriding the system saying that its safe, then sure,
> > > > its better then anything else, why wouldn't we?
> > > > 
> > > 
> > > That's acceptable, maybe add change to Documentation/kernel-parameters.txt
> > > 
> > > 	tsc=		Disable clocksource-must-verify flag for TSC.
> > > 			Format: <string>
> > > 			[x86] reliable: mark tsc clocksource as reliable and
> > >                         makes tsc the default clocksource; this
> > > 			disables clocksource verification at runtime.
> > > 			Used to enable high-resolution timer mode on older
> > > 			hardware, and in virtualized environment.
> > > 
> > 
> > Sounds good. Thanks so much for the bug report and testing!
> > 
> > 
> > This patch makes the tsc=reliable option disable the boot time stability
> > checks. Currently the option only disables the runtime watchdog checks.
> > This change allows folks who want to override the boot time TSC
> > stability checks and use the TSC when the system would otherwise
> > disqualify it.
> > 
> > There still are some situations that the TSC will be disqualified, such
> > as cpufreq scaling. But these are situations where the box will hang if
> > allowed.
> > 
> 
> I had purposefully kept the tsc=reliable separate from the TSC
> synchronous checks. 
> With this patch TSC is marked as usable though the hardware below
> doesn't export a CONSTANT_TSC, it might not be a problem generally, but
> since TSC has the highest rating, don't you think that timekeeping might
> be wayward on such systems ? 

Oh yea, there's a risk of that, but we are telling the kernel to
override its runtime checking of the clocksource, so it seems reasonable
to also include the boot time checks. I worry otherwise the option
becomes too subtle to be really useful to users. 

> Having said that, I don't think I have a particular problem with the
> patch as far as we are explicitly mentioning the fact that TSC=reliable
> means TSC is blindly trusted on this system, and time might be little
> off on some systems.

I think the explicit boot option, along with the kernel-parameters text
makes it clear enough, but if you have a specific wording in mind that
works better, please send a patch and I'll ack it.

thanks
-john

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