riotboot_flashwrite: Failure on the nrf52

[bergzand]

Description

The riotboot_flashwrite test fails to properly boot after an update on the nrf52 chips. This happens because RIOTBOOT_LEN is exactly the size of one full flash page. The rest of the flash is divided between the other two slots, giving each slot 128.5 flash pages. This results in SLOT1_OFFSET not starting at a flash page border, violating some of the assumptions of riotboot.

Steps to reproduce the issue

execute the steps from the README.md in the riotboot_flashwrite test on a nrf52-based board (nrf52840dk for example)

Expected results

Slot1 is booted after a reset

Actual results

Slot0 boots after a result, furthermore debugging slot1 shows that the magic number is not flashed to the first word of the slot.

Quick fix:

Override RIOTBOOT_LEN in the nrf52 Makefile.include to 0x2000.

More generic fix:

Ensure that RIOTBOOT_LEN is always at least 2 flash pages large. Assuming that most chips with a homogeneous flash page layout always have an even number of flash pages, this would result in the remaining number of flash pages for the two slots also be an even number. Having an even number of remaining flash pages ensures that it can be divided into two slots each starting at a flash page border

[kaspar030]

Nice catch!

Ensure that RIOTBOOT_LEN is always at least 2 flash pages large.

I guess that should be "an even number of flash pages large", so the remaining flash pages can be divided cleanly between two. I think on samr21-xpro we were just lucky so far that "$RIOTBOOT_LEN" left an even amount of flash pages for the two slots.

Additionally, slot starts need to be checked against whether they are flash page aligned, as a precaution.

[bergzand]

"an even number of flash pages large"

👍

Additionally, slot starts need to be checked against whether they are flash page aligned, as a precaution.

IMHO we should include this in the tests/riotboot

[cladmi]

Having slots starting on full pages is a requirement and should be at least documented.
Asking them to have a length of full pages is also I think a better thing to do, otherwise the remaining space is lost, so it is sane enough to require "full pages".
A check could be even done at compile time in the bootloader/and modules using "slots" if we require to have the "FLASHPAGE_SIZE" information available for them.

However having a bootloader being a multiple of two is currently not a requirement, there could be cases where you have a 1 page bootloader, and some ROM reserved at the end taking 3 pages so have even number of pages in the middle.
So I would prefer to have the test check only what is required, even if our solution to handle this is currently doing "2 pages bootloader"

The issue comes from the calculation of SLOT_LEN that does not have any information of a cpu page size and so need to assume that ROM_LEN - RIOTBOOT_LEN is a multiple of 2 pages.
I am not sure having the "page size" information in the build system is needed or even wanted.
And a solution doing "lets give 2 pages to the bootloader" is for me good enough.

[cladmi]

The issue comes from the calculation of SLOT_LEN that does not have any information of a cpu page size and so need to assume that ROM_LEN - RIOTBOOT_LEN is a multiple of 2 pages.
I am not sure having the "page size" information in the build system is needed or even wanted.
And a solution doing "lets give 2 pages to the bootloader" is for me good enough.

And this should be at least documented even if it cannot be checked in the build system.

[aabadie]

The fix was merged with #11518 so let's close this.