From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqkGg-0007KG-Ro for qemu-devel@nongnu.org; Mon, 26 Oct 2015 12:05:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZqkGc-00037Q-Qx for qemu-devel@nongnu.org; Mon, 26 Oct 2015 12:05:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51231) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqkGc-00036k-KS for qemu-devel@nongnu.org; Mon, 26 Oct 2015 12:05:02 -0400 Date: Mon, 26 Oct 2015 17:04:57 +0100 From: Andrew Jones Message-ID: <20151026160457.GH30256@hawk.localdomain> References: <1444662470-13045-1-git-send-email-cov@codeaurora.org> <1445873930-9058-1-git-send-email-cov@codeaurora.org> <1445873930-9058-3-git-send-email-cov@codeaurora.org> <20151026155843.GG30256@hawk.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151026155843.GG30256@hawk.localdomain> Subject: Re: [Qemu-devel] [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christopher Covington Cc: wei@redhat.com, alindsay@codeaurora.org, kvm@vger.kernel.org, croberts@codeaurora.org, qemu-devel@nongnu.org, alistair.francis@xilinx.com, shannon.zhao@linaro.org, kvmarm@lists.cs.columbia.edu On Mon, Oct 26, 2015 at 04:58:43PM +0100, Andrew Jones wrote: > On Mon, Oct 26, 2015 at 11:38:49AM -0400, Christopher Covington wrote: > > Ensure that reads of the PMCCNTR_EL0 are monotonically increasing, > > even for the smallest delta of two subsequent reads. > > > > Signed-off-by: Christopher Covington > > --- > > arm/pmu.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > > > diff --git a/arm/pmu.c b/arm/pmu.c > > index 42d0ee1..c44d708 100644 > > --- a/arm/pmu.c > > +++ b/arm/pmu.c > > @@ -14,6 +14,8 @@ > > */ > > #include "libcflat.h" > > > > +#define NR_SAMPLES 10 > > + > > #if defined(__arm__) > > static inline uint32_t get_pmcr(void) > > { > > @@ -22,6 +24,25 @@ static inline uint32_t get_pmcr(void) > > asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret)); > > return ret; > > } > > + > > +static inline void set_pmcr(uint32_t pmcr) > > +{ > > + asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr)); > > +} > > + > > +/* > > + * While PMCCNTR can be accessed as a 64 bit coprocessor register, returning 64 > > + * bits doesn't seem worth the trouble when differential usage of the result is > > + * expected (with differences that can easily fit in 32 bits). So just return > > + * the lower 32 bits of the cycle count in AArch32. > > + */ > > +static inline unsigned long get_pmccntr(void) > > +{ > > + unsigned long cycles; > > + > > + asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles)); > > + return cycles; > > +} > > #elif defined(__aarch64__) > > static inline uint32_t get_pmcr(void) > > { > > @@ -30,6 +51,19 @@ static inline uint32_t get_pmcr(void) > > asm volatile("mrs %0, pmcr_el0" : "=r" (ret)); > > return ret; > > } > > + > > +static inline void set_pmcr(uint32_t pmcr) > > +{ > > + asm volatile("msr pmcr_el0, %0" : : "r" (pmcr)); > > +} > > + > > +static inline unsigned long get_pmccntr(void) > > +{ > > + unsigned long cycles; > > + > > + asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles)); > > + return cycles; > > +} > > #endif > > > > struct pmu_data { > > @@ -72,11 +106,37 @@ static bool check_pmcr(void) > > return pmu.implementer != 0; > > } > > > > +/* > > + * Ensure that the cycle counter progresses between back-to-back reads. > > + */ > > +static bool check_cycles_increase(void) > > +{ > > + struct pmu_data pmu = { {0} }; > > One set of {} is enough, and looks better. Ah, just read your cover letter and now see that this was done on purpose. So your compiler complains about {0}? Is there a problem besides the warning? If not, then I'm still a bit inclined to keep the code neat. The warnings will go away with compiler updates. Thanks, drew > > > + > > + pmu.enable = 1; > > + set_pmcr(pmu.pmcr_el0); > > + > > + for (int i = 0; i < NR_SAMPLES; i++) { > > + unsigned long a, b; > > + > > + a = get_pmccntr(); > > + b = get_pmccntr(); > > + > > + if (a >= b) { > > + printf("Read %ld then %ld.\n", a, b); > > + return false; > > + } > > + } > > + > > + return true; > > +} > > + > > int main(void) > > { > > report_prefix_push("pmu"); > > > > report("Control register", check_pmcr()); > > + report("Monotonically increasing cycle count", check_cycles_increase()); > > > > return report_summary(); > > } > > -- > > Qualcomm Innovation Center, Inc. > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > > a Linux Foundation Collaborative Project > > > > > > Otherwise > Reviewed-by: Andrew Jones