On Wed, Nov 01, 2017 at 10:40:57AM -0400, Stefan Berger wrote:
When timer_calc_usec() is used with large timeout falues, such as 60s, the function lacks precision and produces different results than when using timer_calc(time / 1000) for the same timeout. This patch fixes the precision issue by falling back to timer_calc(time / 1000) for usec values over one second.
Makes sense. The code would need to use DIV_ROUND_UP though (it's okay to go slightly longer, but it must not calculate a shorter time). Also, I think the check should be extended to nsleep/usleep/ndelay/udelay as well - see the patch below.
Is this needed for the current code, or is this only an issue for the future TPM CRB patches? I was planning to make a release this week - I'd prefer to add this to the next release unless it fixes an active bug.
-Kevin
--- a/src/hw/timer.c +++ b/src/hw/timer.c @@ -167,53 +167,60 @@ timer_check(u32 end) }
static void -timer_delay(u32 diff) +timer_delay(u32 end) { - u32 start = timer_read(); - u32 end = start + diff; while (!timer_check(end)) cpu_relax(); }
static void -timer_sleep(u32 diff) +timer_sleep(u32 end) { - u32 start = timer_read(); - u32 end = start + diff; while (!timer_check(end)) yield(); }
+// Return the TSC value that is 'msecs' time in the future. +u32 +timer_calc(u32 msecs) +{ + return timer_read() + (GET_GLOBAL(TimerKHz) * msecs); +} +u32 +timer_calc_usec(u32 usecs) +{ + u32 cur = timer_read(), khz = GET_GLOBAL(TimerKHz); + if (usecs > 500000) + return cur + DIV_ROUND_UP(usecs, 1000) * khz; + return cur + DIV_ROUND_UP(usecs * khz, 1000); +} +static u32 +timer_calc_nsec(u32 nsecs) +{ + u32 cur = timer_read(), khz = GET_GLOBAL(TimerKHz); + if (nsecs > 500000) + return cur + DIV_ROUND_UP(nsecs, 1000000) * khz; + return cur + DIV_ROUND_UP(nsecs * khz, 1000000); +} + void ndelay(u32 count) { - timer_delay(DIV_ROUND_UP(count * GET_GLOBAL(TimerKHz), 1000000)); + timer_delay(timer_calc_nsec(count)); } void udelay(u32 count) { - timer_delay(DIV_ROUND_UP(count * GET_GLOBAL(TimerKHz), 1000)); + timer_delay(timer_calc_usec(count)); } void mdelay(u32 count) { - timer_delay(count * GET_GLOBAL(TimerKHz)); + timer_delay(timer_calc(count)); }
void nsleep(u32 count) { - timer_sleep(DIV_ROUND_UP(count * GET_GLOBAL(TimerKHz), 1000000)); + timer_sleep(timer_calc_nsec(count)); } void usleep(u32 count) { - timer_sleep(DIV_ROUND_UP(count * GET_GLOBAL(TimerKHz), 1000)); + timer_sleep(timer_calc_usec(count)); } void msleep(u32 count) { - timer_sleep(count * GET_GLOBAL(TimerKHz)); -} - -// Return the TSC value that is 'msecs' time in the future. -u32 -timer_calc(u32 msecs) -{ - return timer_read() + (GET_GLOBAL(TimerKHz) * msecs); -} -u32 -timer_calc_usec(u32 usecs) -{ - return timer_read() + DIV_ROUND_UP(GET_GLOBAL(TimerKHz) * usecs, 1000); + timer_sleep(timer_calc(count)); }