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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Feb 10, 2026 at 11:04:59AM +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@xxxxxxxx>
> ---
> 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.

Hm, what I've done lately with {un}likely() is not attempting to
optimize for the most taken path, but rather use it to force the
compiler to optimize the fast path, if the function has one.  The slow
path will be slow anyway, and hence any compiler optimization should
be towards making the fast path possibly faster IMO.

This function doesn't seem to have any fast (or slow) paths, so I
would leave it as-is.

> 
> --- 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;
> +    int64_t delta = 0;

Why do you make delta a signed integer, the value returned by
scale_delta() is unsigned.

>  
> -    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

The capping might want mentioning in the function prototype, as maybe
new users expect stime2tsc() to return TSC values from times in the
past.

Otherwise LGTM.

Thanks, Roger.