Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42784?usp=email )

Change subject: libosmo_emb: type-safe tearfree_u64_t wrapper for LDRD/STRD access ......................................................................

libosmo_emb: type-safe tearfree_u64_t wrapper for LDRD/STRD access

Although types are frowned upon because memorizing all differerent flavors of void* is the usual way to get acquainted with any mature C code base some heretics decided to introduce generics in C11, which can be used to make aligned access (which guarantees tear-free/restartable 64 bit access on cortex m4) less exciting.

Change-Id: I458bfaf53e27c51882c7a8c1326e51d12943b0bb --- M ccid_common/cuart.h M ccid_host/libosmo_emb.h M sysmoOCTSIM/libosmo_emb.c M sysmoOCTSIM/libosmo_emb.h 4 files changed, 45 insertions(+), 20 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/84/42784/1

diff --git a/ccid_common/cuart.h b/ccid_common/cuart.h index a53dc18..d31d2d9 100644 --- a/ccid_common/cuart.h +++ b/ccid_common/cuart.h @@ -102,7 +102,7 @@ uint32_t wtime_etu; /* deadline in jiffies (ms) for card response timeout, 0 = inactive. * Set from IRQ context, checked from main loop */ - volatile uint64_t wtime_deadline __attribute__((aligned(8))); + tearfree_u64_t wtime_deadline; /* expected number of bytes, for timeout */ uint32_t current_wtime_byte;

diff --git a/ccid_host/libosmo_emb.h b/ccid_host/libosmo_emb.h index ffc0324..e811dd3 100644 --- a/ccid_host/libosmo_emb.h +++ b/ccid_host/libosmo_emb.h @@ -1,15 +1,23 @@ #pragma once -/* Host-side jiffies emulation */ +/* Host-side jiffies emulation + * On the host every access is a plain memory access; the typed wrapper exists + * only purely for type compatibility with code shared with the embedded target. */

#include <stdint.h> +#include <stdalign.h>

-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; -} +typedef struct { + alignas(8) volatile uint64_t _v; +} tearfree_u64_t; + +static inline uint64_t _ldrd_u64(const tearfree_u64_t *p) { return p->_v; } +static inline void _strd_u64(tearfree_u64_t *p, uint64_t v) { p->_v = v; } + +#define ldrd_u64(p) _Generic((p), \ + tearfree_u64_t *: _ldrd_u64, \ + const tearfree_u64_t *: _ldrd_u64)(p) + +#define strd_u64(p, v) _Generic((p), \ + tearfree_u64_t *: _strd_u64)((p), (v))

uint64_t get_jiffies(void); diff --git a/sysmoOCTSIM/libosmo_emb.c b/sysmoOCTSIM/libosmo_emb.c index e96f88e..731268b 100644 --- a/sysmoOCTSIM/libosmo_emb.c +++ b/sysmoOCTSIM/libosmo_emb.c @@ -17,7 +17,7 @@ #include "driver_init.h" #include "libosmo_emb.h"

-volatile uint64_t jiffies __attribute__((aligned(8))); +tearfree_u64_t jiffies;

uint64_t get_jiffies(void) { @@ -31,7 +31,7 @@

void SysTick_Handler(void) { - jiffies++; + jiffies._v++; }

int _gettimeofday(struct timeval *tv, void *tz) diff --git a/sysmoOCTSIM/libosmo_emb.h b/sysmoOCTSIM/libosmo_emb.h index 4b576c6..705982c 100644 --- a/sysmoOCTSIM/libosmo_emb.h +++ b/sysmoOCTSIM/libosmo_emb.h @@ -7,42 +7,59 @@ * * Worst-case outcome is being off by one tick (1 ms) which is irrelevant * for the timeout use-case. + * + * To enforce the alignment requirement the underlying + * uint64_t is wrapped in a struct with 8-byte + * alignment via alignas(8) + _Generic to ensure types. */

#include <stdint.h> +#include <stdalign.h> + +typedef struct { + alignas(8) volatile uint64_t _v; +} tearfree_u64_t;

#if defined(__arm__) -static inline uint64_t ldrd_u64(const volatile uint64_t *p) +static inline uint64_t _ldrd_u64(const tearfree_u64_t *p) { uint32_t lo, hi; __asm__ volatile( "ldrd %0, %1, [%2]\n" : "=&r"(lo), "=&r"(hi) - : "r"(p) + : "r"(&p->_v) : "memory"); return ((uint64_t)hi << 32) | lo; }

-static inline void strd_u64(volatile uint64_t *p, uint64_t v) +static inline void _strd_u64(tearfree_u64_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) + : : "r"(&p->_v), "r"(lo), "r"(hi) : "memory"); } #else /* host */ -static inline uint64_t ldrd_u64(const volatile uint64_t *p) +static inline uint64_t _ldrd_u64(const tearfree_u64_t *p) { - return *p; + return p->_v; } -static inline void strd_u64(volatile uint64_t *p, uint64_t v) +static inline void _strd_u64(tearfree_u64_t *p, uint64_t v) { - *p = v; + p->_v = v; } #endif

+/* _Generic ensures matched types or bust */ +#define ldrd_u64(p) _Generic((p), \ + tearfree_u64_t *: _ldrd_u64, \ + const tearfree_u64_t *: _ldrd_u64)(p) + +#define strd_u64(p, v) _Generic((p), \ + tearfree_u64_t *: _strd_u64)((p), (v)) + uint64_t get_jiffies(void); void store_jiffies(uint64_t j);