rp2: per-board custom memmap_mp.ld to enforce configured flash size

[Gadgetoid]

I'm not sure this pertains just to RP2, but I have little experience with other platforms so I'm going to keep the scope narrow unless anyone wants to chime in. That said I think this approach is pretty generic and should be - SDK's willing - portable to other platforms.

The long and short of it is that ports/rp2/boards/PICO/mpconfigboard.h specifies MICROPY_HW_FLASH_STORAGE_BYTES which - at runtime - is the region allocated to the user-facing filesystem on an RP2-based MicroPython board.

This is configurable since boards can have 2MB, 4MB, 8MB and 16MB (and maybe beyond?) of Flash storage.

However this region is not enforced by the memmap_mp.ld linker script used to build the RP2 ports.

The practical upshot of this is that you can build a MicroPython port with too much stuff baked into it (and oh my do we bake in a lot of stuff) which will happily link and flash but then immediately soft-brick the board it's flashed to. (Turns out MicroPython has some pretty important stuff at the high-end of its binary.)

My proposal is either:

1. To pre-process the port's memmap_mp.ld in order to configure the flash size correctly (I don't know how we'd do this, but it prevents duplication)

or

2. To introduce the concept of board-specific memmap_mp.ld files that are auto-detected and used in place of the port one.

The latter would be achieved something like so:

diff --git a/ports/rp2/CMakeLists.txt b/ports/rp2/CMakeLists.txt
index a5e421734..2db097364 100644
--- a/ports/rp2/CMakeLists.txt
+++ b/ports/rp2/CMakeLists.txt
@@ -296,7 +296,11 @@ endif()
 #  a linker script modification) until we explicitly add  macro calls around the function
 #  defs to move them into RAM.
 if (PICO_ON_DEVICE AND NOT PICO_NO_FLASH AND NOT PICO_COPY_TO_RAM)
-    pico_set_linker_script(${MICROPY_TARGET} ${CMAKE_CURRENT_LIST_DIR}/memmap_mp.ld)
+    if(EXISTS ${MICROPY_BOARD_DIR}/memmap_mp.ld)
+        pico_set_linker_script(${MICROPY_TARGET} ${MICROPY_BOARD_DIR}/memmap_mp.ld)
+    else()
+        pico_set_linker_script(${MICROPY_TARGET} ${CMAKE_CURRENT_LIST_DIR}/memmap_mp.ld)
+    endif()
 endif()
 
 pico_add_extra_outputs(${MICROPY_TARGET})

And then boards/PICO/memmap_mp.ld would correctly configure the flash size:

FLASH(rx) : ORIGIN = 0x10000000, LENGTH = 640k

This ensures that an oversized build (for whatever reason) will fail something like this:

/usr/lib/gcc/arm-none-eabi/10.3.1/../../../arm-none-eabi/bin/ld: firmware.elf section `.rodata' will not fit in region `FLASH'
/usr/lib/gcc/arm-none-eabi/10.3.1/../../../arm-none-eabi/bin/ld: region `FLASH' overflowed by 90736 bytes
collect2: error: ld returned 1 exit status
make[3]: *** [CMakeFiles/firmware.dir/build.make:9088: firmware.elf] Error 1
make[2]: *** [CMakeFiles/Makefile2:1553: CMakeFiles/firmware.dir/all] Error 2
make[1]: *** [Makefile:91: all] Error 2
make: *** [Makefile:23: all] Error 2

Rather than failing when it's flashed, run and inevitably overwrites something important!

[Gadgetoid]

For similar reasons, it probably wouldn't hurt to add the following to CMakeLists.txt also:

target_link_options(${MICROPY_TARGET} PRIVATE
    -Wl,--print-memory-usage
)

Producing:

Memory region         Used Size  Region Size  %age Used
           FLASH:      602736 B       640 KB     91.97%
             RAM:      240372 B       256 KB     91.69%
       SCRATCH_X:          0 GB         4 KB      0.00%
       SCRATCH_Y:          0 GB         4 KB      0.00%

Which gives a much more immediate and intuitive picture of flash usage.

[jimmo]

I was just helping someone on Discord with exactly this issue.

As outlined above, the linker is the right place to solve this. Maybe instead of having board-specific .ld files though, we could instead have mpconfigboard.cmake set the flash size, which would then make CMakeLists.txt automatically select one of N pre-defined common linker fragments (2MiB, 4MiB, 8MiB, etc), as well as setting the preprocessor definition MICROPY_HW_FLASH_STORAGE_BYTES.

[jimmo]

Also, at the very least we should make this a runtime error and verify that rp2_flash.c never attempts to write to anything before __flash_binary_end.

(Edit: we already have this! it's just only in debug builds... https://github.com/micropython/micropython/blob/master/ports/rp2/rp2_flash.c#L83 )

[Gadgetoid]

I was totally stonewalled by the Pico bi_decl functions not playing nice with linker symbols in #8761, but I think your suggestion works around this specific issue?

My approach was to set total flash† and "app" sizes in each board config, preprocess the linker file and try to use the linker symbols to handle everything else. The "app" size would configure how much is deserved for MicroPython - rather than have it be implicitly the remaining flash - and trying to link a binary too large for this reserved space would blow up the build with a linker error.

Enabling some variety of the debug feature to catch the overflow on build might be a quick(er) win, though. But that assert check will only prevent the build self-corrupting, and not warn beforehand?

† - The total flash size should - in theory at least - be set by the Pico SDK in PICO_FLASH_SIZE_BYTES set by the relevant board config.