Re: [PATCH 05/12] x86: Migrate spec_ctrl vendor checks to cpu_vendor()

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Thu Feb 12, 2026 at 11:49 AM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> @@ -738,11 +738,10 @@ static bool __init retpoline_calculations(void)
>>      unsigned int ucode_rev = this_cpu(cpu_sig).rev;
>>      bool safe = false;
>>  
>> -    if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
>> +    if ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
>>          return true;
>>  
>> -    if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
>> -         boot_cpu_data.family != 6 )
>> +    if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
>>          return false;
>
> At the example of this (applies throughout this patch): With the panic() in
> patch 03 the transformation looks correct. Without that panic(), or without
> being explicitly aware of it, this gives the impression of explicitly doing
> an unsafe thing:

These patches wouldn't be functional changes without the X86_ENABLED_VENDORS
mask at cpu_vendor() or the single-vendor optimisation. I could split it that
way. Introduce the cpu_vendor() macro early. Transform the entire tree, and then
apply the optimisations.

> being explicitly aware of it, this gives the impression of explicitly doing
> an unsafe thing:

Without the panic, it'd indeed be (intentionally) doing an unsafe thing.
Removing the panic can't be safe, it's not just a matter of features missing.
It'd imply printing a banner saying the current configuration is unsupported.

> Even though by way of boot_cpu_data.vendor we know what
> vendor's CPU we're on, we're acting as if we didn't know.

Not with the panic in place, which is partially why I put it there.

> I'm really
> uncertain whether such changes are worth it with the mere goal of reducing
> code size.

It is unreachable code, and in a safety context every line of unreachable code
must be either removed or justified. And justifications cost time, effort and
are difficult to maintain.

> Even beyond the concern raised, this feels like it might be
> increasing the risk of introducing subtle bugs.

I would've agreed with you with x86_vendor_is(), which is why for v1 I took a
step back to make cpu_vendor(). That macro was complex. Too complex and it took
many trial and errors to fine tune it. cpu_vendor() is comparatively trivial and
relies on the compiler doing the heavy lifting. In the single-vendor case it's
a constant and it's hopefully uncontroversially fine to remove unreachable code
then.

In the multivendor case, the complexity amounts to a mask of available vendors.

I don't think there's an inherent danger in removing unreachable code, so long
as we can prove at boot time the reachability preconditions can't be met.

>
> I wonder what Andrew and Roger think in this regard.

+1

Cheers,
Alejandro