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
