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@citrix.com>
---
 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. */
 	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 ) )
+		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
 
 	if ( opt_arat &&
 	     ( c->cpuid_level >= 0x00000006 ) &&
-- 
2.51.1

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@citrix.com>
> ---
>  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