You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/01/28 17:04:07 UTC
[incubator-nuttx] branch master updated: cortex-m/doirq: do not update the CURRENT_REGS on nested interrupt handling
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 7d8c2c1 cortex-m/doirq: do not update the CURRENT_REGS on nested interrupt handling
7d8c2c1 is described below
commit 7d8c2c1ad6bd8ce5ba37f5198dbc44cb8fd2511b
Author: chao.an <an...@xiaomi.com>
AuthorDate: Thu Jan 27 13:27:27 2022 +0800
cortex-m/doirq: do not update the CURRENT_REGS on nested interrupt handling
current implementation incorrectly update CURRENT_REGS to interrupt context if
trigger nested interrupt, (e.g, hard fault occurs during interrupt handling)
this would ambiguous for programs using CURRENT_REGS, this patch will prohibit
the update of CURRENT_REGS on nested interrupt handling
Signed-off-by: chao.an <an...@xiaomi.com>
---
arch/arm/src/armv6-m/arm_doirq.c | 44 +++++++++++++---------------------------
arch/arm/src/armv7-m/arm_doirq.c | 28 ++++++++++++-------------
arch/arm/src/armv8-m/arm_doirq.c | 28 ++++++++++++-------------
3 files changed, 42 insertions(+), 58 deletions(-)
diff --git a/arch/arm/src/armv6-m/arm_doirq.c b/arch/arm/src/armv6-m/arm_doirq.c
index 3141316..6fce4d0 100644
--- a/arch/arm/src/armv6-m/arm_doirq.c
+++ b/arch/arm/src/armv6-m/arm_doirq.c
@@ -36,22 +36,6 @@
#include "arm_internal.h"
/****************************************************************************
- * Pre-processor Definitions
- ****************************************************************************/
-
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-/****************************************************************************
- * Private Data
- ****************************************************************************/
-
-/****************************************************************************
- * Private Functions
- ****************************************************************************/
-
-/****************************************************************************
* Public Functions
****************************************************************************/
@@ -61,22 +45,22 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
#ifdef CONFIG_SUPPRESS_INTERRUPTS
PANIC();
#else
- uint32_t *savestate;
/* Nested interrupts are not supported in this implementation. If you
* want to implement nested interrupts, you would have to (1) change the
* way that CURRENT_REGS is handled and (2) the design associated with
- * CONFIG_ARCH_INTERRUPTSTACK. The savestate variable will not work for
- * that purpose as implemented here because only the outermost nested
- * interrupt can result in a context switch (it can probably be deleted).
+ * CONFIG_ARCH_INTERRUPTSTACK.
*/
/* Current regs non-zero indicates that we are processing an interrupt;
* CURRENT_REGS is also used to manage interrupt level context switches.
*/
- savestate = (uint32_t *)CURRENT_REGS;
- CURRENT_REGS = regs;
+ if (CURRENT_REGS == NULL)
+ {
+ CURRENT_REGS = regs;
+ regs = NULL;
+ }
/* Acknowledge the interrupt */
@@ -84,7 +68,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
/* Deliver the IRQ */
- irq_dispatch(irq, regs);
+ irq_dispatch(irq, (uint32_t *)CURRENT_REGS);
/* If a context switch occurred while processing the interrupt then
* CURRENT_REGS may have change value. If we return any value different
@@ -92,15 +76,15 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)CURRENT_REGS;
+ if (regs == NULL)
+ {
+ /* Update the return regs and restore the CURRENT_REGS to NULL. */
- /* Restore the previous value of CURRENT_REGS. NULL would indicate that
- * we are no longer in an interrupt handler. It will be non-NULL if we
- * are returning from a nested interrupt.
- */
-
- CURRENT_REGS = savestate;
+ regs = (uint32_t *)CURRENT_REGS;
+ CURRENT_REGS = NULL;
+ }
#endif
+
board_autoled_off(LED_INIRQ);
return regs;
}
diff --git a/arch/arm/src/armv7-m/arm_doirq.c b/arch/arm/src/armv7-m/arm_doirq.c
index 6f53ac3..abea2e6 100644
--- a/arch/arm/src/armv7-m/arm_doirq.c
+++ b/arch/arm/src/armv7-m/arm_doirq.c
@@ -45,22 +45,22 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
#ifdef CONFIG_SUPPRESS_INTERRUPTS
PANIC();
#else
- uint32_t *savestate;
/* Nested interrupts are not supported in this implementation. If you
* want to implement nested interrupts, you would have to (1) change the
* way that CURRENT_REGS is handled and (2) the design associated with
- * CONFIG_ARCH_INTERRUPTSTACK. The savestate variable will not work for
- * that purpose as implemented here because only the outermost nested
- * interrupt can result in a context switch.
+ * CONFIG_ARCH_INTERRUPTSTACK.
*/
/* Current regs non-zero indicates that we are processing an interrupt;
* CURRENT_REGS is also used to manage interrupt level context switches.
*/
- savestate = (uint32_t *)CURRENT_REGS;
- CURRENT_REGS = regs;
+ if (CURRENT_REGS == NULL)
+ {
+ CURRENT_REGS = regs;
+ regs = NULL;
+ }
/* Acknowledge the interrupt */
@@ -68,7 +68,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
/* Deliver the IRQ */
- irq_dispatch(irq, regs);
+ irq_dispatch(irq, (uint32_t *)CURRENT_REGS);
/* If a context switch occurred while processing the interrupt then
* CURRENT_REGS may have change value. If we return any value different
@@ -76,15 +76,15 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)CURRENT_REGS;
+ if (regs == NULL)
+ {
+ /* Update the return regs and restore the CURRENT_REGS to NULL. */
- /* Restore the previous value of CURRENT_REGS. NULL would indicate that
- * we are no longer in an interrupt handler. It will be non-NULL if we
- * are returning from a nested interrupt.
- */
-
- CURRENT_REGS = savestate;
+ regs = (uint32_t *)CURRENT_REGS;
+ CURRENT_REGS = NULL;
+ }
#endif
+
board_autoled_off(LED_INIRQ);
return regs;
}
diff --git a/arch/arm/src/armv8-m/arm_doirq.c b/arch/arm/src/armv8-m/arm_doirq.c
index f1d3c76..bbbba30 100644
--- a/arch/arm/src/armv8-m/arm_doirq.c
+++ b/arch/arm/src/armv8-m/arm_doirq.c
@@ -45,22 +45,22 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
#ifdef CONFIG_SUPPRESS_INTERRUPTS
PANIC();
#else
- uint32_t *savestate;
/* Nested interrupts are not supported in this implementation. If you
* want to implement nested interrupts, you would have to (1) change the
* way that CURRENT_REGS is handled and (2) the design associated with
- * CONFIG_ARCH_INTERRUPTSTACK. The savestate variable will not work for
- * that purpose as implemented here because only the outermost nested
- * interrupt can result in a context switch.
+ * CONFIG_ARCH_INTERRUPTSTACK.
*/
/* Current regs non-zero indicates that we are processing an interrupt;
* CURRENT_REGS is also used to manage interrupt level context switches.
*/
- savestate = (uint32_t *)CURRENT_REGS;
- CURRENT_REGS = regs;
+ if (CURRENT_REGS == NULL)
+ {
+ CURRENT_REGS = regs;
+ regs = NULL;
+ }
/* Acknowledge the interrupt */
@@ -68,7 +68,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
/* Deliver the IRQ */
- irq_dispatch(irq, regs);
+ irq_dispatch(irq, (uint32_t *)CURRENT_REGS);
/* If a context switch occurred while processing the interrupt then
* CURRENT_REGS may have change value. If we return any value different
@@ -76,15 +76,15 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)CURRENT_REGS;
+ if (regs == NULL)
+ {
+ /* Update the return regs and restore the CURRENT_REGS to NULL. */
- /* Restore the previous value of CURRENT_REGS. NULL would indicate that
- * we are no longer in an interrupt handler. It will be non-NULL if we
- * are returning from a nested interrupt.
- */
-
- CURRENT_REGS = savestate;
+ regs = (uint32_t *)CURRENT_REGS;
+ CURRENT_REGS = NULL;
+ }
#endif
+
board_autoled_off(LED_INIRQ);
return regs;
}