From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE Date: Fri, 13 May 2011 08:14:00 +0100 Message-ID: <4DCCF65802000078000412A4@vpn.id2.novell.com> References: <625BA99ED14B2D499DC4E29D8138F1505C8F0A4971@shsmsx502.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: Kevin Tian , xen devel List-Id: xen-devel@lists.xenproject.org >>> On 13.05.11 at 07:55, Keir Fraser wrote: > On 13/05/2011 03:45, "Tian, Kevin" wrote: >=20 >> x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE >>=20 >> 23228:1329d99b4f16 disables deep cstate to avoid restoring tsc when >> tsc msr is not writtable on some old platform, which however also >> adds an assertion on X86_FEATURE_TSC_RELIABLE in cstate_restore_tsc. >> The two don't match as tsc writtable-ness has nothing to do with >> whether it's reliable. As long as Xen can use tsc as the time source >> and it's writable, it should be OK to continue using deep cstate >> with tsc save/restore. >=20 > Looks like I just got the assertion the wrong way round, should be > ASSERT(!boot_cpu_has(X86_FEATURE_TSC_RELIABLE)). No, the assertion is correct imo (since tsc_check_writability() bails immediately when boot_cpu_has(X86_FEATURE_TSC_RELIABLE)). But the problem Kevin reports is exactly what I expected when we discussed the whole change. Nevertheless, simply removing the assertion won't be correct - instead your addition of the early bail out on TSC_RELIABLE is what I'd now put under question (the comment that goes with it, as we now see, isn't correct). Jan