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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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" ?
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

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