On 18.01.2022 13:18, Andrew Cooper wrote:
> On 17/01/2022 15:30, Jan Beulich wrote:
>> As of SDM revision 076 there is a CPUID bit for this functionality. Use
>> it to amend the existing model-based logic.
>>
>> Signed-off-by: Jan Beulich <[email protected]>
>> ---
>> In xen-cpuid.c I wasn't sure whether it's better to put the 7b1 table
>> next to the 7a1 one, or whether tables should continue to be placed by
>> feature set ABI identifier.
>
> They're in FEATURESET_* order, which is also chronological order.
> str_7b1 wants to be after str_e21a.
I can see the ordering being necessary for decodes[], but I have a
hard time seeing why there would be any strict need for a particular
ordering model for the str_...[] objects; there it would seem
slightly more natural to me to have adjacent registers / leaves
next to each other. But since I don't care that much, I'll switch.
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -156,7 +156,7 @@ static const char *const str_e8b[32] =
>> [18] = "ibrs-fast", [19] = "ibrs-same-mode",
>>
>> [20] = "no-lmsl",
>> - /* [22] */ [23] = "ppin",
>> + /* [22] */ [23] = "amd-ppin",
>
> We don't retrofit names like this. If we did, loads of the speculation
> bits would need to change.
>
> The Intel vs AMD split is clear from the leaf index, and the only reason
> we have the distinction is to fit the two bits into the same namespace.
In which case are you also asking to make the new leave simply say
"ppin"?
> Please leave this as was, to match its name in the source code.
By match you mean what? Hardly
XEN_CPUFEATURE(AMD_PPIN, 8*32+23) /* Protected Processor Inventory
Number */
?
>> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
>> @@ -865,6 +865,13 @@ static void intel_init_ppin(const struct
>> {
>> uint64_t val;
>>
>> + default:
>
> Considering the comment above this switch statement, which you haven't
> edited at all, this wants a note saying that a CPUID bit was added in
> Sapphire Rapids, but older CPUs still require model-specific enumeration.
Oh, yes, I should have paid attention to that comment. Will edit.
Jan
