Re: [PATCH 6/6] x86/apic: Convert the TSC deadline errata table to X86_MATCH_*()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/07/2025 9:31 am, Jan Beulich wrote:
> On 16.07.2025 19:31, Andrew Cooper wrote:
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -1051,64 +1051,32 @@ static void setup_APIC_timer(void)
>>      local_irq_restore(flags);
>>  }
>>  
>> -#define DEADLINE_MODEL_MATCH(m, fr) \
>> -    { .vendor = X86_VENDOR_INTEL, .family = 6, .model = (m), \
>> -      .feature = X86_FEATURE_TSC_DEADLINE, \
>> -      .driver_data = (void *)(unsigned long)(fr) }
>> +static const struct x86_cpu_id __initconst deadline_match[] = {
>> +    X86_MATCH_VFMS(INTEL_HASWELL_X,   0x2, 0x3a), /* EP */
>> +    X86_MATCH_VFMS(INTEL_HASWELL_X,   0x4, 0x0f), /* EX */
>>  
>> -static unsigned int __init hsx_deadline_rev(void)
>> -{
>> -    switch ( boot_cpu_data.x86_mask )
>> -    {
>> -    case 0x02: return 0x3a; /* EP */
>> -    case 0x04: return 0x0f; /* EX */
>> -    }
>> +    X86_MATCH_VFM (INTEL_BROADWELL_X,      0x0b000020),
>>  
>> -    return ~0U;
>> -}
>> +    X86_MATCH_VFMS(INTEL_BROADWELL_D, 0x2, 0x00000011),
>> +    X86_MATCH_VFMS(INTEL_BROADWELL_D, 0x3, 0x0700000e),
>> +    X86_MATCH_VFMS(INTEL_BROADWELL_D, 0x4, 0x0f00000c),
>> +    X86_MATCH_VFMS(INTEL_BROADWELL_D, 0x5, 0x0e000003),
> Hmm, actually - why are Broadwell and ...
>
>> -static unsigned int __init bdx_deadline_rev(void)
>> -{
>> -    switch ( boot_cpu_data.x86_mask )
>> -    {
>> -    case 0x02: return 0x00000011;
>> -    case 0x03: return 0x0700000e;
>> -    case 0x04: return 0x0f00000c;
>> -    case 0x05: return 0x0e000003;
>> -    }
>> +    X86_MATCH_VFMS(INTEL_SKYLAKE_X,   0x3, 0x01000136),
>> +    X86_MATCH_VFMS(INTEL_SKYLAKE_X,   0x4, 0x02000014),
> ... Skylake each split ...
>
>> -    return ~0U;
>> -}
>> +    X86_MATCH_VFM (INTEL_HASWELL,          0x22),
>> +    X86_MATCH_VFM (INTEL_HASWELL_L,        0x20),
>> +    X86_MATCH_VFM (INTEL_HASWELL_G,        0x17),
>>  
>> -static unsigned int __init skx_deadline_rev(void)
>> -{
>> -    switch ( boot_cpu_data.x86_mask )
>> -    {
>> -    case 0x00 ... 0x02: return ~0U;
>> -    case 0x03: return 0x01000136;
>> -    case 0x04: return 0x02000014;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>> -static const struct x86_cpu_id __initconstrel deadline_match[] = {
>> -    DEADLINE_MODEL_MATCH(0x3c, 0x22),             /* Haswell */
>> -    DEADLINE_MODEL_MATCH(0x3f, hsx_deadline_rev), /* Haswell EP/EX */
>> -    DEADLINE_MODEL_MATCH(0x45, 0x20),             /* Haswell D */
>> -    DEADLINE_MODEL_MATCH(0x46, 0x17),             /* Haswell H */
>> +    X86_MATCH_VFM (INTEL_BROADWELL,        0x25),
>> +    X86_MATCH_VFM (INTEL_BROADWELL_G,      0x17),
> ... into disjoint groups (continuing ...
>
>> -    DEADLINE_MODEL_MATCH(0x3d, 0x25),             /* Broadwell */
>> -    DEADLINE_MODEL_MATCH(0x47, 0x17),             /* Broadwell H */
>> -    DEADLINE_MODEL_MATCH(0x4f, 0x0b000020),       /* Broadwell EP/EX */
>> -    DEADLINE_MODEL_MATCH(0x56, bdx_deadline_rev), /* Broadwell D */
>> +    X86_MATCH_VFM (INTEL_SKYLAKE_L,        0xb2),
>> +    X86_MATCH_VFM (INTEL_SKYLAKE,          0xb2),
> ... here)? The patch already isn't overly straightforward to review without
> that.

The layout comes from Linux (I was mostly checking that I hadn't broken
anything), although I took the opportunity to optimise the table by
dropping useless rows.

I can't find any way of getting the diff to read nicely.  My normal
trick of reordering with a function doesn't work, although it turns out
that removing the blank lines and moving it into check_deadline_errata()
does render nicely.  I'll do that in v2.

~Andrew