Fix linker script to prevent using not available flash memory

[AlexXZero]

There is a fix for the issue found in the ticket #1463
It replace magic numbers with named constants in the linker script for mcuboot application.

[JF002]

@AlexXZero Thank you very much for your in-depth analysis in #1463 and for this PR!

I had a look at the changes, and I can see that I did a few mistakes when I designed the memory map for the bootloader and the firmware! The image header and trailer were not taken into account!

Also, the last 12KB that are left unused are a bit intriguing... To be honest, I can't remember what was my intent : I either made a calculus error, and I intended to use them as scratch area, but forgot to extend the area size to 16KB...
In the short term, I guess we'll leave it as it is, but we can brainstorm about how to best use those 12KB :)

Have you already had the opportunity to check that those changes in the linker script work correctly, and that it prevents from generating a faulty (too big) firmware?

EDIT : it would also be nice to update the memory map in the documentation of the bootloader. Would you like to do it?

[JF002]

I did a few additional checks : generate the biggest possible firmware file, and then the corresponding image. The goal is to check that everything work fine when flashing the biggest size allowed.

So, according to @AlexXZero's calculation, we should be allowed to build a firmware that is 474672 (0x73e30) bytes of size.

Memory region         Used Size  Region Size  %age Used
           FLASH:      473728 B     474672 B     99.80%
     SPARE_SPACE:          0 GB        12 KB      0.00%
             RAM:       54376 B        64 KB     82.97%
post build steps for pinetime-mcuboot-app-1.11.0
   text	   data	    bss	    dec	    hex	filename
 473728	    940	  53432	 528100	  80ee4	pinetime-mcuboot-app-1.11.0.out
Usage: imgtool.py create [OPTIONS] INFILE OUTFILE
Try 'imgtool.py create -h' for help.

Error: Image size (0x73e74) + trailer (0x630) exceeds requested size 0x74000

In this case, the firmware size is 473728B of code and 940B of data which is 474668B total. It's 4B less than the maximum, but I couldn't reach the exact number of 474672.

Notice that --print-memory-usage does not take constant data into account while arm-none-eabi-size does.

There are 2 issues here :

• why does imgtool think that the image size is 0x73e74 (474740B) while it's either 474668B or 474700B with the image header ? What do those 40B account for?
• why does imgtool want to generate a trailer of 0x630 (1584B) instead of 0x1b0 (432B) calculated by @AlexXZero ?

I do not have the answer to the 1st question.

The 2nd question can be explained by min-write-size (see here and the --align parameter of imgtool.
According to @AlexXZero, min-write-size is set to 1(*), while --align is set to 4. Let's try with a 1 byte align:

Error: Image size (0x73e74) + trailer (0x1b0) exceeds requested size 0x74000

Yep, trailer sizes match, but not the image size.

So, in theory, we should change the imgtool CLI to use an align value of 1, but I would like to understand why there's a difference in the image size between the build and the call to imgtool first.

---

(*) And I think they are right.
In the bootloader, the internal flash is configured this way (see repos/apache-mynewt-core/hw/mcu/nordic/nrf52xxx/src/hal_flash.c:74):

See hf_align=1.

Still in the bootloader, the external spi flash is configured this way : (repos/apache-mynewt-core/hw/drivers/flash/spiflash/src/spiflash.c)

hf_align is also set to 1.

Since both memories are configured with an align value of 1, mcu boot should use a min-write-size equal to 1.

[JF002]

Ok, I found where do those 40bytes come from : imgtool.py adds a few TLV (type-length-value records) at [https://github.com/mcu-tools/mcuboot/blob/v1.6.0/docs/design.md#image-format](the end of the image).

See https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/tools/mcuboot/imgtool/image.py#L374. I added a few prints and figured out that the TLV account for exactly 40Bytes.

So I guess we'll probably take that into account in the linker script by removing 40 bytes to the max size of the firmware.

[AlexXZero]

@JF002 It seems we also need to change the imgtool arguments, I will review how to do it soon and also will test building and running an image with maximum possible size.

I tired to draw the memory map diagram which we can add into documentation:

[JF002]

@AlexXZero The new memory map looks good! Waiting for your test results :)

[AlexXZero]

I have built and tested image with maximum possible size (actually I also couldn't reach the rest 4 bytes, but I guess it is caused by alignment between .text and .data sections)

Test firmware is based on cfc86d7 commit + the following changes:

diff --git a/gcc_nrf52-mcuboot.ld b/gcc_nrf52-mcuboot.ld
index 6c3a769b..0fb3e9e9 100644
--- a/gcc_nrf52-mcuboot.ld
+++ b/gcc_nrf52-mcuboot.ld
@@ -166,6 +166,12 @@ SECTIONS
     PROVIDE(__stop_nrf_balloc = .);
   } > FLASH
 
+  .test_dummy :
+  {
+    . = ALIGN(4);
+    . += 56212;
+  } > FLASH
+
 } INSERT AFTER .text

Tested actions:

1. Uploading DFU image onto pinewatch dev kit
2. Tested if test version is working correctly
   • button is working
   • touch sensor is working
   • watch screen is working
   • menu is working
3. Tested reboot
   • Approved current version first
   • Hold the button to produce software reset
4. Tested if rolling back to previous version
   • Hold the button to produce software reset
   • Hold the button in bootloader mode until it start process of rolling back to previous firmware version

All test results are match expected behavior.
@JF002 I think this PR is ready and I will prepare updating documentation in another branch/PR to make it easier to review. Let me know if I need to do something else.

[JF002]

@AlexXZero Thanks for your test results, it looks good to me! I'm OK to create a new PR for the documentation.

Just one minor detail : I think we should also update gcc_nrf52.ld. This linker script is used in development, when flashing the firmware directly at offset 0x00 so that you don't need to run the bootloader when flashing a new version of the firmware via SWD. The only difference with gcc_nrf52-mcuboot.ld is the flash offset set to 0x00 instead of 0x8000.

[JF002]

I added the changes in gcc_nrf52-mcuboot.ld and also updated the documentation in the bootloader readme file (InfiniTimeOrg/pinetime-mcuboot-bootloader@2bd9ead) so that we can merge this branch.

Thanks for you work, @AlexXZero !