arch: arm: Fix interrupt stack handlings for SMP

Summary:
- Modify arm_intstack_base() to return "top" of the IRQ stack for the current CPU
- This change fixes IRQ stack dump information for ARM SMP
- Add arm_intstack_alloc() to return "bottom" of the IRQ stack for the current CPU
- Also, these functions are now implemented in xxx_irq.c (imx/cxd56/lc823450)
- up_color_intstack() and up_check_intstack() now call arm_intstack_alloc()
- These semantics are now consistent with non-SMP case
- up_color_intstack() now initializes whole IRQ stack region for SMP
- Adjust IRQ stack top address for each CPU (e.g. -8)
- Fix setintstack to handle in case of NCPUS=1 (cxd56, lc823450)
- Adjust INTSTACK_SIZE to 8 bytes alignment (cxd56, lc823450)
- Refactor setintstack for lc823450
- Remove old IRQ stack coloring code from up_irqinitialize() (lc823450)
- Introduce g_cpu_intstack_top for lc823450
- Refactor header files

Impact:
- Affects imx6/cxd56xx/lc823450 SMP with interrupt stack enabled

Testing:
- Tested with sabre-6quad:smp (with QEMU, NCPUS=1 and 4)
- Tested with spresense:wifi_smp (NCPUS=1 and 2)
- Tested with lc823450-xgevk:rndis (NCPUS=1 and 2)

Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>

arch/arm/src/common/arm_checkstack.c

@@ -245,7 +245,7 @@ ssize_t up_check_stack_remain(void)
 size_t up_check_intstack(void)
 {
 #ifdef CONFIG_SMP
-  return do_stackcheck((FAR void *)arm_intstack_base(),
+  return do_stackcheck((FAR void *)arm_intstack_alloc(),
                         INT32_ALIGN_DOWN(CONFIG_ARCH_INTERRUPTSTACK));
 #else
   return do_stackcheck((FAR void *)&g_intstackalloc,

arch/arm/src/common/arm_initialize.c

@@ -62,13 +62,17 @@
 static inline void up_color_intstack(void)
 {
 #ifdef CONFIG_SMP
-  uint32_t *ptr = (uint32_t *)arm_intstack_base();
+  uint32_t *ptr = (uint32_t *)arm_intstack_alloc();
 #else
   uint32_t *ptr = (uint32_t *)&g_intstackalloc;
 #endif
   ssize_t size;
 
+#ifdef CONFIG_SMP
+  for (size = ((CONFIG_ARCH_INTERRUPTSTACK & ~3) * CONFIG_SMP_NCPUS);
+#else
   for (size = (CONFIG_ARCH_INTERRUPTSTACK & ~3);
+#endif
        size > 0;
        size -= sizeof(uint32_t))
     {

arch/arm/src/cxd56xx/chip.h

@@ -56,6 +56,7 @@
 
 #if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
 #  include "cxd56_cpuindex.h"
+#  include "cxd56_irq.h"
 #endif
 
 /****************************************************************************
@@ -75,43 +76,13 @@
 
 #if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
   .macro  setintstack, tmp1, tmp2
-#if CONFIG_SMP_NCPUS > 1
   ldr \tmp1, =CXD56_ADSP_PID
   ldr \tmp1, [\tmp1, 0]
   sub \tmp1, 2                   /* tmp1 = getreg32(CXD56_ADSP_PID) - 2 */
   ldr \tmp2, =g_cpu_intstack_top
   ldr sp, [\tmp2, \tmp1, lsl #2] /* sp = g_cpu_intstack_top[tmp1] */
-#endif
   .endm
 #endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
 
-#endif /* __ASSEMBLY__  */
-
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-#ifndef __ASSEMBLY__
-#if defined(__cplusplus)
-#define EXTERN extern "C"
-extern "C"
-{
-#else
-#define EXTERN extern
-#endif
-
-/****************************************************************************
- * Public Functions Prototypes
- ****************************************************************************/
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-uintptr_t arm_intstack_base(void);
-#endif
-
-#undef EXTERN
-#if defined(__cplusplus)
-}
-#endif
-
 #endif /* __ASSEMBLY__  */
 #endif /* __ARCH_ARM_SRC_CXD56XX_CHIP_H */

arch/arm/src/cxd56xx/cxd56_irq.c

@@ -72,10 +72,7 @@
 
 #define INTC_EN(n) (CXD56_INTC_BASE + 0x10 + (((n) >> 5) << 2))
 
-/* Interrupt stack definitions for SMP */
-
 #if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-#  define INTSTACK_SIZE  CONFIG_ARCH_INTERRUPTSTACK
 #  define INTSTACK_ALLOC (CONFIG_SMP_NCPUS * INTSTACK_SIZE)
 #endif
 
@@ -115,17 +112,17 @@ static uint64_t g_intstack_alloc[INTSTACK_ALLOC >> 3];
 
 const uint32_t g_cpu_intstack_top[CONFIG_SMP_NCPUS] =
 {
-  (uint32_t)g_intstack_alloc + INTSTACK_SIZE,
+  (uint32_t)g_intstack_alloc + INTSTACK_SIZE - 8,
 #if CONFIG_SMP_NCPUS > 1
-  (uint32_t)g_intstack_alloc + (2 * INTSTACK_SIZE),
+  (uint32_t)g_intstack_alloc + (2 * INTSTACK_SIZE) - 8,
 #if CONFIG_SMP_NCPUS > 2
-  (uint32_t)g_intstack_alloc + (3 * INTSTACK_SIZE),
+  (uint32_t)g_intstack_alloc + (3 * INTSTACK_SIZE) - 8,
 #if CONFIG_SMP_NCPUS > 3
-  (uint32_t)g_intstack_alloc + (4 * INTSTACK_SIZE),
+  (uint32_t)g_intstack_alloc + (4 * INTSTACK_SIZE) - 8,
 #if CONFIG_SMP_NCPUS > 4
-  (uint32_t)g_intstack_alloc + (5 * INTSTACK_SIZE),
+  (uint32_t)g_intstack_alloc + (5 * INTSTACK_SIZE) - 8,
 #if CONFIG_SMP_NCPUS > 5
-  (uint32_t)g_intstack_alloc + (6 * INTSTACK_SIZE),
+  (uint32_t)g_intstack_alloc + (6 * INTSTACK_SIZE) - 8,
 #endif /* CONFIG_SMP_NCPUS > 5 */
 #endif /* CONFIG_SMP_NCPUS > 4 */
 #endif /* CONFIG_SMP_NCPUS > 3 */
@@ -647,17 +644,29 @@ int up_prioritize_irq(int irq, int priority)
  *
  * Description:
  *   Return a pointer to the "base" the correct interrupt stack allocation
- *   for the  current CPU.
+ *   for the current CPU. NOTE: Here, the base means "top" of the stack
  *
  ****************************************************************************/
 
 #if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
 uintptr_t arm_intstack_base(void)
 {
-  uintptr_t base = (uintptr_t)g_intstack_alloc;
+  return g_cpu_intstack_top[up_cpu_index()];
-  uint32_t cpu = up_cpu_index();
+}
-  base += cpu * INTSTACK_SIZE;
+#endif
-
+
-  return base;
+/****************************************************************************
+ * Name: arm_intstack_alloc
+ *
+ * Description:
+ *   Return a pointer to the "alloc" the correct interrupt stack allocation
+ *   for the current CPU.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+uintptr_t arm_intstack_alloc(void)
+{
+  return g_cpu_intstack_top[up_cpu_index()] - (INTSTACK_SIZE - 8);
 }
 #endif

arch/arm/src/cxd56xx/cxd56_irq.h

@@ -48,6 +48,12 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* The size of one interrupt stack.  This is the configured value aligned
+ * the 8-bytes as required by the ARM EABI.
+ */
+
+#define INTSTACK_SIZE  (CONFIG_ARCH_INTERRUPTSTACK & ~7)
+
 /****************************************************************************
  * Public Types
  ****************************************************************************/
@@ -67,14 +73,15 @@ extern "C"
 #define EXTERN extern
 #endif
 
-/****************************************************************************
- * Inline Functions
- ****************************************************************************/
-
 /****************************************************************************
  * Public Functions Prototypes
  ****************************************************************************/
 
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+EXTERN uintptr_t arm_intstack_base(void);
+EXTERN uintptr_t arm_intstack_alloc(void);
+#endif
+
 #undef EXTERN
 #if defined(__cplusplus)
 }

arch/arm/src/imx6/chip.h

@@ -61,23 +61,6 @@
 
 #define CHIP_MPCORE_VBASE IMX_ARMMP_VSECTION
 
-/****************************************************************************
- * Public Types
- ****************************************************************************/
-
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-#ifdef __ASSEMBLY__
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-  .globl  g_irqstack_top
-  .globl  g_fiqstack_top
-#endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
-
-#endif /* __ASSEMBLY__ */
-
 /****************************************************************************
  * Macro Definitions
  ****************************************************************************/
@@ -141,51 +124,4 @@
 
 #endif /* __ASSEMBLY__ */
 
-/****************************************************************************
- * Inline Functions
- ****************************************************************************/
-
-#ifndef __ASSEMBLY__
-
-/****************************************************************************
- * Name: arm_intstack_base
- *
- * Description:
- *   Return a pointer to the "base" the correct interrupt stack allocation
- *   for the  current CPU.
- *
- ****************************************************************************/
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-static inline uintptr_t arm_intstack_base(void)
-{
-  uintptr_t base = (uintptr_t)g_irqstack_alloc;
-#if CONFIG_SMP_NCPUS > 1
-  uint32_t cpu = up_cpu_index();
-
-  base += cpu * INTSTACK_SIZE;
-#endif
-
-  return base;
-}
-#endif
-
-/****************************************************************************
- * Name: arm_intstack_top
- *
- * Description:
- *   Return a pointer to the "top" the correct interrupt stack for the
- *   current CPU.
- *
- ****************************************************************************/
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-static inline uintptr_t arm_intstack_top(void)
-{
-  return arm_intstack_base() + INTSTACK_SIZE;
-}
-#endif
-
-#endif /* !__ASSEMBLY__ */
-
 #endif /* __ARCH_ARM_SRC_IMX6_CHIP_H */

arch/arm/src/imx6/imx_irq.c

@@ -88,15 +88,15 @@ uint64_t g_fiqstack_alloc[INTSTACK_ALLOC >> 3];
 
 uintptr_t g_irqstack_top[CONFIG_SMP_NCPUS] =
 {
-  (uintptr_t)g_irqstack_alloc + INTSTACK_SIZE,
+  (uintptr_t)g_irqstack_alloc + INTSTACK_SIZE - 8,
 #if CONFIG_SMP_NCPUS > 1
-  (uintptr_t)g_irqstack_alloc + 2 * INTSTACK_SIZE,
+  (uintptr_t)g_irqstack_alloc + (2 * INTSTACK_SIZE) - 8,
 #endif
 #if CONFIG_SMP_NCPUS > 2
-  (uintptr_t)g_irqstack_alloc + 3 * INTSTACK_SIZE,
+  (uintptr_t)g_irqstack_alloc + (3 * INTSTACK_SIZE) - 8,
 #endif
 #if CONFIG_SMP_NCPUS > 3
-  (uintptr_t)g_irqstack_alloc + 4 * INTSTACK_SIZE
+  (uintptr_t)g_irqstack_alloc + (4 * INTSTACK_SIZE) - 8
 #endif
 };
 
@@ -186,3 +186,35 @@ void up_irqinitialize(void)
   up_irq_enable();
 #endif
 }
+
+/****************************************************************************
+ * Name: arm_intstack_base
+ *
+ * Description:
+ *   Return a pointer to the "base" the correct interrupt stack allocation
+ *   for the current CPU. NOTE: Here, the base means "top" of the stack
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+uintptr_t arm_intstack_base(void)
+{
+  return g_irqstack_top[up_cpu_index()];
+}
+#endif
+
+/****************************************************************************
+ * Name: arm_intstack_alloc
+ *
+ * Description:
+ *   Return a pointer to the "alloc" the correct interrupt stack allocation
+ *   for the current CPU.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+uintptr_t arm_intstack_alloc(void)
+{
+  return g_irqstack_top[up_cpu_index()] - (INTSTACK_SIZE - 8);
+}
+#endif

arch/arm/src/imx6/imx_irq.h

@@ -67,25 +67,15 @@ extern "C"
 #define EXTERN extern
 #endif
 
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-/* In the SMP configuration, we will need custom IRQ and FIQ stacks.
- * These definitions provide the aligned stack allocations.
- */
-
-EXTERN uint64_t g_irqstack_alloc[];
-EXTERN uint64_t g_fiqstack_alloc[];
-
-/* These are arrays that point to the top of each interrupt stack */
-
-EXTERN uintptr_t g_irqstack_top[CONFIG_SMP_NCPUS];
-EXTERN uintptr_t g_irqstack_top[CONFIG_SMP_NCPUS];
-
-#endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
-
 /****************************************************************************
  * Public Function Prototypes
  ****************************************************************************/
 
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+EXTERN uintptr_t arm_intstack_base(void);
+EXTERN uintptr_t arm_intstack_alloc(void);
+#endif
+
 #undef EXTERN
 #if defined(__cplusplus)
 }

arch/arm/src/lc823450/chip.h

@@ -55,18 +55,6 @@
  * Public Types
  ****************************************************************************/
 
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-#ifdef __ASSEMBLY__
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-  .globl  g_instack_alloc
-#endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
-
-#endif /* __ASSEMBLY__ */
-
 /****************************************************************************
  * Macro Definitions
  ****************************************************************************/
@@ -84,77 +72,12 @@
 
 #if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
   .macro  setintstack, tmp1, tmp2
-#if CONFIG_SMP_NCPUS > 1
   ldr \tmp1, =CORE_COREID
-  ldr  \tmp1, [\tmp1, 0]     /* \tmp = getreg32(coreid_reg) */
+  ldr \tmp1, [\tmp1, 0]          /* tmp1 = getreg32(CORE_COREID)  */
-  and  \tmp1, \tmp1, 1       /* \tmp = COREID */
+  ldr \tmp2, =g_cpu_intstack_top
-  cmp  \tmp1, #0
+  ldr sp, [\tmp2, \tmp1, lsl #2] /* sp = g_cpu_intstack_top[tmp1] */
-  bne  1f
-  ldr  \tmp1, =g_cpu0_instack_base
-  ldr  sp, [\tmp1, 0]        /* sp = getreg32(g_cpu0_instack_base) */
-  b    2f
-1:
-  ldr  \tmp1, =g_cpu1_instack_base
-  ldr  sp, [\tmp1, 0]        /* sp = getreg32(g_cpu1_instack_base) */
-2:
-#else
-  ldr  \tmp1, =g_cpu0_instack_base
-  ldr  sp, [\tmp1, 0]        /* sp = getreg32(g_cpu0_instack_base) */
-#endif
   .endm
 #endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
 
 #endif /* __ASSEMBLY__  */
-
-/****************************************************************************
- * Inline Functions
- ****************************************************************************/
-
-#ifndef __ASSEMBLY__
-
-#ifdef __cplusplus
-#define EXTERN extern "C"
-extern "C"
-{
-#else
-#define EXTERN extern
-#endif
-
-/****************************************************************************
- * Name: arm_intstack_base
- *
- * Description:
- *   Set the current stack pointer to the "base" the correct interrupt stack
- *   allocation for the current CPU.
- *
- ****************************************************************************/
-
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-static inline uintptr_t arm_intstack_base(void)
-{
-  uintptr_t base = (uintptr_t)g_instack_alloc;
-#if CONFIG_SMP_NCPUS > 1
-  uint32_t coreid = getreg32(CORE_COREID);
-
-  if ((coreid & CORE_COREID_ID) != 0)
-    {
-      base += INTSTACK_SIZE;
-    }
-#endif
-
-  return base;
-}
-#endif
-
-/****************************************************************************
- * Public Function Prototypes
- ****************************************************************************/
-
-#if defined(__cplusplus)
-}
-#endif
-#undef EXTERN
-
-#endif /* !__ASSEMBLY__ */
-
 #endif /* _ARCH_ARM_SRC_LC823450_CHIP_H */

arch/arm/src/lc823450/lc823450_irq.c

@@ -89,17 +89,17 @@ volatile uint32_t *g_current_regs[1];
  * These definitions provide the aligned stack allocations.
  */
 
-uint64_t g_instack_alloc[INTSTACK_ALLOC >> 3];
+uint64_t g_intstack_alloc[INTSTACK_ALLOC >> 3];
 
 /* These definitions provide the "top" of the push-down stacks. */
 
-const uint32_t g_cpu0_instack_base =
+const uint32_t g_cpu_intstack_top[CONFIG_SMP_NCPUS] =
-  (uint32_t)g_instack_alloc + INTSTACK_SIZE;
+{
-
+  (uint32_t)g_intstack_alloc + INTSTACK_SIZE - 8,
 #if CONFIG_SMP_NCPUS > 1
-const uint32_t g_cpu1_instack_base =
+  (uint32_t)g_intstack_alloc + (2 * INTSTACK_SIZE) - 8,
-  (uint32_t)g_instack_alloc + 2 * INTSTACK_SIZE;
+#endif /* CONFIG_SMP_NCPUS > 1 */
-#endif
+};
 #endif
 
 /****************************************************************************
@@ -470,16 +470,6 @@ void up_irqinitialize(void)
   putreg32(0xffffffff, NVIC_IRQ0_31_CLEAR);
   putreg32(0xffffffff, NVIC_IRQ32_63_CLEAR);
 
-  /* Colorize the interrupt stack for debug purposes */
-
-#if defined(CONFIG_STACK_COLORATION) && CONFIG_ARCH_INTERRUPTSTACK > 3
-    {
-      size_t intstack_size = (CONFIG_ARCH_INTERRUPTSTACK & ~3);
-      arm_stack_color((FAR void *)((uintptr_t)&g_intstackbase -
-                      intstack_size), intstack_size);
-    }
-#endif
-
   /* The standard location for the vector table is at the beginning of FLASH
    * at address 0x0800:0000.  If we are using the STMicro DFU bootloader,
    * then the vector table will be offset to a different location in FLASH
@@ -869,3 +859,35 @@ int lc823450_irq_register(int irq, struct lc823450_irq_ops *ops)
   return OK;
 }
 #endif /* CONFIG_LC823450_VIRQ */
+
+/****************************************************************************
+ * Name: arm_intstack_base
+ *
+ * Description:
+ *   Return a pointer to the "base" the correct interrupt stack allocation
+ *   for the current CPU. NOTE: Here, the base means "top" of the stack
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+uintptr_t arm_intstack_base(void)
+{
+  return g_cpu_intstack_top[up_cpu_index()];
+}
+#endif
+
+/****************************************************************************
+ * Name: arm_intstack_alloc
+ *
+ * Description:
+ *   Return a pointer to the "alloc" the correct interrupt stack allocation
+ *   for the current CPU.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+uintptr_t arm_intstack_alloc(void)
+{
+  return g_cpu_intstack_top[up_cpu_index()] - (INTSTACK_SIZE - 8);
+}
+#endif

arch/arm/src/lc823450/lc823450_irq.h

@@ -67,26 +67,15 @@ extern "C"
 #define EXTERN extern
 #endif
 
-#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
-/* In the SMP configuration, we will need two custom interrupt stacks.
- * These definitions provide the aligned stack allocations.
- */
-
-EXTERN uint64_t g_instack_alloc[];
-
-/* These definitions provide the "top" of the push-down stacks. */
-
-EXTERN const uint32_t g_cpu0_instack_base;
-#if CONFIG_SMP_NCPUS > 1
-EXTERN const uint32_t g_cpu1_instack_base;
-#endif
-
-#endif /* CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 7 */
-
 /****************************************************************************
  * Public Function Prototypes
  ****************************************************************************/
 
+#if defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
+EXTERN uintptr_t arm_intstack_base(void);
+EXTERN uintptr_t arm_intstack_alloc(void);
+#endif
+
 #undef EXTERN
 #if defined(__cplusplus)
 }