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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.11.15 at 15:26, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 11/25/2015 04:55 AM, Jan Beulich wrote:
>>>>> On 25.11.15 at 00:53, <bgregg@xxxxxxxxxxx> 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" ?

No, I was actually referring to the statement in the CPUID table
in Vol 2.

> 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.

Indeed, let's not support v1 then for now and leave the exercise
to add all the if()s to whoever cares for such support.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel