Merge pull request #5346 from scartmell-arm/feature-hal-spec-critical-section

Add Critical Section HAL API specification

features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c

@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2015-2017, ARM Limited, All Rights Reserved
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "cmsis.h"
+#include "nrf_error.h"
+#include "nrf_sdm.h"
+#include "nrf_soc.h"
+
+#include <stdbool.h>
+#include <stdint.h>
+
+static union {
+    uint32_t _PRIMASK_state;
+    uint8_t _sd_state;
+} _state = {0};
+
+static bool _use_softdevice_routine = false;
+static bool _state_saved = false;
+
+void hal_critical_section_enter(void)
+{
+    // Fetch the current state of interrupts
+    uint32_t primask = __get_PRIMASK();
+    uint8_t temp_state = 0;
+
+    // If interrupts are enabled, try to use the soft device
+    uint8_t sd_enabled;
+    if ((primask == 0) &&
+        (sd_softdevice_is_enabled(&sd_enabled) == NRF_SUCCESS) &&
+        (sd_enabled == 1)) {
+        // If the softdevice can be used, use it.
+        sd_nvic_critical_region_enter(&temp_state);
+        _use_softdevice_routine = true;
+
+        if (_state_saved == false) {
+            _state._sd_state = temp_state;
+        }
+    } else {
+        // If interrupts are enabled, disable them.
+        if (primask == 0) {
+            __disable_irq();
+        }
+
+        // Store PRIMASK state, it will be restored when exiting critical
+        // section.
+        _use_softdevice_routine = false;
+
+        if (_state_saved == false) {
+            _state._PRIMASK_state = primask;
+        }
+    }
+
+    _state_saved = true;
+}
+
+void hal_critical_section_exit(void)
+{
+    _state_saved = false;
+
+    // Restore the state as it was prior to entering the critical section.
+    if (_use_softdevice_routine) {
+        sd_nvic_critical_region_exit(_state._sd_state)
+    } else {
+        __set_PRIMASK(_state._PRIMASK_state);
+    }
+}
+
+bool hal_in_critical_section(void)
+{
+    return (_state_saved == true);
+}

features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/nordic_critical.c

@@ -1,80 +0,0 @@
-/*
- * Copyright (c) 2015-2016, ARM Limited, All Rights Reserved
- * SPDX-License-Identifier: Apache-2.0
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdint.h>                  // uint32_t, UINT32_MAX
-#include <assert.h>                  // uint32_t, UINT32_MAX
-#include "cmsis.h"
-#include "nrf_soc.h"
-#include "nrf_sdm.h"
-
-static union {
-    uint32_t _PRIMASK_state;
-    uint8_t  _sd_state;
-} _state = { 0 } ;
-static volatile uint32_t _entry_count = 0;
-static bool _use_softdevice_routine = false;
-
-void core_util_critical_section_enter()
-{
-    // if a critical section has already been entered, just update the counter
-    if (_entry_count) {
-        ++_entry_count;
-        return;
-    }
-
-    // in this path, a critical section has never been entered
-    uint32_t primask = __get_PRIMASK();
-
-    // if interrupts are enabled, try to use the soft device
-    uint8_t sd_enabled;
-    if ((primask == 0) && (sd_softdevice_is_enabled(&sd_enabled) == NRF_SUCCESS) && sd_enabled == 1) {
-        // if the soft device can be use, use it
-        sd_nvic_critical_region_enter(&_state._sd_state);
-        _use_softdevice_routine = true;
-    } else {
-        // if interrupts where enabled, disable them
-        if(primask == 0) {
-            __disable_irq();
-        }
-
-        // store the PRIMASK state, it will be restored at the end of the critical section
-        _state._PRIMASK_state = primask;
-        _use_softdevice_routine = false;
-    }
-
-    assert(_entry_count == 0); // entry count should always be equal to 0 at this point
-    ++_entry_count;
-}
-
-void core_util_critical_section_exit()
-{
-    assert(_entry_count > 0);
-    --_entry_count;
-
-    // If their is other segments which have entered the critical section, just leave
-    if (_entry_count) {
-        return;
-    }
-
-    // This is the last segment of the critical section, state should be restored as before entering
-    // the critical section
-    if (_use_softdevice_routine) {
-        sd_nvic_critical_region_exit(_state._sd_state);
-    } else {
-        __set_PRIMASK(_state._PRIMASK_state);
-    }
-}

hal/critical_section_api.h

@@ -0,0 +1,107 @@
+/** \addtogroup hal */
+/** @{*/
+/* mbed Microcontroller Library
+ * Copyright (c) 2006-2017 ARM Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef MBED_CRITICAL_SECTION_API_H
+#define MBED_CRITICAL_SECTION_API_H
+
+#include <stdbool.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * \defgroup hal_critical Critical Section HAL functions
+ * @{
+ */
+
+/**
+ * Mark the start of a critical section
+ *
+ * This function will be called by core_util_critical_section_enter() each time
+ * the application requests to enter a critical section. The purpose of the
+ * critical section is to ensure mutual-exclusion synchronisation of the
+ * processor by preventing any change in processor control, the default
+ * behaviour requires storing the state of interrupts in the system before
+ * disabling them.
+ *
+ * The critical section code supports nesting. When a thread has entered a
+ * critical section it can make additional calls to
+ * core_util_critical_section_enter() without deadlocking itself. The critical
+ * section driver API tracks the number of nested calls to the critical section.
+ * The critical section will only be exited when
+ * core_util_critical_section_exit() has been called once for each time it
+ * entered the critical section.
+ *
+ * On the first call to enter a critical section this function MUST store the
+ * state of any interrupts or other application settings it will modify to
+ * facilitate the critical section.
+ *
+ * Each successive call to enter the critical section MUST ignore storing or
+ * modifying any application state.
+ *
+ * The default implementation of this function which will save the current state
+ * of interrupts before disabling them. This implementation can be found in
+ * mbed_critical_section_api.c. This behaviour is can be overridden on a per
+ * platform basis by providing a different implementation within the correct
+ * targets directory.
+ */
+void hal_critical_section_enter(void);
+
+
+/** Mark the end of a critical section.
+ *
+ * The purpose of this function is to restore any state that was modified upon
+ * entering the critical section, allowing other threads or interrupts to change
+ * the processor control.
+ *
+ * This function will be called once by core_util_critical_section_exit() per
+ * critical section on last call to exit. When called, the application MUST
+ * restore the saved interrupt/application state that was saved when entering
+ * the critical section.
+ *
+ * There is a default implementation of this function, it will restore the state
+ * of interrupts that were previously saved when hal_critical_section_enter was
+ * first called, this implementation can be found in
+ * mbed_critical_section_api.c. This behaviour is overridable by providing a
+ * different function implementation within the correct targets directory.
+ */
+void hal_critical_section_exit(void);
+
+
+/** Determine if the application is currently running in a critical section
+ *
+ * The purpose of this function is to inform the caller whether or not the
+ * application is running in a critical section. This is done by checking if
+ * the current interrupt state has been saved in the underlying implementation,
+ * this could also be done by checking the state of the interrupts at the time
+ * of calling.
+ *
+ *  @return  True if running in a critical section, false if not.
+ */
+bool hal_in_critical_section(void);
+
+
+/**@}*/
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // MBED_CRITICAL_SECTION_API_H
+
+/** @}*/

hal/mbed_critical_section_api.c

@@ -0,0 +1,67 @@
+/* mbed Microcontroller Library
+ * Copyright (c) 2017 ARM Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "cmsis.h"
+#include "hal/critical_section_api.h"
+#include "platform/mbed_assert.h"
+#include "platform/mbed_toolchain.h"
+
+#include <stdbool.h>
+
+static volatile bool critical_interrupts_enabled = false;
+static volatile bool state_saved = false;
+
+static bool are_interrupts_enabled(void)
+{
+#if defined(__CORTEX_A9)
+    return ((__get_CPSR() & 0x80) == 0);
+#else
+    return ((__get_PRIMASK() & 0x1) == 0);
+#endif
+}
+
+
+MBED_WEAK void hal_critical_section_enter(void)
+{
+    const bool interrupt_state = are_interrupts_enabled();
+
+    __disable_irq();
+
+    if (state_saved == true) {
+        return;
+    }
+
+    critical_interrupts_enabled = interrupt_state;
+    state_saved = true;
+}
+
+MBED_WEAK void hal_critical_section_exit(void)
+{
+#ifndef FEATURE_UVISOR
+    // Interrupts must be disabled on invoking an exit from a critical section
+    MBED_ASSERT(!are_interrupts_enabled());
+#endif
+    state_saved = false;
+
+    // Restore the IRQs to their state prior to entering the critical section
+    if (critical_interrupts_enabled == true) {
+        __enable_irq();
+    }
+}
+
+MBED_WEAK bool hal_in_critical_section(void)
+{
+    return (state_saved == true);
+}

platform/mbed_critical.c

@@ -17,14 +17,14 @@
 
 /* Declare __STDC_LIMIT_MACROS so stdint.h defines UINT32_MAX when using C++ */
 #define __STDC_LIMIT_MACROS
-#include "platform/mbed_critical.h"
+#include "hal/critical_section_api.h"
 
 #include "cmsis.h"
 #include "platform/mbed_assert.h"
+#include "platform/mbed_critical.h"
 #include "platform/mbed_toolchain.h"
 
-static volatile uint32_t interrupt_enable_counter = 0;
+static volatile uint32_t critical_section_reentrancy_counter = 0;
-static volatile bool critical_interrupts_disabled = false;
 
 bool core_util_are_interrupts_enabled(void)
 {
@@ -51,53 +51,42 @@ bool core_util_is_isr_active(void)
 #endif
 }
 
-MBED_WEAK void core_util_critical_section_enter(void)
+bool core_util_in_critical_section(void)
 {
-    bool interrupts_disabled = !core_util_are_interrupts_enabled();
+    return hal_in_critical_section();
-    __disable_irq();
-
-    /* Save the interrupt disabled state as it was prior to any nested critical section lock use */
-    if (!interrupt_enable_counter) {
-        critical_interrupts_disabled = interrupts_disabled;
-    }
-
-    /* If the interrupt_enable_counter overflows or we are in a nested critical section and interrupts
-       are enabled, then something has gone badly wrong thus assert an error.
-    */
-    MBED_ASSERT(interrupt_enable_counter < UINT32_MAX); 
-// FIXME
-#ifndef   FEATURE_UVISOR
-    if (interrupt_enable_counter > 0) {
-        MBED_ASSERT(interrupts_disabled);
-    }
-#else
-#warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
-#endif /* FEATURE_UVISOR */
-    interrupt_enable_counter++;
 }
 
-MBED_WEAK void core_util_critical_section_exit(void)
+void core_util_critical_section_enter(void)
 {
-    /* If critical_section_enter has not previously been called, do nothing */
-    if (interrupt_enable_counter) {
-
 // FIXME
-#ifndef   FEATURE_UVISOR
+#ifdef FEATURE_UVISOR
-        bool interrupts_disabled = !core_util_are_interrupts_enabled(); /* get the current interrupt disabled state */
+    #warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
-
-        MBED_ASSERT(interrupts_disabled); /* Interrupts must be disabled on invoking an exit from a critical section */
 #else
-#warning "core_util_critical_section_exit needs fixing to work from unprivileged code"
+    // If the reentrancy counter overflows something has gone badly wrong.
+    MBED_ASSERT(critical_section_reentrancy_counter < UINT32_MAX);
 #endif /* FEATURE_UVISOR */
 
-        interrupt_enable_counter--;
+    hal_critical_section_enter();
 
-        /* Only re-enable interrupts if we are exiting the last of the nested critical sections and
+    ++critical_section_reentrancy_counter;
-           interrupts were enabled on entry to the first critical section.
+}
-        */
+
-        if (!interrupt_enable_counter && !critical_interrupts_disabled) {
+void core_util_critical_section_exit(void)
-            __enable_irq();
+{
-        }
+// FIXME
+#ifdef FEATURE_UVISOR
+    #warning "core_util_critical_section_exit needs fixing to work from unprivileged code"
+#endif /* FEATURE_UVISOR */
+
+    // If critical_section_enter has not previously been called, do nothing
+    if (critical_section_reentrancy_counter == 0) {
+        return;
+    }
+
+    --critical_section_reentrancy_counter;
+
+    if (critical_section_reentrancy_counter == 0) {
+        hal_critical_section_exit();
     }
 }
 

platform/mbed_critical.h

@@ -82,6 +82,13 @@ void core_util_critical_section_enter(void);
   */
 void core_util_critical_section_exit(void);
 
+/**
+ * Determine if we are currently in a critical section
+ *
+ * @return true if in a critical section, false otherwise.
+ */
+bool core_util_in_critical_section(void);
+
 /**
  * Atomic compare and set. It compares the contents of a memory location to a
  * given value and, only if they are the same, modifies the contents of that

targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c

@@ -19,13 +19,13 @@
 #include "app_util_platform.h"
 
 #if defined(SOFTDEVICE_PRESENT)
-static volatile uint32_t nordic_cr_nested = 0;
+static volatile bool state_saved = false;
 
 static void nordic_nvic_critical_region_enter(void);
 static void nordic_nvic_critical_region_exit(void);
 #endif
 
-void core_util_critical_section_enter()
+void hal_critical_section_enter()
 {
 #ifdef NRF52
     ASSERT(APP_LEVEL_PRIVILEGED == privilege_level_get())
@@ -39,7 +39,7 @@ void core_util_critical_section_enter()
 #endif
 }
 
-void core_util_critical_section_exit()
+void hal_critical_section_exit()
 {
 #ifdef NRF52
     ASSERT(APP_LEVEL_PRIVILEGED == privilege_level_get())
@@ -53,6 +53,13 @@ void core_util_critical_section_exit()
 #endif
 }
 
+
+bool hal_in_critical_section(void)
+{
+    return (state_saved != 0);
+}
+
+
 #if defined(SOFTDEVICE_PRESENT)
 /**@brief Enters critical region.
  *
@@ -63,7 +70,7 @@ static inline void nordic_nvic_critical_region_enter(void)
 {
     int was_masked = __sd_nvic_irq_disable();
 
-    if (nordic_cr_nested == 0) {
+    if (state_saved == false) {
             nrf_nvic_state.__irq_masks[0] = ( NVIC->ICER[0] & __NRF_NVIC_APP_IRQS_0 );
             NVIC->ICER[0] = __NRF_NVIC_APP_IRQS_0;
 #ifdef NRF52
@@ -72,7 +79,7 @@ static inline void nordic_nvic_critical_region_enter(void)
 #endif
     }
 
-    nordic_cr_nested++;
+    state_saved = true;
 
     if (!was_masked) {
         __sd_nvic_irq_enable();
@@ -86,17 +93,15 @@ static inline void nordic_nvic_critical_region_enter(void)
  */
 static inline void nordic_nvic_critical_region_exit(void)
 {
-    nordic_cr_nested--;
+    state_saved = false;
 
-    if (nordic_cr_nested == 0) {
+    int was_masked = __sd_nvic_irq_disable();
-        int was_masked = __sd_nvic_irq_disable();
+    NVIC->ISER[0] = nrf_nvic_state.__irq_masks[0];
-        NVIC->ISER[0] = nrf_nvic_state.__irq_masks[0];
 #ifdef NRF52
-        NVIC->ISER[1] = nrf_nvic_state.__irq_masks[1];
+    NVIC->ISER[1] = nrf_nvic_state.__irq_masks[1];
 #endif
-        if (!was_masked) {
+    if (!was_masked) {
-        __sd_nvic_irq_enable();
+    __sd_nvic_irq_enable();
-        }
     }
 }
 #endif