Re: [PATCH 4/6] x86: Convert users of INTEL_FAM6_MODEL() to X86_MATCH_VFM()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.07.2025 19:31, Andrew Cooper wrote:
> This replaces raw model numbers (and comments in some cases) with names.  For
> probe_mwait_errata(), merge the comments with the table to make it easier to
> see which erratum is which, and drop a stray "Problem" in LNL030.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> There isn't an obvious helper in Linux to use which can avoid the data
> parameter; Linux doesn't really do lists like this which are *just* a VFM
> match with nothing else.
> 
> Either way, I've opted for a few extra NULLs than a proliferation of
> X86_MATCH_*() helpers.

+1

> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -583,7 +583,6 @@ bool errata_c6_workaround(void)
>  
>      if ( unlikely(fix_needed == -1) )
>      {
> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>          /*
>           * Errata AAJ72: EOI Transaction May Not be Sent if Software Enters
>           * Core C6 During an Interrupt Service Routine"
> @@ -594,12 +593,12 @@ bool errata_c6_workaround(void)
>           * there is an EOI pending.
>           */
>          static const struct x86_cpu_id eoi_errata[] = {
> -            INTEL_FAM6_MODEL(0x1a),
> -            INTEL_FAM6_MODEL(0x1e),
> -            INTEL_FAM6_MODEL(0x1f),
> -            INTEL_FAM6_MODEL(0x25),
> -            INTEL_FAM6_MODEL(0x2c),
> -            INTEL_FAM6_MODEL(0x2f),
> +            X86_MATCH_VFM(INTEL_NEHALEM_EP,   NULL),
> +            X86_MATCH_VFM(INTEL_NEHALEM,      NULL),
> +            X86_MATCH_VFM(INTEL_NEHALEM_G,    NULL),
> +            X86_MATCH_VFM(INTEL_WESTMERE,     NULL),
> +            X86_MATCH_VFM(INTEL_WESTMERE_EP,  NULL),
> +            X86_MATCH_VFM(INTEL_WESTMERE_EX,  NULL),
>              { }
>          };

Along the lines of a comment further down, maybe make explicit that Nehalem-EX
is intentionally omitted here (assuming that's not in fact an oversight)?

> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -382,16 +382,12 @@ static void cf_check early_init_intel(struct 
> cpuinfo_x86 *c)
>   */
>  static void probe_c3_errata(const struct cpuinfo_x86 *c)
>  {
> -#define INTEL_FAM6_MODEL(m) { X86_VENDOR_INTEL, 6, m, X86_FEATURE_ALWAYS }
>      static const struct x86_cpu_id models[] = {
> -        /* Nehalem */
> -        INTEL_FAM6_MODEL(0x1a),
> -        INTEL_FAM6_MODEL(0x1e),
> -        INTEL_FAM6_MODEL(0x1f),
> -        INTEL_FAM6_MODEL(0x2e),
> -        /* Westmere (note Westmere-EX is not affected) */
> -        INTEL_FAM6_MODEL(0x2c),
> -        INTEL_FAM6_MODEL(0x25),
> +        X86_MATCH_VFM(INTEL_NEHALEM_EP,   NULL),
> +        X86_MATCH_VFM(INTEL_NEHALEM,      NULL),
> +        X86_MATCH_VFM(INTEL_NEHALEM_G,    NULL),
> +        X86_MATCH_VFM(INTEL_WESTMERE,     NULL),
> +        X86_MATCH_VFM(INTEL_WESTMERE_EP,  NULL),
>          { }
>      };

You lost NEHALEM_EX here. For Westmere-EX I think the comment (part) would
better be retained, to clarify that this isn't an oversight.

Jan