Re: [PATCH v2 1/2] x86/VPMU: return correct fixed PMC count

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 11/25/2015 04:55 AM, Jan Beulich wrote:
>>>> On 25.11.15 at 00:53, <bgregg@netflix.com> wrote:
>> --- a/xen/arch/x86/cpu/vpmu_intel.c
>> +++ b/xen/arch/x86/cpu/vpmu_intel.c
>> @@ -166,10 +166,10 @@ static int core2_get_arch_pmc_count(void)
>>    */
>>   static int core2_get_fixed_pmc_count(void)
>>   {
>> -    u32 eax;
>> +    u32 edx;
>>   
>> -    eax = cpuid_eax(0xa);
>> -    return MASK_EXTR(eax, PMU_FIXED_NR_MASK);
>> +    edx = cpuid_edx(0xa);
>> +    return MASK_EXTR(edx, PMU_FIXED_NR_MASK);
>>   }
>>   
>>   /* edx bits 5-12: Bit width of fixed-function performance counters  */
> I'll commit as is since it's an immediate improvement, but I don't think
> this is sufficient: The SDM clearly says "if Version ID > 1", which isn't
> being tested here or in the immediately following function.

Are you referring to the statement in 18.2.2:
   "The enhanced features provided by architectural performance 
monitoring version 2 include the following" ?

I'd expect CPUID to report zeroes for those enhanced featured in v1.

However, I just noticed that various control and status registers are 
not available for v1. I wonder whether we should even support version 1 
since we'd need to add whole lot of 'if (supported)' throughout the code 
plus there are some assumptions about existence of IA32_PERF_GLOBAL_CTRL 
so we'll need to add additional logic to handle that too. And it's not 
clear to me if it's all worth it.

-boris