From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: patch for restricted vPMU modes Date: Mon, 23 Nov 2015 11:12:04 +0000 Message-ID: <5652F484.9010201@citrix.com> References: <565301D502000078000B7BC4@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <565301D502000078000B7BC4@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: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 23/11/15 11:08, Jan Beulich wrote: >>>> On 21.11.15 at 06:32, wrote: >> I've included the short patch below for Xen 4.6.0, which provides these >> modes (it also fixes a minor copy-and-paste error with >> core2_get_fixed_pmc_count(), which I believe was accessing the wrong >> register). I am not a veteran Xen programmer, so please feel free to edit >> or rewrite this patch. In case this email messes it up, it's also on: >> https://github.com/brendangregg/Misc/blob/master/xen/xen-4.6.0-vpmu-filter.d >> iff > Thanks for the contribution, but I'm sorry - this is not how things work. > Unless someone else want to pick this up (and perhaps even then) the > patch lacks proper attributes (like a Signed-off-by tag), should be > against -unstable instead of any released version, and I don't think > anyone's going to go grab it from a web page to apply (i.e. if you > can't get your mail client to handle it properly when inlined, attach it > in addition to inlining). > > See http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches. > >> --- xen-4.6.0-clean/xen/arch/x86/cpu/vpmu_intel.c 2015-10-05 >> 07:33:39.000000000 -0700 >> +++ xen-4.6.0-brendan/xen/arch/x86/cpu/vpmu_intel.c 2015-11-20 >> 15:29:42.571781176 -0800 >> @@ -166,10 +166,10 @@ >> */ >> 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); >> } > Without going into much detail on the actual patch, this caught my > eye: Either you're fixing a pretty blatant bug here, or this change > just can't be right. In the former case, such a fix should be > submitted as a separate patch. Blatent bug. The number of fixed function perf counters is bits 4:0 of edx. ~Andrew