fix(rp2350): add software spinlocks #5034

mikesmitty · 2025-09-10T01:17:45Z

As it turns out, the RP2350 has hardware spinlocks that can be unlocked by writes to nearby addresses, the lower spinlocks currently in use in TinyGo happen to be unlocked by writes to the doorbell interrupt registers used to signal between cores, very possibly leading to some unexpected unlocks. This was not corrected in the A3 or A4 steppings and instead software spinlocks are used by default on RP2350 in pico-sdk:

RP2350 Warning. Due to erratum RP2350-E2, writes to new SIO registers above an offset of +0x180 alias the spinlocks, causing spurious lock releases. This SDK by default use atomic memory accesses to implement the hardware_sync_spin_lock API, as a workaround on RP2350 A2.

https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#group_hardware_sync

Another thing I noticed in that section, they also always disable interrupts when the spinlocks are being held, we may want to do the same:

[...] the default spinlock related methods here (e.g. spin_lock_blocking) always disable interrupts while the lock is held as use by IRQ handlers and user code is common/desirable, and spin locks are only expected to be held for brief periods.

These are the software spinlock macros ported over:
https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L112
https://github.com/raspberrypi/pico-sdk/blob/2.2.0/src/rp2_common/hardware_sync_spin_lock/include/hardware/sync/spin_lock.h#L197

Writing to the UART takes time and that may not be a good idea inside an interrupt, but it is essential for debugging sometimes (especially since USB-CDC typically doesn't work inside an interrupt). This fixes UART support in interrupts for the RP2040 at least. You can test it with `-serial=uart` and connecting a USB-UART adapter to the right pins.

…he first error is returned In some cases, e.g nothing connected on the bus, repeated resume-stop sequences can lead to the bus never reaching the stop state, hanging Tx. This change ensures the resume-stop sequence is submitted once on error. It also moves the error code read to before the sequence to ensure it's valid. Fixes: tinygo-org#4998

mikesmitty · 2025-09-10T02:08:06Z

Oh, whoops. My go fmt extension has been flaking out on me. Will have the missing rp2040 imports updated in a moment. Here's the lock/unlock disassembled output with inlining disabled:

   0x10001178 <(*runtime.spinLock).Lock+0>:     cbz     r0, 0x10001192 <(*runtime.spinLock).Lock+26>
   0x1000117a <(*runtime.spinLock).Lock+2>:     ldaexb  r2, [r0]
   0x1000117e <(*runtime.spinLock).Lock+6>:     movs    r1, #1
   0x10001180 <(*runtime.spinLock).Lock+8>:     cmp     r2, #0
   0x10001182 <(*runtime.spinLock).Lock+10>:    bne.n   0x1000117a <(*runtime.spinLock).Lock+2>
   0x10001184 <(*runtime.spinLock).Lock+12>:    strexb  r2, r1, [r0]
   0x10001188 <(*runtime.spinLock).Lock+16>:    cmp     r2, #0
   0x1000118a <(*runtime.spinLock).Lock+18>:    bne.n   0x1000117a <(*runtime.spinLock).Lock+2>
   0x1000118c <(*runtime.spinLock).Lock+20>:    dmb     sy
   0x10001190 <(*runtime.spinLock).Lock+24>:    bx      lr
   0x10001192 <(*runtime.spinLock).Lock+26>:    bl      0x100013f4 <runtime.nilPanic>

   0x100013e4 <(*runtime.spinLock).Unlock+0>:   cbz     r0, 0x100013ee <(*runtime.spinLock).Unlock+10>
   0x100013e6 <(*runtime.spinLock).Unlock+2>:   movs    r1, #0
   0x100013e8 <(*runtime.spinLock).Unlock+4>:   stlb    r1, [r0]
   0x100013ec <(*runtime.spinLock).Unlock+8>:   bx      lr
   0x100013ee <(*runtime.spinLock).Unlock+10>:  bl      0x100013f4 <runtime.nilPanic>

eliasnaur

I support the switch to atomic instructions, but if you need something that works right away, RP2350-E2 mentions that some spinlocks are not affected:

The following SIO spinlocks can be used normally because they don’t alias with writable registers: 5, 6, 7,
10, 11, and 18 through 31. Some of the other lock addresses may be used safely depending on which of
the high-addressed SIO registers are in use.
Locks 18 through 24 alias with some read-only TMDS encoder registers, which is safe as only writes are
mis-decoded.

src/runtime/runtime_rp2350.go

aykevl

See my comment below.

Also,

Another thing I noticed in that section, they also always disable interrupts when the spinlocks are being held, we may want to do the same: [...]

The runtime does this in various places if needed. The spinlock implementation doesn't need to disable interrupts too.

src/runtime/runtime_rp2350.go

mikesmitty · 2025-09-10T23:21:55Z

I support the switch to atomic instructions, but if you need something that works right away, RP2350-E2 mentions that some spinlocks are not affected:

The following SIO spinlocks can be used normally because they don’t alias with writable registers: 5, 6, 7,
10, 11, and 18 through 31. Some of the other lock addresses may be used safely depending on which of
the high-addressed SIO registers are in use.
Locks 18 through 24 alias with some read-only TMDS encoder registers, which is safe as only writes are
mis-decoded.

Thank you for tracking those down. I figured there would probably be some, but I couldn't find them

…tests that use Chromium headless browser to avoid use of older incompatible version. Signed-off-by: deadprogram <ron@hybridgroup.com>

…wser used for running the wasm tests. Also add favicon link to avoid extra fetches during test runs. Signed-off-by: deadprogram <ron@hybridgroup.com>

With these flags, the TinyGo binary gets 18.8MB (11.6%) smaller. That seems like a quite useful win for such a small change! This is only for Linux for now. MacOS and Windows can be tested later, the flags for those probably need to be modified. Originally inspired by: https://discourse.llvm.org/t/state-of-the-art-for-reducing-executable-size-with-heavily-optimized-program/87952/18 There are some other flags like -Wl,--pack-dyn-relocs=relr that did not shrink binary size in my testing, so I've left them out. This also switches the linker to prefer mold or lld over the default linker, since the system linker is usually ld.bfd which is very slow. (Also, for some reason mold produces smaller binaries than lld).

The revive command seems to have had a syntax error in the file input glob. It appears to have been broken in a way that did not result in a return code being set. This change uses 'find' to build the input to the linter. Note that it is expected to fail the CI script, because it is uncovering some existing lint issues that were not being caught.

Signed-off-by: deadprogram <ron@hybridgroup.com>

This switches the Espressif fork from LLVM 19 to LLVM 20, so we can use the improvements made between those LLVM versions. It also better aligns with the system-LLVM build method, which currently also defaults to LLVM 20. Note that this disables the machine outliner for RISC-V. It appears there's a bug in there somewhere, with the machine outliner enabled the crypto/elliptic package tests fail with -target=riscv-qemu. This should ideally be investigated and reported upstream.

eliasnaur

Other than a nit, this looks good to me. @aykevl WDYT?

eliasnaur · 2025-09-17T14:40:17Z

src/runtime/runtime_rp2.go

 	printLock     = spinLock{id: 0}
 	schedulerLock = spinLock{id: 1}
 	atomicsLock   = spinLock{id: 2}
 	futexLock     = spinLock{id: 3}


id is an implementation detail of rp2040 spinlocks, whereas on rp2350 ids have some meaning but are unusued. I suggest moving the spinlock variables to the rpXXXX.go files and avoid the id field on rp2350.

eliasnaur · 2025-09-17T14:41:40Z

src/runtime/runtime_rp2350.go

+
+type spinLock struct {
+	atomic.Uint32
+	id uint8


Delete field.

That one's still needed for compatibility with how the rp2040 hardware spinlocks are initialized here:

tinygo/src/runtime/runtime_rp2.go

Lines 295 to 298 in 109e076

printLock = spinLock{id: 20}

schedulerLock = spinLock{id: 21}

atomicsLock = spinLock{id: 22}

futexLock = spinLock{id: 23}

Indeed. Above, I suggested moving that var () block to the runtime_rp2xxx.go files to get rid of that dependency.

Ah yep, I missed that

mikesmitty · 2025-09-18T01:37:49Z

After doing some testing, I think this might not actually be locking properly. Going to do some more thorough testing

- do not install cmake, instead use the version already installed - add macOS 15 to the CI builds - update a could of GH actions to latest release - update Go version being use to Go 1.25.1 Signed-off-by: deadprogram <ron@hybridgroup.com>

GitHub org. The git server for git.musl-libc.org is having troubles, and it also seems like a safer bet to have our own mirror just in case. Signed-off-by: deadprogram <ron@hybridgroup.com>

…d by the test-macos-homebrew job, and it conflicts with the actual build that we want, which is macOS 14 for backwards-compatibility. Signed-off-by: deadprogram <ron@hybridgroup.com>

Signed-off-by: Piotr Bocheński <piotr@bochen.ski>

…fatal

aykevl · 2025-09-23T12:17:49Z

Yes, this looks good. @mikesmitty can you apply the changes proposed by @eliasnaur?

Also, you may want to rebase on the dev branch to hopefully fix the assert-test-linux CI failure.

mikesmitty · 2025-09-24T12:58:24Z

Sure, here's the rebase. I haven't had time to test or diagnose it yet, been stuck working on a project that's consumed all my free time, but I'm pretty sure it's not functioning properly as confirmed by that CI failure

mikesmitty · 2025-09-24T13:11:44Z

Hmm, no I guess it is actually locking. I'm not sure what's happening with that CI test though

sago35 and others added 4 commits September 2, 2025 07:14

chore: update version for 0.40 development cycle

3753193

ci:uninstall cmake before install

13bb59c

eliasnaur reviewed Sep 10, 2025

View reviewed changes

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

aykevl requested changes Sep 10, 2025

View reviewed changes

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

aykevl reviewed Sep 10, 2025

View reviewed changes

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

eliasnaur reviewed Sep 10, 2025

View reviewed changes

src/runtime/runtime_rp2350.go Outdated Show resolved Hide resolved

mikesmitty requested review from aykevl and eliasnaur September 10, 2025 23:24

deadprogram and others added 11 commits September 11, 2025 08:34

chore: create separate go.mod file for testing dependencies for wasm …

e80c6c4

…tests that use Chromium headless browser to avoid use of older incompatible version. Signed-off-by: deadprogram <ron@hybridgroup.com>

fix: increase the timeout for chromedp to connect to the headless bro…

e5bdfb0

…wser used for running the wasm tests. Also add favicon link to avoid extra fetches during test runs. Signed-off-by: deadprogram <ron@hybridgroup.com>

fix: expand RTT debugger compatibility

373ca88

feat: enable //go:linkname pragma for globals

663a94f

fix/typo:makeESPFirmwareImage

eab3dce

fix: add TryLock to sync.RWMutex

3ddba4e

Move the directory list into a variable

2c99692

fix: correct linter issues exposed by the fix in tinygo-org#4679

79ab77f

Signed-off-by: deadprogram <ron@hybridgroup.com>

eliasnaur approved these changes Sep 17, 2025

View reviewed changes

deadprogram and others added 5 commits September 18, 2025 08:24

fix: point the submodule for musl-lib to a mirror in the TinyGo

28a911b

GitHub org. The git server for git.musl-libc.org is having troubles, and it also seems like a safer bet to have our own mirror just in case. Signed-off-by: deadprogram <ron@hybridgroup.com>

fix: remove macOS 15 from CI build matrix. It was already being teste…

0dca016

…d by the test-macos-homebrew job, and it conflicts with the actual build that we want, which is macOS 14 for backwards-compatibility. Signed-off-by: deadprogram <ron@hybridgroup.com>

fix(rp2): disable DBGPAUSE on startup

5d8afa2

fix(rp2): switch spinlock busy loop to wfe

3dce224

mikesmitty and others added 6 commits September 19, 2025 02:22

fix(rp2): use side-effect-free spinlocks

b9387fe

fix(rp2): reset spinlocks at startup

310df7a

fix: use int64 in ReadTemperature to avoid overflow

e29e583

net: update submodule to latest commits

93a11e2

Signed-off-by: Piotr Bocheński <piotr@bochen.ski>

Fix linker errors for runtime.vgetrandom and crypto/internal/sysrand.…

821f2ae

…fatal

Added Gopher ARCADE board

109e076

mikesmitty added 6 commits September 24, 2025 08:39

fix(rp2350): add software spinlocks

d1eca84

chore: clarify comments

fc81160

chore: use atomics instead of artisinal assembly

31696c2

chore: comment cleanup

09fbdc0

fix: add id field for hw spinlock compatibility

ab8bf89

chore: switch to cgo inline asm

cee4537

mikesmitty force-pushed the ms/rp2350-sw-mutex branch from 1f2642c to cee4537 Compare September 24, 2025 12:43

	printLock = spinLock{id: 20}
	schedulerLock = spinLock{id: 21}
	atomicsLock = spinLock{id: 22}
	futexLock = spinLock{id: 23}

fix(rp2350): add software spinlocks #5034

Are you sure you want to change the base?

fix(rp2350): add software spinlocks #5034

Uh oh!

Conversation

mikesmitty commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikesmitty commented Sep 10, 2025

Uh oh!

eliasnaur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aykevl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikesmitty commented Sep 10, 2025

Uh oh!

eliasnaur left a comment

Choose a reason for hiding this comment

Uh oh!

eliasnaur Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

eliasnaur Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

mikesmitty Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

eliasnaur Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

mikesmitty Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

mikesmitty commented Sep 18, 2025

Uh oh!

aykevl commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikesmitty commented Sep 24, 2025

Uh oh!

mikesmitty commented Sep 24, 2025

Uh oh!

Uh oh!

mikesmitty commented Sep 10, 2025 •

edited

Loading

aykevl commented Sep 23, 2025 •

edited

Loading