From afa88b76d4306241690e7b9d8f47e2034b3345ee Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Thu, 3 Oct 2019 17:00:49 -0700 Subject: [PATCH 01/13] Refactor internal flash TDB bounds determination Default the size to the larger of two sectors or 10 pages, so that the computation works better on devices with a low sector to page size ratio. Reduce code duplication. --- .../kvstore/conf/filesystem/mbed_lib.json | 6 +- features/storage/kvstore/conf/kv_config.cpp | 343 +++++++----------- .../kvstore/conf/tdb_external/mbed_lib.json | 4 +- .../kvstore/conf/tdb_internal/mbed_lib.json | 4 +- 4 files changed, 145 insertions(+), 212 deletions(-) diff --git a/features/storage/kvstore/conf/filesystem/mbed_lib.json b/features/storage/kvstore/conf/filesystem/mbed_lib.json index 85b27cbf4d..1758242de7 100644 --- a/features/storage/kvstore/conf/filesystem/mbed_lib.json +++ b/features/storage/kvstore/conf/filesystem/mbed_lib.json @@ -2,11 +2,11 @@ "name": "storage_filesystem", "config": { "rbp_internal_size": { - "help": "Default is the size of the 2 last sectors of internal flash", + "help": "Default is the larger of the last 2 sectors or last 10 pages of flash.", "value": "0" }, "internal_base_address": { - "help": "If default, base address is the first sector after the application code", + "help": "If default, base address is set to internal_size bytes before the end of flash.", "value": "0" }, "filesystem": { @@ -36,7 +36,7 @@ }, "target_overrides": { "MCU_PSOC6": { - "rbp_internal_size": 7168 + "rbp_internal_size": "7168" } } } diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index 5283287a10..9da2b27268 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -270,31 +270,37 @@ FileSystem *_get_filesystem_default(const char *mount) #endif } -//Calculates the start address of FLASHIAP block device for TDB_INTERNAL profile. -//Last two sectors to have a predictable location for the TDBStore -int _get_flashiap_bd_default_addresses_tdb_internal(bd_addr_t *start_address, bd_size_t *size) +int get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size) { int ret = MBED_SUCCESS; #if COMPONENT_FLASHIAP - FlashIAP flash; - static const int STORE_SECTORS = 2; - - if (*start_address || *size) { - return MBED_ERROR_INVALID_ARGUMENT; - } - if (flash.init() != 0) { - return MBED_ERROR_FAILED_OPERATION; + return MBED_ERROR_INITIALIZATION_FAILED; } - // Lets work from end of the flash backwards + // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) + // For each area: must be a minimum of 1 page of reserved and 2 pages for master record + static const int STORE_SECTORS = 2; + static const int STORE_PAGES = 10; + + // Let's work from end of the flash backwards bd_addr_t curr_addr = flash.get_flash_start() + flash.get_flash_size(); + bd_size_t sector_space = 0; for (int i = STORE_SECTORS; i; i--) { bd_size_t sector_size = flash.get_sector_size(curr_addr - 1); - curr_addr -= sector_size; + sector_space += sector_size; + } + + bd_size_t page_space = flash.get_page_size() * STORE_PAGES; + if (sector_space > page_space) { + curr_addr -= sector_space; + *size = sector_space; + } else { + curr_addr -= page_space; + *size = page_space; } // Store- and application-sectors mustn't overlap @@ -309,57 +315,12 @@ int _get_flashiap_bd_default_addresses_tdb_internal(bd_addr_t *start_address, bd } flash.deinit(); - #endif return ret; } -//Calculates address and size for FLASHIAP block device in TDB_EXTERNAL and FILESYSTEM profiles. -//The size of the block device will be 2 sectors at the ends of the internal flash for -//the use of the rbp internal TDBStore. -int _get_flashiap_bd_default_addresses_rbp(bd_addr_t *start_address, bd_size_t *size) -{ -#if COMPONENT_FLASHIAP - - bd_addr_t flash_end_address; - bd_addr_t flash_start_address; - bd_addr_t aligned_start_address; - bd_addr_t flash_first_writable_sector_address; - FlashIAP flash; - - if (*start_address != 0 || *size != 0) { - return MBED_ERROR_INVALID_ARGUMENT; - } - - int ret = flash.init(); - if (ret != 0) { - return MBED_ERROR_INITIALIZATION_FAILED; - } - - flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); - flash_start_address = flash.get_flash_start(); - flash_end_address = flash_start_address + flash.get_flash_size();; - *start_address = flash_end_address - 1; - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - *size = (flash_end_address - aligned_start_address) * 2; - *start_address = (flash_end_address - *size); - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - - flash.deinit(); - - if (aligned_start_address < flash_first_writable_sector_address) { - tr_error("KV Config: Internal block device start address overlapped ROM address "); - return MBED_ERROR_INITIALIZATION_FAILED; - } - -#endif - - return MBED_SUCCESS; - -} - -BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t start_address, bd_size_t size) +int get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size) { #if COMPONENT_FLASHIAP @@ -371,80 +332,91 @@ BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t start_address, bd_size_t size) bd_addr_t end_address; FlashIAP flash; - int ret = flash.init(); - if (ret != 0) { - return NULL; + if (!start_address || !size) { + return MBED_ERROR_INVALID_ARGUMENT; } - //Get flash parameters before starting + int ret = flash.init(); + if (ret != 0) { + return MBED_ERROR_INITIALIZATION_FAILED; + } + + // Get flash parameters flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); flash_start_address = flash.get_flash_start(); - flash_end_address = flash_start_address + flash.get_flash_size();; + flash_end_address = flash_start_address + flash.get_flash_size(); - //Non default configuration - if (start_address != 0) { - - aligned_start_address = align_down(start_address, flash.get_sector_size(start_address)); - if (start_address != aligned_start_address) { - tr_error("KV Config: Internal block device start address is not aligned. Better use %02llx", aligned_start_address); + if (*start_address == 0) { + if (*size == 0) { flash.deinit(); - return NULL; + return MBED_ERROR_INVALID_ARGUMENT; } - if (size == 0) { + *start_address = flash_end_address - *size; + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + } else { + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + + if (*size == 0) { //The block device will have all space from start address to the end of the flash - size = (flash_end_address - start_address); + *size = (flash_end_address - *start_address); + } else { + // Do checks on end address to make sure configured start address/size are good - static FlashIAPBlockDevice bd(start_address, size); - flash.deinit(); - return &bd; - } - - if (size != 0) { - end_address = start_address + size; + end_address = *start_address + *size; if (end_address > flash_end_address) { - tr_error("KV Config: Internal block device end address is out of boundaries"); + // End address is out of flash bounds flash.deinit(); - return NULL; + return MBED_ERROR_INVALID_SIZE; } aligned_end_address = align_up(end_address, flash.get_sector_size(end_address - 1)); if (end_address != aligned_end_address) { - tr_error("KV Config: Internal block device start address is not aligned. Consider changing the size parameter"); + // End address not aligned - size should likely be changed so that it is flash.deinit(); - return NULL; + return MBED_ERROR_INVALID_SIZE; } - - static FlashIAPBlockDevice bd(start_address, size); - flash.deinit(); - return &bd; } } - //Non default configuration start_address = 0 - start_address = flash_end_address - size; - aligned_start_address = align_down(start_address, flash.get_sector_size(start_address)); - if (start_address != aligned_start_address) { - tr_error("KV Config: Internal block device start address is not aligned. Consider changing the size parameter"); - flash.deinit(); - return NULL; - } - flash.deinit(); - if (aligned_start_address < flash_first_writable_sector_address) { - tr_error("KV Config: Internal block device start address overlapped ROM address "); + if (*start_address < flash_first_writable_sector_address) { + // Calculated start address overlaps with ROM + return MBED_ERROR_MEDIA_FULL; + } +#endif + + return MBED_SUCCESS; +} + +BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t &start_address, bd_size_t &size) +{ +#if COMPONENT_FLASHIAP + int ret = get_flash_bounds_from_config(&start_address, &size); + if (ret != 0) { + tr_error("KV Config: Determination of internal block device bounds failed. The configured start address/size is likely invalid."); return NULL; } - static FlashIAPBlockDevice bd(aligned_start_address, size); - return &bd; + static FlashIAPBlockDevice bd(start_address, size); + return &bd; #else return NULL; #endif } -BlockDevice *_get_blockdevice_SPIF(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_SPIF(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_SPIF @@ -474,6 +446,9 @@ BlockDevice *_get_blockdevice_SPIF(bd_addr_t start_address, bd_size_t size) return NULL; } + start_address = aligned_start_address; + size = aligned_end_address - aligned_start_address; + static SlicingBlockDevice sbd(&bd, aligned_start_address, aligned_end_address); return &sbd; @@ -482,7 +457,7 @@ BlockDevice *_get_blockdevice_SPIF(bd_addr_t start_address, bd_size_t size) #endif } -BlockDevice *_get_blockdevice_QSPIF(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_QSPIF(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_QSPIF @@ -515,6 +490,9 @@ BlockDevice *_get_blockdevice_QSPIF(bd_addr_t start_address, bd_size_t size) return NULL; } + start_address = aligned_start_address; + size = aligned_end_address - aligned_start_address; + static SlicingBlockDevice sbd(&bd, aligned_start_address, aligned_end_address); return &sbd; @@ -523,7 +501,7 @@ BlockDevice *_get_blockdevice_QSPIF(bd_addr_t start_address, bd_size_t size) #endif } -BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_DATAFLASH @@ -552,6 +530,9 @@ BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t start_address, bd_size_t size) return NULL; } + start_address = aligned_start_address; + size = aligned_end_address - aligned_start_address; + static SlicingBlockDevice sbd(&bd, aligned_start_address, aligned_end_address); return &sbd; @@ -561,7 +542,7 @@ BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t start_address, bd_size_t size) #endif } -BlockDevice *_get_blockdevice_SD(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_SD(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_SD @@ -618,6 +599,10 @@ BlockDevice *_get_blockdevice_SD(bd_addr_t start_address, bd_size_t size) } aligned_end_address = align_down(aligned_end_address, bd.get_erase_size(aligned_end_address)); + + start_address = aligned_start_address; + size = aligned_end_address - aligned_start_address; + static SlicingBlockDevice sbd(&bd, aligned_start_address, aligned_end_address); return &sbd; @@ -626,7 +611,7 @@ BlockDevice *_get_blockdevice_SD(bd_addr_t start_address, bd_size_t size) #endif } -BlockDevice *_get_blockdevice_default(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_default(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_QSPIF return _get_blockdevice_QSPIF(start_address, size); @@ -644,7 +629,7 @@ BlockDevice *_get_blockdevice_default(bd_addr_t start_address, bd_size_t size) /* Same logic as _get_blockdevice_SD() except block device replaced with from * get_other_blockdevice() */ -BlockDevice *_get_blockdevice_other(bd_addr_t start_address, bd_size_t size) +BlockDevice *_get_blockdevice_other(bd_addr_t &start_address, bd_size_t &size) { bd_addr_t aligned_end_address; bd_addr_t aligned_start_address; @@ -691,6 +676,10 @@ BlockDevice *_get_blockdevice_other(bd_addr_t start_address, bd_size_t size) } aligned_end_address = align_down(aligned_end_address, bd->get_erase_size(aligned_end_address)); + + start_address = aligned_start_address; + size = aligned_end_address - aligned_start_address; + static SlicingBlockDevice sbd(bd, aligned_start_address, aligned_end_address); return &sbd; } @@ -700,57 +689,70 @@ MBED_WEAK BlockDevice *get_other_blockdevice() return NULL; } -int _storage_config_TDB_INTERNAL() -{ -#if COMPONENT_FLASHIAP - bd_size_t internal_size = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_SIZE; - bd_addr_t internal_start_address = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS; +int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_size_t *size, bd_addr_t *start_address) { int ret; - if (internal_size == 0 && internal_start_address == 0) { + //Get the default address and size for the TDBStore + if (*size == 0 && *start_address == 0) { //Calculate the block device size and start address in case default values are used. - ret = _get_flashiap_bd_default_addresses_tdb_internal(&internal_start_address, &internal_size); + ret = get_default_flash_addresses(start_address, size); if (ret != MBED_SUCCESS) { - return ret; + return MBED_ERROR_FAILED_OPERATION; } } - //Get internal memory FLASHIAP block device. - kvstore_config.internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, internal_start_address, internal_size); - if (kvstore_config.internal_bd == NULL) { + //Create internal FLASHIAP block device + *internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, *start_address, *size); + if (*internal_bd == NULL) { tr_error("KV Config: Fail to get internal BlockDevice."); - return MBED_ERROR_FAILED_OPERATION; + return MBED_ERROR_FAILED_OPERATION ; } - - ret = kvstore_config.internal_bd->init(); + // Initialize internal block device + ret = (*internal_bd)->init(); if (ret != MBED_SUCCESS) { tr_error("KV Config: Fail to init internal BlockDevice."); - return MBED_ERROR_FAILED_OPERATION; + return MBED_ERROR_FAILED_OPERATION ; } - //Check that internal flash has 2 or more sectors - if (_calculate_blocksize_match_tdbstore(kvstore_config.internal_bd) != MBED_SUCCESS) { + //Check if TDBStore has at least 2 sector. + if (_calculate_blocksize_match_tdbstore(*internal_bd) != MBED_SUCCESS) { tr_error("KV Config: Can not create TDBStore with less then 2 sector."); return MBED_ERROR_INVALID_ARGUMENT; } - //Deinitialize internal block device and TDB will reinitialize and take control on it. - ret = kvstore_config.internal_bd->deinit(); + //Deinitialize internal block device and TDB will reinitialize and take control of it. + ret = (*internal_bd)->deinit(); if (ret != MBED_SUCCESS) { tr_error("KV Config: Fail to deinit internal BlockDevice."); return MBED_ERROR_FAILED_OPERATION; } - //Create a TDBStore in the internal FLASHIAP block device. - static TDBStore tdb_internal(kvstore_config.internal_bd); - kvstore_config.internal_store = &tdb_internal; + //Create internal TDBStore + static TDBStore tdb_internal(*internal_bd); + *internal_tdb = &tdb_internal; - ret = kvstore_config.internal_store->init(); + ret = (*internal_tdb)->init(); if (ret != MBED_SUCCESS) { tr_error("KV Config: Fail to init internal TDBStore."); return ret; } + + return MBED_SUCCESS; +} + +int _storage_config_TDB_INTERNAL() +{ +#if COMPONENT_FLASHIAP + bd_size_t internal_size = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_SIZE; + bd_addr_t internal_start_address = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS; + + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_size, &internal_start_address); + if (MBED_SUCCESS != ret) { + tr_error("KV Config: Fail to create internal TDBStore"); + return ret; + } + kvstore_config.kvstore_main_instance = kvstore_config.internal_store; @@ -790,40 +792,9 @@ int _storage_config_TDB_EXTERNAL() bd_size_t internal_rbp_size = MBED_CONF_STORAGE_TDB_EXTERNAL_RBP_INTERNAL_SIZE; bd_addr_t internal_start_address = MBED_CONF_STORAGE_TDB_EXTERNAL_INTERNAL_BASE_ADDRESS; - //Get the default address and size for internal rbp TDBStore - if (internal_rbp_size == 0 && internal_start_address == 0) { - //Calculate the block device size and start address in case default values are used. - if (_get_flashiap_bd_default_addresses_rbp(&internal_start_address, &internal_rbp_size) != MBED_SUCCESS) { - return MBED_ERROR_FAILED_OPERATION; - } - } - - //Create internal FLASHIAP block device - kvstore_config.internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, internal_start_address, internal_rbp_size); - if (kvstore_config.internal_bd == NULL) { - tr_error("KV Config: Fail to get internal BlockDevice."); - return MBED_ERROR_FAILED_OPERATION ; - } - - int ret = kvstore_config.internal_bd->init(); - if (ret != MBED_SUCCESS) { - tr_error("KV Config: Fail to init internal BlockDevice."); - return MBED_ERROR_FAILED_OPERATION ; - } - - //Check if TDBStore has at least 2 sector. - if (_calculate_blocksize_match_tdbstore(kvstore_config.internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sector."); - return MBED_ERROR_INVALID_ARGUMENT; - } - - //Create internal TDBStore - static TDBStore tdb_internal(kvstore_config.internal_bd); - kvstore_config.internal_store = &tdb_internal; - - ret = kvstore_config.internal_store->init(); - if (ret != MBED_SUCCESS) { - tr_error("KV Config: Fail to init internal TDBStore."); + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_rbp_size, &internal_start_address); + if (MBED_SUCCESS != ret) { + tr_error("KV Config: Fail to create internal TDBStore"); return ret; } @@ -965,47 +936,9 @@ int _storage_config_FILESYSTEM() bd_size_t internal_rbp_size = MBED_CONF_STORAGE_FILESYSTEM_RBP_INTERNAL_SIZE; bd_addr_t internal_start_address = MBED_CONF_STORAGE_FILESYSTEM_INTERNAL_BASE_ADDRESS; - //Get the default address and size for internal rbp TDBStore - if (internal_rbp_size == 0 && internal_start_address == 0) { - //Calculate the block device size and start address in case default values are used. - if (_get_flashiap_bd_default_addresses_rbp(&internal_start_address, &internal_rbp_size) != MBED_SUCCESS) { - return MBED_ERROR_FAILED_OPERATION; - } - } - - //Get internal FLASHIAP block device - kvstore_config.internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, internal_start_address, internal_rbp_size); - if (kvstore_config.internal_bd == NULL) { - tr_error("KV Config: Fail to get internal BlockDevice "); - return MBED_ERROR_FAILED_OPERATION ; - } - - int ret = kvstore_config.internal_bd->init(); - if (ret != MBED_SUCCESS) { - tr_error("KV Config: Fail to init internal BlockDevice "); - return MBED_ERROR_FAILED_OPERATION ; - } - - //Check that internal flash has 2 or more sectors - if (_calculate_blocksize_match_tdbstore(kvstore_config.internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sector."); - return MBED_ERROR_INVALID_ARGUMENT; - } - - //Deinitialize internal block device and TDB will reinitialize and take control on it. - ret = kvstore_config.internal_bd->deinit(); - if (ret != MBED_SUCCESS) { - tr_error("KV Config: Fail to deinit internal BlockDevice."); - return MBED_ERROR_FAILED_OPERATION; - } - - //Create internal TDBStore for rbp - static TDBStore tdb_internal(kvstore_config.internal_bd); - kvstore_config.internal_store = &tdb_internal; - - ret = kvstore_config.internal_store->init(); - if (ret != MBED_SUCCESS) { - tr_error("KV Config: Fail to init internal TDBStore"); + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_rbp_size, &internal_start_address); + if (MBED_SUCCESS != ret) { + tr_error("KV Config: Fail to create internal TDBStore"); return ret; } diff --git a/features/storage/kvstore/conf/tdb_external/mbed_lib.json b/features/storage/kvstore/conf/tdb_external/mbed_lib.json index b4d0fb6923..97486e841a 100644 --- a/features/storage/kvstore/conf/tdb_external/mbed_lib.json +++ b/features/storage/kvstore/conf/tdb_external/mbed_lib.json @@ -2,11 +2,11 @@ "name": "storage_tdb_external", "config": { "rbp_internal_size": { - "help": "Default is the size of the 2 last sectors of internal flash", + "help": "Default is the larger of the last 2 sectors or last 10 pages of flash.", "value": "0" }, "internal_base_address": { - "help": "If default, the base address is set to the first sector after the application code ends.", + "help": "If default, the base address is set to internal_size bytes before the end of flash.", "value": "0" }, "blockdevice": { diff --git a/features/storage/kvstore/conf/tdb_internal/mbed_lib.json b/features/storage/kvstore/conf/tdb_internal/mbed_lib.json index f0948de855..06f4cd43e9 100644 --- a/features/storage/kvstore/conf/tdb_internal/mbed_lib.json +++ b/features/storage/kvstore/conf/tdb_internal/mbed_lib.json @@ -2,11 +2,11 @@ "name": "storage_tdb_internal", "config": { "internal_size": { - "help": "Size of the FlashIAP block device. default size will be from internal_base_address till the end of the internal flash.", + "help": "Size of the FlashIAP block device. Default size will be the larger of the last 2 sectors or last 10 pages of flash.", "value": "0" }, "internal_base_address": { - "help": "If default, the base address is set to the first sector after the application code ends.", + "help": "If default, the base address is set to internal_size bytes before the end of flash.", "value": "0" } }, From cda0af66ebd19a93bcacdbc7a38904df10c0b6e2 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Thu, 3 Oct 2019 17:05:58 -0700 Subject: [PATCH 02/13] Move TDB bounds computation for better reuse Migrate into TDBStore so that DirectAccessDeviceKey can use it as well. --- features/storage/kvstore/conf/kv_config.cpp | 137 +----------------- .../DirectAccessDevicekey.cpp | 63 +++----- .../storage/kvstore/tdbstore/TDBStore.cpp | 136 +++++++++++++++++ features/storage/kvstore/tdbstore/TDBStore.h | 27 ++++ 4 files changed, 185 insertions(+), 178 deletions(-) diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index 9da2b27268..bdb681962c 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -270,140 +270,10 @@ FileSystem *_get_filesystem_default(const char *mount) #endif } -int get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size) -{ - int ret = MBED_SUCCESS; - -#if COMPONENT_FLASHIAP - FlashIAP flash; - if (flash.init() != 0) { - return MBED_ERROR_INITIALIZATION_FAILED; - } - - // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) - // For each area: must be a minimum of 1 page of reserved and 2 pages for master record - static const int STORE_SECTORS = 2; - static const int STORE_PAGES = 10; - - // Let's work from end of the flash backwards - bd_addr_t curr_addr = flash.get_flash_start() + flash.get_flash_size(); - bd_size_t sector_space = 0; - - for (int i = STORE_SECTORS; i; i--) { - bd_size_t sector_size = flash.get_sector_size(curr_addr - 1); - sector_space += sector_size; - } - - bd_size_t page_space = flash.get_page_size() * STORE_PAGES; - if (sector_space > page_space) { - curr_addr -= sector_space; - *size = sector_space; - } else { - curr_addr -= page_space; - *size = page_space; - } - - // Store- and application-sectors mustn't overlap - uint32_t first_wrtbl_sector_addr = - (uint32_t)(align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR))); - - MBED_ASSERT(curr_addr >= first_wrtbl_sector_addr); - if (curr_addr < first_wrtbl_sector_addr) { - ret = MBED_ERROR_MEDIA_FULL; - } else { - *start_address = curr_addr; - } - - flash.deinit(); -#endif - - return ret; -} - -int get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size) -{ -#if COMPONENT_FLASHIAP - - bd_addr_t flash_end_address; - bd_addr_t flash_start_address; - bd_addr_t flash_first_writable_sector_address; - bd_addr_t aligned_start_address; - bd_addr_t aligned_end_address; - bd_addr_t end_address; - FlashIAP flash; - - if (!start_address || !size) { - return MBED_ERROR_INVALID_ARGUMENT; - } - - int ret = flash.init(); - if (ret != 0) { - return MBED_ERROR_INITIALIZATION_FAILED; - } - - // Get flash parameters - flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); - flash_start_address = flash.get_flash_start(); - flash_end_address = flash_start_address + flash.get_flash_size(); - - if (*start_address == 0) { - if (*size == 0) { - flash.deinit(); - return MBED_ERROR_INVALID_ARGUMENT; - } - - *start_address = flash_end_address - *size; - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - if (*start_address != aligned_start_address) { - // Start address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - } else { - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - if (*start_address != aligned_start_address) { - // Start address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - - if (*size == 0) { - //The block device will have all space from start address to the end of the flash - *size = (flash_end_address - *start_address); - } else { - // Do checks on end address to make sure configured start address/size are good - - end_address = *start_address + *size; - if (end_address > flash_end_address) { - // End address is out of flash bounds - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - - aligned_end_address = align_up(end_address, flash.get_sector_size(end_address - 1)); - if (end_address != aligned_end_address) { - // End address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - } - } - - flash.deinit(); - - if (*start_address < flash_first_writable_sector_address) { - // Calculated start address overlaps with ROM - return MBED_ERROR_MEDIA_FULL; - } -#endif - - return MBED_SUCCESS; -} - BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t &start_address, bd_size_t &size) { #if COMPONENT_FLASHIAP - int ret = get_flash_bounds_from_config(&start_address, &size); + int ret = TDBStore::get_flash_bounds_from_config(&start_address, &size); if (ret != 0) { tr_error("KV Config: Determination of internal block device bounds failed. The configured start address/size is likely invalid."); return NULL; @@ -689,13 +559,14 @@ MBED_WEAK BlockDevice *get_other_blockdevice() return NULL; } -int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_size_t *size, bd_addr_t *start_address) { +int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_size_t *size, bd_addr_t *start_address) +{ int ret; //Get the default address and size for the TDBStore if (*size == 0 && *start_address == 0) { //Calculate the block device size and start address in case default values are used. - ret = get_default_flash_addresses(start_address, size); + ret = TDBStore::get_default_flash_addresses(start_address, size); if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } diff --git a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp index d5a0a3a476..f1dfd23b0c 100644 --- a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp +++ b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp @@ -23,6 +23,7 @@ #include "mbed_error.h" #include "MbedCRC.h" #include "mbed_trace.h" +#include "TDBStore.h" #define TRACE_GROUP "DADK" using namespace mbed; @@ -82,7 +83,7 @@ int direct_access_to_devicekey(uint32_t tdb_start_offset, uint32_t tdb_end_offse status = calc_area_params(&flash, tdb_start_offset, tdb_end_offset, area_params); if (status != MBED_SUCCESS) { - tr_error("Couldn't calulate Area Params - err: %d", status); + tr_error("Couldn't calculate Area Params - err: %d", status); goto exit_point; } @@ -108,21 +109,17 @@ exit_point: return status; } -int get_expected_internal_TDBStore_position(uint32_t *out_tdb_start_offset, uint32_t *out_tdb_end_offset) +int get_expected_internal_TDBStore_position(uint32_t *out_tdb_start_offset, uint32_t *out_tdb_end_offset) { - uint32_t flash_end_address; - uint32_t flash_start_address; - uint32_t aligned_start_address; - uint32_t aligned_end_address; + bd_addr_t tdb_start_address; + bd_size_t tdb_size; FlashIAP flash; - bool is_default_configuration = false; - uint32_t tdb_size; if (strcmp(STR(MBED_CONF_STORAGE_STORAGE_TYPE), "FILESYSTEM") == 0) { #ifndef MBED_CONF_STORAGE_FILESYSTEM_INTERNAL_BASE_ADDRESS return MBED_ERROR_ITEM_NOT_FOUND; #else - *out_tdb_start_offset = MBED_CONF_STORAGE_FILESYSTEM_INTERNAL_BASE_ADDRESS; + tdb_start_address = MBED_CONF_STORAGE_FILESYSTEM_INTERNAL_BASE_ADDRESS; tdb_size = MBED_CONF_STORAGE_FILESYSTEM_RBP_INTERNAL_SIZE; #endif @@ -130,7 +127,7 @@ int get_expected_internal_TDBStore_position(uint32_t *out_tdb_start_offset, uin #ifndef MBED_CONF_STORAGE_TDB_EXTERNAL_INTERNAL_BASE_ADDRESS return MBED_ERROR_ITEM_NOT_FOUND; #else - *out_tdb_start_offset = MBED_CONF_STORAGE_TDB_EXTERNAL_INTERNAL_BASE_ADDRESS; + tdb_start_address = MBED_CONF_STORAGE_TDB_EXTERNAL_INTERNAL_BASE_ADDRESS; tdb_size = MBED_CONF_STORAGE_TDB_EXTERNAL_RBP_INTERNAL_SIZE; #endif @@ -139,52 +136,28 @@ int get_expected_internal_TDBStore_position(uint32_t *out_tdb_start_offset, uin #ifndef MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS return MBED_ERROR_ITEM_NOT_FOUND; #else - *out_tdb_start_offset = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS; + tdb_start_address = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS; tdb_size = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_SIZE; #endif } else { return MBED_ERROR_UNSUPPORTED; } - *out_tdb_end_offset = (*out_tdb_start_offset) + tdb_size; - - if ((*out_tdb_start_offset == 0) && (tdb_size == 0)) { - is_default_configuration = true; - } else if ((*out_tdb_start_offset == 0) || (tdb_size == 0)) { - return MBED_ERROR_UNSUPPORTED; + int ret; + if ((tdb_start_address == 0) && (tdb_size == 0)) { + ret = TDBStore::get_default_flash_addresses(&tdb_start_address, &tdb_size); + if (ret != MBED_SUCCESS) { + return MBED_ERROR_FAILED_OPERATION; + } } - int ret = flash.init(); - if (ret != 0) { + ret = TDBStore::get_flash_bounds_from_config(&tdb_start_address, &tdb_size); + if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } - uint32_t flash_first_writable_sector_address = (uint32_t)(align_up(FLASHIAP_APP_ROM_END_ADDR, - flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR))); - - //Get flash parameters before starting - flash_start_address = flash.get_flash_start(); - flash_end_address = flash_start_address + flash.get_flash_size(); - - if (!is_default_configuration) { - aligned_start_address = align_down(*out_tdb_start_offset, flash.get_sector_size(*out_tdb_start_offset)); - aligned_end_address = align_up(*out_tdb_end_offset, flash.get_sector_size(*out_tdb_end_offset - 1)); - if ((*out_tdb_start_offset != aligned_start_address) || (*out_tdb_end_offset != aligned_end_address) - || (*out_tdb_end_offset > flash_end_address)) { - flash.deinit(); - return MBED_ERROR_INVALID_OPERATION; - } - } else { - aligned_start_address = flash_end_address - (flash.get_sector_size(flash_end_address - 1) * 2); - if (aligned_start_address < flash_first_writable_sector_address) { - flash.deinit(); - return MBED_ERROR_INVALID_OPERATION; - } - *out_tdb_start_offset = aligned_start_address; - *out_tdb_end_offset = flash_end_address; - } - - flash.deinit(); + *out_tdb_start_offset = tdb_start_address; + *out_tdb_end_offset = tdb_start_address + tdb_size; return MBED_SUCCESS; } diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index 43424b1d50..877f5da56c 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -25,6 +25,7 @@ #include "mbed_assert.h" #include "mbed_wait_api.h" #include "MbedCRC.h" +#include "FlashIAP.h" using namespace mbed; @@ -110,6 +111,11 @@ static inline uint32_t align_up(uint32_t val, uint32_t size) return (((val - 1) / size) + 1) * size; } +static inline uint32_t align_down(uint64_t val, uint64_t size) +{ + return (((val) / size)) * size; +} + static uint32_t calc_crc(uint32_t init_crc, uint32_t data_size, const void *data_buf) { @@ -1422,6 +1428,136 @@ int TDBStore::reserved_data_get(void *reserved_data, size_t reserved_data_buf_si return ret; } +int TDBStore::get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size) +{ + int ret = MBED_SUCCESS; + +#if COMPONENT_FLASHIAP + FlashIAP flash; + if (flash.init() != 0) { + return MBED_ERROR_INITIALIZATION_FAILED; + } + + // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) + // For each area: must be a minimum of 1 page of reserved and 2 pages for master record + static const int STORE_SECTORS = 2; + static const int STORE_PAGES = 10; + + // Let's work from end of the flash backwards + bd_addr_t curr_addr = flash.get_flash_start() + flash.get_flash_size(); + bd_size_t sector_space = 0; + + for (int i = STORE_SECTORS; i; i--) { + bd_size_t sector_size = flash.get_sector_size(curr_addr - 1); + sector_space += sector_size; + } + + bd_size_t page_space = flash.get_page_size() * STORE_PAGES; + if (sector_space > page_space) { + curr_addr -= sector_space; + *size = sector_space; + } else { + curr_addr -= page_space; + *size = page_space; + } + + // Store- and application-sectors mustn't overlap + uint32_t first_wrtbl_sector_addr = + (uint32_t)(align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR))); + + MBED_ASSERT(curr_addr >= first_wrtbl_sector_addr); + if (curr_addr < first_wrtbl_sector_addr) { + ret = MBED_ERROR_MEDIA_FULL; + } else { + *start_address = curr_addr; + } + + flash.deinit(); +#endif + + return ret; +} + +int TDBStore::get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size) +{ +#if COMPONENT_FLASHIAP + + bd_addr_t flash_end_address; + bd_addr_t flash_start_address; + bd_addr_t flash_first_writable_sector_address; + bd_addr_t aligned_start_address; + bd_addr_t aligned_end_address; + bd_addr_t end_address; + FlashIAP flash; + + if (!start_address || !size) { + return MBED_ERROR_INVALID_ARGUMENT; + } + + int ret = flash.init(); + if (ret != 0) { + return MBED_ERROR_INITIALIZATION_FAILED; + } + + // Get flash parameters + flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); + flash_start_address = flash.get_flash_start(); + flash_end_address = flash_start_address + flash.get_flash_size(); + + if (*start_address == 0) { + if (*size == 0) { + flash.deinit(); + return MBED_ERROR_INVALID_ARGUMENT; + } + + *start_address = flash_end_address - *size; + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + } else { + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + + if (*size == 0) { + //The block device will have all space from start address to the end of the flash + *size = (flash_end_address - *start_address); + } else { + // Do checks on end address to make sure configured start address/size are good + + end_address = *start_address + *size; + if (end_address > flash_end_address) { + // End address is out of flash bounds + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + + aligned_end_address = align_up(end_address, flash.get_sector_size(end_address - 1)); + if (end_address != aligned_end_address) { + // End address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + } + } + + flash.deinit(); + + if (*start_address < flash_first_writable_sector_address) { + // Calculated start address overlaps with ROM + return MBED_ERROR_MEDIA_FULL; + } +#endif + + return MBED_SUCCESS; +} + void TDBStore::offset_in_erase_unit(uint8_t area, uint32_t offset, uint32_t &offset_from_start, uint32_t &dist_to_end) diff --git a/features/storage/kvstore/tdbstore/TDBStore.h b/features/storage/kvstore/tdbstore/TDBStore.h index 71b92b1fa4..f5c1b8be90 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.h +++ b/features/storage/kvstore/tdbstore/TDBStore.h @@ -280,6 +280,33 @@ public: virtual int reserved_data_get(void *reserved_data, size_t reserved_data_buf_size, size_t *actual_data_size = 0); + /** + * @brief Get the default TDBStore flash start address and size. + * + * @param[out] start_address Default TDBStore start address in flash. + * @param[out] size Default TDBStore size. + * + * @returns MBED_SUCCESS Success. + * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. + * MBED_ERROR_MEDIA_FULL Default TDBStore space overlaps with program memory. + */ + static int get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size); + + /** + * @brief Get the TDBStore flash bounds from the configured start address and size. + * Configured start address/size must not both be 0. + * + * @param[inout] start_address Configured TDBStore start address in flash. + * @param[inout] size Configured TDBStore size. + * + * @returns MBED_SUCCESS Success. + * MBED_ERROR_INVALID_ARGUMENT One of the arguments is NULL or both the configured start address and size are 0. + * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. + * MBED_ERROR_INVALID_SIZE Configured size results in misaligned start/end address or end address past the end of flash. + * MBED_ERROR_MEDIA_FULL Configured start address/size results in bounds overlapping with program memory. + */ + static int get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size); + #if !defined(DOXYGEN_ONLY) private: From 7cd4d11a8aac8ae4993bd6d88bf6c62cc9eee79f Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Tue, 29 Oct 2019 16:20:47 -0700 Subject: [PATCH 03/13] Expand error checks in _calculate_blocksize_match_tdbstore The minimum size required by tdbstore is either 2 sectors or 10 pages, whichever is larger. Correspondingly, adjust the error checks in _calculate_blocksize_match_tdbstore to match this requirement. --- features/storage/kvstore/conf/kv_config.cpp | 18 ++++++++++++------ features/storage/kvstore/tdbstore/TDBStore.cpp | 5 ----- features/storage/kvstore/tdbstore/TDBStore.h | 7 +++++++ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index bdb681962c..cfa2f96cb2 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -180,13 +180,19 @@ int _calculate_blocksize_match_tdbstore(BlockDevice *bd) { bd_size_t size = bd->size(); bd_size_t erase_size = bd->get_erase_size(); + bd_size_t page_size = bd->get_program_size(); bd_size_t number_of_sector = size / erase_size; - - if (number_of_sector < 2) { + bd_size_t number_of_page = size / page_size; + if (number_of_sector < TDBStore::STORE_SECTORS) { tr_warning("KV Config: There are less than two sectors - TDBStore will not work."); return -1; } + if (number_of_page < TDBStore::STORE_PAGES) { + tr_warning("KV Config: There are less than ten pages sectors - TDBStore will not work."); + return -1; + } + if (number_of_sector % 2 != 0) { tr_warning("KV Config: Number of sectors is not an even number. Consider changing the BlockDevice size"); @@ -586,9 +592,9 @@ int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_s return MBED_ERROR_FAILED_OPERATION ; } - //Check if TDBStore has at least 2 sector. + //Check if TDBStore has at least 2 sectors or 10 pages. if (_calculate_blocksize_match_tdbstore(*internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sector."); + tr_error("KV Config: Can not create TDBStore with less then 2 sectors or 10 pages."); return MBED_ERROR_INVALID_ARGUMENT; } @@ -754,9 +760,9 @@ int _storage_config_tdb_external_common() return MBED_ERROR_FAILED_OPERATION ; } - //Check that there is at least 2 sector for the external TDBStore + //Check that there is at least 2 sectors for the external TDBStore if (_calculate_blocksize_match_tdbstore(kvstore_config.external_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sector."); + tr_error("KV Config: Can not create TDBStore with less then 2 sectors or 10 pages."); return MBED_ERROR_INVALID_SIZE; } diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index 877f5da56c..f367283887 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -1438,11 +1438,6 @@ int TDBStore::get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *s return MBED_ERROR_INITIALIZATION_FAILED; } - // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) - // For each area: must be a minimum of 1 page of reserved and 2 pages for master record - static const int STORE_SECTORS = 2; - static const int STORE_PAGES = 10; - // Let's work from end of the flash backwards bd_addr_t curr_addr = flash.get_flash_start() + flash.get_flash_size(); bd_size_t sector_space = 0; diff --git a/features/storage/kvstore/tdbstore/TDBStore.h b/features/storage/kvstore/tdbstore/TDBStore.h index f5c1b8be90..519891d923 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.h +++ b/features/storage/kvstore/tdbstore/TDBStore.h @@ -37,6 +37,13 @@ public: static const uint32_t RESERVED_AREA_SIZE = 64; + // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) + // For each area: must be a minimum of 1 page of reserved and 2 pages for master record + /** Minimum number of internal flash sectors required for TDBStore */ + static const int STORE_SECTORS = 2; + /** Minimum number of internal flash pages required for TDBStore */ + static const int STORE_PAGES = 10; + /** * @brief Class constructor * From 9d414316da3d92c791f49df61099920dddc48bd4 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Tue, 29 Oct 2019 16:25:32 -0700 Subject: [PATCH 04/13] TDBStore: Fix potential alignment issue in default addresses When 10 pages is larger than 2 sectors, align the selected size down to be an even multiple of the sector size, to ensure that the allocated space divides cleanly in half for garbage collection. --- features/storage/kvstore/tdbstore/TDBStore.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index f367283887..a3ccc7a700 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -1439,7 +1439,8 @@ int TDBStore::get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *s } // Let's work from end of the flash backwards - bd_addr_t curr_addr = flash.get_flash_start() + flash.get_flash_size(); + bd_addr_t end_of_flash = flash.get_flash_start() + flash.get_flash_size(); + bd_addr_t curr_addr = end_of_flash; bd_size_t sector_space = 0; for (int i = STORE_SECTORS; i; i--) { @@ -1453,7 +1454,9 @@ int TDBStore::get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *s *size = sector_space; } else { curr_addr -= page_space; - *size = page_space; + // Align to 2 sector boundary so that garbage collection works properly + curr_addr = align_down(curr_addr, 2 * flash.get_sector_size(curr_addr)); + *size = end_of_flash - curr_addr; } // Store- and application-sectors mustn't overlap From 0002830c037e30fdac720c24e962b395904cfd83 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Tue, 29 Oct 2019 16:27:33 -0700 Subject: [PATCH 05/13] TDBStore: remove get_flash_bounds input constraint Handle the case where the entirety of flash (size = 0) is required for a flash memory starting at address 0, instead of erroring out. --- features/storage/kvstore/tdbstore/TDBStore.cpp | 4 ++-- features/storage/kvstore/tdbstore/TDBStore.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index a3ccc7a700..4e82dd3fe6 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -1504,8 +1504,8 @@ int TDBStore::get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t * if (*start_address == 0) { if (*size == 0) { - flash.deinit(); - return MBED_ERROR_INVALID_ARGUMENT; + //The block device will have all space from start address to the end of the flash + *size = flash.get_flash_size(); } *start_address = flash_end_address - *size; diff --git a/features/storage/kvstore/tdbstore/TDBStore.h b/features/storage/kvstore/tdbstore/TDBStore.h index 519891d923..a0603d37fd 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.h +++ b/features/storage/kvstore/tdbstore/TDBStore.h @@ -301,10 +301,9 @@ public: /** * @brief Get the TDBStore flash bounds from the configured start address and size. - * Configured start address/size must not both be 0. * * @param[inout] start_address Configured TDBStore start address in flash. - * @param[inout] size Configured TDBStore size. + * @param[inout] size Configured TDBStore size. If 0, the size will be from the start address to the end of flash * * @returns MBED_SUCCESS Success. * MBED_ERROR_INVALID_ARGUMENT One of the arguments is NULL or both the configured start address and size are 0. From 3ef04db1a3333bc46706a174a2605f4dae44b923 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Wed, 30 Oct 2019 16:09:28 -0700 Subject: [PATCH 06/13] TDBStore: Increase min pages to 14 Increase minimum page count from 10 to 14 based on further experiments with features-storage-tests-kvstore-static_tests. --- .../storage/kvstore/conf/filesystem/mbed_lib.json | 2 +- features/storage/kvstore/conf/kv_config.cpp | 12 ++++++------ .../storage/kvstore/conf/tdb_external/mbed_lib.json | 2 +- .../storage/kvstore/conf/tdb_internal/mbed_lib.json | 2 +- features/storage/kvstore/tdbstore/TDBStore.h | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/features/storage/kvstore/conf/filesystem/mbed_lib.json b/features/storage/kvstore/conf/filesystem/mbed_lib.json index 1758242de7..836da52074 100644 --- a/features/storage/kvstore/conf/filesystem/mbed_lib.json +++ b/features/storage/kvstore/conf/filesystem/mbed_lib.json @@ -2,7 +2,7 @@ "name": "storage_filesystem", "config": { "rbp_internal_size": { - "help": "Default is the larger of the last 2 sectors or last 10 pages of flash.", + "help": "Default is the larger of the last 2 sectors or last 14 pages of flash.", "value": "0" }, "internal_base_address": { diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index cfa2f96cb2..d19afa62cf 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -184,12 +184,12 @@ int _calculate_blocksize_match_tdbstore(BlockDevice *bd) bd_size_t number_of_sector = size / erase_size; bd_size_t number_of_page = size / page_size; if (number_of_sector < TDBStore::STORE_SECTORS) { - tr_warning("KV Config: There are less than two sectors - TDBStore will not work."); + tr_warning("KV Config: There are less than %d sectors - TDBStore will not work.", TDBStore::STORE_SECTORS); return -1; } if (number_of_page < TDBStore::STORE_PAGES) { - tr_warning("KV Config: There are less than ten pages sectors - TDBStore will not work."); + tr_warning("KV Config: There are less than %d pages - TDBStore will not work.", TDBStore::STORE_PAGES); return -1; } @@ -592,9 +592,9 @@ int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_s return MBED_ERROR_FAILED_OPERATION ; } - //Check if TDBStore has at least 2 sectors or 10 pages. + //Check if TDBStore has at least 2 sectors or 14 pages. if (_calculate_blocksize_match_tdbstore(*internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sectors or 10 pages."); + tr_error("KV Config: Can not create TDBStore with less then %d sectors or %d pages.", TDBStore::STORE_SECTORS, TDBStore::STORE_PAGES); return MBED_ERROR_INVALID_ARGUMENT; } @@ -760,9 +760,9 @@ int _storage_config_tdb_external_common() return MBED_ERROR_FAILED_OPERATION ; } - //Check that there is at least 2 sectors for the external TDBStore + //Check that there is at least 2 sectors or 14 pages for the external TDBStore if (_calculate_blocksize_match_tdbstore(kvstore_config.external_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then 2 sectors or 10 pages."); + tr_error("KV Config: Can not create TDBStore with less then 2 sectors or 14 pages."); return MBED_ERROR_INVALID_SIZE; } diff --git a/features/storage/kvstore/conf/tdb_external/mbed_lib.json b/features/storage/kvstore/conf/tdb_external/mbed_lib.json index 97486e841a..af9b80ec5d 100644 --- a/features/storage/kvstore/conf/tdb_external/mbed_lib.json +++ b/features/storage/kvstore/conf/tdb_external/mbed_lib.json @@ -2,7 +2,7 @@ "name": "storage_tdb_external", "config": { "rbp_internal_size": { - "help": "Default is the larger of the last 2 sectors or last 10 pages of flash.", + "help": "Default is the larger of the last 2 sectors or last 14 pages of flash.", "value": "0" }, "internal_base_address": { diff --git a/features/storage/kvstore/conf/tdb_internal/mbed_lib.json b/features/storage/kvstore/conf/tdb_internal/mbed_lib.json index 06f4cd43e9..4289423c38 100644 --- a/features/storage/kvstore/conf/tdb_internal/mbed_lib.json +++ b/features/storage/kvstore/conf/tdb_internal/mbed_lib.json @@ -2,7 +2,7 @@ "name": "storage_tdb_internal", "config": { "internal_size": { - "help": "Size of the FlashIAP block device. Default size will be the larger of the last 2 sectors or last 10 pages of flash.", + "help": "Size of the FlashIAP block device. Default size will be the larger of the last 2 sectors or last 14 pages of flash.", "value": "0" }, "internal_base_address": { diff --git a/features/storage/kvstore/tdbstore/TDBStore.h b/features/storage/kvstore/tdbstore/TDBStore.h index a0603d37fd..48c94eefb3 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.h +++ b/features/storage/kvstore/tdbstore/TDBStore.h @@ -37,12 +37,12 @@ public: static const uint32_t RESERVED_AREA_SIZE = 64; - // Use the last 2 sectors or 10 pages of flash for the TDBStore by default (whichever is larger) + // Use the last 2 sectors or 14 pages of flash for the TDBStore by default (whichever is larger) // For each area: must be a minimum of 1 page of reserved and 2 pages for master record /** Minimum number of internal flash sectors required for TDBStore */ static const int STORE_SECTORS = 2; /** Minimum number of internal flash pages required for TDBStore */ - static const int STORE_PAGES = 10; + static const int STORE_PAGES = 14; /** * @brief Class constructor From e1b857078a9bc96da618c75222d534ddf5bbe109 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Fri, 8 Nov 2019 17:51:59 -0800 Subject: [PATCH 07/13] Remove unnecessary reference-typed arguments No callers make use of the modified argument values, so change them to a more straightforward pass by value. --- features/storage/kvstore/conf/kv_config.cpp | 28 ++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index d19afa62cf..9926aa3a28 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -276,7 +276,7 @@ FileSystem *_get_filesystem_default(const char *mount) #endif } -BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_FLASHIAP int ret = TDBStore::get_flash_bounds_from_config(&start_address, &size); @@ -292,7 +292,7 @@ BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t &start_address, bd_size_t &size #endif } -BlockDevice *_get_blockdevice_SPIF(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_SPIF(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_SPIF @@ -333,7 +333,7 @@ BlockDevice *_get_blockdevice_SPIF(bd_addr_t &start_address, bd_size_t &size) #endif } -BlockDevice *_get_blockdevice_QSPIF(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_QSPIF(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_QSPIF @@ -377,7 +377,7 @@ BlockDevice *_get_blockdevice_QSPIF(bd_addr_t &start_address, bd_size_t &size) #endif } -BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_DATAFLASH @@ -418,7 +418,7 @@ BlockDevice *_get_blockdevice_DATAFLASH(bd_addr_t &start_address, bd_size_t &siz #endif } -BlockDevice *_get_blockdevice_SD(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_SD(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_SD @@ -487,7 +487,7 @@ BlockDevice *_get_blockdevice_SD(bd_addr_t &start_address, bd_size_t &size) #endif } -BlockDevice *_get_blockdevice_default(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_default(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_QSPIF return _get_blockdevice_QSPIF(start_address, size); @@ -505,7 +505,7 @@ BlockDevice *_get_blockdevice_default(bd_addr_t &start_address, bd_size_t &size) /* Same logic as _get_blockdevice_SD() except block device replaced with from * get_other_blockdevice() */ -BlockDevice *_get_blockdevice_other(bd_addr_t &start_address, bd_size_t &size) +BlockDevice *_get_blockdevice_other(bd_addr_t start_address, bd_size_t size) { bd_addr_t aligned_end_address; bd_addr_t aligned_start_address; @@ -565,21 +565,21 @@ MBED_WEAK BlockDevice *get_other_blockdevice() return NULL; } -int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_size_t *size, bd_addr_t *start_address) +int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_size_t size, bd_addr_t start_address) { int ret; //Get the default address and size for the TDBStore - if (*size == 0 && *start_address == 0) { + if (size == 0 && start_address == 0) { //Calculate the block device size and start address in case default values are used. - ret = TDBStore::get_default_flash_addresses(start_address, size); + ret = TDBStore::get_default_flash_addresses(&start_address, &size); if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } } //Create internal FLASHIAP block device - *internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, *start_address, *size); + *internal_bd = GET_BLOCKDEVICE(INTERNAL_BLOCKDEVICE_NAME, start_address, size); if (*internal_bd == NULL) { tr_error("KV Config: Fail to get internal BlockDevice."); return MBED_ERROR_FAILED_OPERATION ; @@ -624,7 +624,7 @@ int _storage_config_TDB_INTERNAL() bd_size_t internal_size = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_SIZE; bd_addr_t internal_start_address = MBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS; - int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_size, &internal_start_address); + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, internal_size, internal_start_address); if (MBED_SUCCESS != ret) { tr_error("KV Config: Fail to create internal TDBStore"); return ret; @@ -669,7 +669,7 @@ int _storage_config_TDB_EXTERNAL() bd_size_t internal_rbp_size = MBED_CONF_STORAGE_TDB_EXTERNAL_RBP_INTERNAL_SIZE; bd_addr_t internal_start_address = MBED_CONF_STORAGE_TDB_EXTERNAL_INTERNAL_BASE_ADDRESS; - int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_rbp_size, &internal_start_address); + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, internal_rbp_size, internal_start_address); if (MBED_SUCCESS != ret) { tr_error("KV Config: Fail to create internal TDBStore"); return ret; @@ -813,7 +813,7 @@ int _storage_config_FILESYSTEM() bd_size_t internal_rbp_size = MBED_CONF_STORAGE_FILESYSTEM_RBP_INTERNAL_SIZE; bd_addr_t internal_start_address = MBED_CONF_STORAGE_FILESYSTEM_INTERNAL_BASE_ADDRESS; - int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, &internal_rbp_size, &internal_start_address); + int ret = _create_internal_tdb(&kvstore_config.internal_bd, &kvstore_config.internal_store, internal_rbp_size, internal_start_address); if (MBED_SUCCESS != ret) { tr_error("KV Config: Fail to create internal TDBStore"); return ret; From 622a50ff6a9d1c26c888172769cc3e424d9cadb7 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Fri, 8 Nov 2019 17:53:31 -0800 Subject: [PATCH 08/13] KV_CONFIG: Change errors to use tr_error not tr_warning --- features/storage/kvstore/conf/kv_config.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index 9926aa3a28..0cc1f292b3 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -184,12 +184,12 @@ int _calculate_blocksize_match_tdbstore(BlockDevice *bd) bd_size_t number_of_sector = size / erase_size; bd_size_t number_of_page = size / page_size; if (number_of_sector < TDBStore::STORE_SECTORS) { - tr_warning("KV Config: There are less than %d sectors - TDBStore will not work.", TDBStore::STORE_SECTORS); + tr_error("KV Config: There are less than %d sectors - TDBStore will not work.", TDBStore::STORE_SECTORS); return -1; } if (number_of_page < TDBStore::STORE_PAGES) { - tr_warning("KV Config: There are less than %d pages - TDBStore will not work.", TDBStore::STORE_PAGES); + tr_error("KV Config: There are less than %d pages - TDBStore will not work.", TDBStore::STORE_PAGES); return -1; } From 926423c109219e759a19b5c150fd9c3f5424d56b Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Fri, 8 Nov 2019 18:10:31 -0800 Subject: [PATCH 09/13] Reuse TDBStore default size computation in devicekey test Replace custom caluation that always assumed two sectors with the standard calculation exposed on TDBStore. --- .../direct_access_devicekey_test/main.cpp | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp b/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp index 8a695eb590..e58983716b 100644 --- a/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp +++ b/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp @@ -58,7 +58,6 @@ int get_virtual_TDBStore_position(uint32_t conf_start_address, uint32_t conf_si uint32_t flash_start_address; uint32_t aligned_start_address; FlashIAP flash; - static const int STORE_SECTORS = 2; int ret = flash.init(); if (ret != 0) { @@ -93,19 +92,11 @@ int get_virtual_TDBStore_position(uint32_t conf_start_address, uint32_t conf_si } } } else { - // Assumption is that last two sectors are reserved for the TDBStore - aligned_start_address = flash.get_flash_start() + flash.get_flash_size(); - - for (int i = STORE_SECTORS; i; i--) { - bd_size_t sector_size = flash.get_sector_size(aligned_start_address - 1); - aligned_start_address -= sector_size; - } - - if (aligned_start_address < flash_first_writable_sector_address) { - flash.deinit(); - return -2; - } - bd_final_size = (flash_end_address - aligned_start_address); + bd_addr_t default_start; + bd_size_t default_size; + TDBStore::get_default_flash_addresses(&default_start, &default_size); + aligned_start_address = (uint32_t)default_start; + bd_final_size = (uint32_t)default_size; } (*tdb_start_address) = aligned_start_address; From 7f18a6ce491fc859aabd6530d984b2e6876420ef Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Mon, 2 Mar 2020 16:34:01 -0800 Subject: [PATCH 10/13] Move flash bounds helpers from TDBStore to kv_config --- .../direct_access_devicekey_test/main.cpp | 2 +- features/storage/kvstore/conf/kv_config.cpp | 148 +++++++++++++++++- features/storage/kvstore/conf/kv_config.h | 28 ++++ .../DirectAccessDevicekey.cpp | 5 +- .../storage/kvstore/tdbstore/TDBStore.cpp | 129 --------------- features/storage/kvstore/tdbstore/TDBStore.h | 33 ---- 6 files changed, 173 insertions(+), 172 deletions(-) diff --git a/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp b/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp index e58983716b..869e335f0b 100644 --- a/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp +++ b/features/storage/TESTS/kvstore/direct_access_devicekey_test/main.cpp @@ -94,7 +94,7 @@ int get_virtual_TDBStore_position(uint32_t conf_start_address, uint32_t conf_si } else { bd_addr_t default_start; bd_size_t default_size; - TDBStore::get_default_flash_addresses(&default_start, &default_size); + kv_get_default_flash_addresses(&default_start, &default_size); aligned_start_address = (uint32_t)default_start; bd_final_size = (uint32_t)default_size; } diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index 0cc1f292b3..c6c3c79db6 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -150,6 +150,12 @@ static const char *filesystemstore_folder_path = NULL; using namespace mbed; +// Use the last 2 sectors or 14 pages of flash for the TDBStore by default (whichever is larger) +// For each area: must be a minimum of 1 page of reserved and 2 pages for master record +/** Minimum number of internal flash sectors required for TDBStore */ +static const int STORE_SECTORS = 2; +/** Minimum number of internal flash pages required for TDBStore */ +static const int STORE_PAGES = 14; static SingletonPtr mutex; static bool is_kv_config_initialize = false; @@ -183,13 +189,13 @@ int _calculate_blocksize_match_tdbstore(BlockDevice *bd) bd_size_t page_size = bd->get_program_size(); bd_size_t number_of_sector = size / erase_size; bd_size_t number_of_page = size / page_size; - if (number_of_sector < TDBStore::STORE_SECTORS) { - tr_error("KV Config: There are less than %d sectors - TDBStore will not work.", TDBStore::STORE_SECTORS); + if (number_of_sector < STORE_SECTORS) { + tr_error("KV Config: There are less than %d sectors - TDBStore will not work.", STORE_SECTORS); return -1; } - if (number_of_page < TDBStore::STORE_PAGES) { - tr_error("KV Config: There are less than %d pages - TDBStore will not work.", TDBStore::STORE_PAGES); + if (number_of_page < STORE_PAGES) { + tr_error("KV Config: There are less than %d pages - TDBStore will not work.", STORE_PAGES); return -1; } @@ -279,7 +285,7 @@ FileSystem *_get_filesystem_default(const char *mount) BlockDevice *_get_blockdevice_FLASHIAP(bd_addr_t start_address, bd_size_t size) { #if COMPONENT_FLASHIAP - int ret = TDBStore::get_flash_bounds_from_config(&start_address, &size); + int ret = kv_get_flash_bounds_from_config(&start_address, &size); if (ret != 0) { tr_error("KV Config: Determination of internal block device bounds failed. The configured start address/size is likely invalid."); return NULL; @@ -572,7 +578,7 @@ int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_s //Get the default address and size for the TDBStore if (size == 0 && start_address == 0) { //Calculate the block device size and start address in case default values are used. - ret = TDBStore::get_default_flash_addresses(&start_address, &size); + ret = kv_get_default_flash_addresses(&start_address, &size); if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } @@ -594,7 +600,7 @@ int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_s //Check if TDBStore has at least 2 sectors or 14 pages. if (_calculate_blocksize_match_tdbstore(*internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then %d sectors or %d pages.", TDBStore::STORE_SECTORS, TDBStore::STORE_PAGES); + tr_error("KV Config: Can not create TDBStore with less then %d sectors or %d pages.", STORE_SECTORS, STORE_PAGES); return MBED_ERROR_INVALID_ARGUMENT; } @@ -991,3 +997,131 @@ exit: mutex->unlock(); return ret; } + +int kv_get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size) +{ + int ret = MBED_SUCCESS; + +#if COMPONENT_FLASHIAP + FlashIAP flash; + if (flash.init() != 0) { + return MBED_ERROR_INITIALIZATION_FAILED; + } + + // Let's work from end of the flash backwards + bd_addr_t end_of_flash = flash.get_flash_start() + flash.get_flash_size(); + bd_addr_t curr_addr = end_of_flash; + bd_size_t sector_space = 0; + + for (int i = STORE_SECTORS; i; i--) { + bd_size_t sector_size = flash.get_sector_size(curr_addr - 1); + sector_space += sector_size; + } + + bd_size_t page_space = flash.get_page_size() * STORE_PAGES; + if (sector_space > page_space) { + curr_addr -= sector_space; + *size = sector_space; + } else { + curr_addr -= page_space; + // Align to 2 sector boundary so that garbage collection works properly + curr_addr = align_down(curr_addr, 2 * flash.get_sector_size(curr_addr)); + *size = end_of_flash - curr_addr; + } + + // Store- and application-sectors mustn't overlap + uint32_t first_wrtbl_sector_addr = + (uint32_t)(align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR))); + + MBED_ASSERT(curr_addr >= first_wrtbl_sector_addr); + if (curr_addr < first_wrtbl_sector_addr) { + ret = MBED_ERROR_MEDIA_FULL; + } else { + *start_address = curr_addr; + } + + flash.deinit(); +#endif + + return ret; +} + +int kv_get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size) +{ +#if COMPONENT_FLASHIAP + + bd_addr_t flash_end_address; + bd_addr_t flash_start_address; + bd_addr_t flash_first_writable_sector_address; + bd_addr_t aligned_start_address; + bd_addr_t aligned_end_address; + bd_addr_t end_address; + FlashIAP flash; + + if (!start_address || !size) { + return MBED_ERROR_INVALID_ARGUMENT; + } + + int ret = flash.init(); + if (ret != 0) { + return MBED_ERROR_INITIALIZATION_FAILED; + } + + // Get flash parameters + flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); + flash_start_address = flash.get_flash_start(); + flash_end_address = flash_start_address + flash.get_flash_size(); + + if (*start_address == 0) { + if (*size == 0) { + //The block device will have all space from start address to the end of the flash + *size = flash.get_flash_size(); + } + + *start_address = flash_end_address - *size; + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + } else { + aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); + if (*start_address != aligned_start_address) { + // Start address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + + if (*size == 0) { + //The block device will have all space from start address to the end of the flash + *size = (flash_end_address - *start_address); + } else { + // Do checks on end address to make sure configured start address/size are good + + end_address = *start_address + *size; + if (end_address > flash_end_address) { + // End address is out of flash bounds + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + + aligned_end_address = align_up(end_address, flash.get_sector_size(end_address - 1)); + if (end_address != aligned_end_address) { + // End address not aligned - size should likely be changed so that it is + flash.deinit(); + return MBED_ERROR_INVALID_SIZE; + } + } + } + + flash.deinit(); + + if (*start_address < flash_first_writable_sector_address) { + // Calculated start address overlaps with ROM + return MBED_ERROR_MEDIA_FULL; + } +#endif + + return MBED_SUCCESS; +} diff --git a/features/storage/kvstore/conf/kv_config.h b/features/storage/kvstore/conf/kv_config.h index 660a6bdd88..f16688fd5e 100644 --- a/features/storage/kvstore/conf/kv_config.h +++ b/features/storage/kvstore/conf/kv_config.h @@ -16,6 +16,8 @@ #ifndef _KV_CONFIG #define _KV_CONFIG +#include "features/storage/blockdevice/BlockDevice.h" + #ifdef __cplusplus extern "C" { #endif @@ -42,6 +44,32 @@ int kv_init_storage_config(); */ const char *get_filesystemstore_folder_path(); +/** + * @brief Get the default TDBStore flash start address and size. + * + * @param[out] start_address Default TDBStore start address in flash. + * @param[out] size Default TDBStore size. + * + * @returns MBED_SUCCESS Success. + * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. + * MBED_ERROR_MEDIA_FULL Default TDBStore space overlaps with program memory. + */ +int kv_get_default_flash_addresses(mbed::bd_addr_t *start_address, mbed::bd_size_t *size); + +/** + * @brief Get the TDBStore flash bounds from the configured start address and size. + * + * @param[inout] start_address Configured TDBStore start address in flash. + * @param[inout] size Configured TDBStore size. If 0, the size will be from the start address to the end of flash + * + * @returns MBED_SUCCESS Success. + * MBED_ERROR_INVALID_ARGUMENT One of the arguments is NULL or both the configured start address and size are 0. + * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. + * MBED_ERROR_INVALID_SIZE Configured size results in misaligned start/end address or end address past the end of flash. + * MBED_ERROR_MEDIA_FULL Configured start address/size results in bounds overlapping with program memory. + */ +int kv_get_flash_bounds_from_config(mbed::bd_addr_t *start_address, mbed::bd_size_t *size); + #ifdef __cplusplus } // closing brace for extern "C" #endif diff --git a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp index f1dfd23b0c..2c7add9a59 100644 --- a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp +++ b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp @@ -24,6 +24,7 @@ #include "MbedCRC.h" #include "mbed_trace.h" #include "TDBStore.h" +#include "kv_config.h" #define TRACE_GROUP "DADK" using namespace mbed; @@ -145,13 +146,13 @@ int get_expected_internal_TDBStore_position(uint32_t *out_tdb_start_offset, uint int ret; if ((tdb_start_address == 0) && (tdb_size == 0)) { - ret = TDBStore::get_default_flash_addresses(&tdb_start_address, &tdb_size); + ret = kv_get_default_flash_addresses(&tdb_start_address, &tdb_size); if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } } - ret = TDBStore::get_flash_bounds_from_config(&tdb_start_address, &tdb_size); + ret = kv_get_flash_bounds_from_config(&tdb_start_address, &tdb_size); if (ret != MBED_SUCCESS) { return MBED_ERROR_FAILED_OPERATION; } diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index 4e82dd3fe6..fd7d8ce3c0 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -1428,135 +1428,6 @@ int TDBStore::reserved_data_get(void *reserved_data, size_t reserved_data_buf_si return ret; } -int TDBStore::get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size) -{ - int ret = MBED_SUCCESS; - -#if COMPONENT_FLASHIAP - FlashIAP flash; - if (flash.init() != 0) { - return MBED_ERROR_INITIALIZATION_FAILED; - } - - // Let's work from end of the flash backwards - bd_addr_t end_of_flash = flash.get_flash_start() + flash.get_flash_size(); - bd_addr_t curr_addr = end_of_flash; - bd_size_t sector_space = 0; - - for (int i = STORE_SECTORS; i; i--) { - bd_size_t sector_size = flash.get_sector_size(curr_addr - 1); - sector_space += sector_size; - } - - bd_size_t page_space = flash.get_page_size() * STORE_PAGES; - if (sector_space > page_space) { - curr_addr -= sector_space; - *size = sector_space; - } else { - curr_addr -= page_space; - // Align to 2 sector boundary so that garbage collection works properly - curr_addr = align_down(curr_addr, 2 * flash.get_sector_size(curr_addr)); - *size = end_of_flash - curr_addr; - } - - // Store- and application-sectors mustn't overlap - uint32_t first_wrtbl_sector_addr = - (uint32_t)(align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR))); - - MBED_ASSERT(curr_addr >= first_wrtbl_sector_addr); - if (curr_addr < first_wrtbl_sector_addr) { - ret = MBED_ERROR_MEDIA_FULL; - } else { - *start_address = curr_addr; - } - - flash.deinit(); -#endif - - return ret; -} - -int TDBStore::get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size) -{ -#if COMPONENT_FLASHIAP - - bd_addr_t flash_end_address; - bd_addr_t flash_start_address; - bd_addr_t flash_first_writable_sector_address; - bd_addr_t aligned_start_address; - bd_addr_t aligned_end_address; - bd_addr_t end_address; - FlashIAP flash; - - if (!start_address || !size) { - return MBED_ERROR_INVALID_ARGUMENT; - } - - int ret = flash.init(); - if (ret != 0) { - return MBED_ERROR_INITIALIZATION_FAILED; - } - - // Get flash parameters - flash_first_writable_sector_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR)); - flash_start_address = flash.get_flash_start(); - flash_end_address = flash_start_address + flash.get_flash_size(); - - if (*start_address == 0) { - if (*size == 0) { - //The block device will have all space from start address to the end of the flash - *size = flash.get_flash_size(); - } - - *start_address = flash_end_address - *size; - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - if (*start_address != aligned_start_address) { - // Start address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - } else { - aligned_start_address = align_down(*start_address, flash.get_sector_size(*start_address)); - if (*start_address != aligned_start_address) { - // Start address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - - if (*size == 0) { - //The block device will have all space from start address to the end of the flash - *size = (flash_end_address - *start_address); - } else { - // Do checks on end address to make sure configured start address/size are good - - end_address = *start_address + *size; - if (end_address > flash_end_address) { - // End address is out of flash bounds - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - - aligned_end_address = align_up(end_address, flash.get_sector_size(end_address - 1)); - if (end_address != aligned_end_address) { - // End address not aligned - size should likely be changed so that it is - flash.deinit(); - return MBED_ERROR_INVALID_SIZE; - } - } - } - - flash.deinit(); - - if (*start_address < flash_first_writable_sector_address) { - // Calculated start address overlaps with ROM - return MBED_ERROR_MEDIA_FULL; - } -#endif - - return MBED_SUCCESS; -} - - void TDBStore::offset_in_erase_unit(uint8_t area, uint32_t offset, uint32_t &offset_from_start, uint32_t &dist_to_end) { diff --git a/features/storage/kvstore/tdbstore/TDBStore.h b/features/storage/kvstore/tdbstore/TDBStore.h index 48c94eefb3..71b92b1fa4 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.h +++ b/features/storage/kvstore/tdbstore/TDBStore.h @@ -37,13 +37,6 @@ public: static const uint32_t RESERVED_AREA_SIZE = 64; - // Use the last 2 sectors or 14 pages of flash for the TDBStore by default (whichever is larger) - // For each area: must be a minimum of 1 page of reserved and 2 pages for master record - /** Minimum number of internal flash sectors required for TDBStore */ - static const int STORE_SECTORS = 2; - /** Minimum number of internal flash pages required for TDBStore */ - static const int STORE_PAGES = 14; - /** * @brief Class constructor * @@ -287,32 +280,6 @@ public: virtual int reserved_data_get(void *reserved_data, size_t reserved_data_buf_size, size_t *actual_data_size = 0); - /** - * @brief Get the default TDBStore flash start address and size. - * - * @param[out] start_address Default TDBStore start address in flash. - * @param[out] size Default TDBStore size. - * - * @returns MBED_SUCCESS Success. - * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. - * MBED_ERROR_MEDIA_FULL Default TDBStore space overlaps with program memory. - */ - static int get_default_flash_addresses(bd_addr_t *start_address, bd_size_t *size); - - /** - * @brief Get the TDBStore flash bounds from the configured start address and size. - * - * @param[inout] start_address Configured TDBStore start address in flash. - * @param[inout] size Configured TDBStore size. If 0, the size will be from the start address to the end of flash - * - * @returns MBED_SUCCESS Success. - * MBED_ERROR_INVALID_ARGUMENT One of the arguments is NULL or both the configured start address and size are 0. - * MBED_ERROR_INITIALIZATION_FAILED Failed to initialize flash driver. - * MBED_ERROR_INVALID_SIZE Configured size results in misaligned start/end address or end address past the end of flash. - * MBED_ERROR_MEDIA_FULL Configured start address/size results in bounds overlapping with program memory. - */ - static int get_flash_bounds_from_config(bd_addr_t *start_address, bd_size_t *size); - #if !defined(DOXYGEN_ONLY) private: From ae7c6203ccc30669dab7cd21ae7a9b780cad81a7 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Mon, 2 Mar 2020 16:35:32 -0800 Subject: [PATCH 11/13] kv_config: Remove hard fail on too few pages STORE_SECTORS is a hard requirement. If there are fewer than 2 pages then the kvstore will not work, because the garbage collection process relies on having at least two sectors to work with. STORE_PAGES is a heuristic. It is a reasonable default to use if the application does not specify the amount of flash to use for TDBStore. But if an application knows that a smaller number of pages will suffice for its specific needs, then that is valid and should be permitted. --- features/storage/kvstore/conf/kv_config.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/features/storage/kvstore/conf/kv_config.cpp b/features/storage/kvstore/conf/kv_config.cpp index c6c3c79db6..4102c0a67b 100644 --- a/features/storage/kvstore/conf/kv_config.cpp +++ b/features/storage/kvstore/conf/kv_config.cpp @@ -194,12 +194,6 @@ int _calculate_blocksize_match_tdbstore(BlockDevice *bd) return -1; } - if (number_of_page < STORE_PAGES) { - tr_error("KV Config: There are less than %d pages - TDBStore will not work.", STORE_PAGES); - return -1; - } - - if (number_of_sector % 2 != 0) { tr_warning("KV Config: Number of sectors is not an even number. Consider changing the BlockDevice size"); } @@ -600,7 +594,7 @@ int _create_internal_tdb(BlockDevice **internal_bd, KVStore **internal_tdb, bd_s //Check if TDBStore has at least 2 sectors or 14 pages. if (_calculate_blocksize_match_tdbstore(*internal_bd) != MBED_SUCCESS) { - tr_error("KV Config: Can not create TDBStore with less then %d sectors or %d pages.", STORE_SECTORS, STORE_PAGES); + tr_error("KV Config: Can not create TDBStore with less then %d sectors.", STORE_SECTORS); return MBED_ERROR_INVALID_ARGUMENT; } From ab8ac8871cf0051692fba478946ba68aeee48243 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Thu, 19 Mar 2020 09:06:18 -0700 Subject: [PATCH 12/13] Remove stray include of TDBStore.h --- .../kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp index 2c7add9a59..2bd1930dfd 100644 --- a/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp +++ b/features/storage/kvstore/direct_access_devicekey/DirectAccessDevicekey.cpp @@ -23,7 +23,6 @@ #include "mbed_error.h" #include "MbedCRC.h" #include "mbed_trace.h" -#include "TDBStore.h" #include "kv_config.h" #define TRACE_GROUP "DADK" From 85d2e8f2f21b953d079c9071253bc1ac59f44609 Mon Sep 17 00:00:00 2001 From: Kyle Kearney Date: Tue, 24 Mar 2020 12:36:36 -0700 Subject: [PATCH 13/13] Add kvstore/conf to unittest includes The unittests compile DirectAccessDeviceKey.cpp which depends on kv_config.h, which lives in features/storage/kvstore/conf --- UNITTESTS/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/UNITTESTS/CMakeLists.txt b/UNITTESTS/CMakeLists.txt index 1ef06cff65..52f46f5f9d 100644 --- a/UNITTESTS/CMakeLists.txt +++ b/UNITTESTS/CMakeLists.txt @@ -145,6 +145,7 @@ set(unittest-includes-base "${PROJECT_SOURCE_DIR}/../features/mbedtls" "${PROJECT_SOURCE_DIR}/../features/mbedtls/inc" "${PROJECT_SOURCE_DIR}/../features/mbedtls/mbed-crypto/inc" + "${PROJECT_SOURCE_DIR}/../features/storage/kvstore/conf" ) # Create a list for test suites.