raspberrypi / pico-feedback

25 stars 2 forks source link

Exclusive Access to PSRAM on RP2350 #398

Open thejpster opened 3 weeks ago

thejpster commented 3 weeks ago

Based on our reading of the RP2350 datasheet, we believe that any Exclusive Load/Stores to the PSRAM region will always fail (thus any code with an Exclusive Load/Store loop, e.g. to do a CAS operation, will either loop indefinitely or raise a fault).

Is our understanding correct? If so, creating a Rust AtomicU32 value (or similar) in PSRAM is going to end badly and we need to try and stop people doing that. Perhasps the same applies to a C11 _Atomic value.

jamesmunns commented 3 weeks ago

Here's the chat history on matrix where we discuss this.

thejpster commented 3 weeks ago

We note that the comment in https://github.com/raspberrypi/pico-sdk/blob/efe2103f9b28458a1615ff096054479743ade236/src/rp2_common/hardware_sync_spin_lock/sync_spin_lock.c#L28 seems relevant, along with https://developer.arm.com/documentation/100235/0004/the-cortex-m33-peripherals/system-control-block/auxiliary-control-register?lang=en.

liamfraser commented 3 weeks ago

You are correct, PSRAM does not support exclusive transactions.

lurch commented 3 weeks ago

I asked about this internally, and was pointed at section 2.1.6.2. of https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf which says:

The Global Exclusive monitor only supports exclusive transactions on certain address ranges. The main system SRAM supports exclusive transactions throughout its entire range: 0x20000000 through 0x20082000.

and

Exclusive transactions are not supported outside of this range: all exclusive accesses report exclusive failure (both exclusive reads and exclusive writes), and exclusive writes will not be suppressed.

and

It is recommended not to perform exclusive accesses on regions outside of main SRAM.

thejpster commented 3 weeks ago

Thank you for the quick response.

Are we correct that the failure mode is an infinite loop rather than an exception? I haven't got PARAM working so I can't test.

jamesmunns commented 3 weeks ago

Our assumption that this will endlessly loop comes from the LLVM implmentation of various "compare and swap" primitives:

https://github.com/llvm/llvm-project/blob/c7a54bfd1d25330199c96dd0a46cef1644b1b1ce/compiler-rt/lib/builtins/arm/sync-ops.h#L31-L35

  LOCAL_LABEL(tryatomic_##op) : ldrex r0, [r12];                               \
  op(r2, r0, r1);                                                              \
  strex r3, r2, [r12];                                                         \
  cmp r3, #0;                                                                  \
  bne LOCAL_LABEL(tryatomic_##op);  

strex places the result of the exclusive store in r3, which should be 0 when the exclusive store succeeds, and 1 if it fails. If r3 != 0, we branch back and retry the operation (unbounded).

If this does not cause some kind of fault, it's likely users of _Atomic (in C), std::atomic (in C++), or core::sync::atomic (in Rust) will observe lockups if CAS operations are performed in the PSRAM mapped region.

jamesmunns commented 3 weeks ago

But answering @thejpster's question from datasheet section 2.1.6.2:

Outside of regions with exclusive transaction support, load/store exclusive loops run forever while still affecting SRAM contents. This applies to both Arm processors performing exclusive reads/writes and RISC-V processors performing lr.w/sc.w instructions. However, an amo*.w instruction on Hazard3 will result in a Store/AMO Fault, as the hardware detects the failed exclusive read and bails out to avoid an infinite loop.

So I think the answer is pretty clearly "yes, expect endless loops".