Re: [PATCH RFC 05/31] xen/x86: Collect more CPUID feature words

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/12/15 16:46, Jan Beulich wrote:
>>>> On 16.12.15 at 22:24, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/cpu/amd.c
>> +++ b/xen/arch/x86/cpu/amd.c
>> @@ -481,7 +481,7 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
>>  
>>  	if (c->extended_cpuid_level >= 0x80000007) {
>>  		c->x86_power = cpuid_edx(0x80000007);
>> -		if (c->x86_power & (1<<8)) {
>> +		if (c->x86_power & cpufeat_mask(X86_FEATURE_ITSC)) {
> generic_identify() has already run by the time we get here, so no
> need to re-read cpuid_edx() (interestingly enough you leverage
> this in init_intel()).

I didn't change either in this regard (observe that it is just in
context), although it is now obvious that x86_power can be dropped
completely, given that isn't used anywhere else.

andrewcoop@andrewcoop:/local/xen.git/xen$ git grep x86_power
arch/x86/cpu/amd.c:541:         c->x86_power = cpuid_edx(0x80000007);
arch/x86/cpu/amd.c:542:         if (c->x86_power &
cpufeat_mask(X86_FEATURE_ITSC)) {
include/asm-x86/processor.h:194:    int  x86_power;


>
>> --- a/xen/arch/x86/cpuid/cpuid.c
>> +++ b/xen/arch/x86/cpuid/cpuid.c
>> @@ -103,6 +103,12 @@ const uint32_t known_features[XEN_NR_FEATURESET_ENTRIES] =
>>                                              cpufeat_mask(X86_FEATURE_RDSEED)     |
>>                                              cpufeat_mask(X86_FEATURE_ADX)        |
>>                                              cpufeat_mask(X86_FEATURE_SMAP)),
>> +
>> +    [cpufeat_word(X86_FEATURE_PREFETCHWT1)] = (cpufeat_mask(X86_FEATURE_PREFETCHWT1)),
>> +
>> +    [cpufeat_word(X86_FEATURE_ITSC)] = (cpufeat_mask(X86_FEATURE_ITSC)),
>> +
>> +    [cpufeat_word(X86_FEATURE_CLZERO)] = (cpufeat_mask(X86_FEATURE_CLZERO)),
>>  };
> Are these indeed the only ones in their groups that can be safely
> marked "known"?

I am going to go out on a limb and say yes here.  The point of "known"
is "someone has actively thought about what the hyperivsor implications
of using this feature is".  These are the only ones which I feel safe
declaring to be such at the time of writing.

Rebasing onto staging has introduced the PKU bits, which are adjacent to
PREFETCHWT1, and altered this patch quite a lot.

~Andrew