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: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
From: Keir Fraser <keir.xen@xxxxxxxxx>
Date: Fri, 13 May 2011 09:29:27 +0100
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, xen devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 13 May 2011 01:30:21 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:user-agent:date:subject:from:to:cc:message-id :thread-topic:thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=iUezDtZ446FS14+gh21b48J48+DGH9Wx90tGP85Wlj8=; b=v8IjbXmYp6ve9BAHtKGnHkE4j6yQ5R8njfns1XDodhvhuqFo3fDwSbkpfT1Sz3rMvT RdM2mFiGVuGQ1YwDChv2+nj86ohJkciaiG/rXXxHiLBr0hudmM+w/XexTdDv12jeOFKe jsE8W43i3nnJQa1ib3RhNgiGDcQtVDDaQkjbw=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=KSPe7X1SDwePfVQm/sYa6ANM2IpB5t8jfdL4qZdd8EO3VOxYnO3ZaxNFJFCbfBz/YH q/1Kjl5f/loNwtTqZCa/KeEIq9UgQ8HVxBnMtaoe4llrTzRmcEMY4eFfeDW4MzxuZrkf 99KjnUL+zr/1BRQYnqzs74HK6+3VpXVvMkk8I=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4DCCF65802000078000412A4@xxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwRR98VCeyRBP9eDEy6fQ1rA4OPkQ==
Thread-topic: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
User-agent: Microsoft-Entourage/
On 13/05/2011 08:14, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:

>> 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)).

The current idea of TSC_RELIABLE is it means the platform ensures that all
TSCs are in lock step, at constant rate, never stopping even in C3. Hence we
don't need to modify TSCs, hence we don't need to check TSC writability. And
also, hence we shouldn't get to the write_tsc() in cstate_restore_tsc()
(since TSC_RELIABLE should imply NONSTOP_TSC, and hence we should bail early
from cstate_restore_tsc()).

> But the problem Kevin reports is exactly what I expected when
> we discussed the whole change.

Well I don't understand that.

Nevertheless, I feel I'm playing devil's advocate here and batting on DanM's
side for something I don't consider a major issue. If someone wants to clean
this up and come up with (possibly different and new) documented and
consistently applied semantics for these TSC feature flags, please go ahead
and propose it. And we'll see who comes out to care and bat against it.

As it is, I'm still of the opinion that the smallest correct fix would be to
invert the assertion predicate.

 -- Keir

> 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