patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 8 Packages Projects Releases Wiki Activity

drivers/timer: Clamp after tick adjustment, not before

Browse source 
Some early tickless drivers had a common pattern where they would
compute a tick maximum for the request (i.e. the maximum the hardware
counter can handle) but apply it only on the input tick value and not
on the adjusted final value, opening up the overflow condition it was
supposed to have prevented.

Fixes #20939 (Strictly it fixes the specific pattern that was
discovered in that bug.  It's not impossible that other drivers with
alternative implementations have a similar issue, though they look OK
to me via a quick audit).

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2019-11-26 11:27:19 -08:00 committed by Carles Cufí
parent 44bbdd0a94
commit 5f63c9d907
4 changed files with 33 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
drivers/timer/cc13x2_cc26x2_rtc_timer.c
View file

@ -39,7 +39,8 @@
/*
* Maximum number of ticks.
*/
#define MAX_TICKS (0x7FFFFFFFFFFFULL / RTC_COUNTS_PER_TICK)
#define MAX_CYC 0x7FFFFFFFFFFFULL
#define MAX_TICKS (MAX_CYC / RTC_COUNTS_PER_TICK)
/*
* Due to the nature of clock synchronization, the comparator cannot be set
@ -217,7 +218,7 @@ void z_clock_set_timeout(s32_t ticks, bool idle)
/* Round to the nearest tick boundary. */
timeout = (timeout + RTC_COUNTS_PER_TICK - 1) / RTC_COUNTS_PER_TICK
* RTC_COUNTS_PER_TICK;
timeout = MIN(timeout, MAX_CYC);
timeout += rtc_last;
/* Set the comparator */

13
drivers/timer/hpet.c
View file

@ -136,10 +136,17 @@ void z_clock_set_timeout(s32_t ticks, bool idle)
ticks = MAX(MIN(ticks - 1, (s32_t)max_ticks), 0);
k_spinlock_key_t key = k_spin_lock(&lock);
u32_t now = MAIN_COUNTER_REG, cyc;
u32_t now = MAIN_COUNTER_REG, cyc, adj;
u32_t max_cyc = max_ticks * cyc_per_tick;
/* Round up to next tick boundary */
cyc = ticks * cyc_per_tick + (now - last_count) + (cyc_per_tick - 1);
/* Round up to next tick boundary. */
cyc = ticks * cyc_per_tick;
adj = (now - last_count) + (cyc_per_tick - 1);
if (cyc <= max_cyc - adj) {
cyc += adj;
} else {
cyc = max_cyc;
}
cyc = (cyc / cyc_per_tick) * cyc_per_tick;
cyc += last_count;

16
drivers/timer/riscv_machine_timer.c
View file

@ -10,7 +10,8 @@
#define CYC_PER_TICK ((u32_t)((u64_t)sys_clock_hw_cycles_per_sec() \
/ (u64_t)CONFIG_SYS_CLOCK_TICKS_PER_SEC))
#define MAX_TICKS ((0xffffffffu - CYC_PER_TICK) / CYC_PER_TICK)
#define MAX_CYC 0xffffffffu
#define MAX_TICKS ((MAX_CYC - CYC_PER_TICK) / CYC_PER_TICK)
#define MIN_DELAY 1000
#define TICKLESS (IS_ENABLED(CONFIG_TICKLESS_KERNEL) && \
@ -99,12 +100,15 @@ void z_clock_set_timeout(s32_t ticks, bool idle)
k_spinlock_key_t key = k_spin_lock(&lock);
u64_t now = mtime();
u32_t cyc = ticks * CYC_PER_TICK;
u32_t adj, cyc = ticks * CYC_PER_TICK;
/* Round up to next tick boundary. Note use of 32 bit math,
* max_ticks is calibrated to permit this.
*/
cyc += (u32_t)(now - last_count) + (CYC_PER_TICK - 1);
/* Round up to next tick boundary. */
adj = (u32_t)(now - last_count) + (CYC_PER_TICK - 1);
if (cyc <= MAX_CYC - adj) {
cyc += adj;
} else {
cyc = MAX_CYC;
}
cyc = (cyc / CYC_PER_TICK) * CYC_PER_TICK;
if ((s32_t)(cyc + last_count - now) < MIN_DELAY) {

13
drivers/timer/xtensa_sys_timer.c
View file

@ -13,7 +13,8 @@
#define CYC_PER_TICK (sys_clock_hw_cycles_per_sec() \
/ CONFIG_SYS_CLOCK_TICKS_PER_SEC)
#define MAX_TICKS ((0xffffffffu - CYC_PER_TICK) / CYC_PER_TICK)
#define MAX_CYC 0xffffffffu
#define MAX_TICKS ((MAX_CYC - CYC_PER_TICK) / CYC_PER_TICK)
#define MIN_DELAY 1000
static struct k_spinlock lock;
@ -74,10 +75,16 @@ void z_clock_set_timeout(s32_t ticks, bool idle)
ticks = MAX(MIN(ticks - 1, (s32_t)MAX_TICKS), 0);
k_spinlock_key_t key = k_spin_lock(&lock);
u32_t curr = ccount(), cyc;
u32_t curr = ccount(), cyc, adj;
/* Round up to next tick boundary */
cyc = ticks * CYC_PER_TICK + (curr - last_count) + (CYC_PER_TICK - 1);
cyc = ticks * CYC_PER_TICK;
adj = (curr - last_count) + (CYC_PER_TICK - 1);
if (cyc <= MAX_CYC - adj) {
cyc += adj;
} else {
cyc = MAX_CYC;
}
cyc = (cyc / CYC_PER_TICK) * CYC_PER_TICK;
cyc += last_count;