Re: [PATCH 8/9] x86/amd: Introduce is_zen{1,2}_uarch() predicates

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/09/2023 12:07 pm, Jan Beulich wrote:
> On 15.09.2023 17:00, Andrew Cooper wrote:
>> --- a/xen/arch/x86/include/asm/amd.h
>> +++ b/xen/arch/x86/include/asm/amd.h
>> @@ -140,6 +140,17 @@
>>                         AMD_MODEL_RANGE(0x11, 0x0, 0x0, 0xff, 0xf),  \
>>                         AMD_MODEL_RANGE(0x12, 0x0, 0x0, 0xff, 0xf))
>>  
>> +/*
>> + * The Zen1 and Zen2 microarchitectures are implemented by AMD (Fam17h) and
>> + * 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.
>> + */
>> +#define is_zen1_uarch() (!boot_cpu_has(X86_FEATURE_AMD_STIBP))
>> +#define is_zen2_uarch()   boot_cpu_has(X86_FEATURE_AMD_STIBP)
>> +
>>  struct cpuinfo_x86;
>>  int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
> With one simply the opposite of the other, and with the requirement of a
> family check up front, do we really need both? Personally I'd prefer if
> we had just the latter. Yet in any event
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

We specifically do want both, because they're use is not symmetric at
callsites.

In particular, having only one would make the following patch illogical
to read.

~Andrew