From d1eca84edb6eb095d14d8dac41ae57503b0807b5 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Tue, 9 Sep 2025 20:13:09 -0400 Subject: [PATCH 1/6] fix(rp2350): add software spinlocks --- src/runtime/runtime_rp2.go | 30 ------------------ src/runtime/runtime_rp2040.go | 33 ++++++++++++++++++++ src/runtime/runtime_rp2350.go | 58 +++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 30 deletions(-) diff --git a/src/runtime/runtime_rp2.go b/src/runtime/runtime_rp2.go index 08ae865699..9a6a1c1bda 100644 --- a/src/runtime/runtime_rp2.go +++ b/src/runtime/runtime_rp2.go @@ -298,36 +298,6 @@ var ( futexLock = spinLock{id: 23} ) -func resetSpinLocks() { - for i := uint8(0); i < numSpinlocks; i++ { - l := &spinLock{id: i} - l.spinlock().Set(0) - } -} - -// A hardware spinlock, one of the 32 spinlocks defined in the SIO peripheral. -type spinLock struct { - id uint8 -} - -// Return the spinlock register: rp.SIO.SPINLOCKx -func (l *spinLock) spinlock() *volatile.Register32 { - return (*volatile.Register32)(unsafe.Add(unsafe.Pointer(&rp.SIO.SPINLOCK0), l.id*4)) -} - -func (l *spinLock) Lock() { - // Wait for the lock to be available. - spinlock := l.spinlock() - for spinlock.Get() == 0 { - arm.Asm("wfe") - } -} - -func (l *spinLock) Unlock() { - l.spinlock().Set(0) - arm.Asm("sev") -} - // Wait until a signal is received, indicating that it can resume from the // spinloop. func spinLoopWait() { diff --git a/src/runtime/runtime_rp2040.go b/src/runtime/runtime_rp2040.go index 2ca3605e03..84377ace37 100644 --- a/src/runtime/runtime_rp2040.go +++ b/src/runtime/runtime_rp2040.go @@ -3,10 +3,43 @@ package runtime import ( + "device/arm" "device/rp" + "runtime/volatile" + "unsafe" ) const ( sioIrqFifoProc0 = rp.IRQ_SIO_IRQ_PROC0 sioIrqFifoProc1 = rp.IRQ_SIO_IRQ_PROC1 ) + +func resetSpinLocks() { + for i := uint8(0); i < numSpinlocks; i++ { + l := &spinLock{id: i} + l.spinlock().Set(0) + } +} + +// A hardware spinlock, one of the 32 spinlocks defined in the SIO peripheral. +type spinLock struct { + id uint8 +} + +// Return the spinlock register: rp.SIO.SPINLOCKx +func (l *spinLock) spinlock() *volatile.Register32 { + return (*volatile.Register32)(unsafe.Add(unsafe.Pointer(&rp.SIO.SPINLOCK0), l.id*4)) +} + +func (l *spinLock) Lock() { + // Wait for the lock to be available. + spinlock := l.spinlock() + for spinlock.Get() == 0 { + arm.Asm("wfe") + } +} + +func (l *spinLock) Unlock() { + l.spinlock().Set(0) + arm.Asm("sev") +} diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index 91af23212e..facf88b680 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -3,6 +3,7 @@ package runtime import ( + "device/arm" "device/rp" ) @@ -14,3 +15,60 @@ const ( sioIrqFifoProc0 = rp.IRQ_SIO_IRQ_FIFO sioIrqFifoProc1 = rp.IRQ_SIO_IRQ_FIFO ) + +// Due to hardware errata RP2350-E2 spinlocks are emulated in software as in pico-sdk. +type spinLock struct { + // lock field must be first field so its address is the same as the struct address. + lock uint8 + id uint8 + uint16 // Padding to prevent false sharing +} + +func (l *spinLock) Lock() { + // Original reference: + // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L112 + + // r0 is automatically filled with the pointer value "l" here. + // We create a variable to allow access the lock byte and avoid a memory + // fault when accessing l.lock in assembly. + lock := &l.lock + _ = lock + + // Set a loop start point + arm.Asm("1:") + // Exclusively load the state variable (l.lock) and put its value in r2. + arm.Asm("ldaexb r2, [r0]") + // Store the "locked" state value (1) into r1 to keep things moving. + arm.Asm("movs r1, #1") + // Check if the lock was already taken (r2 != 0). + arm.Asm("cmp r2, #0") + // Jump back to "1:" if the lock is already held + arm.Asm("bne 1b") + + // Attempt to store '1' into the lock address. + // The return code (0 for success, 1 for failure) is placed in r2. + arm.Asm("strexb r2, r1, [r0]") + // Check if the result was successful (r2 == 0). + arm.Asm("cmp r2, #0") + // Jump back to "1:" if the lock was not acquired. + arm.Asm("bne 1b") + + // Memory barrier to ensure everyone knows we're holding the lock now. + arm.Asm("dmb") +} + +func (l *spinLock) Unlock() { + // Original reference: + // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L197 + + // r0 is automatically filled with the pointer value l here. + // We create a variable to allow access the lock byte and avoid a memory + // fault when accessing l.lock in assembly. + lock := &l.lock + _ = lock + // Fill r1 with 0 and store it to the lock address in r0. stlb requires a + // register as a source so we can't use a literal 0 directly. + arm.Asm("movs r1, #0") + // Release the pseudo-spinlock by writing 0 to and releasing l.lock. + arm.Asm("stlb r1, [r0]") +} From fc8116054befe5925f8516f1aef18a8f5eab63e4 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Tue, 9 Sep 2025 21:52:08 -0400 Subject: [PATCH 2/6] chore: clarify comments --- src/runtime/runtime_rp2350.go | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index facf88b680..b9b5decd2a 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -18,9 +18,9 @@ const ( // Due to hardware errata RP2350-E2 spinlocks are emulated in software as in pico-sdk. type spinLock struct { - // lock field must be first field so its address is the same as the struct address. - lock uint8 - id uint8 + // state field must be first field so its address is the same as the struct address. + state uint8 + id uint8 uint16 // Padding to prevent false sharing } @@ -29,28 +29,28 @@ func (l *spinLock) Lock() { // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L112 // r0 is automatically filled with the pointer value "l" here. - // We create a variable to allow access the lock byte and avoid a memory - // fault when accessing l.lock in assembly. - lock := &l.lock - _ = lock + // We create a variable to permit access to the state byte (l.state) and + // avoid a memory fault when accessing it in assembly. + state := &l.state + _ = state - // Set a loop start point + // Set the loop start point. arm.Asm("1:") - // Exclusively load the state variable (l.lock) and put its value in r2. + // Exclusively load (lock) the state byte and put its value in r2. arm.Asm("ldaexb r2, [r0]") - // Store the "locked" state value (1) into r1 to keep things moving. + // Set the r1 register to '1' for later use. arm.Asm("movs r1, #1") // Check if the lock was already taken (r2 != 0). arm.Asm("cmp r2, #0") - // Jump back to "1:" if the lock is already held + // Jump back to the loop start ("1:") if the lock is already held. arm.Asm("bne 1b") - // Attempt to store '1' into the lock address. + // Attempt to store '1' into the lock state byte. // The return code (0 for success, 1 for failure) is placed in r2. arm.Asm("strexb r2, r1, [r0]") // Check if the result was successful (r2 == 0). arm.Asm("cmp r2, #0") - // Jump back to "1:" if the lock was not acquired. + // Jump back to the loop start ("1:") if the lock was not acquired. arm.Asm("bne 1b") // Memory barrier to ensure everyone knows we're holding the lock now. @@ -62,13 +62,13 @@ func (l *spinLock) Unlock() { // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L197 // r0 is automatically filled with the pointer value l here. - // We create a variable to allow access the lock byte and avoid a memory - // fault when accessing l.lock in assembly. - lock := &l.lock - _ = lock - // Fill r1 with 0 and store it to the lock address in r0. stlb requires a - // register as a source so we can't use a literal 0 directly. + // We create a variable to permit access to the state byte (l.state) and + // avoid a memory fault when accessing it in assembly. + state := &l.state + _ = state + // Fill r1 with 0 and store it to the state byte address in r0. stlb + // requires a register as a source so we can't use a literal 0 directly. arm.Asm("movs r1, #0") - // Release the pseudo-spinlock by writing 0 to and releasing l.lock. + // Release the pseudo-spinlock by writing 0 to and releasing l.state. arm.Asm("stlb r1, [r0]") } From 31696c2df5dfa0279a8a5445f504307b56b90537 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Sep 2025 18:58:50 -0400 Subject: [PATCH 3/6] chore: use atomics instead of artisinal assembly --- src/runtime/runtime_rp2350.go | 59 +++++++---------------------------- 1 file changed, 12 insertions(+), 47 deletions(-) diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index b9b5decd2a..aa6c38a188 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -3,8 +3,8 @@ package runtime import ( - "device/arm" "device/rp" + "sync/atomic" ) const ( @@ -18,57 +18,22 @@ const ( // Due to hardware errata RP2350-E2 spinlocks are emulated in software as in pico-sdk. type spinLock struct { - // state field must be first field so its address is the same as the struct address. - state uint8 - id uint8 - uint16 // Padding to prevent false sharing + atomic.Uint32 } func (l *spinLock) Lock() { - // Original reference: - // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L112 - - // r0 is automatically filled with the pointer value "l" here. - // We create a variable to permit access to the state byte (l.state) and - // avoid a memory fault when accessing it in assembly. - state := &l.state - _ = state - - // Set the loop start point. - arm.Asm("1:") - // Exclusively load (lock) the state byte and put its value in r2. - arm.Asm("ldaexb r2, [r0]") - // Set the r1 register to '1' for later use. - arm.Asm("movs r1, #1") - // Check if the lock was already taken (r2 != 0). - arm.Asm("cmp r2, #0") - // Jump back to the loop start ("1:") if the lock is already held. - arm.Asm("bne 1b") - - // Attempt to store '1' into the lock state byte. - // The return code (0 for success, 1 for failure) is placed in r2. - arm.Asm("strexb r2, r1, [r0]") - // Check if the result was successful (r2 == 0). - arm.Asm("cmp r2, #0") - // Jump back to the loop start ("1:") if the lock was not acquired. - arm.Asm("bne 1b") - - // Memory barrier to ensure everyone knows we're holding the lock now. - arm.Asm("dmb") + // Try to replace 0 with 1. Once we succeed, the lock has been acquired. + for !l.Uint32.CompareAndSwap(0, 1) { + spinLoopWait() + } } func (l *spinLock) Unlock() { - // Original reference: - // https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L197 + // Safety check: the spinlock should have been locked. + if schedulerAsserts && l.Uint32.Load() != 1 { + runtimePanic("unlock of unlocked spinlock") + } - // r0 is automatically filled with the pointer value l here. - // We create a variable to permit access to the state byte (l.state) and - // avoid a memory fault when accessing it in assembly. - state := &l.state - _ = state - // Fill r1 with 0 and store it to the state byte address in r0. stlb - // requires a register as a source so we can't use a literal 0 directly. - arm.Asm("movs r1, #0") - // Release the pseudo-spinlock by writing 0 to and releasing l.state. - arm.Asm("stlb r1, [r0]") + // Unlock the lock. Simply write 0, because we already know it is locked. + l.Uint32.Store(0) } From 09fbdc0afa7f06a12cd5fb94b741a7d16740d27f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Sep 2025 19:00:44 -0400 Subject: [PATCH 4/6] chore: comment cleanup --- src/runtime/runtime_rp2350.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index aa6c38a188..09507ccd5c 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -16,7 +16,6 @@ const ( sioIrqFifoProc1 = rp.IRQ_SIO_IRQ_FIFO ) -// Due to hardware errata RP2350-E2 spinlocks are emulated in software as in pico-sdk. type spinLock struct { atomic.Uint32 } From ab8bf89c052854c804232a5b6ff2963e4c0c596e Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Sep 2025 19:16:21 -0400 Subject: [PATCH 5/6] fix: add id field for hw spinlock compatibility --- src/runtime/runtime_rp2350.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index 09507ccd5c..bb1b477fda 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -18,6 +18,7 @@ const ( type spinLock struct { atomic.Uint32 + id uint8 } func (l *spinLock) Lock() { From cee45376b65e3ad83972dc58c874b437d937c35e Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 13 Sep 2025 21:06:12 -0400 Subject: [PATCH 6/6] chore: switch to cgo inline asm --- src/runtime/runtime_rp2350.go | 49 +++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/src/runtime/runtime_rp2350.go b/src/runtime/runtime_rp2350.go index bb1b477fda..93ac2f7ff7 100644 --- a/src/runtime/runtime_rp2350.go +++ b/src/runtime/runtime_rp2350.go @@ -2,9 +2,40 @@ package runtime +// #include +// +// static void spinlock_lock(void *lock) { +// uint32_t _tmp0, _tmp1; +// __asm volatile ( \ +// "1:\n" \ +// "ldaexb %1, [%2]\n" \ +// "movs %0, #1\n" /* fill dependency slot */ \ +// "cmp %1, #0\n" \ +// /* Immediately retry if lock is seen to be taken */ \ +// "bne 1b\n" \ +// /* Attempt to claim */ \ +// "strexb %1, %0, [%2]\n" \ +// "cmp %1, #0\n" \ +// /* Claim failed due to intervening write, so retry */ \ +// "bne 1b\n" \ +// : "=&r" (_tmp0), "=&r" (_tmp1) : "r" (lock) \ +// ); \ +// __asm volatile ("dmb" : : : "memory"); +// } +// +// static void spinlock_unlock(void *lock) { +// /* Release-ordered store is available: use instead of separate fence */ \ +// uint32_t zero = 0; \ +// __asm volatile ( \ +// "stlb %0, [%1]\n" \ +// : : "r" (zero), "r" (lock) \ +// ); \ +// } +import "C" + import ( "device/rp" - "sync/atomic" + "unsafe" ) const ( @@ -16,24 +47,26 @@ const ( sioIrqFifoProc1 = rp.IRQ_SIO_IRQ_FIFO ) +// Software spinlocks don't persist across soft resets so this is a no-op. +func resetSpinLocks() {} + type spinLock struct { - atomic.Uint32 - id uint8 + state uint8 + id uint8 + _ [2]uint8 } func (l *spinLock) Lock() { // Try to replace 0 with 1. Once we succeed, the lock has been acquired. - for !l.Uint32.CompareAndSwap(0, 1) { - spinLoopWait() - } + C.spinlock_lock(unsafe.Pointer(&l.state)) } func (l *spinLock) Unlock() { // Safety check: the spinlock should have been locked. - if schedulerAsserts && l.Uint32.Load() != 1 { + if schedulerAsserts && l.state != 1 { runtimePanic("unlock of unlocked spinlock") } // Unlock the lock. Simply write 0, because we already know it is locked. - l.Uint32.Store(0) + C.spinlock_unlock(unsafe.Pointer(&l.state)) }