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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.07.2025 12:55, Andrew Cooper wrote:
> On 18/07/2025 11:23 am, Andrew Cooper wrote:
>> On 18/07/2025 11:19 am, Jan Beulich wrote:
>>> On 18.07.2025 12:07, Andrew Cooper wrote:
>>>> With the ability to match on steppings, introduce a new X86_MATCH_VFMS()
>>>> helper to match a specific stepping, and use it to rework deadline_match[].
>>>>
>>>> Notably this removes the overloading of driver_data possibly being a 
>>>> function
>>>> pointer, and removes the latent bug where the target functions are missing
>>>> ENDBR instructions owing to the lack of the cf_check attribute.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Thanks.
> 
> Actually, this isn't as no-functional-change as I thought.
> 
> X86_FEATURE_TSC_DEADLINE has been swapped for X86_FEATURE_ANY in the table.
> 
> check_deadline_errata() is called unconditionally, without checking for
> TSC_DEADLINE, yet the rows in the table are the CPUs for which an
> erratum is known, so they all have the feature.
> 
> It does make a difference if e.g. one were to boot with
> cpuid=no-tsc-deadline.  Previously we'd have exited early, while now
> we'll emit the warning.
> 
> We could switch back to using TSC_DEADLINE (requiring a more complicated
> X86_MATCH_*() wrapper), although a better option would be to predicate
> the call to check_deadline_errata() with a feature check, because it's a
> much more recent addition to AMD CPUs, and there's no point searching
> the errata list on CPUs which lack the feature.

To be honest in this case I'd be fine with either adjustment. Switching the
feature back is more consistent with the overall purpose of X86_MATCH_*(),
but as you say a table with every entry having the same feature named isn't
very useful to go through when the feature isn't there.

One option to keep things table based, yet still avoiding to run through
the entire table, would be to allow for a "negative" entry (which here
would simply be placed first in the table).

Jan