On Wed, Jan 25, 2023 at 08:55:50AM +0100, Eric Auger wrote: > Hi, > > On 1/25/23 05:11, Reiji Watanabe wrote: > > On Tue, Jan 24, 2023 at 6:19 PM Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > >> On Wed, Jan 18, 2023 at 09:58:38PM -0800, Reiji Watanabe wrote: > >>> Hi Ricardo, > >>> > >>> On Mon, Jan 9, 2023 at 1:18 PM Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > >>>> @@ -898,12 +913,12 @@ static void test_overflow_interrupt(bool overflow_at_64bits) > >>>> > >>>> pmu_reset_stats(); > >>> This isn't directly related to the patch. > >>> But, as bits of pmovsclr_el0 are already set (although interrupts > >>> are disabled), I would think it's good to clear pmovsclr_el0 here. > >>> > >>> Thank you, > >>> Reiji > >>> > >> There's no need in this case as there's this immediately before the > >> pmu_reset_stats(); > >> > >> report(expect_interrupts(0), "no overflow interrupt after counting"); > >> > >> so pmovsclr_el0 should be clear. > > In my understanding, it means that no overflow *interrupt* was reported, > > as the interrupt is not enabled yet (pmintenset_el1 is not set). > > But, (as far as I checked the test case,) the both counters should be > > overflowing here. So, pmovsclr_el0 must be 0x3. > > I would tend to agree with Reiji. The PMOVSSET_EL0<idx> will impact the > next test according to aarch64/debug/pmu/AArch64.CheckForPMUOverflow and > I think the overflow reg should be reset. Ahh, right. The overflow interrupt could fire as soon as they are enabled again. OK, will add a fix in the next version. > > Eric > > > > Or am I misunderstanding something? > > > > Thank you, > > Reiji > > > > > >>>> - write_regn_el0(pmevcntr, 0, PRE_OVERFLOW); > >>>> - write_regn_el0(pmevcntr, 1, PRE_OVERFLOW); > >>>> + write_regn_el0(pmevcntr, 0, pre_overflow); > >>>> + write_regn_el0(pmevcntr, 1, pre_overflow); > >>>> write_sysreg(ALL_SET, pmintenset_el1); > >>>> isb(); > >>>> >
