On 27.01.2022 00:30, Andrew Cooper wrote:
> On 20/01/2022 14:17, Jan Beulich wrote:
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -195,6 +195,11 @@ static const char *const str_e21a[32] =
>> [ 6] = "nscb",
>> };
>>
>> +static const char *const str_7b1[32] =
>> +{
>> + [ 0] = "ppin",
>> +};
>
> I hadn't realised what a mess we had with the prefixes.
>
> We have AMD_PPIN rendered as simply "ppin", while we also have
> AMD_{STIBP,SSBD} which are rendered with an amd- prefix. This patch is
> the first introduction of an INTEL_ prefixed feature.
>
> We should figure out a consistency rule and fix the logic, before adding
> more confusion.
>
> Given the AMD MSR_SPEC_CTRL series just posted, use of CPUID bits will
> often be symmetrical and it's awkward to have one or with a prefix and
> the other without.
IOW you suggest I add a kind-of-prereq patch to drop the prefixes?
>> --- a/xen/include/xen/lib/x86/cpuid.h
>> +++ b/xen/include/xen/lib/x86/cpuid.h
>> @@ -16,6 +16,7 @@
>> #define FEATURESET_7d0 9 /* 0x00000007:0.edx */
>> #define FEATURESET_7a1 10 /* 0x00000007:1.eax */
>> #define FEATURESET_e21a 11 /* 0x80000021.eax */
>> +#define FEATURESET_7b1 12 /* 0x00000007:1.ebx */
>>
>> struct cpuid_leaf
>> {
>> @@ -188,6 +189,10 @@ struct cpuid_policy
>> uint32_t _7a1;
>> struct { DECL_BITFIELD(7a1); };
>> };
>> + union {
>> + uint32_t _7b1;
>> + struct { DECL_BITFIELD(7b1); };
>> + };
>> };
>> } feat;
>>
>>
>
> Looking at a related patch I've got, at a minimum, you also need:
> * collect the leaf in generic_identify()
I'll need to make a patch first to collect 7a1, as it seems. It was
actually 7a1 that I used as a reference, iirc.
> * extend cpuid_policy_to_featureset() and cpuid_featureset_to_policy()
Yeah, I missed those. Presumably by looking for instances only under
arch/x86/. Especially with per-arch include/ now living under
arch/<arch>/, having separate x86 bits elsewhere is a little unhelpful.
> However I've got an idea to help us split "add new leaf" from "first bit
> in new leaf" which I'd like to experiment with first. It is rather
> awkward having the two mostly-unrelated changes forced together in a
> single patch.
I'll make the necessary adjustments here anyway. I can always re-base
on top of what you may come up with.
Jan
