Xen devel - Oct 2009 - [PATCH] trust new architecturally-defined TSC Invariant bit on Intel systems

Trust new architecturally-defined TSC Invariant bit (on
Intel systems only for now, AMD TBD).

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff -r 1d7221667204 xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c	Thu Oct 08 09:24:32 2009 +0100
+++ b/xen/arch/x86/smpboot.c	Thu Oct 08 11:48:15 2009 -0600
@@ -187,6 +187,11 @@ static void __init synchronize_tsc_bp (v
 	unsigned int one_usec;
 	int buggy = 0;
 
+	if (tsc_is_reliable()) {
+		printk("TSC is reliable, synchronization unnecessary\n");
+		return;
+	}
+       
 	printk("checking TSC synchronization across %u CPUs: ",
num_booting_cpus());
 
 	/* convert from kcyc/sec to cyc/usec */
@@ -278,6 +283,9 @@ static void __init synchronize_tsc_ap (v
 static void __init synchronize_tsc_ap (void)
 {
 	int i;
+
+	if (tsc_is_reliable())
+		return;
 
 	/*
 	 * Not every cpu is online at the time
diff -r 1d7221667204 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Thu Oct 08 09:24:32 2009 +0100
+++ b/xen/arch/x86/time.c	Thu Oct 08 11:48:15 2009 -0600
@@ -43,6 +43,12 @@ string_param("clocksource", opt_clocksou
  */
 static int opt_consistent_tscs;
 boolean_param("consistent_tscs", opt_consistent_tscs);
+
+/*
+ * opt_tsc_broken: Override all tests and force TSC to be assumed unreliable
+ */
+static int opt_tsc_broken;
+boolean_param("tsc_broken", opt_tsc_broken);
 
 unsigned long cpu_khz;  /* CPU clock frequency in kHz. */
 DEFINE_SPINLOCK(rtc_lock);
@@ -692,6 +698,11 @@ static void __init init_platform_timer(v
            freq_string(pts->frequency), pts->name);
 }
 
+int tsc_is_reliable(void)
+{
+    return boot_cpu_has(X86_FEATURE_TSC_RELIABLE) && !opt_tsc_broken;
+}
+
 void cstate_restore_tsc(void)
 {
     struct cpu_time *t = &this_cpu(cpu_time);
@@ -699,7 +710,7 @@ void cstate_restore_tsc(void)
     s_time_t stime_delta;
     u64 new_tsc;
 
-    if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
+    if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) && !opt_tsc_broken )
         return;
 
     stime_delta = read_platform_stime() - t->stime_master_stamp;
@@ -1117,6 +1128,9 @@ static void time_calibration_tsc_rendezv
     struct calibration_rendezvous *r = _r;
     unsigned int total_cpus = cpus_weight(r->cpu_calibration_map);
 
+    if ( tsc_is_reliable() )
+        goto skip_tsc_sync;
+
     /* Loop to get rid of cache effects on TSC skew. */
     for ( i = 4; i >= 0; i-- )
     {
@@ -1153,6 +1167,8 @@ static void time_calibration_tsc_rendezv
                 mb();
         }
     }
+
+skip_tsc_sync:
 
     rdtscll(c->local_tsc_stamp);
     c->stime_local_stamp = get_s_time();
diff -r 1d7221667204 xen/include/asm-x86/time.h
--- a/xen/include/asm-x86/time.h	Thu Oct 08 09:24:32 2009 +0100
+++ b/xen/include/asm-x86/time.h	Thu Oct 08 11:48:15 2009 -0600
@@ -43,4 +43,6 @@ uint64_t ns_to_acpi_pm_tick(uint64_t ns)
 
 void pv_soft_rdtsc(struct vcpu *v, struct cpu_user_regs *regs);
 
+int tsc_is_reliable(void);
+
 #endif /* __X86_TIME_H__ */


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

On 08/10/2009 19:05, "Dan Magenheimer"
<dan.magenheimer@oracle.com> wrote:
> Trust new architecturally-defined TSC Invariant bit (on
> Intel systems only for now, AMD TBD).
> 
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
Reworked this a bunch, so we always look to the X86_FEATURE_ bits to decide
what to do, rather than also needing to interpret a growing bunch of boot
flags.

 -- Keir



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

> > Trust new architecturally-defined TSC Invariant bit (on
> > Intel systems only for now, AMD TBD).
> > 
> > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> Reworked this a bunch, so we always look to the X86_FEATURE_ 
> bits to decide
> what to do, rather than also needing to interpret a growing 
> bunch of boot
> flags.
Eyeballed in staging, looks good, but...

If TSC is reliable, is it still necessary to rendezvous?
I thought the rendezvous was only needed if the slopes
differ (but not sure I''ve thought it all the way through).
No sense stealing all those pcpu cycles to rendezvous if
it''s not necessary, especially on a large system.
(Maybe TSC reliability IS useful for Xen, not just for
exposing to userland :-)

Also, if TSC is constant and no deep-C events have
happened (need a global counter), the rendezvous
might also be avoidable.

Dan

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

On 09/10/2009 18:47, "Dan Magenheimer"
<dan.magenheimer@oracle.com> wrote:
> If TSC is reliable, is it still necessary to rendezvous?
> I thought the rendezvous was only needed if the slopes
> differ (but not sure I''ve thought it all the way through).
> No sense stealing all those pcpu cycles to rendezvous if
> it''s not necessary, especially on a large system.
> (Maybe TSC reliability IS useful for Xen, not just for
> exposing to userland :-)
Possibly not, but I was only reworking your original patch which didn''t
avoid rendezvous either.

 -- Keir



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

> > If TSC is reliable, is it still necessary to rendezvous?
> > I thought the rendezvous was only needed if the slopes
> > differ (but not sure I''ve thought it all the way through).
> > No sense stealing all those pcpu cycles to rendezvous if
> > it''s not necessary, especially on a large system.
> > (Maybe TSC reliability IS useful for Xen, not just for
> > exposing to userland :-)
> 
> Possibly not, but I was only reworking your original patch 
> which didn''t
> avoid rendezvous either.
Actually it did (avoid rendezvous), but I have to admit
it was accidental: I was trying to avoid the write_tsc''s
not the rendezvous, and I didn''t notice the additional
implication until I was looking at the difference between
my patch and yours.

I''ll look at this more in a few days...

Dan

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

On 09/10/2009 20:03, "Dan Magenheimer"
<dan.magenheimer@oracle.com> wrote:
> Actually it did (avoid rendezvous), but I have to admit
> it was accidental: I was trying to avoid the write_tsc''s
> not the rendezvous, and I didn''t notice the additional
> implication until I was looking at the difference between
> my patch and yours.
> 
> I''ll look at this more in a few days...
If you mean your test-and-goto in time_calibration_tsc_rendezvous(), that
was just broken. You still raised the TIME_CALIBRATE_SOFTIRQ, but with
uninitialised r->master_stime. And at that point you''d already
rendezvoused
all the CPUs, so done the expensive bit anyway.

 -- Keir



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