On 13/10/2023 1:26 am, Alejandro Vallejo wrote: > It slightly simplifies the code that uses them at no real cost because the > code is very rarely executed. This makes it harder to confuse zen uarches > due to missing or mistaken family checks.
I'm afraid I disagree. It's bogus to do a family check without a vendor check. By making this change, you're separating (spatially in code, and therefore cognitively) the two checks that it's important to be able to cross-check. > diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h > index d862cb7972..5a40bcc2ba 100644 > --- a/xen/arch/x86/include/asm/amd.h > +++ b/xen/arch/x86/include/asm/amd.h > @@ -145,11 +145,12 @@ > * Hygon (Fam18h) but without simple model number rules. Instead, use STIBP > * as a heuristic that distinguishes the two. > * > - * The caller is required to perform the appropriate vendor/family checks > - * first. > + * The caller is required to perform the appropriate vendor check first. > */ > -#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP)) > -#define is_zen2_uarch() boot_cpu_has(X86_FEATURE_AMD_STIBP) > +#define is_zen1_uarch() ((boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == > 0x18) && \ > + !boot_cpu_has(X86_FEATURE_AMD_STIBP)) > +#define is_zen2_uarch() (boot_cpu_data.x86 == 0x17 && \ > + boot_cpu_has(X86_FEATURE_AMD_STIBP)) What leads you to believe there aren't Hygon Zen2's ? ~Andrew
