RE: [Xen-devel] [PATCH] X86: Prefer TSC-deadline timer in Xen

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

The change to cstate_restore_tsc reminds me...

TSC should never be written if it is being used to
generate system time, and I think cstate_restore_tsc
is only used on systems where another clocksource
is used to generate system time.   (And it generates
a rather poor approximation of TSC.)  Are there any
machines where cstate_restore_tsc might get executed
but TSC_Deadline is in use?  If so, might there be
a race condition here?  Also, are there any
issues with using TSC Deadline on a system with
the potential for hot-add physical CPUs?

I suspect cstate_restore_tsc doesn't get executed
if TSC is reliable, and the TSC Deadline feature
is probably only available on systems where TSC
is reliable, but am not sure.

Thanks,
Dan

> -----Original Message-----
> From: Wei, Gang [mailto:gang.wei@xxxxxxxxx]
> Sent: Thursday, October 28, 2010 9:14 AM
> To: Keir Fraser; Tim Deegan
> Cc: Brown, Len; xen-devel@xxxxxxxxxxxxxxxxxxx; Wei, Gang; Jan Beulich
> Subject: RE: [Xen-devel] [PATCH] X86: Prefer TSC-deadline timer in Xen
> 
> Resend.
> 
> Jimmy
> ---------------------------------------------------------
> X86: Prefer TSC-deadline timer in Xen
> 
> The new TSC Deadline Timer offers system software a low overhead
> per-logical-thread deadline timer in TSC units.
> 
> The timer is implemented via a new architectural 64-bit register,
> IA32_TSC_DEADLINE_MSR. Reads and writes of this MSR occur in program
> order,
> but are non-serializing.
> 
> The support for this feature is indicated by
> CPUID.01H:ECX.TSC_Deadline[bit 24] = 1 as documented in the Intel
> Architectures
> Software Developer's Manual.
> 
> The LOCAL APIC on new processors has a mode where its underlying
> hardware timer
> can now be accessed via the non-serializing IA32_TSC_DEADLINE_MSR in
> TSC tick
> units.
> 
> If this mode is present, prefer it over the traditional LAPIC timer
> mode.
> KERN_DEBUG dmesg will print "TSC deadline timer enabled" when TDT is
> used.
> 
> Bootparam "tdt=off" is available to revert to LAPIC timer mode.
> 
> This patch is based on original work by Len Brown for Linux kernel.
> 
> cc: Len Brown <len.brown@xxxxxxxxx>
> Signed-off-by: Wei Gang <gang.wei@xxxxxxxxx>
> 
> diff -r 0dc0bc411035 xen/arch/x86/apic.c
> --- a/xen/arch/x86/apic.c     Thu Oct 21 18:51:36 2010 +0100
> +++ b/xen/arch/x86/apic.c     Sat Oct 30 05:09:41 2010 +0800
> @@ -37,6 +37,14 @@
>  #include <asm/asm_defns.h> /* for BUILD_SMP_INTERRUPT */
>  #include <mach_apic.h>
>  #include <io_ports.h>
> +
> +#define APIC_TIMER_MODE_ONESHOT         (0 << 17)
> +#define APIC_TIMER_MODE_PERIODIC        (1 << 17)
> +#define APIC_TIMER_MODE_TSC_DEADLINE    (2 << 17)
> +#define APIC_TIMER_MODE_MASK            (3 << 17)
> +
> +static int tdt_enabled __read_mostly = 1;
> +boolean_param("tdt", tdt_enabled);
> 
>  static struct {
>      int active;
> @@ -1198,6 +1206,13 @@ static void __setup_APIC_LVTT(unsigned i
>      lvtt_value = /*APIC_LVT_TIMER_PERIODIC |*/ LOCAL_TIMER_VECTOR;
>      if (!APIC_INTEGRATED(ver))
>          lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);
> +
> +    if ( tdt_enabled )
> +    {
> +        lvtt_value &= (~APIC_TIMER_MODE_MASK);
> +        lvtt_value |= APIC_TIMER_MODE_TSC_DEADLINE;
> +    }
> +
>      apic_write_around(APIC_LVTT, lvtt_value);
> 
>      tmp_value = apic_read(APIC_TDCR);
> @@ -1309,7 +1324,15 @@ void __init setup_boot_APIC_clock(void)
> 
>      local_irq_save(flags);
> 
> -    calibrate_APIC_clock();
> +    if ( tdt_enabled && boot_cpu_has(X86_FEATURE_TSC_DEADLINE) )
> +    {
> +        printk(KERN_DEBUG "TSC deadline timer enabled\n");
> +    }
> +    else
> +    {
> +        tdt_enabled = 0;
> +        calibrate_APIC_clock();
> +    }
> 
>      setup_APIC_timer();
> 
> @@ -1359,6 +1382,18 @@ int reprogram_timer(s_time_t timeout)
>      /* No local APIC: timer list is polled via the PIT interrupt. */
>      if ( !cpu_has_apic )
>          return 1;
> +
> +    if ( tdt_enabled )
> +    {
> +        u64 tsc = 0;
> +
> +        if ( timeout )
> +            tsc = stime2tsc(timeout);
> +
> +        wrmsrl(MSR_IA32_TSC_DEADLINE, tsc);
> +
> +        return 1;
> +    }
> 
>      if ( timeout && ((expire = timeout - NOW()) > 0) )
>          apic_tmict = min_t(u64, (bus_scale * expire) >> 18, UINT_MAX);
> diff -r 0dc0bc411035 xen/arch/x86/time.c
> --- a/xen/arch/x86/time.c     Thu Oct 21 18:51:36 2010 +0100
> +++ b/xen/arch/x86/time.c     Sat Oct 30 04:28:14 2010 +0800
> @@ -662,26 +662,31 @@ static void __init init_platform_timer(v
>             freq_string(pts->frequency), pts->name);
>  }
> 
> -void cstate_restore_tsc(void)
> +u64 stime2tsc(s_time_t stime)
>  {
>      struct cpu_time *t;
>      struct time_scale sys_to_tsc;
>      s_time_t stime_delta;
> -    u64 new_tsc;
> -
> +    u64 tsc;
> +
> +    t = &this_cpu(cpu_time);
> +    sys_to_tsc = scale_reciprocal(t->tsc_scale);
> +
> +    stime_delta = stime - t->stime_local_stamp;
> +    if ( stime_delta < 0 )
> +        stime_delta = 0;
> +
> +    tsc = t->local_tsc_stamp + scale_delta(stime_delta, &sys_to_tsc);
> +
> +    return tsc;
> +}
> +
> +void cstate_restore_tsc(void)
> +{
>      if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
>          return;
> 
> -    t = &this_cpu(cpu_time);
> -    sys_to_tsc = scale_reciprocal(t->tsc_scale);
> -
> -    stime_delta = read_platform_stime() - t->stime_master_stamp;
> -    if ( stime_delta < 0 )
> -        stime_delta = 0;
> -
> -    new_tsc = t->local_tsc_stamp + scale_delta(stime_delta,
> &sys_to_tsc);
> -
> -    write_tsc(new_tsc);
> +    write_tsc(stime2tsc(read_platform_stime()));
>  }
> 
> 
> /**********************************************************************
> *****
> diff -r 0dc0bc411035 xen/include/asm-x86/cpufeature.h
> --- a/xen/include/asm-x86/cpufeature.h        Thu Oct 21 18:51:36 2010
> +0100
> +++ b/xen/include/asm-x86/cpufeature.h        Sat Oct 30 04:28:14 2010
> +0800
> @@ -99,6 +99,7 @@
>  #define X86_FEATURE_SSE4_2   (4*32+20) /* Streaming SIMD Extensions
> 4.2 */
>  #define X86_FEATURE_X2APIC   (4*32+21) /* Extended xAPIC */
>  #define X86_FEATURE_POPCNT   (4*32+23) /* POPCNT instruction */
> +#define X86_FEATURE_TSC_DEADLINE (4*32+24) /* "tdt" TSC Deadline Timer
> */
>  #define X86_FEATURE_XSAVE    (4*32+26) /* XSAVE/XRSTOR/XSETBV/XGETBV
> */
>  #define X86_FEATURE_OSXSAVE  (4*32+27) /* OSXSAVE */
>  #define X86_FEATURE_HYPERVISOR       (4*32+31) /* Running under some
> hypervisor */
> diff -r 0dc0bc411035 xen/include/asm-x86/msr-index.h
> --- a/xen/include/asm-x86/msr-index.h Thu Oct 21 18:51:36 2010
> +0100
> +++ b/xen/include/asm-x86/msr-index.h Sat Oct 30 04:28:14 2010
> +0800
> @@ -327,6 +327,8 @@
>  #define MSR_IA32_MISC_ENABLE_MONITOR_ENABLE (1<<18)
>  #define MSR_IA32_MISC_ENABLE_LIMIT_CPUID  (1<<22)
>  #define MSR_IA32_MISC_ENABLE_XTPR_DISABLE (1<<23)
> +
> +#define MSR_IA32_TSC_DEADLINE                0x000006E0
> 
>  /* Intel Model 6 */
>  #define MSR_P6_EVNTSEL0                      0x00000186
> diff -r 0dc0bc411035 xen/include/xen/time.h
> --- a/xen/include/xen/time.h  Thu Oct 21 18:51:36 2010 +0100
> +++ b/xen/include/xen/time.h  Sat Oct 30 04:28:14 2010 +0800
> @@ -31,6 +31,7 @@ typedef s64 s_time_t;
>  typedef s64 s_time_t;
> 
>  s_time_t get_s_time(void);
> +u64 stime2tsc(s_time_t stime);
>  unsigned long get_localtime(struct domain *d);
> 
>  struct tm {

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