[PATCH] hw/timer/npcm7xx_timer: Prevent timer from counting down past zero

[PATCH] hw/timer/npcm7xx_timer: Prevent timer from counting down past zero

[Chris Rauer]

The counter register is only 24-bits and counts down.  If the timer is
running but the qtimer to reset it hasn't fired off yet, there is a chance
the regster read can return an invalid result.

Signed-off-by: Chris Rauer <crauer@google.com>
---
 hw/timer/npcm7xx_timer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/timer/npcm7xx_timer.c b/hw/timer/npcm7xx_timer.c
index 32f5e021f8..a8bd93aeb2 100644
--- a/hw/timer/npcm7xx_timer.c
+++ b/hw/timer/npcm7xx_timer.c
@@ -138,6 +138,9 @@ static int64_t npcm7xx_timer_count_to_ns(NPCM7xxTimer *t, uint32_t count)
 /* Convert a time interval in nanoseconds to a timer cycle count. */
 static uint32_t npcm7xx_timer_ns_to_count(NPCM7xxTimer *t, int64_t ns)
 {
+    if (ns < 0) {
+        return 0;
+    }
     return clock_ns_to_ticks(t->ctrl->clock, ns) /
         npcm7xx_tcsr_prescaler(t->tcsr);
 }
-- 
2.42.0.515.g380fc7ccd1-goog

Re: [PATCH] hw/timer/npcm7xx_timer: Prevent timer from counting down past zero

[Hao Wu]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Is this related to this error?

https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04903.html

On Fri, Sep 22, 2023 at 11:14 AM Chris Rauer <crauer@google.com> wrote:

> The counter register is only 24-bits and counts down.  If the timer is
> running but the qtimer to reset it hasn't fired off yet, there is a chance
> the regster read can return an invalid result.
>
> Signed-off-by: Chris Rauer <crauer@google.com>
> ---
>  hw/timer/npcm7xx_timer.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/timer/npcm7xx_timer.c b/hw/timer/npcm7xx_timer.c
> index 32f5e021f8..a8bd93aeb2 100644
> --- a/hw/timer/npcm7xx_timer.c
> +++ b/hw/timer/npcm7xx_timer.c
> @@ -138,6 +138,9 @@ static int64_t npcm7xx_timer_count_to_ns(NPCM7xxTimer
> *t, uint32_t count)
>  /* Convert a time interval in nanoseconds to a timer cycle count. */
>  static uint32_t npcm7xx_timer_ns_to_count(NPCM7xxTimer *t, int64_t ns)
>  {
> +    if (ns < 0) {
> +        return 0;
> +    }
>      return clock_ns_to_ticks(t->ctrl->clock, ns) /
>          npcm7xx_tcsr_prescaler(t->tcsr);
>  }
> --
> 2.42.0.515.g380fc7ccd1-goog
>
>

[-- Attachment #2: Type: text/html, Size: 1665 bytes --]

Re: [PATCH] hw/timer/npcm7xx_timer: Prevent timer from counting down past zero

[Chris Rauer]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

No.  This patch does not address that issue and is not related.  I was able
to reproduce it about 2/1000 iterations with and without this patch.  I
will look into that issue separately.

-Chris


On Fri, Sep 22, 2023 at 11:24 AM Hao Wu <wuhaotsh@google.com> wrote:

> Is this related to this error?
>
> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04903.html
>
> On Fri, Sep 22, 2023 at 11:14 AM Chris Rauer <crauer@google.com> wrote:
>
>> The counter register is only 24-bits and counts down.  If the timer is
>> running but the qtimer to reset it hasn't fired off yet, there is a chance
>> the regster read can return an invalid result.
>>
>> Signed-off-by: Chris Rauer <crauer@google.com>
>> ---
>>  hw/timer/npcm7xx_timer.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/timer/npcm7xx_timer.c b/hw/timer/npcm7xx_timer.c
>> index 32f5e021f8..a8bd93aeb2 100644
>> --- a/hw/timer/npcm7xx_timer.c
>> +++ b/hw/timer/npcm7xx_timer.c
>> @@ -138,6 +138,9 @@ static int64_t npcm7xx_timer_count_to_ns(NPCM7xxTimer
>> *t, uint32_t count)
>>  /* Convert a time interval in nanoseconds to a timer cycle count. */
>>  static uint32_t npcm7xx_timer_ns_to_count(NPCM7xxTimer *t, int64_t ns)
>>  {
>> +    if (ns < 0) {
>> +        return 0;
>> +    }
>>      return clock_ns_to_ticks(t->ctrl->clock, ns) /
>>          npcm7xx_tcsr_prescaler(t->tcsr);
>>  }
>> --
>> 2.42.0.515.g380fc7ccd1-goog
>>
>>

[-- Attachment #2: Type: text/html, Size: 2290 bytes --]

Re: [PATCH] hw/timer/npcm7xx_timer: Prevent timer from counting down past zero

[Peter Maydell]

On Fri, 22 Sept 2023 at 19:14, Chris Rauer <crauer@google.com> wrote:
>
> The counter register is only 24-bits and counts down.  If the timer is
> running but the qtimer to reset it hasn't fired off yet, there is a chance
> the regster read can return an invalid result.
>
> Signed-off-by: Chris Rauer <crauer@google.com>

Applied to target-arm.next, thanks.

(As a side note, if you'd used the ptimer countdown-timer functions
to implement this timer, this is one of the corner cases that it
would have got right for you ;-))

-- PMM