| Message ID | 20230922181411.2697135-1-crauer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/timer/npcm7xx_timer: Prevent timer from counting down past zero | expand |
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 > >
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 >> >>
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
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); }
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(+)