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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.09.2023 16:02, Andrew Cooper wrote:
> 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.

I don't think it would, but that's perhaps one more of the many areas where
we take different perspectives.

Jan