On 2/11/2025 1:09 PM, Dave Hansen wrote: > On 2/11/25 11:44, Sohil Mehta wrote: >> + if (c->x86_vendor == X86_VENDOR_INTEL && >> + ((c->x86_vfm >= INTEL_PENTIUM_PRO && c->x86_vfm <= INTEL_PENTIUM_M_DOTHAN) || >> + (c->x86_vfm >= INTEL_P4_WILLAMETTE && c->x86_vfm <= INTEL_P4_CEDARMILL))) { > > Since these are both closed checks and not open-ended, is the > > if (c->x86_vendor == X86_VENDOR_INTEL && > > bit needed or superfluous? > You are right, since it is close ended on both sides we should be able to remove the X86_VENDOR_INTEL. I was thinking if we should leave it there to avoid confusion. But, INTEL_* in the VFM string is a good enough hint that the checks are Intel specific. Also, it's not like this check is going to be modified frequently. > Also, super nit, can you vertically align the two range checks, please? > > ((c->x86_vfm >= INTEL_PENTIUM_PRO && c->x86_vfm <= > INTEL_PENTIUM_M_DOTHAN) || > (c->x86_vfm >= INTEL_P4_WILLAMETTE && c->x86_vfm <= > INTEL_P4_CEDARMILL))) { > >