runtime/cores: add secondary core startup barrier

[rdon-key]

This fixes a startup race with scheduler=cores that was identified during the review of the RP2040 XIP / flash-safe changes in #5411.

startSecondaryCores() can return after a secondary core/hart has started, but before secondaryCoresStarted is set. During that window, the secondary core can enter the scheduler and run Go code while the runtime still considers the system single-core.

This is especially problematic for the GC. If GC starts on a secondary core while secondaryCoresStarted is still false, the single-core path can use the secondary core's system stack pointer together with the primary core's stackTop, resulting in an invalid stack range.

This change adds a startup barrier so secondary cores cannot enter the scheduler until the primary core has completed startSecondaryCores() and marked the multicore runtime as ready.

Changes

• Change secondaryCoresStarted to atomic.Uint32
• Set it after startSecondaryCores() returns
• Add waitForSecondaryCoresReady()
• On RP2, wait after enabling the FIFO interrupt and before entering the scheduler
• On RISC-V QEMU, wait after startup interrupt handling and interrupt setup, before entering the scheduler
• Update GC and RP2040 flash-safe checks to use atomic loads

The intended invariant is:

secondaryCoresStarted == 0
=> secondary cores cannot execute Go scheduler code

Reproducer

RP2040 / Pico

The race can be reproduced naturally, without calling runtime internals, by queueing a goroutine during init() and checking whether core 1 runs it before secondaryCoresStarted becomes true. A repeating timer alarm interrupt busy-waits in its handler until then, widening the race window.

Note: the reproducer reads the flag through //go:extern as a bool with a single-byte volatile load. Since the target is little-endian, this still works after the variable becomes atomic.Uint32, so the same reproducer runs unchanged on both dev and this change.

Details

package main

import (
        "runtime/interrupt"
        "runtime/volatile"
        "time"
        "unsafe"
)

//go:extern runtime.secondaryCoresStarted
var secondaryCoresStarted bool

const (
        sioCPUID = uintptr(0xd0000000)

        timerBase = uintptr(0x40054000)

        timerAlarm1 = timerBase + 0x14
        timerRawL   = timerBase + 0x28
        timerIntr   = timerBase + 0x34
        timerInte   = timerBase + 0x38

        alarmBit = uint32(1 << 1)

        irqPeriodUS = uint32(2)
        irqHoldUS   = uint32(100)
)

var (
        workerCore uint32 = 99
        workerFlag bool
        workerRan  bool

        irqCount uint32
)

func reg32(addr uintptr) *volatile.Register32 {
        return (*volatile.Register32)(unsafe.Pointer(addr))
}

func coreID() uint32 {
        return *(*uint32)(unsafe.Pointer(sioCPUID))
}

// Read runtime.secondaryCoresStarted using an actual volatile load.
func coresStarted() bool {
        reg := (*volatile.Register8)(unsafe.Pointer(&secondaryCoresStarted))
        return reg.Get() != 0
}

var alarmIRQ = interrupt.New(1, alarmHandler)

func alarmHandler(interrupt.Interrupt) {
        reg32(timerIntr).Set(alarmBit)

        irqCount++

        if coresStarted() {
                reg32(timerInte).ClearBits(alarmBit)
                return
        }

        start := reg32(timerRawL).Get()
        for uint32(reg32(timerRawL).Get()-start) < irqHoldUS {
        }

        now := reg32(timerRawL).Get()
        reg32(timerAlarm1).Set(now + irqPeriodUS)
}

func init() {
        go func() {
                workerCore = coreID()
                workerFlag = coresStarted()
                workerRan = true
        }()

        reg32(timerIntr).Set(alarmBit)
        reg32(timerInte).SetBits(alarmBit)
        alarmIRQ.Enable()

        now := reg32(timerRawL).Get()
        reg32(timerAlarm1).Set(now + irqPeriodUS)
}

func main() {
        time.Sleep(2 * time.Second)

        println("============================")
        println("A06 VOLATILE STARTUP RACE")
        println("============================")
        println("worker ran:", workerRan)
        println("worker core:", workerCore)
        println("flag seen by worker:", workerFlag)
        println("flag now:", coresStarted())
        println("IRQ count:", irqCount)
}

On dev:

worker ran: true
worker core: 1
flag seen by worker: false
flag now: true
IRQ count: 7

With this change:

worker ran: true
worker core: 1
flag seen by worker: true
flag now: true
IRQ count: 7

RISC-V QEMU

This reproducer is intentionally artificial: it calls startSecondaryCores() directly from init() via //go:linkname and exits before the runtime would set secondaryCoresStarted. It only checks whether a secondary hart can reach the scheduler before the flag is set. With this change, the hart stays blocked at the barrier for the whole test.

Details

package main

import (
        "device/riscv"
        "os"
        "runtime/volatile"
        "sync/atomic"
        "unsafe"
)

//go:linkname startSecondaryCores runtime.startSecondaryCores
func startSecondaryCores()

var (
        workerStarted atomic.Uint32
        workerHart    atomic.Uint32
)

func qemuTicks() uint64 {
        low := (*volatile.Register32)(unsafe.Pointer(uintptr(0x0200bff8)))
        high := (*volatile.Register32)(unsafe.Pointer(uintptr(0x0200bffc)))

        hi := high.Get()
        for {
                lo := low.Get()
                hi2 := high.Get()
                if hi == hi2 {
                        return uint64(lo) | uint64(hi)<<32
                }
                hi = hi2
        }
}

func worker() {
        workerHart.Store(uint32(riscv.MHARTID.Get()))
        workerStarted.Store(1)
}

func init() {
        println("============================")
        println("A16 RISC-V STARTUP BARRIER")
        println("============================")

        go worker()

        // Deliberately start secondary harts.
        startSecondaryCores()

        // If there is no startup barrier, a secondary hart can enter the scheduler
        // and run this worker during init().
        start := qemuTicks()
        for qemuTicks()-start < 500_000 && workerStarted.Load() == 0 {
                riscv.Asm("pause")
        }

        if workerStarted.Load() != 0 {
                println("FAIL: secondary ran during startup")
                println("worker hart:", workerHart.Load())
        } else {
                println("PASS: startup barrier blocked secondary during startup")
        }

        println("test completed.")
        os.Exit(0)
}

func main() {}

On dev:

FAIL: secondary ran during startup
worker hart: 1
test completed.

With this change:

PASS: startup barrier blocked secondary during startup
test completed.

Testing

Tested with -scheduler=cores.

• RP2040 / Pico: see the reproducer above.
• RP2350 / Pico 2: the same reproducer did not observe the race in 10 runs on dev. The reason is unclear, but RP2350 shares the same RP2 startup path as RP2040, so the same race window exists. With this change, the worker observed secondaryCoresStarted=true.
• RISC-V QEMU: see the reproducer above. Normal multicore GC also completed successfully in 10/10 runs.

[deadprogram]

Thanks for the fix and the clear reproducers @rdon-key! The following is edited from an automated review:

1. waitForSecondaryCoresReady() spins on an empty loop. The other wait loops in gc_stack_cores.go and runtime_rp2040_flashsafe_cores.go call spinLoopWait(), which both RP2 and RISC-V QEMU already define. Perhaps use it here too for consistency?

2. secondaryCoresStarted now means "secondary cores may enter the scheduler", as the new comment says. Should you rename it to something like secondaryCoresReady so the name matches?

3. Each target has to remember to call waitForSecondaryCoresReady() between its interrupt setup and schedulerLock.Lock(). Any future scheduler=cores port (for example ESP32-S3) will need the same call. Perhaps a short comment at the call sites saying it must come after the stop-the-world interrupt is enabled would help the next person? Seems like a good idea maybe?

[rdon-key]

Thank you for the review.

Item 1:
I kept the empty polling loop because spinLoopWait() uses WFE on RP2, but setting the ready flag does not issue SEV.

Items 2 and 3:
I renamed the flag and added comments at the call sites.

[rdon-key]

This PR has been rebased, and binary-size.txt has been updated.

[deadprogram]

Thanks for the improvement @rdon-key and for handling all the feedback items. Now squash/merging.