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>
2020-10-19 21:48:47 +09:00 · 2020-10-19 21:48:47 +09:00 · dad5a79bf3
parent 56a081515c
commit dad5a79bf3
11 changed files with 131 additions and 248 deletions
--- a/arch/arm/src/common/arm_checkstack.c
+++ b/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,
--- a/arch/arm/src/common/arm_initialize.c
+++ b/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))
    {
--- a/arch/arm/src/cxd56xx/chip.h
+++ b/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 */
--- a/arch/arm/src/cxd56xx/cxd56_irq.c
+++ b/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;
-  uint32_t cpu = up_cpu_index();
-  base += cpu * INTSTACK_SIZE;
-
-  return base;
+  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
--- a/arch/arm/src/cxd56xx/cxd56_irq.h
+++ b/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)
 }
--- a/arch/arm/src/imx6/chip.h
+++ b/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 */
--- a/arch/arm/src/imx6/imx_irq.c
+++ b/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
--- a/arch/arm/src/imx6/imx_irq.h
+++ b/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)
 }
--- a/arch/arm/src/lc823450/chip.h
+++ b/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) */
-  and  \tmp1, \tmp1, 1       /* \tmp = COREID */
-  cmp  \tmp1, #0
-  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
+  ldr \tmp1, =CORE_COREID
+  ldr \tmp1, [\tmp1, 0]          /* tmp1 = getreg32(CORE_COREID)  */
+  ldr \tmp2, =g_cpu_intstack_top
+  ldr sp, [\tmp2, \tmp1, lsl #2] /* sp = g_cpu_intstack_top[tmp1] */
  .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 */
--- a/arch/arm/src/lc823450/lc823450_irq.c
+++ b/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 =
-  (uint32_t)g_instack_alloc + INTSTACK_SIZE;
-
+const uint32_t g_cpu_intstack_top[CONFIG_SMP_NCPUS] =
+{
+  (uint32_t)g_intstack_alloc + INTSTACK_SIZE - 8,
 #if CONFIG_SMP_NCPUS > 1
-const uint32_t g_cpu1_instack_base =
-  (uint32_t)g_instack_alloc + 2 * INTSTACK_SIZE;
-#endif
+  (uint32_t)g_intstack_alloc + (2 * INTSTACK_SIZE) - 8,
+#endif /* CONFIG_SMP_NCPUS > 1 */
+};
 #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
--- a/arch/arm/src/lc823450/lc823450_irq.h
+++ b/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)
 }