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:23, 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.
> 
>>
>>> -static const struct x86_cpu_id __initconstrel deadline_match[] = {
>> Seeing this transformation ...
>>
>>>  static void __init check_deadline_errata(void)
>>>  {
>>> +    static const struct x86_cpu_id __initconst deadline_match[] = {
>> ... of the section placement, we may want to investigate whether with the
>> toolchain baseline bump we can actually do away with __initconstrel, using
>> __initconst uniformly everywhere.
> 
> To be honest, I'm not even sure why we needed the split in the first
> place.  We merge both sections together, so it isn't about section
> attributes.

It is about section attributes, but at assembly time. Even an up-to-date
gas will choke on certain conflicting section attributes, when multiple
section "declarations" are present. (Oddly enough I did fiddle with that
code earlier in the day, hence why I have a fresh impression of this
error appearing in practice.)

When you have only constant data (no relocations), the compiler ought to
request an "a" section, whereas when there are relocations it would
request an "aw" one (along the lines of why there is .data.rel.ro). Some
gcc versions and/or some gas versions conflicted in how custom
(__attribute__((section(...)))) sections would have their attributes
specified, causing assembly to fail.

> But, if you think it's safe to remove, it will definitely be a good
> amplification.

As to "think" - I'm not sure, but my recollection is that the issue was
with some gcc 4.x only (or binutils from that time frame).

Jan