laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42504?usp=email )
Change subject: firmware: add tear-free 64bit load/store helpers for jiffies ......................................................................
firmware: add tear-free 64bit load/store helpers for jiffies
On Cortex-M4 a 64bit load may compile to two separate LDR instructions which can lead to torn reads due to interrupts. LDRD/STRD are restartable, they either complete or restart from scratch, so the result is always consistent. The only "downside" is the required alignment, which is fine.
Change-Id: I729c0fdfb5b228b03c2df1cf098743100b1ea625 --- M sysmoOCTSIM/libosmo_emb.c A sysmoOCTSIM/libosmo_emb.h 2 files changed, 63 insertions(+), 3 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved lynxis lazus: Looks good to me, but someone else must approve
diff --git a/sysmoOCTSIM/libosmo_emb.c b/sysmoOCTSIM/libosmo_emb.c index 81b29e3..37f6126 100644 --- a/sysmoOCTSIM/libosmo_emb.c +++ b/sysmoOCTSIM/libosmo_emb.c @@ -15,8 +15,19 @@
#include <sys/time.h> #include "driver_init.h" +#include "libosmo_emb.h"
-volatile uint64_t jiffies; +volatile uint64_t jiffies __attribute__((aligned(8))); + +uint64_t get_jiffies(void) +{ + ldrd_u64(&jiffies); +} + +void store_jiffies(uint64_t j) +{ + strd_u64(&jiffies, j); +}
void SysTick_Handler(void) { @@ -25,8 +36,9 @@
int _gettimeofday(struct timeval *tv, void *tz) { - tv->tv_sec = jiffies / 1000; - tv->tv_usec = (jiffies % 1000) * 1000; + uint64_t j = get_jiffies(); + tv->tv_sec = j / 1000; + tv->tv_usec = (j % 1000) * 1000; return 0; }
diff --git a/sysmoOCTSIM/libosmo_emb.h b/sysmoOCTSIM/libosmo_emb.h new file mode 100644 index 0000000..4b576c6 --- /dev/null +++ b/sysmoOCTSIM/libosmo_emb.h @@ -0,0 +1,48 @@ +/* Tear-free 64-bit load/store for Cortex-M4. + * + * LDRD/STRD are restartable: if an interrupt arrives mid-instruction the + * core restarts the access from scratch, so the result is always consistent. + * This requires 8-byte alignment (unaligned LDRD silently becomes two + * separate word accesses, losing the restart guarantee). + * + * Worst-case outcome is being off by one tick (1 ms) which is irrelevant + * for the timeout use-case. + */ + +#include <stdint.h> + +#if defined(__arm__) +static inline uint64_t ldrd_u64(const volatile uint64_t *p) +{ + uint32_t lo, hi; + __asm__ volatile( + "ldrd %0, %1, [%2]\n" + : "=&r"(lo), "=&r"(hi) + : "r"(p) + : "memory"); + return ((uint64_t)hi << 32) | lo; +} + +static inline void strd_u64(volatile uint64_t *p, uint64_t v) +{ + uint32_t lo = (uint32_t)v; + uint32_t hi = (uint32_t)(v >> 32); + __asm__ volatile( + "strd %1, %2, [%0]\n" + : : "r"(p), "r"(lo), "r"(hi) + : "memory"); +} +#else +/* host */ +static inline uint64_t ldrd_u64(const volatile uint64_t *p) +{ + return *p; +} +static inline void strd_u64(volatile uint64_t *p, uint64_t v) +{ + *p = v; +} +#endif + +uint64_t get_jiffies(void); +void store_jiffies(uint64_t j);