[ip/timer] `mtime` overflow(possibly underflow too) about opentitan HOT 4 CLOSED

Eunchan wrote: <#290 (comment):>
we could have phase bit in this case, but it means it is not matched to privileged spec. IMHO, we can leave it as it is.

Why would a phase bit not match the privileged spec? "Bit 65" doesn't need to be visible to software, you only need to use it when doing the comparison beyond the 64 bit boundary. And you can always reset it to 0 when mtime or mtimecmp is written. (I didn't fully think it through, but it should work?) Of course that makes the adder and the comparison logic one bit larger.

My assumption is that one Hart can have multiple mtimecmp but having single mtime.(for general usage) In this case, phase bit should be maintained. When the firmware writes to mtimecmp, it can calculate phase bit as below:

1. If mtimecmp is greater than mtime then phase bit is same as mtime.
2. If mtimecmp is smaller than mtime then flip the phase bit.

It is not that difficult but invisible phase bit can cause problem when the Hart updates mtime. If we apply same concept as above (new mtime is greater than mtime then p bit same), it may be okay. But it is not quite spec conforming behavior.

As RISC-V spec doesn't specify this precisely. I would like to make it simple. For next version of the timer IP, we will have a few more features such as count down timer (to be used as Watchdog), one shot (stop when reaches all mtimecmp expired). Both features are configurable per Hart.

Mark wrote: <#290 (comment)>
The close to max wrap problem only happens if step is not 1. So maybe we can document it as only being a problem if step > 1 (and mtimecmp not being a multiple of step)?

I don't expect software to freely set step, but instead I'd expect software to set step as a consequence of the timer's frequency in order to have a timer of a given wall-clock time period (e.g. 1 us). Given that step is mostly not a design choice in software, but a consequence, I'd hate to have buggy behavior in there which can go unnoticed quite easily.

Another option would be to remove step as a register altogether, and making it a hardware parameter, i.e. creating a timer module which always ticks at a fixed wall-clock time period (e.g. 100 ns or 1 us). This way we can optimize the prescaler and step values together to have "nice" properties to make the hardware implementation easier.

step and prescaler must be software configurable (maybe in machine mode only). We can determine the Xtal or PLL clock frequency when floor planning the SoC but it is not guaranteed operating frequency. For some reason, it can use slower frequency later (or after taped-out). And low-power mode clock (FROSC) is quite unstable and affected by power consumption, process variation, voltages, and so on. So, there should be a way to compensate them.

This is quite off the spec, but the Hart always set it to default which meets RISC-V spec. If we can give a custom timer driver, then we can give wall-clock time period.

from opentitan.

Behavior seen in rv_timer Testbench that when mTime rollover to 0, interrupt is kept asserted until its cleared explicitly.

from opentitan.

@eunchan , did we reach a conclusion whether to have a phase bit for the overflow case? Please update status accordingly. Thanks.

from opentitan.

@eunchan , did we reach a conclusion whether to have a phase bit for the overflow case? Please update status accordingly. Thanks.

We decided not to implement phase bit. Let me close this issue.

from opentitan.