Hi Andrew,

29.08.2024 11:40, Andrew Jones wrote:
> While the spec doesn't state it, setting timecmp to UINT64_MAX is
> another way to stop a timer, as it's considered setting the next
> timer event to occur at infinity. 

I think this should be explicitly stated in the spec, since some software
may initially set time and timecmp to big values just to check how overflow
is handled. And without it no chance that all HW implementations will interpret
UINT64_MAX as 'stop timer'.

Do we need github issue on SSTC/privileged?

Thank you,
Vladimir Isaev

> And, even if the time CSR does
> eventually reach UINT64_MAX, the very next tick will bring it back to
> zero, once again less than timecmp. For this reason
> riscv_timer_write_timecmp() special cases UINT64_MAX. However, if a
> previously set timecmp has not yet expired, then setting timecmp to
> UINT64_MAX to disable / stop it would not work, as the special case
> left the previous QEMU timer active, which would then still deliver
> an interrupt at that previous timecmp time. Ensure the stopped timer
> will not still deliver an interrupt by also deleting the QEMU timer
> in the UINT64_MAX special case.
> 
> Fixes: ae0edf2188b3 ("target/riscv: No need to re-start QEMU timer when 
> timecmp == UINT64_MAX")
> Signed-off-by: Andrew Jones <ajo...@ventanamicro.com>
> ---
>  target/riscv/time_helper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/time_helper.c b/target/riscv/time_helper.c
> index 8d245bed3ae3..bc0d9a0c4c35 100644
> --- a/target/riscv/time_helper.c
> +++ b/target/riscv/time_helper.c
> @@ -92,6 +92,7 @@ void riscv_timer_write_timecmp(CPURISCVState *env, 
> QEMUTimer *timer,
>       * equals UINT64_MAX.
>       */
>      if (timecmp == UINT64_MAX) {
> +        timer_del(timer);
>          return;
>      }
>  




Reply via email to