On Thu, Aug 04, 2022 at 09:55:01AM +0100, Alexandru Elisei wrote: > Hi, > > On Wed, Aug 03, 2022 at 11:23:26AM -0700, Ricardo Koller wrote: > > There are various pmu tests that require an isb() between enabling > > counting and the actual counting. This can lead to count registers > > reporting less events than expected; the actual enabling happens after > > some events have happened. For example, some missing isb()'s in the > > pmu-sw-incr test lead to the following errors on bare-metal: > > > > INFO: pmu: pmu-sw-incr: SW_INCR counter #0 has value 4294967280 > > PASS: pmu: pmu-sw-incr: PWSYNC does not increment if PMCR.E is unset > > FAIL: pmu: pmu-sw-incr: counter #1 after + 100 SW_INCR > > FAIL: pmu: pmu-sw-incr: counter #0 after + 100 SW_INCR > > INFO: pmu: pmu-sw-incr: counter values after 100 SW_INCR #0=82 #1=98 > > PASS: pmu: pmu-sw-incr: overflow on counter #0 after 100 SW_INCR > > SUMMARY: 4 tests, 2 unexpected failures > > > > Add the missing isb()'s on all failing tests, plus some others that seem > > required: > > - after clearing the overflow signal in the IRQ handler to avoid > > spurious interrupts. > > Nitpick, but it doesn't avoid (eliminates) spurious interrupts, it makes > them less likely. > > > - after direct writes to PMSWINC_EL0 for software to read the correct > > value for PMEVNCTR0_EL0 (from ARM DDI 0487H.a, page D13-5237). > > > > Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx> > > --- > > arm/pmu.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/arm/pmu.c b/arm/pmu.c > > index 15c542a2..76156f78 100644 > > --- a/arm/pmu.c > > +++ b/arm/pmu.c > > [..] > > @@ -821,10 +832,13 @@ static void test_overflow_interrupt(void) > > report(expect_interrupts(0), "no overflow interrupt after preset"); > > > > set_pmcr(pmu.pmcr_ro | PMU_PMCR_E); > > + isb(); > > + > > for (i = 0; i < 100; i++) > > write_sysreg(0x2, pmswinc_el0); > > > > set_pmcr(pmu.pmcr_ro); > > + isb(); > > A context synchronization event affects system register writes that come > before the context synchronization event in program order, but if there are > multiple system register writes, it doesn't perform them in program order > (if that makes sense). Good point, didn't think of that case. Added the missing isb() in v3. Thanks, Ricardo > > So it might happen that the CPU decides to perform the write to PMCR_EL1 > which disables the PMU *before* the writes to PMSWINC_EL0. Which means that > even if PMINTENSET_EL1 allows the PMU to assert interrupts when it > shouldn't (thus causing the test to fail), those interrupt won't be > asserted by the PMU because the PMU is disabled and the test would pass. > > You need an ISB after the PMSWINC_EL0 writes and before disabling the PMU. > > Thanks, > Alex > > > report(expect_interrupts(0), "no overflow interrupt after counting");
