[v3,4/6] x86/time: refactor read_platform_stime()

Commit Message

Joao Martins Aug. 24, 2016, 12:43 p.m. UTC

To fetch the last read from the clocksource which was used to
calculate system_time. In the case of clocksource=tsc we will
use it to set tsc_timestamp.

Signed-off-by: Joao Martins <[e-mail]>
---
Cc: Jan Beulich <[e-mail]>
Cc: Andrew Cooper <[e-mail]>

Changes since v2:
 - s/plt_stamp_counter/plt_counter/g
 - Move conditional write of stamp out of the locked region.

Changes since v1:
 - Add missing spaces between brackets
 - Move plt_stamp_counter to read_platform_stime()
 - Add signature change in time_calibration_std_rendezvous()
---
 xen/arch/x86/time.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Jan Beulich Aug. 25, 2016, 10:17 a.m. UTC | #1

>>> On 24.08.16 at 14:43, <[e-mail]> wrote:
> To fetch the last read from the clocksource which was used to
> calculate system_time.

DYM "To allow the caller to fetch ..."?

> In the case of clocksource=tsc we will
> use it to set tsc_timestamp.

I don't see how this relates to the patch here.

The change itself looks reasonable assuming its a prereq for a
subsequent patch.

Jan

Joao Martins Aug. 26, 2016, 3:13 p.m. UTC | #2

On 08/25/2016 11:17 AM, Jan Beulich wrote:
>>>> On 24.08.16 at 14:43, <[e-mail]> wrote:
>> To fetch the last read from the clocksource which was used to
>> calculate system_time.
> 
> DYM "To allow the caller to fetch ..."?
Yeap, sounds better that way.

> 
>> In the case of clocksource=tsc we will
>> use it to set tsc_timestamp.
> 
> I don't see how this relates to the patch here.
> 
> The change itself looks reasonable assuming its a prereq for a
> subsequent patch.
OK, I can remove it, and could perhaps add this instead?

"This is a prerequisite for a subsequent patch that will use this last read."

Joao

Jan Beulich Aug. 29, 2016, 9:42 a.m. UTC | #3

>>> On 26.08.16 at 17:13, <[e-mail]> wrote:
> On 08/25/2016 11:17 AM, Jan Beulich wrote:
>>>>> On 24.08.16 at 14:43, <[e-mail]> wrote:
>>> In the case of clocksource=tsc we will
>>> use it to set tsc_timestamp.
>> 
>> I don't see how this relates to the patch here.
>> 
>> The change itself looks reasonable assuming its a prereq for a
>> subsequent patch.
> OK, I can remove it, and could perhaps add this instead?
> 
> "This is a prerequisite for a subsequent patch that will use this last 
> read."

Sounds good.

Jan

Patch

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index a03127e..57c1b47 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -575,18 +575,22 @@  static void plt_overflow(void *unused)
     set_timer(&plt_overflow_timer, NOW() + plt_overflow_period);
 }
 
-static s_time_t read_platform_stime(void)
+static s_time_t read_platform_stime(u64 *stamp)
 {
-    u64 count;
+    u64 plt_counter, count;
     s_time_t stime;
 
     ASSERT(!local_irq_is_enabled());
 
     spin_lock(&platform_timer_lock);
-    count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
+    plt_counter = plt_src.read_counter();
+    count = plt_stamp64 + ((plt_counter - plt_stamp) & plt_mask);
     stime = __read_platform_stime(count);
     spin_unlock(&platform_timer_lock);
 
+    if ( stamp )
+        *stamp = plt_counter;
+
     return stime;
 }
 
@@ -731,7 +735,7 @@  void cstate_restore_tsc(void)
     if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
         return;
 
-    write_tsc(stime2tsc(read_platform_stime()));
+    write_tsc(stime2tsc(read_platform_stime(NULL)));
 }
 
 /***************************************************************************
@@ -1050,7 +1054,7 @@  int cpu_frequency_change(u64 freq)
 
     local_irq_disable();
     /* Platform time /first/, as we may be delayed by platform_timer_lock. */
-    t->stamp.master_stime = read_platform_stime();
+    t->stamp.master_stime = read_platform_stime(NULL);
     curr_tsc = rdtsc_ordered();
     /* TSC-extrapolated time may be bogus after frequency change. */
     /*t->stamp.local_stime = get_s_time_fixed(curr_tsc);*/
@@ -1355,7 +1359,7 @@  static void time_calibration_tsc_rendezvous(void *_r)
 
             if ( r->master_stime == 0 )
             {
-                r->master_stime = read_platform_stime();
+                r->master_stime = read_platform_stime(NULL);
                 r->master_tsc_stamp = rdtsc_ordered();
             }
             atomic_inc(&r->semaphore);
@@ -1395,7 +1399,7 @@  static void time_calibration_std_rendezvous(void *_r)
     {
         while ( atomic_read(&r->semaphore) != (total_cpus - 1) )
             cpu_relax();
-        r->master_stime = read_platform_stime();
+        r->master_stime = read_platform_stime(NULL);
         smp_wmb(); /* write r->master_stime /then/ signal */
         atomic_inc(&r->semaphore);
     }
@@ -1434,7 +1438,7 @@  void time_latch_stamps(void)
     unsigned long flags;
 
     local_irq_save(flags);
-    ap_bringup_ref.master_stime = read_platform_stime();
+    ap_bringup_ref.master_stime = read_platform_stime(NULL);
     ap_bringup_ref.local_tsc = rdtsc_ordered();
     local_irq_restore(flags);
 
@@ -1452,7 +1456,7 @@  void init_percpu_time(void)
     t->tsc_scale = per_cpu(cpu_time, 0).tsc_scale;
 
     local_irq_save(flags);
-    now = read_platform_stime();
+    now = read_platform_stime(NULL);
     tsc = rdtsc_ordered();
     local_irq_restore(flags);