Re: [PATCH v3] x86: Remove x86 prefixed names from cpuinfo for intel.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/11/2025 7:19 pm, Kevin Lampis wrote:
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 6f71365b7e..ff20f5667f 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -137,28 +138,29 @@ static void __init probe_masking_msrs(void)
>       unsigned int exp_msr_basic, exp_msr_ext, exp_msr_xsave;
>  
>       /* Only family 6 supports this feature. */
> -     if (c->x86 != 6)
> +     if (c->family != 6)
>               return;

The f in vfm is Family.  This condition should be dropped, and the
switch() gain a default which simply returns.

> @@ -265,7 +267,7 @@ static void __init noinline intel_init_levelling(void)
>        * so skip it altogether. In the case where Xen is virtualized these
>        * MSRs may be emulated though, so we allow it in that case.
>        */
> -     if ((boot_cpu_data.x86 != 0xf || cpu_has_hypervisor) &&
> +     if ((boot_cpu_data.family != 0xf || cpu_has_hypervisor) &&

Everywhere else, you're turning 0xf into 15.

I've pushed an interim
https://gitlab.com/xen-project/hardware/xen-staging/-/commits/andrew/intel-vfm
which has the style corrections and these two fixes.

> @@ -657,14 +670,16 @@ static void cf_check init_intel(struct cpuinfo_x86 *c)
>       /* Work around errata */
>       Intel_errata_workarounds(c);
>  
> -     if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> -             (c->x86 == 0x6 && c->x86_model >= 0x0e))
> +     if ( ( c->family == 15 && c->model >= 0x03 ) ||
> +          ( c->family == 6 && c->model >= 0x0e ) )
>               __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +

This is logic that becomes buggy with Fam 18/19.

https://git.kernel.org/tip/fadb6f569b10bf668677add876ed50586931b8f3 was
the Linux fix, which will want adapting as a prerequisite fix.

>       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);
>       }

Note that Linux's opencoded "c->x86_power & (1 << 8)" is really the ITSC
check above.

I'm about to commit the resync of intel-family.h so that will rebase out
shortly.

~Andrew