Re: [PATCH 1/2] x86/time: use native TSC scaling factors when TSC is not scaled

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2026 12:33, Roger Pau Monne wrote:
> When running HVM guest in native TSC mode avoid using the recalculated vTSC
> scaling factors based on the cpu_khz value.  Using the kHz based frequency
> leads to the TSC scaling values possibly not being the same as the ones
> used by the per CPU cpu_time->tsc_scale field, which introduces skew
> between the guest and Xen's calculations of the system time.
> 
> On a 2gHz system, where the frequency is possibly detected as 1999999999Hz
> (note this is a worse-case scenario), the cpu_khz variable will be set to
> 1999999kHz, and hence 999Hz cycles will be not accounted for per second.
> Over a second (the time synchronization period), this leads to a skew of:
> 
> cycles * 1 / (Hz freq) = 999 / 1999999999 = 499,5ns
> 
> So far this has gone unnoticed because the time synchronization rendezvous
> forces the update of the tsc_timestamp and system_time fields in the vCPU
> time info area, and hence the skew only accumulates up to the rendezvous
> period.  Attempting to remove the rendezvous causes the skew to grow
> unbounded.
> 
> Fix by using the native TSC scaling values (as used by Xen) when the guest
> TSC is not scaled.
> 
> Fixes: eab8a90be723 ("x86/time: scale host TSC in pvclock properly")
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> I'm worried about the usage of cpu_khz beyond simple printing it for
> informational purposes.  Overall I think it would be safer to store the
> frequency in Hz, as to avoid losing the least significant digits.
> 
> In any case, that's a different change.

I'm not quite sure - improving accuracy is of course a good thing, but will
we ever be able to do any such calculations error free, when already the
detected frequency isn't exactly precise?

> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1710,17 +1710,25 @@ static void collect_time_info(const struct vcpu *v,
>      else
>      {
>          if ( is_hvm_domain(d) && hvm_tsc_scaling_supported )
> -        {
>              tsc_stamp            = hvm_scale_tsc(d, t->stamp.local_tsc);

This is a potentially imprecise calculation. How likely is it that its result
will indeed ...

> -            u->tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
> -            u->tsc_shift         = d->arch.vtsc_to_ns.shift;
> -        }
>          else
> -        {
>              tsc_stamp            = t->stamp.local_tsc;
> +
> +        /*
> +         * HVM guests using the native TSC ratio should use the same per-CPU
> +         * scaling factors as Xen.  This ensures time keeping is always in 
> sync
> +         * between Xen and the guest.
> +         */
> +        if ( tsc_stamp == t->stamp.local_tsc )

... exactly match t->stamp.local_tsc? Don't we possibly need a (small) error
margin? (In which case of course the next question would be: How to establish
such a margin?)

Jan

> +        {
>              u->tsc_to_system_mul = t->tsc_scale.mul_frac;
>              u->tsc_shift         = t->tsc_scale.shift;
>          }
> +        else
> +        {
> +            u->tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
> +            u->tsc_shift         = d->arch.vtsc_to_ns.shift;
> +        }
>      }
>  
>      u->tsc_timestamp = tsc_stamp;