Re: [Qemu-devel] [kvm-unit-tests PATCHv7 1/3] arm: Add PMU test
"PMU implementer: %c\n", pmu.implementer);
> > + printf("Identification code: 0x%x\n", pmu.identification_code);
> > + printf("Event counters: %d\n", pmu.counters);
> > +
> > + return pmu.implementer != 0;
> > +}
> > +
> > +int main(void)
> > +{
> > + report_prefix_push("pmu");
> > +
> > + report("Control register", check_pmcr());
> > +
> > + return report_summary();
> > +}
> > diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> > index 3f6fa45..b647b69 100644
> > --- a/arm/unittests.cfg
> > +++ b/arm/unittests.cfg
> > @@ -54,3 +54,23 @@ file = selftest.flat
> > smp = $MAX_SMP
> > extra_params = -append 'smp'
> > groups = selftest
> > +
> > +# Test PMU support (KVM)
> > +[pmu-kvm]
> > +file = pmu.flat
> > +groups = pmu
> > +accel = kvm
>
> No need to specify kvm when it works for both. Both is assumed.
> tcg-only or kvm-only tests are exceptions requiring the 'accel'
> parameter and a comment explaining why it doesn't work on the
> other.
>
> > +
> > +# Test PMU support (TCG) with -icount IPC=1
> > +[pmu-tcg-icount-1]
> > +file = pmu.flat
> > +extra_params = -icount 0 -append '1'
> > +groups = pmu
> > +accel = tcg
> > +
> > +# Test PMU support (TCG) with -icount IPC=256
> > +[pmu-tcg-icount-256]
> > +file = pmu.flat
> > +extra_params = -icount 8 -append '256'
> > +groups = pmu
> > +accel = tcg
>
> Why are these entries added now. These tests aren't yet implemented.
What makes you say they aren't implemented? They're running the
same binary with a different command line arguments (that turns on
stricter TCG-specific checking).
Not in this patch, they're not. 'int main(void)' <-- arguments are
ignored. Please only introduce unittests.cfg blocks with the patch
that implements them.
Whoops, that's a rebase error. Sorry about that.
Cov
Re: [Qemu-devel] [kvm-unit-tests PATCHv7 1/3] arm: Add PMU test
u +accel = tcg Why are these entries added now. These tests aren't yet implemented. What makes you say they aren't implemented? They're running the same binary with a different command line arguments (that turns on stricter TCG-specific checking). Thanks, Cov
Re: [Qemu-devel] [kvm-unit-tests PATCHv7 2/3] arm: pmu: Check cycle count increases
On 2016-11-03 04:35, Andrew Jones wrote:
+/*
+ * Ensure that the cycle counter progresses between back-to-back
reads.
+ */
+static bool check_cycles_increase(void)
+{
+ struct pmu_data pmu = {{0}};
+
+ enable_counter(ARMV8_PMU_CYCLE_IDX);
+ set_pmccfiltr(0); /* count cycles in EL0, EL1, but not EL2 */
+
+ 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;
+ }
+ }
+
+ pmu.enable = 0;
+ set_pmcr(pmu.pmcr_el0);
+
+ 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();
}
What happens with this test running on tcg? Do we just fail? Does it
explode? Is there a register we can probe and when it indicates things
won't work we can invoke a report_skip?
A monotonically increasing value (but not any attempt at approximating
actual cycle
values) in cycle counter is pretty much the only piece of the PMU it's
had implemented
for the past while. We'll have to check whether TCG can handle the
filter and enable set
registers, but if it doesn't yet, that's something we can improve :).
Thanks,
Cov
Re: [Qemu-devel] [kvm-unit-tests PATCHv7 2/3] arm: pmu: Check cycle count increases
Hi Wei,
Thanks for your work on this.
On 2016-11-02 16:22, Wei Huang 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 <c...@codeaurora.org>
Signed-off-by: Wei Huang <w...@redhat.com>
---
arm/pmu.c | 100
++
1 file changed, 100 insertions(+)
diff --git a/arm/pmu.c b/arm/pmu.c
index 42d0ee1..65b7df1 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -14,6 +14,9 @@
*/
#include "libcflat.h"
+#define NR_SAMPLES 10
+#define ARMV8_PMU_CYCLE_IDX 31
+
#if defined(__arm__)
static inline uint32_t get_pmcr(void)
{
@@ -22,6 +25,43 @@ 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));
+}
+
+static inline void set_pmccfiltr(uint32_t filter)
+{
+ uint32_t cycle_idx = ARMV8_PMU_CYCLE_IDX;
+
+ asm volatile("mcr p15, 0, %0, c9, c12, 5" : : "r" (cycle_idx));
+ asm volatile("mcr p15, 0, %0, c9, c13, 1" : : "r" (filter));
+}
Down the road I'd like to add tests for the regular events. What if you
added
separate PMSELR and PMXEVTYPER accessors now and used them (with
PMSELR.SEL = 31)
for setting PMCCFILTR? Then we wouldn't need a specialized set_pmccfiltr
function for the cycle counter versus PMSELR and PMXEVTYPER for the
regular events.
+/*
+ * 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;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
My personal preference, that I think would make this function look and
act like the other system register accessor functions, would be to
name the function "set_pmcntenset" and do a plain write of the input
parameter without a shift, letting the shift be done in the C code.
(As we scale up, the system register accessor functions should probably
be generated by macros from a concise table.)
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("mrc p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
This function doesn't seem to be used yet. Consider whether it might
make sense to defer introducing it until there is a user.
#elif defined(__aarch64__)
static inline uint32_t get_pmcr(void)
{
@@ -30,6 +70,34 @@ 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 void set_pmccfiltr(uint32_t filter)
+{
+ asm volatile("msr pmccfiltr_el0, %0" : : "r" (filter));
+}
As above, consider whether using PMSELR and PMXEVTYPER might be a more
reusable pair of accessors.
+static inline unsigned long get_pmccntr(void)
+{
+ unsigned long cycles;
+
+ asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntenset_el0, %0" : : "r" (1 << idx));
+}
Same thought as above about uniformity and generatability.
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntensclr_el0, %0" : : "r" (1 << idx));
+}
As above, this function doesn't seem to be used yet.
Thanks,
Cov
