Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
arch: Remove the abuse of spinlock in smp boot
Browse files Browse the repository at this point in the history
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 once

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
xiaoxiang781216 committed Jan 30, 2025
1 parent 80e545a commit bbd6b4d
Showing 8 changed files with 99 additions and 164 deletions.
11 changes: 3 additions & 8 deletions arch/arm/src/cxd56xx/cxd56_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}

11 changes: 3 additions & 8 deletions arch/arm/src/lc823450/lc823450_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}
11 changes: 3 additions & 8 deletions arch/arm/src/rp2040/rp2040_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}

11 changes: 3 additions & 8 deletions arch/arm/src/rp23xx/rp23xx_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}

10 changes: 3 additions & 7 deletions arch/arm/src/sam34/sam4cm_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}

11 changes: 3 additions & 8 deletions arch/sparc/src/s698pm/s698pm_cpustart.c
Original file line number Diff line number Diff line change
@@ -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;
}
98 changes: 40 additions & 58 deletions arch/xtensa/src/esp32/esp32_cpustart.c
Original file line number Diff line number Diff line change
@@ -32,7 +32,6 @@

#include <nuttx/arch.h>
#include <nuttx/sched.h>
#include <nuttx/spinlock.h>
#include <nuttx/sched_note.h>

#include "sched/sched.h"
@@ -51,7 +50,6 @@
****************************************************************************/

static volatile bool g_appcpu_started;
static volatile spinlock_t g_appcpu_interlock;

/****************************************************************************
* ROM function prototypes
@@ -128,12 +126,9 @@
sched_note_cpu_started(tcb);
#endif

/* Release the spinlock to signal to the PRO CPU that the APP CPU has
* started.
*/
/* Signal to the PRO CPU that the APP CPU has started. */

g_appcpu_started = true;
spin_unlock(&g_appcpu_interlock);

/* Reset scheduler parameters */

@@ -220,81 +215,68 @@

int up_cpu_start(int cpu)
{
uint32_t regval;

DEBUGASSERT(cpu >= 0 && cpu < CONFIG_SMP_NCPUS && cpu != this_cpu());

if (!g_appcpu_started)
{
uint32_t regval;

Check failure on line 222 in arch/xtensa/src/esp32/esp32_cpustart.c

GitHub Actions / check

Too many blank lines
/* Start CPU1 */
/* Start CPU1 */

sinfo("Starting CPU%d\n", cpu);
sinfo("Starting CPU%d\n", cpu);

#ifdef CONFIG_SCHED_INSTRUMENTATION
/* Notify of the start event */
/* Notify of the start event */

sched_note_cpu_start(this_task(), cpu);
sched_note_cpu_start(this_task(), cpu);
#endif

/* This spinlock will be used as a handshake between the two CPUs.
* It's first initialized to its locked state, later the PRO CPU will
* try to lock it but spins until the APP CPU starts and unlocks it.
*/

spin_lock_init(&g_appcpu_interlock);
spin_lock(&g_appcpu_interlock);

/* Unstall the APP CPU */
/* Unstall the APP CPU */

regval = getreg32(RTC_CNTL_SW_CPU_STALL_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C1_M;
putreg32(regval, RTC_CNTL_SW_CPU_STALL_REG);
regval = getreg32(RTC_CNTL_SW_CPU_STALL_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C1_M;
putreg32(regval, RTC_CNTL_SW_CPU_STALL_REG);

regval = getreg32(RTC_CNTL_OPTIONS0_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C0_M;
putreg32(regval, RTC_CNTL_OPTIONS0_REG);
regval = getreg32(RTC_CNTL_OPTIONS0_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C0_M;
putreg32(regval, RTC_CNTL_OPTIONS0_REG);

/* OpenOCD might have already enabled clock gating and taken APP CPU
* out of reset. Don't reset the APP CPU if that's the case as this
* will clear the breakpoints that may have already been set.
*/

regval = getreg32(DPORT_APPCPU_CTRL_B_REG);
if ((regval & DPORT_APPCPU_CLKGATE_EN_M) == 0)
{
/* Enable clock gating for the APP CPU */
/* OpenOCD might have already enabled clock gating and taken APP CPU
* out of reset. Don't reset the APP CPU if that's the case as this
* will clear the breakpoints that may have already been set.
*/

regval |= DPORT_APPCPU_CLKGATE_EN;
putreg32(regval, DPORT_APPCPU_CTRL_B_REG);
regval = getreg32(DPORT_APPCPU_CTRL_B_REG);
if ((regval & DPORT_APPCPU_CLKGATE_EN_M) == 0)
{
/* Enable clock gating for the APP CPU */

regval = getreg32(DPORT_APPCPU_CTRL_C_REG);
regval &= ~DPORT_APPCPU_RUNSTALL;
putreg32(regval, DPORT_APPCPU_CTRL_C_REG);
regval |= DPORT_APPCPU_CLKGATE_EN;
putreg32(regval, DPORT_APPCPU_CTRL_B_REG);

/* Reset the APP CPU */
regval = getreg32(DPORT_APPCPU_CTRL_C_REG);
regval &= ~DPORT_APPCPU_RUNSTALL;
putreg32(regval, DPORT_APPCPU_CTRL_C_REG);

regval = getreg32(DPORT_APPCPU_CTRL_A_REG);
regval |= DPORT_APPCPU_RESETTING;
putreg32(regval, DPORT_APPCPU_CTRL_A_REG);
/* Reset the APP CPU */

regval = getreg32(DPORT_APPCPU_CTRL_A_REG);
regval &= ~DPORT_APPCPU_RESETTING;
putreg32(regval, DPORT_APPCPU_CTRL_A_REG);
}
regval = getreg32(DPORT_APPCPU_CTRL_A_REG);
regval |= DPORT_APPCPU_RESETTING;
putreg32(regval, DPORT_APPCPU_CTRL_A_REG);

/* Set the CPU1 start address */
regval = getreg32(DPORT_APPCPU_CTRL_A_REG);
regval &= ~DPORT_APPCPU_RESETTING;
putreg32(regval, DPORT_APPCPU_CTRL_A_REG);
}

ets_set_appcpu_boot_addr((uint32_t)xtensa_appcpu_start);
/* Set the CPU1 start address */

/* And wait until the APP CPU starts and releases the spinlock. */
ets_set_appcpu_boot_addr((uint32_t)xtensa_appcpu_start);

spin_lock(&g_appcpu_interlock);
/* And wait until the APP CPU starts */

/* prev cpu boot done */
while (!g_appcpu_started);

spin_unlock(&g_appcpu_interlock);
DEBUGASSERT(g_appcpu_started);
}
/* prev cpu boot done */

return OK;
}
100 changes: 41 additions & 59 deletions arch/xtensa/src/esp32s3/esp32s3_cpustart.c
Original file line number Diff line number Diff line change
@@ -33,9 +33,8 @@
#include <nuttx/arch.h>
#include <nuttx/sched.h>
#include <nuttx/sched_note.h>
#include <nuttx/spinlock.h>
#include <sched/sched.h>

#include "sched/sched.h"
#include "xtensa.h"
#include "esp32s3_irq.h"
#include "esp32s3_region.h"
@@ -48,7 +47,6 @@
****************************************************************************/

static volatile bool g_appcpu_started;
static volatile spinlock_t g_appcpu_interlock;

/****************************************************************************
* ROM function prototypes
@@ -127,12 +125,9 @@
sched_note_cpu_started(tcb);
#endif

/* Release the spinlock to signal to the PRO CPU that the APP CPU has
* started.
*/
/* Signal to the PRO CPU that the APP CPU has started. */

g_appcpu_started = true;
spin_unlock(&g_appcpu_interlock);

/* Reset scheduler parameters */

@@ -205,80 +200,67 @@

int up_cpu_start(int cpu)
{
uint32_t regval;

DEBUGASSERT(cpu >= 0 && cpu < CONFIG_SMP_NCPUS && cpu != this_cpu());

if (!g_appcpu_started)
{
uint32_t regval;

Check failure on line 207 in arch/xtensa/src/esp32s3/esp32s3_cpustart.c

GitHub Actions / check

Too many blank lines
/* Start CPU1 */
/* Start CPU1 */

sinfo("Starting CPU%d\n", cpu);
sinfo("Starting CPU%d\n", cpu);

#ifdef CONFIG_SCHED_INSTRUMENTATION
/* Notify of the start event */
/* Notify of the start event */

sched_note_cpu_start(this_task(), cpu);
sched_note_cpu_start(this_task(), cpu);
#endif

/* This spinlock will be used as a handshake between the two CPUs.
* It's first initialized to its locked state, later the PRO CPU will
* try to lock it but spins until the APP CPU starts and unlocks it.
*/

spin_lock_init(&g_appcpu_interlock);
spin_lock(&g_appcpu_interlock);

/* OpenOCD might have already enabled clock gating and taken APP CPU
* out of reset. Don't reset the APP CPU if that's the case as this
* will clear the breakpoints that may have already been set.
*/

regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
if ((regval & SYSTEM_CONTROL_CORE_1_CLKGATE_EN) == 0)
{
regval = getreg32(RTC_CNTL_RTC_SW_CPU_STALL_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C1_M;
putreg32(regval, RTC_CNTL_RTC_SW_CPU_STALL_REG);
/* OpenOCD might have already enabled clock gating and taken APP CPU
* out of reset. Don't reset the APP CPU if that's the case as this
* will clear the breakpoints that may have already been set.
*/

regval = getreg32(RTC_CNTL_RTC_OPTIONS0_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C0_M;
putreg32(regval, RTC_CNTL_RTC_OPTIONS0_REG);
regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
if ((regval & SYSTEM_CONTROL_CORE_1_CLKGATE_EN) == 0)
{
regval = getreg32(RTC_CNTL_RTC_SW_CPU_STALL_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C1_M;
putreg32(regval, RTC_CNTL_RTC_SW_CPU_STALL_REG);

/* Enable clock gating for the APP CPU */
regval = getreg32(RTC_CNTL_RTC_OPTIONS0_REG);
regval &= ~RTC_CNTL_SW_STALL_APPCPU_C0_M;
putreg32(regval, RTC_CNTL_RTC_OPTIONS0_REG);

regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval |= SYSTEM_CONTROL_CORE_1_CLKGATE_EN;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);
/* Enable clock gating for the APP CPU */

regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval &= ~SYSTEM_CONTROL_CORE_1_RUNSTALL;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);
regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval |= SYSTEM_CONTROL_CORE_1_CLKGATE_EN;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);

/* Reset the APP CPU */
regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval &= ~SYSTEM_CONTROL_CORE_1_RUNSTALL;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);

regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval |= SYSTEM_CONTROL_CORE_1_RESETING;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);
/* Reset the APP CPU */

regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval &= ~SYSTEM_CONTROL_CORE_1_RESETING;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);
}
regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval |= SYSTEM_CONTROL_CORE_1_RESETING;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);

/* Set the CPU1 start address */
regval = getreg32(SYSTEM_CORE_1_CONTROL_0_REG);
regval &= ~SYSTEM_CONTROL_CORE_1_RESETING;
putreg32(regval, SYSTEM_CORE_1_CONTROL_0_REG);
}

ets_set_appcpu_boot_addr((uint32_t)xtensa_appcpu_start);
/* Set the CPU1 start address */

/* And wait until the APP CPU starts and releases the spinlock. */
ets_set_appcpu_boot_addr((uint32_t)xtensa_appcpu_start);

spin_lock(&g_appcpu_interlock);
/* And wait until the APP CPU starts */

/* prev cpu boot done */
while (!g_appcpu_started);

spin_unlock(&g_appcpu_interlock);
DEBUGASSERT(g_appcpu_started);
}
/* prev cpu boot done */

return OK;
}

0 comments on commit bbd6b4d

Please sign in to comment.