WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <JBeulich@xxxxxxxxxx>, Keir Fraser <keir.xen@xxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
Date: Fri, 13 May 2011 15:28:18 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: xen devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 13 May 2011 00:30:20 -0700
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>
References: <625BA99ED14B2D499DC4E29D8138F1505C8F0A4971@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <C9F2865E.1A39D%keir.xen@xxxxxxxxx> <4DCCF65802000078000412A4@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwRPVd+XKGJMyYuR7uN6fAKWdXZLAAAOsqw
Thread-topic: [Xen-devel] [PATCH] x86, cpuidle: remove assertion on X86_FEATURE_TSC_RELIABLE
> From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx]
> Sent: Friday, May 13, 2011 3:14 PM
> 
> >>> 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)).

here we may need a definition about what a reliable TSC means here. no tsc skew
among cpus, or stably incremented on the bus clock? It looks that we have some
assumption behind various TSC flags, and use them with implicit assumptions.
Here I can see that tsc_check_writability may disable deep cstate when it's not
writable, but it doesn't mean that the assertion on X86_FEATURE_TSC_RELIABLE
is correct since even when this flag is cleared the tsc could still be 
writable. That
way the assertion absolutely is wrong.

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

I still don't understand why deep cstate must be disabled when TSC is not 
reliable.
Also the early bail out doesn't impact my error, since in my case TSC_RELIABLE 
is
not set but it's simply writable.

Thanks
Kevin

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