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

[Keir Fraser <keir.xen@gmail.com>]

On 13/05/2011 03:45, "Tian, Kevin" <kevin.tian@intel.com> 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)).

> Also mark tsc as reliable for X86_FEATURE_CONSTANT_TSC.

Unrelated change? Also, TSC_RELIABLE should only be asserted on NONSTOP_TSC
platforms. I suspect this change is not correct.

 -- Keir

> Without this fix, one of our platform hits the assertion which
> only has constant tsc feature.
> 
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> 
> diff -r 0c446850d85e xen/arch/x86/cpu/intel.c
> --- a/xen/arch/x86/cpu/intel.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/cpu/intel.c Fri May 13 10:01:20 2011 +0800
> @@ -221,8 +221,10 @@
> if (c->x86 == 6) 
> set_bit(X86_FEATURE_P3, c->x86_capability);
> if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> -  (c->x86 == 0x6 && c->x86_model >= 0x0e))
> +  (c->x86 == 0x6 && c->x86_model >= 0x0e)) {
> set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +  set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
> + }
> if (cpuid_edx(0x80000007) & (1u<<8)) {
> set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
> diff -r 0c446850d85e xen/arch/x86/time.c
> --- a/xen/arch/x86/time.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/time.c Fri May 13 10:01:20 2011 +0800
> @@ -686,8 +686,6 @@
>      if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
>          return;
>  
> -    ASSERT(boot_cpu_has(X86_FEATURE_TSC_RELIABLE));
> -
>      write_tsc(stime2tsc(read_platform_stime()));
>  }
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel