arch: Remove the abuse of spinlock in smp boot

What's really need is a done signal sent from the secondary cpu to the boot cpu, so let's simplify the logic by: 1.Change the spinlock to a bool flag 2.Set the flag to true in the secondary cpu 3.Wait the flag set in the boot cpu before continue booting This also remove all bad usage of spinlock from the code base: 1.Lock spinlock in one thread, but unlock in a different thread 2.Lock spinlock twice in one thread, but unlock only once Signed-off-by: Xiang Xiao <[email protected]>
apache · Jan 30, 2025 · 9c4b043 · 9c4b043
1 parent 80e545a
commit 9c4b043
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 166 deletions.
diff --git a/arch/arm/src/cxd56xx/cxd56_cpustart.c b/arch/arm/src/cxd56xx/cxd56_cpustart.c
@@ -34,7 +34,6 @@
 #include <errno.h>
 
 #include <nuttx/arch.h>
-#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
 #include "nvic.h"
@@ -72,7 +71,7 @@
  * Public Data
  ****************************************************************************/
 
-volatile static spinlock_t g_appdsp_boot;
+static volatile bool g_appdsp_boot;
 
 extern int cxd56_smp_call_handler(int irq, void *c, void *arg);
 
@@ -122,7 +121,7 @@ static void appdsp_boot(void)
   irq_attach(CXD56_IRQ_SMP_CALL, cxd56_smp_call_handler, NULL);
   up_enable_irq(CXD56_IRQ_SMP_CALL);
 
-  spin_unlock(&g_appdsp_boot);
+  g_appdsp_boot = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -189,8 +188,6 @@ int up_cpu_start(int cpu)
                      tcb->adj_stack_size, VECTOR_ISTACK);
   putreg32((uint32_t)appdsp_boot, VECTOR_RESETV);
 
-  spin_lock(&g_appdsp_boot);
-
   /* See 3.13.4.16.3 ADSP Startup */
 
   /* 2. Clock supply */
@@ -238,12 +235,10 @@ int up_cpu_start(int cpu)
       up_enable_irq(CXD56_IRQ_SMP_CALL);
     }
 
-  spin_lock(&g_appdsp_boot);
+  while (!g_appdsp_boot);
 
   /* APP_DSP(cpu) boot done */
 
-  spin_unlock(&g_appdsp_boot);
-
   return 0;
 }
 

diff --git a/arch/arm/src/lc823450/lc823450_cpustart.c b/arch/arm/src/lc823450/lc823450_cpustart.c
@@ -35,7 +35,6 @@
 #include <stdio.h>
 
 #include <nuttx/arch.h>
-#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
 #include "nvic.h"
@@ -65,7 +64,7 @@
  * Public Data
  ****************************************************************************/
 
-static volatile spinlock_t g_cpu_wait[CONFIG_SMP_NCPUS];
+static volatile bool g_cpu_wait[CONFIG_SMP_NCPUS];
 
 /****************************************************************************
  * Private Functions
@@ -112,7 +111,7 @@ static void cpu1_boot(void)
       up_enable_irq(LC823450_IRQ_SMP_CALL_01);
     }
 
-  spin_unlock(&g_cpu_wait[0]);
+  g_cpu_wait[0] = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -177,8 +176,6 @@ int up_cpu_start(int cpu)
                      tcb->adj_stack_size, CPU1_VECTOR_ISTACK);
   putreg32((uint32_t)cpu1_boot, CPU1_VECTOR_RESETV);
 
-  spin_lock(&g_cpu_wait[0]);
-
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify of the start event */
 
@@ -198,7 +195,7 @@ int up_cpu_start(int cpu)
   irq_attach(LC823450_IRQ_SMP_CALL_11, lc823450_smp_call_handler, NULL);
   up_enable_irq(LC823450_IRQ_SMP_CALL_11);
 
-  spin_lock(&g_cpu_wait[0]);
+  while (!g_cpu_wait[0]);
 
   /* CPU1 boot done */
 
@@ -208,7 +205,5 @@ int up_cpu_start(int cpu)
   putreg32(backup[1], CPU1_VECTOR_RESETV);
   putreg32(0x0, REMAP); /* remap disable */
 
-  spin_unlock(&g_cpu_wait[0]);
-
   return 0;
 }
diff --git a/arch/arm/src/rp2040/rp2040_cpustart.c b/arch/arm/src/rp2040/rp2040_cpustart.c
@@ -34,7 +34,6 @@
 #include <errno.h>
 
 #include <nuttx/arch.h>
-#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
 #include "nvic.h"
@@ -67,7 +66,7 @@
  * Public Data
  ****************************************************************************/
 
-volatile static spinlock_t g_core1_boot;
+static volatile bool g_core1_boot;
 
 extern int rp2040_smp_call_handler(int irq, void *c, void *arg);
 
@@ -156,7 +155,7 @@ static void core1_boot(void)
   irq_attach(RP2040_SMP_CALL_PROC1, rp2040_smp_call_handler, NULL);
   up_enable_irq(RP2040_SMP_CALL_PROC1);
 
-  spin_unlock(&g_core1_boot);
+  g_core1_boot = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -221,8 +220,6 @@ int up_cpu_start(int cpu)
     ;
   clrbits_reg32(RP2040_PSM_PROC1, RP2040_PSM_FRCE_OFF);
 
-  spin_lock(&g_core1_boot);
-
   /* Send initial VTOR, MSP, PC for Core 1 boot */
 
   core1_boot_msg[0] = 0;
@@ -252,12 +249,10 @@ int up_cpu_start(int cpu)
   irq_attach(RP2040_SMP_CALL_PROC0, rp2040_smp_call_handler, NULL);
   up_enable_irq(RP2040_SMP_CALL_PROC0);
 
-  spin_lock(&g_core1_boot);
+  while (!g_core1_boot);
 
   /* CPU Core 1 boot done */
 
-  spin_unlock(&g_core1_boot);
-
   return 0;
 }
 

diff --git a/arch/arm/src/rp23xx/rp23xx_cpustart.c b/arch/arm/src/rp23xx/rp23xx_cpustart.c
@@ -34,7 +34,6 @@
 #include <errno.h>
 
 #include <nuttx/arch.h>
-#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
 #include "nvic.h"
@@ -67,7 +66,7 @@
  * Public Data
  ****************************************************************************/
 
-volatile static spinlock_t g_core1_boot;
+static volatile bool g_core1_boot;
 
 extern int rp23xx_smp_call_handler(int irq, void *c, void *arg);
 
@@ -156,7 +155,7 @@ static void core1_boot(void)
   irq_attach(RP23XX_SIO_IRQ_FIFO, rp23xx_smp_call_handler, NULL);
   up_enable_irq(RP23XX_SIO_IRQ_FIFO);
 
-  spin_unlock(&g_core1_boot);
+  g_core1_boot = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -221,8 +220,6 @@ int up_cpu_start(int cpu)
     ;
   clrbits_reg32(RP23XX_PSM_PROC1, RP23XX_PSM_FRCE_OFF);
 
-  spin_lock(&g_core1_boot);
-
   /* Send initial VTOR, MSP, PC for Core 1 boot */
 
   core1_boot_msg[0] = 0;
@@ -252,12 +249,10 @@ int up_cpu_start(int cpu)
   irq_attach(RP23XX_SIO_IRQ_FIFO, rp23xx_smp_call_handler, NULL);
   up_enable_irq(RP23XX_SIO_IRQ_FIFO);
 
-  spin_lock(&g_core1_boot);
+  while (!g_core1_boot);
 
   /* CPU Core 1 boot done */
 
-  spin_unlock(&g_core1_boot);
-
   return 0;
 }
 

diff --git a/arch/arm/src/sam34/sam4cm_cpustart.c b/arch/arm/src/sam34/sam4cm_cpustart.c
@@ -65,7 +65,7 @@
  * Public Data
  ****************************************************************************/
 
-volatile static spinlock_t g_cpu1_boot;
+static volatile bool g_cpu1_boot;
 extern int sam4cm_smp_call_handler(int irq, void *c, void *arg);
 
 /****************************************************************************
@@ -114,7 +114,7 @@ static void cpu1_boot(void)
       up_enable_irq(SAM_IRQ_SMP_CALL1);
     }
 
-  spin_unlock(&g_cpu1_boot);
+  g_cpu1_boot = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -209,8 +209,6 @@ int up_cpu_start(int cpu)
                      tcb->adj_stack_size, CPU1_VECTOR_ISTACK);
   putreg32((uint32_t)cpu1_boot, CPU1_VECTOR_RESETV);
 
-  spin_lock(&g_cpu1_boot);
-
   /* Unreset coprocessor */
 
   putreg32(0x5a000011, SAM_RSTC_CPMR);
@@ -223,12 +221,10 @@ int up_cpu_start(int cpu)
   irq_attach(SAM_IRQ_SMP_CALL0, sam4cm_smp_call_handler, NULL);
   up_enable_irq(SAM_IRQ_SMP_CALL0);
 
-  spin_lock(&g_cpu1_boot);
+  while (!g_cpu1_boot);
 
   /* CPU1 boot done */
 
-  spin_unlock(&g_cpu1_boot);
-
   return 0;
 }
 

diff --git a/arch/sparc/src/s698pm/s698pm_cpustart.c b/arch/sparc/src/s698pm/s698pm_cpustart.c
@@ -34,7 +34,6 @@
 #include <errno.h>
 
 #include <nuttx/arch.h>
-#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
 #include "sched/sched.h"
@@ -51,7 +50,7 @@
  * Public Data
  ****************************************************************************/
 
-volatile static spinlock_t g_cpu_boot;
+static volatile bool g_cpu_boot;
 
 /****************************************************************************
  * Public Functions
@@ -79,7 +78,7 @@ void s698pm_cpu_boot(void)
 
   s698pm_cpuint_initialize();
 
-  spin_unlock(&g_cpu_boot);
+  g_cpu_boot = true;
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
   /* Notify that this CPU has started */
@@ -150,17 +149,13 @@ int up_cpu_start(int cpu)
   regaddr = S698PM_DSU_BASE + (0x1000000 * cpu) + S698PM_DSU_NPC_OFFSET;
   putreg32(0x40001004, regaddr);
 
-  spin_lock(&g_cpu_boot);
-
   /* set 1 to bit n of multiprocessor status register to active cpu n */
 
   putreg32(1 << cpu, S698PM_IRQREG_MPSTATUS);
 
-  spin_lock(&g_cpu_boot);
+  while (!g_cpu_boot);
 
   /* prev cpu boot done */
 
-  spin_unlock(&g_cpu_boot);
-
   return 0;
 }