5 ms·

Nice writeup with coverage of both a discovery and fix. It makes me wonder where the best distribution point to this ends up. The most convenient, but hidden, i

Nice writeup with coverage of both a discovery and fix. It makes me wonder where the best distribution point to this ends up. The most convenient, but hidden, is that it ends up in some embedded dev kit somewhere. But I suppose it might go into the FreeRTOS software, but then it seems like it's not globally applicable to the ARM platform, just the iMX.7 (and likely not even all variants of iMX.7).

wallacoloo 9y ago
I believe that the only generic fixes are to either (a) ensure DRAM is never cached, (b) ensure no atomic operations address DRAM (as the author proposed by using TCM), (c) implement atomics by disabling interrupts, performing a normal RMW operation (i.e. no LL/SC, as even with IRQs disabled that would cause cache incoherency), and re-enabling interrupts. No library like FreeRTOS can guarantee (a). Not even the compiler can guarantee (a), since the user can control the cache by memory-mapped registers. (b) can't be guaranteed by a library, nor can it be guaranteed statically by a compiler, since only the linker knows where the atomic variables will reside in memory (and, atomic operations could be performed on an address that isn't a compile-time constant, e.g. dynamically allocated memory). (c) also can't be guaranteed by any library, but it could be guaranteed by a compiler that has access to the full source of the binary. That's a hefty limitation though, since it means you can't mix any other compiler/language (e.g. assembly, which is almost always used for the startup sequence) into the binary and still have these guarantees. For method (c), I believe gcc allows one to somehow override __atomic_load & other "builtins" - the use-case being that atomics can be implemented for new or uncommon architectures without modifying the compiler itself. If this is the case, then a potential fix could be shipped by a library (e.g. FreeRTOS) which defines __atomic_load as something like void __atomic_load (type *ptr, type *ret, int memorder) { #ifdef IMX7_<partnumber> __disable_irq(); *ret = *ptr; __enable_irq(); #else // insert code to perform a normal atomic load. #endif } In fact, gcc allows one to "wrap" functions - it might be possible to do something like void __wrap___atomic_load (type *ptr, type *ret, int memorder) { #ifdef IMX7_<partnumber> __disable_irq(); *ret = *ptr; __enable_irq(); #else __real___atomic_load(ptr, ret, memorder); #endif } which would have the benefit that the library doesn't need to know how to implement atomic ops on other platforms. This approach could also be used to implement (b) by performing a runtime assert that `ptr` lives in a cache-coherent section of memory. But again, this approach only works if you're not relying on any binary blobs that perform atomic ops. In the end, if you're doing anything nontrivial (e.g. atomic ops on heap-allocated memory, or even stack-allocated memory), it's impossible for a dev kit to completely hide this bug from the developer. Alternatively, do these M4 processors have some type of updatable microcode like X86 processors do? NXP might be able to push a fix that somehow patches LL/SC primitives, or traps when they're encountered at runtime and allows the user to decide how to handle them (e.g. putting them in a no-IRQ critical section like above, but since it's done at runtime now you can mix multiple languages / binary blobs, etc).
- rschaefer2 9y ago
  The M4, to my knowledge, has no microcode like x86 processors. For solution (b), while it can't be guaranteed by the compiler, it can be guaranteed with external tooling that manually ensures that all atomic variables have a gcc section attribute specifying the linker section in the TCM when all sources are available. This also will prevent heap and stack allocated atomics, as I believe the linker will error when specifying a section attribute that the linker cannot respect. Solution (c), that is actually the solution developed for use with some M0 implementations that don't support LL/SC. It works with gcc as the gcc functions implementations have the __weak attribute, meaning that your implementation takes priority. An example override of fetch_add: uint32_t __atomic_fetch_add_4(uint32_t* addr, uint32_t value, int memmodel) { (void)memmodel; uint32_t mask = __get_PRIMASK(); __disable_irq(); uint32_t temp = *addr; *addr = temp + value; if (mask) { __enable_irq(); } return temp; }
- burfog 9y ago
  I think there is another fix: avoid all normal loads and stores to cache lines that contain atomic data structures. When allocating the data structure, make sure to grab a whole cache line or more. Flush and invalidate that, even if you have to do the entire cache. Now simply use the LL/SC operations (apparently named LDREX/STREX on ARM) exclusively. The bug will protect you from itself, preventing the data structure from ever being cached. The only danger left is that some processors will predict memory access, leading to a speculative load of the cache line. I don't know if this CPU has such a feature. If so, a larger allocation might be needed.

wallacoloo 9y ago

I believe that the only generic fixes are to either (a) ensure DRAM is never cached, (b) ensure no atomic operations address DRAM (as the author proposed by using TCM), (c) implement atomics by disabling interrupts, performing a normal RMW operation (i.e. no LL/SC, as even with IRQs disabled that would cause cache incoherency), and re-enabling interrupts. No library like FreeRTOS can guarantee (a). Not even the compiler can guarantee (a), since the user can control the cache by memory-mapped registers. (b) can't be guaranteed by a library, nor can it be guaranteed statically by a compiler, since only the linker knows where the atomic variables will reside in memory (and, atomic operations could be performed on an address that isn't a compile-time constant, e.g. dynamically allocated memory). (c) also can't be guaranteed by any library, but it could be guaranteed by a compiler that has access to the full source of the binary. That's a hefty limitation though, since it means you can't mix any other compiler/language (e.g. assembly, which is almost always used for the startup sequence) into the binary and still have these guarantees. For method (c), I believe gcc allows one to somehow override __atomic_load & other "builtins" - the use-case being that atomics can be implemented for new or uncommon architectures without modifying the compiler itself. If this is the case, then a potential fix could be shipped by a library (e.g. FreeRTOS) which defines __atomic_load as something like void __atomic_load (type *ptr, type *ret, int memorder) { #ifdef IMX7_<partnumber> __disable_irq(); *ret = *ptr; __enable_irq(); #else // insert code to perform a normal atomic load. #endif } In fact, gcc allows one to "wrap" functions - it might be possible to do something like void __wrap___atomic_load (type *ptr, type *ret, int memorder) { #ifdef IMX7_<partnumber> __disable_irq(); *ret = *ptr; __enable_irq(); #else __real___atomic_load(ptr, ret, memorder); #endif } which would have the benefit that the library doesn't need to know how to implement atomic ops on other platforms. This approach could also be used to implement (b) by performing a runtime assert that `ptr` lives in a cache-coherent section of memory. But again, this approach only works if you're not relying on any binary blobs that perform atomic ops. In the end, if you're doing anything nontrivial (e.g. atomic ops on heap-allocated memory, or even stack-allocated memory), it's impossible for a dev kit to completely hide this bug from the developer. Alternatively, do these M4 processors have some type of updatable microcode like X86 processors do? NXP might be able to push a fix that somehow patches LL/SC primitives, or traps when they're encountered at runtime and allows the user to decide how to handle them (e.g. putting them in a no-IRQ critical section like above, but since it's done at runtime now you can mix multiple languages / binary blobs, etc).

rschaefer2 9y ago
The M4, to my knowledge, has no microcode like x86 processors. For solution (b), while it can't be guaranteed by the compiler, it can be guaranteed with external tooling that manually ensures that all atomic variables have a gcc section attribute specifying the linker section in the TCM when all sources are available. This also will prevent heap and stack allocated atomics, as I believe the linker will error when specifying a section attribute that the linker cannot respect. Solution (c), that is actually the solution developed for use with some M0 implementations that don't support LL/SC. It works with gcc as the gcc functions implementations have the __weak attribute, meaning that your implementation takes priority. An example override of fetch_add: uint32_t __atomic_fetch_add_4(uint32_t* addr, uint32_t value, int memmodel) { (void)memmodel; uint32_t mask = __get_PRIMASK(); __disable_irq(); uint32_t temp = *addr; *addr = temp + value; if (mask) { __enable_irq(); } return temp; }
burfog 9y ago
I think there is another fix: avoid all normal loads and stores to cache lines that contain atomic data structures. When allocating the data structure, make sure to grab a whole cache line or more. Flush and invalidate that, even if you have to do the entire cache. Now simply use the LL/SC operations (apparently named LDREX/STREX on ARM) exclusively. The bug will protect you from itself, preventing the data structure from ever being cached. The only danger left is that some processors will predict memory access, leading to a speculative load of the cache line. I don't know if this CPU has such a feature. If so, a larger allocation might be needed.