From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v2 1/2] x86/VPMU: return correct fixed PMC count Date: Wed, 25 Nov 2015 09:26:17 -0500 Message-ID: <5655C509.1040205@oracle.com> References: <1448409192-4914-1-git-send-email-bgregg@netflix.com> <1448409192-4914-2-git-send-email-bgregg@netflix.com> <565593A602000078000B8DF3@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <565593A602000078000B8DF3@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Brendan Gregg Cc: dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 11/25/2015 04:55 AM, Jan Beulich wrote: >>>> On 25.11.15 at 00:53, 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