Re: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 13.05.11 at 07:55, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> On 13/05/2011 03:45, "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
> 
>> x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
>> 
>> 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.
> 
> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel