qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle coun


From: Andrew Jones
Subject: Re: [Qemu-devel] [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases
Date: Mon, 26 Oct 2015 17:04:57 +0100
User-agent: Mutt/1.5.23.1 (2014-03-12)

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 <address@hidden>
> > ---
> >  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 <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]