This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: "Keir Fraser" <keir.xen@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Fri, 13 May 2011 08:14:00 +0100
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, xen devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 13 May 2011 00:14:48 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C9F2865E.1A39D%keir.xen@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <625BA99ED14B2D499DC4E29D8138F1505C8F0A4971@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <C9F2865E.1A39D%keir.xen@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> 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).


Xen-devel mailing list