lancaster-university / microbit-dal

http://lancaster-university.github.io/microbit-docs
Other
254 stars 130 forks source link

Problem with FLASH_PROGRAM_END and target bbc-microbit-classic-gcc #457

Open martinwork opened 4 years ago

martinwork commented 4 years ago

MicroBitFileSystem uses FLASH_PROGRAM_END, rounded up to the next page to calculate the address to start storing the file system data in flash. https://github.com/lancaster-university/microbit-dal/blob/master/inc/core/MicroBitConfig.h#L109 https://github.com/lancaster-university/microbit-dal/blob/master/source/drivers/MicroBitFileSystem.cpp#L187

Using target bbc-microbit-classic-gcc, I've found that writing to flash at that address can sometimes cause a problem: after a reset, the firmware crashes very soon.

It appears that FLASH_PROGRAM_END = (&__etext) is the location for data copied to RAM in the initialisation code from startup_NRF51822.S (see below). When __etext is close to a page boundary, the usual rounding to the next page may not get past the initialisation data.

I think FLASH_PROGRAM_END should be defined something like the following, but I'm not sure this will always be safe. Is __data_start__ >= __data_end__ always?

FLASH_PROGRAM_END also occurs in MicroBitMemoryMap, used by MicroBitPartialFlashingService. I haven't checked whether fixing the calculation of FLASH_PROGRAM_END will cause a problem there.

extern uint32_t __etext;
extern uint32_t __data_start__;
extern uint32_t __data_end__;
#define FLASH_PROGRAM_END ( (uint32_t) (&__etext) + ( uint32_t)(&__data_end__) - ( uint32_t) (&__data_start__))

From startup_NRF51822.S:

/*     Loop to copy data from read only memory to RAM. The ranges
 *      of copy from/to are specified by following symbols evaluated in 
 *      linker script.
 *      __etext: End of code section, i.e., begin of data sections to copy from.
 *      __data_start__/__data_end__: RAM address range that data should be
 *      copied to. Both must be aligned to 4 bytes boundary.  */

    ldr    r1, =__etext
    ldr    r2, =__data_start__
    ldr    r3, =__data_end__

    subs    r3, r2
    ble     .LC0

.LC1:
    subs    r3, 4
    ldr    r0, [r1,r3]
    str    r0, [r2,r3]
    bgt    .LC1
.LC0:
finneyj commented 4 years ago

Hi @martinwork

This is a good spot! it does indeed look like _etext is not a good indicator of the end of flash usage. Curious that the only GCC linker symbols of the data segment are the RAM pointers, but checking the linker script I agree that this is indeed the case.

Fix looks good to me.