From 07a394ee8bf97961b774b5fda61276fb1fdf9d2c Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Thu, 19 Oct 2017 13:32:18 +0100
Subject: [PATCH 1/8] Add Critical Section HAL API specification

- Define header functions for Critical Section HAL API
  - hal_critical_section_enter()
  - hal_critical_section_exit()

- Add weak default implementation for HAL API. The default implementation
  matches the previous behaviour stored in mbed_critical:
  - The first call to enter a critical section stores the state of interrupts
    before disabling and each successive call re-disables interrupts.
  - The last call (non-nested) will restore the IRQ state that was set on the
    enter to the critical section. Nested calls are ignored.

- Add function 'core_util_in_critical_section' to User facing API to determine
  if the program is currently in a critical section, instead of depending on
  'core_util_interrupts_enabled'.
---
 hal/critical_section_api.h      | 70 +++++++++++++++++++++++++++++++++
 hal/mbed_critical_section_api.c | 65 ++++++++++++++++++++++++++++++
 platform/mbed_critical.c        | 65 +++++++++++-------------------
 platform/mbed_critical.h        |  7 ++++
 4 files changed, 165 insertions(+), 42 deletions(-)
 create mode 100644 hal/critical_section_api.h
 create mode 100644 hal/mbed_critical_section_api.c

diff --git a/hal/critical_section_api.h b/hal/critical_section_api.h
new file mode 100644
index 0000000000..5e99ba36f1
--- /dev/null
+++ b/hal/critical_section_api.h
@@ -0,0 +1,70 @@
+/** \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
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * \defgroup hal_critical Critical Section HAL functions
+ * @{
+ */
+
+/**
+  * Mark the start of a critical section
+  *
+  * This function is called directly by core_util_critical_section_enter on
+  * first entrance to a critical section.
+  *
+  * The default behavior of this function is to save the current state of
+  * interrupts before disabling them.
+  *
+  * The function is only called once per critical section by
+  * core_util_critical_section_enter. When implementing this function for a
+  * platform you must store any state that you intend to alter within this
+  * function so it can be restored when exiting the critical section.
+  *
+  */
+void hal_critical_section_enter(void);
+
+/** Mark the end of a critical section
+  *
+  * This function is called directly by core_util_critical_section_exit on the
+  * final exit from a critical section.
+  *
+  * The default behavior of this function is to restore the state of interrupts
+  * as they were prior to entering this critical section.
+  *
+  * This function is only called once per critical section. When implemented
+  * for a specific platform it must restore any state that was saved upon
+  * entering the current critical section.
+  *
+  */
+void hal_critical_section_exit(void);
+
+/**@}*/
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // MBED_CRITICAL_SECTION_API_H
+
+/** @}*/
diff --git a/hal/mbed_critical_section_api.c b/hal/mbed_critical_section_api.c
new file mode 100644
index 0000000000..a78a078791
--- /dev/null
+++ b/hal/mbed_critical_section_api.c
@@ -0,0 +1,65 @@
+/* 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 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)
+{
+    critical_interrupts_enabled = are_interrupts_enabled();
+
+#ifndef FEATURE_UVISOR
+    // If we are in a nested critical section and interrupts are still enabled
+    // something has gone wrong.
+    MBED_ASSERT(!are_interrupts_enabled());
+#else
+    #warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
+#endif /* FEATURE_UVISOR */
+
+    __disable_irq();
+}
+
+
+MBED_WEAK void hal_critical_section_exit()
+{
+// FIXME
+#ifndef FEATURE_UVISOR
+    // Interrupts must be disabled on invoking an exit from a critical section
+    MBED_ASSERT(!are_interrupts_enabled());
+#else
+    #warning "core_util_critical_section_exit needs fixing to work from unprivileged code"
+#endif /* FEATURE_UVISOR */
+
+    // Restore the IRQs to their state prior to entering the critical section
+    if (critical_interrupts_enabled == true) {
+      __enable_irq();
+    }
+}
diff --git a/platform/mbed_critical.c b/platform/mbed_critical.c
index d9aa0963ab..18a686f754 100644
--- a/platform/mbed_critical.c
+++ b/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 bool critical_interrupts_disabled = false;
+static volatile uint32_t critical_section_reentrancy_counter = 0;
 
 bool core_util_are_interrupts_enabled(void)
 {
@@ -51,53 +51,34 @@ 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();
-    __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++;
+    return (critical_section_reentrancy_counter != 0);
 }
 
-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) {
+    // If the reentrancy counter overflows something has gone badly wrong.
+    MBED_ASSERT(critical_section_reentrancy_counter < UINT32_MAX);
 
-// FIXME
-#ifndef   FEATURE_UVISOR
-        bool interrupts_disabled = !core_util_are_interrupts_enabled(); /* get the current interrupt disabled state */
+    if (critical_section_reentrancy_counter == 0) {
+        hal_critical_section_enter();
+    }
 
-        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"
-#endif /* FEATURE_UVISOR */
+    critical_section_reentrancy_counter++;
+}
 
-        interrupt_enable_counter--;
+void core_util_critical_section_exit(void)
+{
+    // If critical_section_enter has not previously been called, do nothing
+    if (critical_section_reentrancy_counter == 0) {
+        return;
+    }
 
-        /* Only re-enable interrupts if we are exiting the last of the nested critical sections and
-           interrupts were enabled on entry to the first critical section.
-        */
-        if (!interrupt_enable_counter && !critical_interrupts_disabled) {
-            __enable_irq();
-        }
+    critical_section_reentrancy_counter--;
+
+    if (critical_section_reentrancy_counter == 0) {
+        hal_critical_section_exit();
     }
 }
 
diff --git a/platform/mbed_critical.h b/platform/mbed_critical.h
index c1c5689889..f428e86032 100644
--- a/platform/mbed_critical.h
+++ b/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

From 3c9ae7bf1c17e00ded17c5e8efe94c52b932aeac Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Wed, 25 Oct 2017 16:35:42 +0100
Subject: [PATCH 2/8] NRF51_DK: Add Critical Section HAL implementation

---
 .../hal_patch/critical_section_api.c          | 66 +++++++++++++++
 .../hal_patch/nordic_critical.c               | 80 -------------------
 .../TARGET_NRF5/critical_section_api.c        | 31 +++++++
 3 files changed, 97 insertions(+), 80 deletions(-)
 create mode 100644 features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
 delete mode 100644 features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/nordic_critical.c
 create mode 100644 targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c

diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
new file mode 100644
index 0000000000..237b14817c
--- /dev/null
+++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
@@ -0,0 +1,66 @@
+/*
+ * 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;
+
+void hal_critical_section_enter(void)
+{
+    // Fetch the current state of interrupts
+    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 softdevice can be used, use it.
+        sd_nvic_critical_region_enter(&_state._sd_state);
+        _use_softdevice_routine = true;
+    } else {
+        // If interrupts are enabled, disable them.
+        if (primask == 0) {
+            __disable_irq();
+        }
+
+        // Store PRIMASK state, it will be restored when exiting critical
+        // section.
+        _state._PRIMASK_state = primask;
+        _use_softdevice_routine = false;
+    }
+}
+
+void hal_critical_section_exit(void)
+{
+    // 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);
+    }
+}
diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/nordic_critical.c b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/nordic_critical.c
deleted file mode 100644
index c8ebae99a7..0000000000
--- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/nordic_critical.c
+++ /dev/null
@@ -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);
-    }
-}
diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c b/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
new file mode 100644
index 0000000000..b560f13b6b
--- /dev/null
+++ b/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
@@ -0,0 +1,31 @@
+/*
+ * 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 "nrf_nvic.h"
+
+#include <stdint.h> // uint32_t, UINT32_MAX
+
+static uint8_t _sd_state = 0;
+
+void hal_critical_section_enter(void)
+{
+    sd_nvic_critical_region_enter(&_sd_state);
+}
+
+void hal_critical_section_exit(void)
+{
+    sd_nvic_critical_region_exit(_sd_state);
+}

From 84391f0b64a4f0309e95f1b4338583e269ccb647 Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Wed, 25 Oct 2017 18:04:40 +0100
Subject: [PATCH 3/8] Remove invalid assert and move uVisor warning to correct
 function

---
 hal/mbed_critical_section_api.c | 15 +--------------
 platform/mbed_critical.c        | 10 ++++++++++
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/hal/mbed_critical_section_api.c b/hal/mbed_critical_section_api.c
index a78a078791..fb89fadf3a 100644
--- a/hal/mbed_critical_section_api.c
+++ b/hal/mbed_critical_section_api.c
@@ -36,30 +36,17 @@ MBED_WEAK void hal_critical_section_enter(void)
 {
     critical_interrupts_enabled = are_interrupts_enabled();
 
-#ifndef FEATURE_UVISOR
-    // If we are in a nested critical section and interrupts are still enabled
-    // something has gone wrong.
-    MBED_ASSERT(!are_interrupts_enabled());
-#else
-    #warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
-#endif /* FEATURE_UVISOR */
-
     __disable_irq();
 }
 
 
 MBED_WEAK void hal_critical_section_exit()
 {
-// FIXME
-#ifndef FEATURE_UVISOR
     // Interrupts must be disabled on invoking an exit from a critical section
     MBED_ASSERT(!are_interrupts_enabled());
-#else
-    #warning "core_util_critical_section_exit needs fixing to work from unprivileged code"
-#endif /* FEATURE_UVISOR */
 
     // Restore the IRQs to their state prior to entering the critical section
     if (critical_interrupts_enabled == true) {
-      __enable_irq();
+        __enable_irq();
     }
 }
diff --git a/platform/mbed_critical.c b/platform/mbed_critical.c
index 18a686f754..87e71ca996 100644
--- a/platform/mbed_critical.c
+++ b/platform/mbed_critical.c
@@ -58,6 +58,11 @@ bool core_util_in_critical_section(void)
 
 void core_util_critical_section_enter(void)
 {
+// FIXME
+#ifdef FEATURE_UVISOR
+    #warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
+#endif /* FEATURE_UVISOR */
+
     // If the reentrancy counter overflows something has gone badly wrong.
     MBED_ASSERT(critical_section_reentrancy_counter < UINT32_MAX);
 
@@ -70,6 +75,11 @@ void core_util_critical_section_enter(void)
 
 void core_util_critical_section_exit(void)
 {
+// 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;

From a07c07fa630496a5dbf11aa1b8e0c4258fa78edf Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Wed, 25 Oct 2017 19:32:07 +0100
Subject: [PATCH 4/8] Update HAL API header information

---
 hal/critical_section_api.h | 14 ++++++++++----
 platform/mbed_critical.c   |  4 ++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hal/critical_section_api.h b/hal/critical_section_api.h
index 5e99ba36f1..3ca51cfd2a 100644
--- a/hal/critical_section_api.h
+++ b/hal/critical_section_api.h
@@ -33,8 +33,11 @@ extern "C" {
   * This function is called directly by core_util_critical_section_enter on
   * first entrance to a critical section.
   *
-  * The default behavior of this function is to save the current state of
-  * interrupts before disabling them.
+  * There is a 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.
   *
   * The function is only called once per critical section by
   * core_util_critical_section_enter. When implementing this function for a
@@ -49,8 +52,11 @@ void hal_critical_section_enter(void);
   * This function is called directly by core_util_critical_section_exit on the
   * final exit from a critical section.
   *
-  * The default behavior of this function is to restore the state of interrupts
-  * as they were prior to entering this critical section.
+  * There is a default implementation of this function, it will restore the
+  * state of the interrupts as they were prior to entering this critical
+  * section, this implementation can be found in mbed_critical_section_api.c.
+  * This behavior is overridable by providing a different function
+  * implementation within the correct targets directory.
   *
   * This function is only called once per critical section. When implemented
   * for a specific platform it must restore any state that was saved upon
diff --git a/platform/mbed_critical.c b/platform/mbed_critical.c
index 87e71ca996..39f2adcebf 100644
--- a/platform/mbed_critical.c
+++ b/platform/mbed_critical.c
@@ -61,10 +61,10 @@ void core_util_critical_section_enter(void)
 // FIXME
 #ifdef FEATURE_UVISOR
     #warning "core_util_critical_section_enter needs fixing to work from unprivileged code"
-#endif /* FEATURE_UVISOR */
-
+#else
     // If the reentrancy counter overflows something has gone badly wrong.
     MBED_ASSERT(critical_section_reentrancy_counter < UINT32_MAX);
+#endif /* FEATURE_UVISOR */
 
     if (critical_section_reentrancy_counter == 0) {
         hal_critical_section_enter();

From e14bee52095dac5a41f770604f3fabb8dbaaadf8 Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Tue, 19 Dec 2017 13:35:43 +0000
Subject: [PATCH 5/8] Fix potential race condition in critical section HAL API

Call underlying HAL implementation to enter critical section/disable interrupts
before incrementing the global critical section counter.

Modify HAL implementations to track first entrances to the critical section and
only update the saved state on first enter.
---
 .../hal_patch/critical_section_api.c            | 17 +++++++++++++++--
 hal/mbed_critical_section_api.c                 | 13 +++++++++++--
 platform/mbed_critical.c                        |  8 +++-----
 .../TARGET_NRF5/critical_section_api.c          | 14 +++++++++++++-
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
index 237b14817c..d3d2a9afa9 100644
--- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
+++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
@@ -28,11 +28,13 @@ static union {
 } _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;
@@ -40,8 +42,12 @@ void hal_critical_section_enter(void)
         (sd_softdevice_is_enabled(&sd_enabled) == NRF_SUCCESS) &&
         (sd_enabled == 1)) {
         // If the softdevice can be used, use it.
-        sd_nvic_critical_region_enter(&_state._sd_state);
+        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) {
@@ -50,13 +56,20 @@ void hal_critical_section_enter(void)
 
         // Store PRIMASK state, it will be restored when exiting critical
         // section.
-        _state._PRIMASK_state = primask;
         _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)
diff --git a/hal/mbed_critical_section_api.c b/hal/mbed_critical_section_api.c
index fb89fadf3a..39f2398622 100644
--- a/hal/mbed_critical_section_api.c
+++ b/hal/mbed_critical_section_api.c
@@ -21,6 +21,7 @@
 #include <stdbool.h>
 
 static volatile bool critical_interrupts_enabled = false;
+static volatile bool state_saved = false;
 
 static bool are_interrupts_enabled(void)
 {
@@ -34,17 +35,25 @@ static bool are_interrupts_enabled(void)
 
 MBED_WEAK void hal_critical_section_enter(void)
 {
-    critical_interrupts_enabled = are_interrupts_enabled();
+    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()
 {
     // Interrupts must be disabled on invoking an exit from a critical section
     MBED_ASSERT(!are_interrupts_enabled());
 
+    state_saved = false;
+
     // Restore the IRQs to their state prior to entering the critical section
     if (critical_interrupts_enabled == true) {
         __enable_irq();
diff --git a/platform/mbed_critical.c b/platform/mbed_critical.c
index 39f2adcebf..fcdcca60b7 100644
--- a/platform/mbed_critical.c
+++ b/platform/mbed_critical.c
@@ -66,11 +66,9 @@ void core_util_critical_section_enter(void)
     MBED_ASSERT(critical_section_reentrancy_counter < UINT32_MAX);
 #endif /* FEATURE_UVISOR */
 
-    if (critical_section_reentrancy_counter == 0) {
-        hal_critical_section_enter();
-    }
+    hal_critical_section_enter();
 
-    critical_section_reentrancy_counter++;
+    ++critical_section_reentrancy_counter;
 }
 
 void core_util_critical_section_exit(void)
@@ -85,7 +83,7 @@ void core_util_critical_section_exit(void)
         return;
     }
 
-    critical_section_reentrancy_counter--;
+    --critical_section_reentrancy_counter;
 
     if (critical_section_reentrancy_counter == 0) {
         hal_critical_section_exit();
diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c b/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
index b560f13b6b..a329d18941 100644
--- a/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
+++ b/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
@@ -16,16 +16,28 @@
  */
 #include "nrf_nvic.h"
 
+#include <stdbool.h>
 #include <stdint.h> // uint32_t, UINT32_MAX
 
 static uint8_t _sd_state = 0;
+static volatile bool _state_saved = false;
 
 void hal_critical_section_enter(void)
 {
-    sd_nvic_critical_region_enter(&_sd_state);
+    uint8_t temp_state = 0;
+    sd_nvic_critical_region_enter(&temp_state);
+
+    if (_state_saved == true) {
+        return;
+    }
+
+    _sd_state = temp_state;
+    _state_saved = true;
 }
 
 void hal_critical_section_exit(void)
 {
+    _state_saved = false;
+
     sd_nvic_critical_region_exit(_sd_state);
 }

From 04d2f3de7814d11e8bf2a79de3557f0e9c03265c Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Wed, 20 Dec 2017 16:21:58 +0000
Subject: [PATCH 6/8] Amend critical section function descriptions

---
 hal/critical_section_api.h | 80 ++++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 33 deletions(-)

diff --git a/hal/critical_section_api.h b/hal/critical_section_api.h
index 3ca51cfd2a..e16ac7f962 100644
--- a/hal/critical_section_api.h
+++ b/hal/critical_section_api.h
@@ -28,41 +28,55 @@ extern "C" {
  */
 
 /**
-  * Mark the start of a critical section
-  *
-  * This function is called directly by core_util_critical_section_enter on
-  * first entrance to a critical section.
-  *
-  * There is a 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.
-  *
-  * The function is only called once per critical section by
-  * core_util_critical_section_enter. When implementing this function for a
-  * platform you must store any state that you intend to alter within this
-  * function so it can be restored when exiting the critical section.
-  *
-  */
+ * 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
-  *
-  * This function is called directly by core_util_critical_section_exit on the
-  * final exit from a critical section.
-  *
-  * There is a default implementation of this function, it will restore the
-  * state of the interrupts as they were prior to entering this critical
-  * section, this implementation can be found in mbed_critical_section_api.c.
-  * This behavior is overridable by providing a different function
-  * implementation within the correct targets directory.
-  *
-  * This function is only called once per critical section. When implemented
-  * for a specific platform it must restore any state that was saved upon
-  * entering the current critical section.
-  *
-  */
+/** 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);
 
 /**@}*/

From 061795c489b99146d3d86d560fa919735da573d1 Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Thu, 4 Jan 2018 13:59:44 +0000
Subject: [PATCH 7/8] Move in_critical_section implementation into the HAL

- Add function to HAL hal_in_critical_section()
- Wrap assert in FEATURE_UVISOR macro
---
 .../hal_patch/critical_section_api.c          |  5 +++
 hal/critical_section_api.h                    | 17 ++++++++
 hal/mbed_critical_section_api.c               | 10 ++++-
 platform/mbed_critical.c                      |  2 +-
 .../TARGET_NRF5/critical_section_api.c        | 43 -------------------
 .../TARGET_NRF5/nordic_critical.c             | 11 ++++-
 6 files changed, 40 insertions(+), 48 deletions(-)
 delete mode 100644 targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c

diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
index d3d2a9afa9..32245e1a2b 100644
--- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
+++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/critical_section_api.c
@@ -77,3 +77,8 @@ void hal_critical_section_exit(void)
         __set_PRIMASK(_state._PRIMASK_state);
     }
 }
+
+bool hal_in_critical_section(void)
+{
+    return (_state_saved == true);
+}
diff --git a/hal/critical_section_api.h b/hal/critical_section_api.h
index e16ac7f962..d5cb24296f 100644
--- a/hal/critical_section_api.h
+++ b/hal/critical_section_api.h
@@ -18,6 +18,8 @@
 #ifndef MBED_CRITICAL_SECTION_API_H
 #define MBED_CRITICAL_SECTION_API_H
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -60,6 +62,7 @@ extern "C" {
  */
 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
@@ -79,6 +82,20 @@ void hal_critical_section_enter(void);
  */
 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
diff --git a/hal/mbed_critical_section_api.c b/hal/mbed_critical_section_api.c
index 39f2398622..c8c19d7e4b 100644
--- a/hal/mbed_critical_section_api.c
+++ b/hal/mbed_critical_section_api.c
@@ -47,11 +47,12 @@ MBED_WEAK void hal_critical_section_enter(void)
     state_saved = true;
 }
 
-MBED_WEAK void hal_critical_section_exit()
+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
@@ -59,3 +60,8 @@ MBED_WEAK void hal_critical_section_exit()
         __enable_irq();
     }
 }
+
+MBED_WEAK bool hal_in_critical_section(void)
+{
+    return (state_saved == true);
+}
diff --git a/platform/mbed_critical.c b/platform/mbed_critical.c
index fcdcca60b7..49cb3b7430 100644
--- a/platform/mbed_critical.c
+++ b/platform/mbed_critical.c
@@ -53,7 +53,7 @@ bool core_util_is_isr_active(void)
 
 bool core_util_in_critical_section(void)
 {
-    return (critical_section_reentrancy_counter != 0);
+    return hal_in_critical_section();
 }
 
 void core_util_critical_section_enter(void)
diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c b/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
deleted file mode 100644
index a329d18941..0000000000
--- a/targets/TARGET_NORDIC/TARGET_NRF5/critical_section_api.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * 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 "nrf_nvic.h"
-
-#include <stdbool.h>
-#include <stdint.h> // uint32_t, UINT32_MAX
-
-static uint8_t _sd_state = 0;
-static volatile bool _state_saved = false;
-
-void hal_critical_section_enter(void)
-{
-    uint8_t temp_state = 0;
-    sd_nvic_critical_region_enter(&temp_state);
-
-    if (_state_saved == true) {
-        return;
-    }
-
-    _sd_state = temp_state;
-    _state_saved = true;
-}
-
-void hal_critical_section_exit(void)
-{
-    _state_saved = false;
-
-    sd_nvic_critical_region_exit(_sd_state);
-}
diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c b/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
index e3a1938164..ff5f00d9b7 100644
--- a/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
+++ b/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
@@ -25,7 +25,7 @@ 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 (nordic_cr_nested != 0);
+}
+
+
 #if defined(SOFTDEVICE_PRESENT)
 /**@brief Enters critical region.
  *

From 643c8926d52a2060d9e3cd445faf67df4212ffb4 Mon Sep 17 00:00:00 2001
From: Steven Cartmell <steve.cartmell@arm.com>
Date: Tue, 9 Jan 2018 15:35:12 +0000
Subject: [PATCH 8/8] Remove counter from nordic critical HAL implementation

hal_critical_section_enter() is safe to call multiple times, however
hal_critical_section_exit() is only called on the last exit from a
critical section. This will cause a mismatch in the counter and the
interrupt state will never be restored if the critical section is
nested. Change this to a bool so that the interrupt save state is
tracked for hal_in_critical_section() to work correctly.
---
 .../TARGET_NRF5/nordic_critical.c             | 22 +++++++++----------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c b/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
index ff5f00d9b7..f1937d3787 100644
--- a/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
+++ b/targets/TARGET_NORDIC/TARGET_NRF5/nordic_critical.c
@@ -19,7 +19,7 @@
 #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);
@@ -56,7 +56,7 @@ void hal_critical_section_exit()
 
 bool hal_in_critical_section(void)
 {
-    return (nordic_cr_nested != 0);
+    return (state_saved != 0);
 }
 
 
@@ -70,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
@@ -79,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();
@@ -93,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();
-        NVIC->ISER[0] = nrf_nvic_state.__irq_masks[0];
+    int was_masked = __sd_nvic_irq_disable();
+    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) {
-        __sd_nvic_irq_enable();
-        }
+    if (!was_masked) {
+    __sd_nvic_irq_enable();
     }
 }
 #endif