qemu-devel
[Top][All Lists]
Advanced

[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: Tyler Ng
Subject: Re: [PATCH v2 1/3] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog for the OpenTitan
Date: Tue, 27 Sep 2022 17:13:22 -0700

Hi Eddie,


On Tue, Sep 27, 2022 at 3:04 PM Dong, Eddie <eddie.dong@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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]