RE: [Xen-devel] [PATCH] Adjust time init sequence

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>From: Tian, Kevin
>Sent: Thursday, December 11, 2008 8:47 AM
>>
>>init_platform_time() -> plt_overflow() -> NOW()
>>
>>Perhaps the above is safe though? Will NOW() return zero for an
>>uninitialised per-cpu time sstructure (since stime_local_stamp 
>>and tsc_scale
>>are both zero)?
>>
>
>I guess not, due to same reason as why I sent out 1st patch idle 
>vcpu state entry. The point is the current TSC value, which count 
>from power on and is translated to a dozens of seconds for elapsed
>time upon a zero tsc stamp. :-( I didn't realize that point in 
>the start...
>

The net effect of first plt_overflow is to take current count
of platform time as init stamps, and thus following simple
change IMO would be acceptable. Of course this leaves to
you to decide whether it's worthy change. :-)

----

Adjust time init sequence

percpu timer init for BSP happens within init_xen_time,
while CMOS access at end may take up to 1s. This may
make 1st trigger to calibration timer happens >1s interval
and elapsed local stime for BSP also exceed 1s. However
percpu timer init happens after that point for APs, which
will then have a <1s elapsed local stime at 1st calibration.
That leads to distinct mul_frac among cores, which can
cause up to 6ms system time skew in the start. This is
not big issue, since gradually xen calibration framework
closes that gap. But it's still better to avoid that big
skew in early boot phase.

Signed-off-by Kevin Tian <kevin.tian@xxxxxxxxx>

diff -r f8ccab7036c7 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c       Wed Dec 10 16:48:10 2008 -0500
+++ b/xen/arch/x86/time.c       Wed Dec 10 17:39:27 2008 -0500
@@ -656,10 +656,9 @@
         1ull << (pts->counter_bits-1), &plt_scale);
     init_timer(&plt_overflow_timer, plt_overflow, NULL, 0);
     plt_src = *pts;
-    plt_overflow(NULL);
 
-    platform_timer_stamp = plt_stamp64;
-
+    plt_stamp = pts->read_counter();
+    platform_timer_stamp = plt_stamp64 = (plt_stamp & plt_mask);
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);
 }
@@ -1109,7 +1108,7 @@
 
     local_irq_save(flags);
     rdtscll(t->local_tsc_stamp);
-    now = !plt_src.read_counter ? 0 : read_platform_stime();
+    now = read_platform_stime();
     local_irq_restore(flags);
 
     t->stime_master_stamp = now;
@@ -1132,13 +1131,15 @@
 
     open_softirq(TIME_CALIBRATE_SOFTIRQ, local_time_calibration);
 
-    init_percpu_time();
+    do_settime(get_cmos_time(), 0, 0);
 
     stime_platform_stamp = 0;
     init_platform_timer();
 
-    do_settime(get_cmos_time(), 0, NOW());
+    init_percpu_time();
 
+    /* Make sure NOW used after percpu time init */
+    set_timer(&plt_overflow_timer, NOW() + plt_overflow_period);
     return 0;
 }

Attachment: time_init_seq.patch
Description: time_init_seq.patch

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