From c5c83cfe3cc25eae0c3cdc342bcb0f564e1929d1 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 25 Jun 2020 21:32:17 +0200 Subject: [PATCH] cpu/msp430_common: Update to inline-able IRQ API - Updated to inline-able IRQ API - Improved robustness of functions - Added memory barrier to prevent the compiler from moving code outside of a critical section guarded by irq_disable() ... irq_restore() - Reduced overhead of `irq_disable()` - After clearing the global interrupt enable (GIE) bit, IRQs remain enabled for up to one CPU cycle - The previous implementation just added a nop to fill that cycle - This implementation uses the cycle for masking the return value - Reduced overhead of `irq_restore()` - Now only one CPU cycle is needed - `irq_disable()`, `irq_restore()`, and `irq_enable()` work now in constant time --- cpu/msp430_common/include/cpu_conf.h | 5 ++ cpu/msp430_common/include/irq_arch.h | 101 +++++++++++++++++++++++++++ cpu/msp430_common/irq.c | 42 +---------- 3 files changed, 108 insertions(+), 40 deletions(-) create mode 100644 cpu/msp430_common/include/irq_arch.h diff --git a/cpu/msp430_common/include/cpu_conf.h b/cpu/msp430_common/include/cpu_conf.h index 7f61ffd67b..d427dd950e 100644 --- a/cpu/msp430_common/include/cpu_conf.h +++ b/cpu/msp430_common/include/cpu_conf.h @@ -22,6 +22,11 @@ extern "C" { #endif +/** + * @brief This arch uses the inlined IRQ API. + */ +#define IRQ_API_INLINED (1) + /** * @name Configure the internal flash memory * @{ diff --git a/cpu/msp430_common/include/irq_arch.h b/cpu/msp430_common/include/irq_arch.h new file mode 100644 index 0000000000..d890f26ca9 --- /dev/null +++ b/cpu/msp430_common/include/irq_arch.h @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2014 Freie Universität Berlin + * 2020 Otto-von-Guericke-Universität Magdeburg + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup cpu_msp430_common +* @{ + * + * @file + * @brief ISR related functions + * + * @author Kaspar Schleiser + * @author Oliver Hahm + * @author Marian Buschsieweke + * + */ + +#ifndef IRQ_ARCH_H +#define IRQ_ARCH_H + +#include "irq.h" +#include "cpu.h" + +#ifdef __cplusplus +extern "C" { +#endif + +extern volatile int __irq_is_in; + +__attribute__((always_inline)) static inline unsigned int irq_disable(void) +{ + unsigned int state; + __asm__ volatile( + "mov.w r2, %[state]" "\n\t" + "bic %[gie], r2" "\n\t" + /* + * BEWARE: IRQs remain enabled for one instruction after clearing the + * GIE bit in the status register (r2). Thus, the next instruction is + * not only used to sanitize the IRQ state, but also delays the actual + * critical section by one CPU cycle, so that IRQs are indeed disabled + * by then. + */ + "and %[gie], %[state]" "\n\t" + : [state] "=r"(state) + : [gie] "i"(GIE) + : "memory" + ); + + return state; +} + +__attribute__((always_inline)) static inline unsigned int irq_enable(void) +{ + unsigned int state; + __asm__ volatile( + "mov.w r2, %[state]" "\n\t" + "bis %[gie], r2" "\n\t" + /* + * BEWARE: IRQs remain disabled for one instruction after setting the + * GIE bit in the status register (r2). Thus, the next instruction is + * not only used to sanitize the IRQ state, but also ensures that the + * first instruction after this function is run with IRQs enabled. + */ + "and %[gie], %[state]" "\n\t" + : [state] "=r"(state) + : [gie] "i"(GIE) + : "memory" + ); + + return state; +} + +__attribute__((always_inline)) static inline void irq_restore(unsigned int state) +{ + __asm__ volatile( + "bis %[state], r2" "\n\t" + : /* no outputs */ + : [state] "r"(state) + : "memory" + ); + /* BEWARE: IRQs remain disabled for up to one CPU cycle after this function + * call. But that doesn't seem to be harmful. + */ +} + +__attribute__((always_inline)) static inline int irq_is_in(void) +{ + return __irq_is_in; +} + +#ifdef __cplusplus +} +#endif + +/** @} */ +#endif /* IRQ_ARCH_H */ diff --git a/cpu/msp430_common/irq.c b/cpu/msp430_common/irq.c index b251f74473..86abfd7c48 100644 --- a/cpu/msp430_common/irq.c +++ b/cpu/msp430_common/irq.c @@ -7,11 +7,11 @@ */ /** - * @ingroup cpu + * @ingroup cpu_msp430_common * @{ * * @file - * @brief ISR related functions + * @brief ISR related variables * * @author Kaspar Schleiser * @author Oliver Hahm @@ -25,41 +25,3 @@ volatile int __irq_is_in = 0; char __isr_stack[ISR_STACKSIZE]; - -unsigned int irq_disable(void) -{ - unsigned int state; - __asm__("mov.w r2,%0" : "=r"(state)); - state &= GIE; - - if (state) { - __disable_irq(); - } - - return state; -} - -unsigned int irq_enable(void) -{ - unsigned int state; - __asm__("mov.w r2,%0" : "=r"(state)); - state &= GIE; - - if (!state) { - __enable_irq(); - } - - return state; -} - -void irq_restore(unsigned int state) -{ - if (state) { - __enable_irq(); - } -} - -int irq_is_in(void) -{ - return __irq_is_in; -}