Hi Eddie,
On Tue, Sep 27, 2022 at 3:04 PM Dong, Eddie <eddie.d...@intel.com> wrote: > Hi Tyler: > > > +} > > + > > +/* Called when the bark timer expires */ static void > > +ibex_aon_barker_expired(void *opaque) { > This may happen during ibex_aon_update_count(), right? > > > + IbexAONTimerState *s = IBEX_AON_TIMER(opaque); > > + if (ibex_aon_update_count(s) && > This may happen during ibex_aon_update_count(). > Nested call may lead to incorrect s->regs[R_WDOG_COUNT] & > s->wdog_last. > > Can you elaborate? The timers for bark and bite are not updated in > "update_count". > > When 1st ibex_aon_update_count() is executing, and is in the place PPP > (after updating s->regs[R_WDOG_COUNT] but before updating s->wdog_last), a > timer (barker) may happen. > Inside ibex_aon_barker_expired(), it calls ibex_aon_update_count() again > (nest call), and update s->regs[R_WDOG_COUNT] & s->wdog_last, with the new > value. > After the nest execution ends, and returns to the initial point (PPP) , > the s->wdog_last is updated (with the value of 1st execution time), this > leads to mismatch of s->regs[R_WDOG_COUNT] & s->wdog_last. > > This case may not be triggered at normal case, but if the guest read > A_WDOG_COUNT, the 1st ibex_aon_update_count() does execute, and bring the > potential issue. > I see, I wasn't aware that the virtual clock continued running while the device address was being read. > I think we can solve the problem, by not updating s->regs[R_WDOG_COUNT] & > s->wdog_last in the timer call back API. The update is not necessary given > that the stored value is anyway not the current COUNT. We only need to > update when the guest write the COUNT. > > My initial concern about this is that the HW does the comparison check to determine a bark/bite occurred, which is why I think the count should be updated on a timer expiration, > > > + s->regs[R_WDOG_COUNT] >= s->regs[R_WDOG_BARK_THOLD]) { > > + s->regs[R_INTR_STATE] |= (1 << 1); > > + qemu_irq_raise(s->bark_irq); > > + } > > +} > > + > > > THX Eddie > Thanks, -Tyler