[PATCH v2] time: Fix get_ticks being non-monotonic

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] time: Fix get_ticks being non-monotonic

Sean Anderson
get_ticks does not always succeed. Sometimes it can be called before the
timer has been initialized. If it does, it returns a negative errno.
This causes the timer to appear non-monotonic, because the value will
become much smaller after the timer is initialized.

No users of get_ticks which I checked handle errors of this kind. Further,
functions like tick_to_time mangle the result of get_ticks, making it very
unlikely that one could check for an error without suggesting a patch such
as this one.

This patch panics if we ever get an error. There are two cases in which
this can occur. The first is if we couldn't find/probe the timer for some
reason. One reason for this is if the timer is not available so early. This
likely indicates misconfiguration. Another reason is that the timer has an
invalid/missing device tree binding. In this case, panicing is also
correct. The second case covers errors calling get_count. This can only
occur if the timer is missing a get_count function (or on RISC-V, but that
should be fixed soon).

Fixes: c8a7ba9e6a5
Signed-off-by: Sean Anderson <[hidden email]>
---

Changes in v2:
- Panic on error

 lib/time.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/time.c b/lib/time.c
index 47f8c84327..88bc50405f 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -91,13 +91,13 @@ uint64_t notrace get_ticks(void)
 
  ret = dm_timer_init();
  if (ret)
- return ret;
+ panic("Could not initialize timer (err %d)\n", ret);
 #endif
  }
 
  ret = timer_get_count(gd->timer, &count);
  if (ret)
- return ret;
+ panic("Could not read count from timer (err %d)\n", ret);
 
  return count;
 }
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] time: Fix get_ticks being non-monotonic

Simon Glass-3
On Wed, 9 Sep 2020 at 14:25, Sean Anderson <[hidden email]> wrote:

>
> get_ticks does not always succeed. Sometimes it can be called before the
> timer has been initialized. If it does, it returns a negative errno.
> This causes the timer to appear non-monotonic, because the value will
> become much smaller after the timer is initialized.
>
> No users of get_ticks which I checked handle errors of this kind. Further,
> functions like tick_to_time mangle the result of get_ticks, making it very
> unlikely that one could check for an error without suggesting a patch such
> as this one.
>
> This patch panics if we ever get an error. There are two cases in which
> this can occur. The first is if we couldn't find/probe the timer for some
> reason. One reason for this is if the timer is not available so early. This
> likely indicates misconfiguration. Another reason is that the timer has an
> invalid/missing device tree binding. In this case, panicing is also
> correct. The second case covers errors calling get_count. This can only
> occur if the timer is missing a get_count function (or on RISC-V, but that
> should be fixed soon).
>
> Fixes: c8a7ba9e6a5
> Signed-off-by: Sean Anderson <[hidden email]>
> ---
>
> Changes in v2:
> - Panic on error
>
>  lib/time.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] time: Fix get_ticks being non-monotonic

Tom Rini-4
In reply to this post by Sean Anderson
On Wed, Sep 09, 2020 at 04:24:56PM -0400, Sean Anderson wrote:

> get_ticks does not always succeed. Sometimes it can be called before the
> timer has been initialized. If it does, it returns a negative errno.
> This causes the timer to appear non-monotonic, because the value will
> become much smaller after the timer is initialized.
>
> No users of get_ticks which I checked handle errors of this kind. Further,
> functions like tick_to_time mangle the result of get_ticks, making it very
> unlikely that one could check for an error without suggesting a patch such
> as this one.
>
> This patch panics if we ever get an error. There are two cases in which
> this can occur. The first is if we couldn't find/probe the timer for some
> reason. One reason for this is if the timer is not available so early. This
> likely indicates misconfiguration. Another reason is that the timer has an
> invalid/missing device tree binding. In this case, panicing is also
> correct. The second case covers errors calling get_count. This can only
> occur if the timer is missing a get_count function (or on RISC-V, but that
> should be fixed soon).
>
> Fixes: c8a7ba9e6a5
> Signed-off-by: Sean Anderson <[hidden email]>
> Reviewed-by: Simon Glass <[hidden email]>
Applied to u-boot/master, thanks!

--
Tom

signature.asc (673 bytes) Download Attachment