Note; I have created custom linker scripts for PICO and our boards which I know the flash sizes of. If this stands a chance of being merged before the next release I could try and find the time to update the other boards.

Right now this is a very us-packing-everything-and-the-kitchen-sink-into-MicroPython problem, but I think having linker scripts that correctly reflect the actual FLASH available (and not allocated to the user filesystem) makes sense generally.

I'm not super happy about copying memmap_mp.ld for every board, but I don't really see a better way to do this and preprocessing is non-trivial CMake spaghetti.

It's definitely not ideal. Would be best to try and eliminate duplication. Linker scripts can include other linker script files, see eg ports/stm32/boards/common_bss_heap_stack.ld and related ld files.

You can also inject variables into linker scripts, see eg ports/nrf/boards/memory.ld. That might be the best way to do it (have a variable for the flash size, which is set in the cmake file...?).

Aha, thank you for the examples, I shall look into those when I have time and see if I can make this workable.

Updated to a much cleaner solution.

The pico_set_linker_script function does not permit multiple linker scripts, so the variable approach that works elsewhere doesn't quite work as gracefully here.

In lieu of this I have detected mpconfigboard.ld and swapped out to it, that file is then responsible (assuming the user doesn't want to write their own complete linker script) for including memmap_mp.ld. The linker is given the port directory so this can be done easily with just INCLUDE memmap_mp.ld.

It feels a bit icky that we're defining both MICROPY_HW_FLASH_STORAGE_BYTES and _user_fs_size.

TODO

• Agree on naming convention for linker file, I have gone with mpconfigboard.ld
• Settle on a naming convention for variables describing user filesystem and flash section sizes
• Configure all other boards with their flash sizes

In stm32 the flash layout (what's for the bootloader, the app firmware, the filesystem etc) is done in the linker script. And then those values are passed up to C. I think it makes good sense to do a similar thing for the rp2 port, define everything at the linker level.

In stm32 an example is:

MEMORY
{
    FLASH (rx)      : ORIGIN = 0x08000000, LENGTH = 1024K /* entire flash */
    FLASH_ISR (rx)  : ORIGIN = 0x08000000, LENGTH = 16K /* sector 0 */
    FLASH_FS (rx)   : ORIGIN = 0x08004000, LENGTH = 112K /* sectors 1,2,3,4 are for filesystem */
    FLASH_TEXT (rx) : ORIGIN = 0x08020000, LENGTH = 896K /* sectors 5,6,7,8,9,10,11 */
    CCMRAM (xrw)    : ORIGIN = 0x10000000, LENGTH = 64K
    RAM (xrw)       : ORIGIN = 0x20000000, LENGTH = 128K
}

Sometimes FLASH_APP is also defined for boards that use a bootloader.

I think we should use similar names for rp2, eg FLASH for the whole flash, FLASH_FS for the filesystem region, and FLASH_APP (or FLASH_TEXT) for the firmware region.

It feels a bit icky that we're defining both MICROPY_HW_FLASH_STORAGE_BYTES and _user_fs_size

I agree. So we would remove MICROPY_HW_FLASH_STORAGE_BYTES and let it be set automatically by the linker/compiler. Eg we can define this in mpconfigboard.mk and then use -D... to pass it through to C, and --defsym=_user_fs_size=$(MICROPY_HW_FLASH_STORAGE_BYTES) to pass it through to the linker.

Your suggested approach has the consequence of making it easier to locate objects in the filesystem - ie an initialized filesystem image - at compile time. That could prove a handy jumping off point for some other ideas floating around. I like it!

I'm not totally sure --defsym does what we want here, it seems extraordinarily difficult to pass any values from the CMake tooling into the linker, outside of loading a per-board custom linker script.

Similarly rpi_flash.c is written such that it cannot depend upon linker symbols in order to set the constant values required for both bi_decl(bi_block_device()) and STATIC rp2_flash_obj_t rp2_flash_obj.

The latter might be fixable with some not-insignificant refactoring, but the former is likely to be tricky.

I ended up with a per-board configuration in mpconfigboard.cmake like so:

set(MICROPY_HW_FLASH_SIZE_BYTES "2 * 1024 * 1024")
set(MICROPY_HW_APP_SIZE_BYTES        "640 * 1024")

Which is a hack around the fact that PICO_FLASH_SIZE_BYTES is defined in an .h file in Pico SDK (as an aside: I find having to look in two places for the flash configuration to be irritating anyway).

But with no way to pass these values into the Linker script, it's all academic.

The linker script now looks something like this:

_flash_start    = 0x10000000;
_flash_size     = DEFINED(MICROPY_HW_FLASH_SIZE_BYTES) ? MICROPY_HW_FLASH_SIZE_BYTES : 2 * 1024 * 1024;
_flash_app_size = DEFINED(MICROPY_HW_APP_SIZE_BYTES)   ? MICROPY_HW_APP_SIZE_BYTES   :      640 * 1024;
_flash_fs_size  = _flash_size - _flash_app_size;
_flash_fs_start = _flash_start + _flash_app_size;


MEMORY
{
    FLASH(rx)      : ORIGIN = _flash_start,    LENGTH = _flash_size
    APP(rx)        : ORIGIN = _flash_start,    LENGTH = _flash_app_size
    FILESYSTEM(r)  : ORIGIN = _flash_fs_start, LENGTH = _flash_fs_size
    RAM(rwx)       : ORIGIN = 0x20000000,      LENGTH = 256k
    SCRATCH_X(rwx) : ORIGIN = 0x20040000,      LENGTH = 4k
    SCRATCH_Y(rwx) : ORIGIN = 0x20041000,      LENGTH = 4k
}

And in CMakeLists.txt I am attempting to do something like this:

# supply user-configurable app size and Pico board size to the linker script
target_link_options(pico_standard_link INTERFACE "-Wl,--defsym,MICROPY_HW_FLASH_SIZE_BYTES=${MICROPY_HW_FLASH_SIZE_BYTES}")
target_link_options(pico_standard_link INTERFACE "-Wl,--defsym,MICROPY_HW_APP_SIZE_BYTES=${MICROPY_HW_APP_SIZE_BYTES}")

But symbols passed in this way neither qualify as DEFINED nor function as a variable, unless it's possible to get their address and use that as a value in memmap_mp.ld, but that also feels clunky.

in rp2_flash.c I also have a basic check to ensure our locally defined flash size and the Pico board definition flash size are in sync:

static_assert(PICO_FLASH_SIZE_BYTES == MICROPY_HW_FLASH_SIZE_BYTES, "MICROPY_HW_FLASH_SIZE_BYTES does not match PICO_FLASH_SIZE_BYTES");

I feel like I've painted myself into a corner here, but there may be light at the end of the tunnel,

It seems that the mimxrt port actually runs the C preprocessor over its common.ld and board-specific .ld files, using this as an opportunity to pass in preprocessor macros. That's the intent behind this rather cryptic line:

$(Q)$(CC) -E -x c $(LDDEFINES) $(LD_FILES) | grep -v '^#' > $(BUILD)/link.ld

I have...borrowed.. this and hung a custom command off the MPY_TARGET in CMakeLists.txt. This allows the linker script to be preprocessed with user specified flash config.

Issues:

1. This feels fragile.
2. Duplication of PICO_FLASH_SIZE_BYTES into MICROPY_HW_FLASH_SIZE_BYTES
   3 rp2_flash.c does not and cannot easily (insofar as I can tell) use the linker symbols to configure flash

Okay I have replaced the ugly preprocessor hack with CMake's configure_file which is likely to be much, much less fragile and potentially allows for more config vars to make it through to the linker script in future without having to modify a spaghetti custom build command.

I still don't like the duplication of PICO_FLASH_SIZE_BYTES to MICROPY_HW_FLASH_SIZE_BYTES but I don't see an easy way around this. The Pico SDK toolchain preprocesses these header files and amalgamates them into one configuration header (as far as I can tell). Using the...slightly tenuous preprocessor solution I have just replaced would potentially let us #include the Pico SDK config but that feels backwards.

On the upside, having both these vars configured by MicroPython's board config means they're right there under your nose if you ever want to tweak the app sizes for existing boards.

Would it help to adjust the code in rp2_flash.c so that it used linker symbols to find the extents of the filesystem, rather than constants in rp2_flash_obj? This is how stm32 does it: https://github.com/micropython/micropython/blob/master/ports/stm32/flashbdev.c#L39-L61

The main issue with adjusting rp2_flash is the inability to use the “dynamic” linker symbols in the “bi_decl” macro. I don’t see an obvious way around this, though that may be a lack of knowledge or imagination on my part.

Apart from this not being marked "Ready for review" is there anything that needs to be addressed?

Getting input is tricky here because I'm probably the only person screaming out for this 😆

It looks like this commit c80e7c1 takes another step toward relying on memmap_mp.ld as a source of truth for board memory layout.

As I mentioned in the comment there, this causes some difficulty with modules that - up until now - have been able to safely assume that some C++ heap is available for general use. This is no longer generally the case.

Of course I'm always going to err on the side of not wanting to update our few dozen modules and expect push-back, but I think th is is another potential argument for this per-board memmap_mp.ld support. It would make it easier for us to ship interim MicroPython builds that leave some RAM available for our modules while we figure out how to "fix" them properly.

It's going to take some amount of effort to rebase this PR, so before I go launching into it-

1. What can we do about bi_decl not being compatible with dynamic linker symbols? The suggestion to make the .ld file the source of truth seems reasonable, but is incompatible with Pico SDK's binary descriptor macros. Does anyone know if there's a reasonable way to work around this? - the problem code is

   micropython/ports/rp2/rp2_flash.c

   Lines 61 to 70 in 668a7bd

   	// Tag the flash drive in the binary as readable/writable (but not reformatable)
   	bi_decl(bi_block_device(
   	BINARY_INFO_TAG_MICROPYTHON,
   	"MicroPython",
   	XIP_BASE + MICROPY_HW_FLASH_STORAGE_BASE,
   	MICROPY_HW_FLASH_STORAGE_BYTES,
   	NULL,
   	BINARY_INFO_BLOCK_DEV_FLAG_READ |
   	BINARY_INFO_BLOCK_DEV_FLAG_WRITE |
   	BINARY_INFO_BLOCK_DEV_FLAG_PT_UNKNOWN));

2. Are there any other blockers here, other than this being a pretty much only-I-seem-to-want-this change?

It should be noted that I've switched away from trying to wrangle linker scripts and have instead written a pure Python tool to analyze the .uf2 output files and check the binary metadata doesn't describe a filesystem that overlaps the flash binary.

That tool lives here: https://github.com/gadgetoid/py_decl

Is invoked in our CI like so:

python3 py_decl/py_decl.py --to-json --verify build-${{ matrix.name }}/${{ env.RELEASE_FILE }}.uf2

And will produce an error like:

Run python3 py_decl/py_decl.py --to-json --verify build-pico/pico-6cbf0fa0654672e92c30b670069e87383523d95c-pimoroni-micropython.uf2
CRITICAL ERROR: Block device / binary overlap!
Binary ends at 0x100a0560, block device starts at 0x100a0000

It's still not ideal and I'm not dealing with romfs well, but I've rebased this PR so we can better see how feasible this is and what the challenges might be.

The issues with bi_decl persist, since they require compile time constants and the linker symbols are only known at link time. This is reflected in the singleton definitions of both flash and romfs, which also require compile time constants- I've... hacked... around these.

Note: should probably have a bi_decl for romfs where it's included?

TODO

Assuming we move forward with this, we need to:

• Unpick the mess I've made with MICROPY_HW_ROMFS_BYTES vs MICROPY_HW_ROMFS_SIZE_BYTES
• Port remaining boards to use the linker defines
• Figure out if there's an alternative to in-source bi_decl?
• Unpick this mess of variables start/size/end 😬
• Add asserts to the linker script to check bounds, overlaps, etc (I think this is possible?)

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:   +32 +0.003% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32