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
