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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.07.2025 19:57, Andrew Cooper wrote:
> On 17/07/2025 8:44 am, Jan Beulich wrote:
>> On 16.07.2025 19:31, Andrew Cooper wrote:
>>> --- 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)?
> 
> It looks to be an oversight.  I've submitted a separate patch, so it can
> be backported more easily.
> 
> In practice, it's covered by probe_c3_errata() which blanket disables C3
> and C6 on Nehalem.
> 
>>
>>> --- 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.
> 
> Oops, too much copy/paste.
> 
>>  For Westmere-EX I think the comment (part) would
>> better be retained, to clarify that this isn't an oversight.
> 
> I can't find anything which looks related for Westmere EX.  I'll retain
> the comment.

With the adjustments:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan