Commit d1233e1b authored by George Beckstein's avatar George Beckstein Committed by David Brown
Browse files

Add reference counting to Mbed OS flash backend 



The internal functions of mcuboot are not consistent in opening/closing flash areas and sometimes nested calls happen to `flash_area_open` and `flash_area_close`. With the previous implementation, a nested call to `flash_area_close` would deinitialize the underlying `BlockDevice`. This could cause subsequent flash operations on an "open" flash area to fail.

This PR adds a simple open counter for each flash area and ensures the underlying `BlockDevice` is initialized and deinitialized appropriately. The `BlockDevice` is only initialized when transitioning from an open count of 0 to 1. The `BlockDevice` is only deinitialized when the open count falls to 0.

Signed-off-by: default avatarGeorge Beckstein <becksteing@embeddedplanet.com>
parent ae097709

boot/mbed/src/flash_map_backend.cpp

+32 −7
Original line number Diff line number Diff line
@@ -53,6 +53,8 @@ static mbed::BlockDevice* flash_map_bd[FLASH_AREAS] = { 


static struct flash_area flash_areas[FLASH_AREAS];



static unsigned int open_count[FLASH_AREAS] = {0};



int flash_area_open(uint8_t id, const struct flash_area** fapp) {



    *fapp = &flash_areas[id];
@@ -77,18 +79,41 @@ int flash_area_open(uint8_t id, const struct flash_area** fapp) { 
            return -1;

    }



    open_count[id]++;

    MCUBOOT_LOG_DBG("flash area %d open count: %d (+)", id, open_count[id]);



    fap->fa_id = id;

    fap->fa_device_id = 0; // not relevant



    mbed::BlockDevice* bd = flash_map_bd[id];

    fap->fa_size = (uint32_t) bd->size();



    /* Only initialize if this isn't a nested call to open the flash area */

    if (open_count[id] == 1) {

        MCUBOOT_LOG_DBG("initializing flash area %d...", id);

        return bd->init();

    } else {

        return 0;

    }

}



void flash_area_close(const struct flash_area* fap) {

    mbed::BlockDevice* bd = flash_map_bd[fap->fa_id];

    uint8_t id = fap->fa_id;

    /* No need to close an unopened flash area, avoid an overflow of the counter */

    if (!open_count[id]) {

        return;

    }



    open_count[id]--;

    MCUBOOT_LOG_DBG("flash area %d open count: %d (-)", id, open_count[id]);

    if (!open_count[id]) {

        /* mcuboot is not currently consistent in opening/closing flash areas only once at a time

         * so only deinitialize the BlockDevice if all callers have closed the flash area. */

        MCUBOOT_LOG_DBG("deinitializing flash area block device %d...", id);

        mbed::BlockDevice* bd = flash_map_bd[id];

        bd->deinit();

    }

}



/*

 * Read/write/erase. Offset is relative from beginning of flash area.
@@ -96,8 +121,8 @@ void flash_area_close(const struct flash_area* fap) { 
int flash_area_read(const struct flash_area* fap, uint32_t off, void* dst, uint32_t len) {

    mbed::BlockDevice* bd = flash_map_bd[fap->fa_id];



    // Note: The address must be aligned to bd->get_read_size(). If MCUBOOT_READ_GRANULARITY

    // is defined, the length does not need to be aligned.

    /* Note: The address must be aligned to bd->get_read_size(). If MCUBOOT_READ_GRANULARITY

       is defined, the length does not need to be aligned. */

#ifdef MCUBOOT_READ_GRANULARITY

    uint32_t read_size = bd->get_read_size();

    if (read_size == 0) {
@@ -121,7 +146,7 @@ int flash_area_read(const struct flash_area* fap, uint32_t off, void* dst, uint3 
        else {

            int ret = bd->read(dst, off, len);

            if (ret != 0) {

                MCUBOOT_LOG_ERR("Read failed: fa_id %d offset 0x%x len 0x%x", fap->fa_id, off, len);

                MCUBOOT_LOG_ERR("Read failed: fa_id %d offset 0x%x len 0x%x (%d)", fap->fa_id, off, len, ret);

                return ret;

            }

        }
@@ -171,7 +196,7 @@ uint8_t flash_area_erased_val(const struct flash_area* fap) { 
int flash_area_get_sectors(int fa_id, uint32_t* count, struct flash_sector* sectors) {

    mbed::BlockDevice* bd = flash_map_bd[fa_id];



    // Loop through sectors and collect information on them

    /* Loop through sectors and collect information on them */

    bd_addr_t offset = 0;

    *count = 0;

    while (*count < MCUBOOT_MAX_IMG_SECTORS && bd->is_valid_read(offset, bd->get_read_size())) {

docs/PORTING.md

+2 −0
Original line number Diff line number Diff line
@@ -132,6 +132,8 @@ int flash_area_id_from_multi_image_slot(int image_index, int slot); 
int     flash_area_id_to_multi_image_slot(int image_index, int area_id);

```



**Note:** As of writing, it is possible that mcuboot will open a flash area multiple times simultaneously (through nested calls to `flash_area_open`). As a result, mcuboot may call `flash_area_close` on a flash area that is still opened by another part of mcuboot. As a workaround when porting, it may be necessary to implement a counter of the number of times a given flash area has been opened by mcuboot. The `flash_area_close` implementation should only fully deinitialize the underlying flash area when the open counter is decremented to 0. See [this GitHub PR](https://github.com/mcu-tools/mcuboot/pull/894/) for a more detailed discussion.



## Memory management for mbed TLS



`mbed TLS` employs dynamic allocation of memory, making use of the pair

CRA Git | Maintained and supported by SUSTech CRA and CCSE