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 <[email protected]>
> ---
> 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
