Re: [PATCH] x86: Limit the non-architectural constant TSC model checks

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Wed Dec 3, 2025 at 5:06 PM CET, Kevin Lampis wrote:
> There are some older Intel CPUs which have CONSTANT_TSC behavior but
> don't advertise it through CPUID. This change replaces the previous
> open-ended check with a definitive range to make it clear that this only
> applies to a specific set of CPUs and that later CPUs like Family 18+
> won't need to be included.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
> ---
>  xen/arch/x86/cpu/intel.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 2bb9956a79..1c37179bc5 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -673,15 +673,15 @@ static void cf_check init_intel(struct cpuinfo_x86 *c)
>       /* Work around errata */
>       Intel_errata_workarounds(c);
>  
> -     if ( ( c->family == 15 && c->model >= 0x03 ) ||
> -          ( c->family == 6 && c->model >= 0x0e ) )
> -             __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> -
> +     /* Use a model specific check for some older CPUs that have
> +      * constant TSC but may not report it via CPUID. */

nit:  This comment, or some variation of it, should (imo) be inside the branch
you add instead. Also, multiline comments follow Xen style elsewhere in
the file.

>       if (cpu_has(c, X86_FEATURE_ITSC)) {
>               __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
>               __set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
>               __set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
> -     }
> +     } else if ( ( c->vfm >= INTEL_P4_PRESCOTT && c->vfm <= 
> INTEL_P4_CEDARMILL ) ||
> +                 ( c->vfm >= INTEL_CORE_YONAH && c->vfm <= INTEL_IVYBRIDGE ) 
> )

I understand this is code motion + macro usage, but it might be worth gating
everything by !cpu_has(c, X86_FEATURE_HYPERVISOR). Otherwise you risk getting
your assumptions wrong when running virtualised.

This might be QEMU TCG, or other shenanigans with live migration when running
virtualised and subject to guest-unaware live-migrations guest-unaware live
migration.

IOW: If you're running virtualised and you have no ITSC it's probably because
your hypervisor didn't want you making assumptions about it based on fam/model
checks.

Not that any CSP would use those processors, but my point stands.

Cheers,
Alejandro