[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v2 1/3] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog f
From: |
Dong, Eddie |
Subject: |
RE: [PATCH v2 1/3] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog for the OpenTitan |
Date: |
Tue, 27 Sep 2022 22:04:28 +0000 |
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 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.
> + 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