[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Jan Beulich <JBeulich@xxxxxxxxxx>
  • 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
  • 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=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcwRR98VCeyRBP9eDEy6fQ1rA4OPkQ==
  • Thread-topic: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE

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



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.