[PATCH v2] x86/time: adjust handling of negative delta in stime2tsc()

[PATCH v2] x86/time: adjust handling of negative delta in stime2tsc()

[Jan Beulich]

When we cap negative values to 0 (see code comment as to why), going
through scale_delta() is pointless - it'll return 0 anyway. Therefore make
the call conditional (and then also the one to scale_reciprocal()), adding
a comment as to why there is this capping.

Modernize types used while there, and switch to usiong initializers for
the local variables.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Adding likely() to the conditional here does make a difference. Question
is whether to do so, seeing that there looks to be a possibility (of
unknown frequency) for the delta to be non-positive.
---
v2: Make delta unsigned. Add comment to function decl. Make the function
    decl also use uint64_t.

--- a/xen/arch/x86/include/asm/time.h
+++ b/xen/arch/x86/include/asm/time.h
@@ -50,7 +50,12 @@ void force_update_vcpu_system_time(struc
 
 bool clocksource_is_tsc(void);
 int host_tsc_is_safe(void);
-u64 stime2tsc(s_time_t stime);
+
+/*
+ * Note: This function "caps" times ahead of the local CPU's stime stamp,
+ * supplying the corresponding TSC stamp in that case.
+ */
+uint64_t stime2tsc(s_time_t stime);
 
 struct time_scale;
 void set_time_scale(struct time_scale *ts, u64 ticks_per_sec);
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1176,20 +1176,26 @@ uint64_t __init calibrate_apic_timer(voi
     return elapsed * CALIBRATE_FRAC;
 }
 
-u64 stime2tsc(s_time_t stime)
+uint64_t stime2tsc(s_time_t stime)
 {
-    struct cpu_time *t;
-    struct time_scale sys_to_tsc;
-    s_time_t stime_delta;
+    const struct cpu_time *t = &this_cpu(cpu_time);
+    s_time_t stime_delta = stime - t->stamp.local_stime;
+    uint64_t delta = 0;
 
-    t = &this_cpu(cpu_time);
-    sys_to_tsc = scale_reciprocal(t->tsc_scale);
+    /*
+     * While for reprogram_timer() the capping at 0 isn't relevant (the returned
+     * value is likely in the past anyway then, by the time it is used), for
+     * cstate_restore_tsc() this is relevant: We need to avoid moving the TSC
+     * backwards (relative to when it may last have been read).
+     */
+    if ( stime_delta > 0 )
+    {
+        struct time_scale sys_to_tsc = scale_reciprocal(t->tsc_scale);
 
-    stime_delta = stime - t->stamp.local_stime;
-    if ( stime_delta < 0 )
-        stime_delta = 0;
+        delta = scale_delta(stime_delta, &sys_to_tsc);
+    }
 
-    return t->stamp.local_tsc + scale_delta(stime_delta, &sys_to_tsc);
+    return t->stamp.local_tsc + delta;
 }
 
 void cstate_restore_tsc(void)

Re: [PATCH v2] x86/time: adjust handling of negative delta in stime2tsc()

[Roger Pau Monné]

On Fri, Mar 27, 2026 at 03:39:42PM +0100, Jan Beulich wrote:
> When we cap negative values to 0 (see code comment as to why), going
> through scale_delta() is pointless - it'll return 0 anyway. Therefore make
> the call conditional (and then also the one to scale_reciprocal()), adding
> a comment as to why there is this capping.
> 
> Modernize types used while there, and switch to usiong initializers for
> the local variables.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.