From 6dd54da93f3932eef57faebb4c8129b993cb48d8 Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Mon, 11 Nov 2024 14:31:57 +0100 Subject: [PATCH 1/4] bootutil: Add support for devices without erase and reduced erases The commit adds two MCUboot configuration options: - MCUBOOT_DEV_WITHOUT_ERASE_THEN_NO_ERASE - MCUBOOT_MINIMAL_SCRAMBLE The first one should be enabled to support devices that do not require erase. When such devices are used in system then MCUboot will avoid erasing such device, which is not needed by hardware, and will just write data to it. This allows to both improve device lifetime and reduce time of operations like swap. The second option allows to reduce amount of removed data. When enabled, MCUboot will remove enough of data, depending on the purpose of the removal, to just fulfill the purpose; for example if removal of data is done to make image unrecognizable for MCUboot, with this option, it will only remove header. Signed-off-by: Dominik Ermel --- boot/boot_serial/src/boot_serial.c | 4 +- boot/boot_serial/src/boot_serial_encryption.c | 2 +- boot/bootutil/src/bootutil_priv.h | 8 ++ boot/bootutil/src/loader.c | 105 ++++++++++++++++-- boot/bootutil/src/ram_load.c | 2 +- boot/bootutil/src/swap_misc.c | 2 +- 6 files changed, 109 insertions(+), 14 deletions(-) diff --git a/boot/boot_serial/src/boot_serial.c b/boot/boot_serial/src/boot_serial.c index ff1fa8a02..4573840db 100644 --- a/boot/boot_serial/src/boot_serial.c +++ b/boot/boot_serial/src/boot_serial.c @@ -759,7 +759,7 @@ static off_t erase_range(const struct flash_area *fap, off_t start, off_t end) BOOT_LOG_DBG("Erasing range 0x%jx:0x%jx", (intmax_t)start, (intmax_t)(start + size - 1)); - rc = flash_area_erase(fap, start, size); + rc = boot_erase_region(fap, start, size); if (rc != 0) { BOOT_LOG_ERR("Error %d while erasing range", rc); return -EINVAL; @@ -895,7 +895,7 @@ bs_upload(char *buf, int len) /* Non-progressive erase erases entire image slot when first chunk of * an image is received. */ - rc = flash_area_erase(fap, 0, area_size); + rc = boot_erase_region(fap, 0, area_size); if (rc) { goto out_invalid_data; } diff --git a/boot/boot_serial/src/boot_serial_encryption.c b/boot/boot_serial/src/boot_serial_encryption.c index c4bd7d87b..b75e4560a 100644 --- a/boot/boot_serial/src/boot_serial_encryption.c +++ b/boot/boot_serial/src/boot_serial_encryption.c @@ -175,7 +175,7 @@ decrypt_region_inplace(struct boot_loader_state *state, (off + bytes_copied + idx) - hdr->ih_hdr_size, blk_sz, blk_off, &buf[idx]); } - rc = flash_area_erase(fap, off + bytes_copied, chunk_sz); + rc = boot_erase_region(fap, off + bytes_copied, chunk_sz); if (rc != 0) { return BOOT_EFLASH; } diff --git a/boot/bootutil/src/bootutil_priv.h b/boot/bootutil/src/bootutil_priv.h index 345933a5f..7045f771a 100644 --- a/boot/bootutil/src/bootutil_priv.h +++ b/boot/bootutil/src/bootutil_priv.h @@ -302,7 +302,15 @@ int boot_copy_region(struct boot_loader_state *state, const struct flash_area *fap_src, const struct flash_area *fap_dst, uint32_t off_src, uint32_t off_dst, uint32_t sz); +/* Prepare for write device that requires erase prior to write. This will + * do nothing on devices without erase requirement. + */ int boot_erase_region(const struct flash_area *fap, uint32_t off, uint32_t sz); +/* SImilar to boot_erase_region but will alwasy remove data */ +int boot_scramble_region(const struct flash_area *fap, uint32_t off, uint32_t sz); +/* Makes slot unbootable, either by scrambling header magic, header sector + * or entire slot, depending on settings */ +int boot_scramble_slot(const struct flash_area *fap); bool boot_status_is_reset(const struct boot_status *bs); #ifdef MCUBOOT_ENC_IMAGES diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c index 1303394ad..ff27b60fe 100644 --- a/boot/bootutil/src/loader.c +++ b/boot/bootutil/src/loader.c @@ -1020,7 +1020,7 @@ boot_validate_slot(struct boot_loader_state *state, int slot, &boot_img_hdr(state, BOOT_PRIMARY_SLOT)->ih_ver); if (rc < 0 && boot_check_header_erased(state, BOOT_PRIMARY_SLOT)) { BOOT_LOG_ERR("insufficient version in secondary slot"); - flash_area_erase(fap, 0, flash_area_get_size(fap)); + boot_scramble_slot(fap); /* Image in the secondary slot does not satisfy version requirement. * Erase the image and continue booting from the primary slot. */ @@ -1040,7 +1040,7 @@ boot_validate_slot(struct boot_loader_state *state, int slot, } if (FIH_NOT_EQ(fih_rc, FIH_SUCCESS)) { if ((slot != BOOT_PRIMARY_SLOT) || ARE_SLOTS_EQUIVALENT()) { - flash_area_erase(fap, 0, flash_area_get_size(fap)); + boot_scramble_slot(fap); /* Image is invalid, erase it to prevent further unnecessary * attempts to validate and boot it. */ @@ -1081,7 +1081,7 @@ boot_validate_slot(struct boot_loader_state *state, int slot, * * Erase the image and continue booting from the primary slot. */ - flash_area_erase(fap, 0, fap->fa_size); + boot_scramble_slot(fap); fih_rc = FIH_NO_BOOTABLE_IMAGE; goto out; } @@ -1174,12 +1174,25 @@ boot_validated_swap_type(struct boot_loader_state *state, } #endif +/* Helper macro to avoid compile errors with systems that do not + * provide function to check device type. + * Note: it used to be inline, but somehow compiler would not + * optimize out branches that were impossible when this evaluated to + * just "true". + */ +#ifdef MCUBOOT_DEV_WITHOUT_ERASE_THEN_NO_ERASE +#define device_requires_erase(fa) (flash_area_erase_required(fa)) +#else +#define device_requires_erase(fa) (true) +#endif + /** - * Erases a region of flash. + * Erases a region of device that requires erase prior to write; does + * nothing on devices without erase. * - * @param flash_area The flash_area containing the region to erase. + * @param fap The flash_area containing the region to erase. * @param off The offset within the flash area to start the - * erase. + * erase. * @param sz The number of bytes to erase. * * @return 0 on success; nonzero on failure. @@ -1187,7 +1200,82 @@ boot_validated_swap_type(struct boot_loader_state *state, int boot_erase_region(const struct flash_area *fap, uint32_t off, uint32_t sz) { - return flash_area_erase(fap, off, sz); + if (device_requires_erase(fap)) { + return flash_area_erase(fap, off, sz); + } + return 0; +} + +/** + * Removes data from specified region either by writing 0x00 in place of + * data or by doing erase, if device has such hardware requirement. + * + * @param fa The flash_area containing the region to erase. + * @param off The offset within the flash area to start the + * erase. + * @param size The number of bytes to erase. + * + * @return 0 on success; nonzero on failure. + */ +int +boot_scramble_region(const struct flash_area *fa, uint32_t off, uint32_t size) +{ + int ret = 0; + + if (device_requires_erase(fa)) { + return flash_area_erase(fa, off, size); + } else { + uint8_t buf[BOOT_MAX_ALIGN]; + size_t size_done = 0; + + memset(buf, flash_area_erased_val(fa), sizeof(buf)); + + while (size_done < size) { + ret = flash_area_write(fa, size_done + off, buf, sizeof(buf)); + if (ret != 0) { + break; + } + size_done += sizeof(buf); + } + } + return ret; +} + +/** + * Remove enough data from slot to mark is as unused + * + * @param fa Pointer to flash area object for slot + * + * @return 0 on success; nonzero on failure. + */ +int +boot_scramble_slot(const struct flash_area *fa) +{ + size_t size; + + /* Whether device with erase or not, without minimal scramble + * removing deata in entire slot. + */ +#if !defined(MCUBOOT_MINIMAL_SCRAMBLE) + size = flash_area_get_size(fa); +#else + if (device_requires_erase(fa)) { + int ret = 0; + struct flash_sector header; + + ret = flash_area_get_sector(fa, 0, &header); + size = flash_sector_get_size(&header); + + if (ret != 0) { + return ret; + } + } else { + size = MAX(sizeof(((struct image_header *)0)->ih_magic), + BOOT_MAX_ALIGN); + size = (size + BOOT_MAX_ALIGN - 1) & ~(BOOT_MAX_ALIGN - 1); + } +#endif + return boot_scramble_region(fa, 0, size); } #if !defined(MCUBOOT_DIRECT_XIP) && !defined(MCUBOOT_RAM_LOAD) @@ -2112,8 +2200,7 @@ check_downgrade_prevention(struct boot_loader_state *state) if (rc < 0) { /* Image in slot 0 prevents downgrade, delete image in slot 1 */ BOOT_LOG_INF("Image %d in slot 1 erased due to downgrade prevention", BOOT_CURR_IMG(state)); - flash_area_erase(BOOT_IMG(state, 1).area, 0, - flash_area_get_size(BOOT_IMG(state, 1).area)); + boot_scramble_slot(BOOT_IMG(state, 1).area); } else { rc = 0; } diff --git a/boot/bootutil/src/ram_load.c b/boot/bootutil/src/ram_load.c index 72255367a..29c79cb60 100644 --- a/boot/bootutil/src/ram_load.c +++ b/boot/bootutil/src/ram_load.c @@ -432,7 +432,7 @@ boot_remove_image_from_flash(struct boot_loader_state *state, uint32_t slot) area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot); rc = flash_area_open(area_id, &fap); if (rc == 0) { - flash_area_erase(fap, 0, flash_area_get_size(fap)); + boot_scramble_slot(fap); flash_area_close(fap); } diff --git a/boot/bootutil/src/swap_misc.c b/boot/bootutil/src/swap_misc.c index 733a39744..ef1222909 100644 --- a/boot/bootutil/src/swap_misc.c +++ b/boot/bootutil/src/swap_misc.c @@ -69,7 +69,7 @@ swap_erase_trailer_sectors(const struct boot_loader_state *state, do { sz = boot_img_sector_size(state, slot, sector); off = boot_img_sector_off(state, slot, sector); - rc = boot_erase_region(fap, off, sz); + rc = boot_scramble_region(fap, off, sz); assert(rc == 0); sector--; From e1a441d7c03d0d4007de27513684e7d6599c9fef Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Tue, 12 Nov 2024 21:23:29 +0100 Subject: [PATCH 2/4] zephyr: Add support for devices without erase and reduced erases Add Kconfig options: - CONFIG_MCUBOOT_STORAGE_NO_UNNEEDED_ERASE that enables MCUboot configuration MCUBOOT_DEV_WITHOUT_ERASE_THEN_NO_ERASE - CONFIG_MCUBOOT_STORAGE_MINIMAL_SCRAMBLE that enables MCUboot configuration MCUBOOT_MINIMAL_SCRAMBLE Adds implementation of flash_area_erase_required, which is required when MCUBOOT_DEV_WITHOUT_ERASE_THEN_NO_ERASE is enabled. Signed-off-by: Dominik Ermel --- boot/zephyr/Kconfig | 30 +++++++++++++++++++ .../flash_map_backend/flash_map_backend.h | 18 +++++++++++ .../include/mcuboot_config/mcuboot_config.h | 19 ++++++++++++ 3 files changed, 67 insertions(+) diff --git a/boot/zephyr/Kconfig b/boot/zephyr/Kconfig index 7ff11ee90..1b1a823c3 100644 --- a/boot/zephyr/Kconfig +++ b/boot/zephyr/Kconfig @@ -832,6 +832,36 @@ endif # BOOT_DECOMPRESSION_SUPPORT endmenu +config MCUBOOT_STORAGE_NO_UNNEEDED_ERASE + bool "Erase is performed only on devices that require it by design" + depends on FLASH_HAS_NO_EXPLICIT_ERASE + help + Not all devices require erase before write and, depending on driver, + may emulate erase by write, as a way to scramble data rather then + by hardware requirement. This unfortunately means that code that + does erase before write, when not needed, will write device twice + which not only reduces device life time but also doubles time + of any write operation (one write for erase and one write for actual + write). + When this option is enabled, MCUboot will check for type of device + and will avoid erase where not needed. + +config MCUBOOT_STORAGE_MINIMAL_SCRAMBLE + bool "Do minimal required work to remove data" + help + In some cases MCUboot has to remove data, which usually means make + it non-viable for MCUboot rather then completely destroyed. + For example when MCUboot does not want to bother with broken image + in some slot it will remove it. + The same can be achieved with just removal of header, leaving the + rest of image untouched, as without header MCUboot will not be able + to recognize image in slot as bootable. + When this option is enabled, MCUboot will not attempt to erase + entire slot or image, instead it will just remove enough of + data from slot to not recognize it as image anymore. + Depending on type of device this may be done by erase of minimal + number of pages or overwrite of part of image. + config MCUBOOT_DEVICE_SETTINGS # Hidden selector for device-specific settings bool diff --git a/boot/zephyr/include/flash_map_backend/flash_map_backend.h b/boot/zephyr/include/flash_map_backend/flash_map_backend.h index e5408a18a..20347cc7e 100644 --- a/boot/zephyr/include/flash_map_backend/flash_map_backend.h +++ b/boot/zephyr/include/flash_map_backend/flash_map_backend.h @@ -106,6 +106,24 @@ static inline uint32_t flash_sector_get_size(const struct flash_sector *fs) int flash_area_get_sector(const struct flash_area *fa, off_t off, struct flash_sector *fs); + +#if defined(CONFIG_MCUBOOT) +static inline bool flash_area_erase_required(const struct flash_area *fa) +{ +#if defined(CONFIG_FLASH_HAS_EXPLICIT_ERASE) && defined(CONFIG_FLASH_HAS_NO_EXPLICIT_ERASE) + const struct flash_parameters *fp = flash_get_parameters(flash_area_get_device(fa)); + + return flash_params_get_erase_cap(flash_get_parameters(flash_area_get_device(fa))) & FLASH_ERASE_C_EXPLICIT; +#else +#if defined(CONFIG_FLASH_HAS_EXPLICIT_ERASE) + return true; +#else + return false; +#endif +#endif +} +#endif + #ifdef __cplusplus } #endif diff --git a/boot/zephyr/include/mcuboot_config/mcuboot_config.h b/boot/zephyr/include/mcuboot_config/mcuboot_config.h index 573155b39..79327a37a 100644 --- a/boot/zephyr/include/mcuboot_config/mcuboot_config.h +++ b/boot/zephyr/include/mcuboot_config/mcuboot_config.h @@ -280,6 +280,25 @@ #define MCUBOOT_ERASE_PROGRESSIVELY #endif +/* + * Devices that do not require erase should not call the function to + * avoid emulation of erase by additional write. + * The emulation is also taking time which doubles required write time + * for such devices. + */ +#ifdef CONFIG_MCUBOOT_STORAGE_NO_UNNEEDED_ERASE +#define MCUBOOT_DEV_WITHOUT_ERASE_THEN_NO_ERASE +#endif + +/* + * MCUboot often calls erase on device just to remove data or make application + * image not recognizable. In such instances it may be faster to just remove + * portion of data to make image unrecognizable. + */ +#ifdef CONFIG_MCUBOOT_STORAGE_MINIMAL_SCRAMBLE +#define MCUBOOT_MINIMAL_SCRAMBLE +#endif + /* * Enabling this option uses newer flash map APIs. This saves RAM and * avoids deprecated API usage. From 4edb7fc08ea8177b7614bd8ff10705b08a381bc7 Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Tue, 12 Nov 2024 21:40:20 +0100 Subject: [PATCH 3/4] zephyr: Use flash_area_flatten in bs_custom_storage_erase The intention of bs_custom_storage_erase is to remove data from device; to support devices that do not require erase, without calling erase, so that devices that do not implement such functions could work, the flash_area_erase has been replaced with flash_area_flatten. Signed-off-by: Dominik Ermel --- boot/zephyr/boot_serial_extension_zephyr_basic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/zephyr/boot_serial_extension_zephyr_basic.c b/boot/zephyr/boot_serial_extension_zephyr_basic.c index b0c75f4a7..e0f978adf 100644 --- a/boot/zephyr/boot_serial_extension_zephyr_basic.c +++ b/boot/zephyr/boot_serial_extension_zephyr_basic.c @@ -47,7 +47,7 @@ static int bs_custom_storage_erase(const struct nmgr_hdr *hdr, if (rc < 0) { BOOT_LOG_ERR("failed to open flash area"); } else { - rc = flash_area_erase(fa, 0, flash_area_get_size(fa)); + rc = flash_area_flatten(fa, 0, flash_area_get_size(fa)); if (rc < 0) { BOOT_LOG_ERR("failed to erase flash area"); } From b96fe4a6aa5e0bcf24cd11df436d7f5ceda802b6 Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Wed, 13 Nov 2024 20:50:50 +0100 Subject: [PATCH 4/4] zephyr: Use flash_area_flatten in boot_set_next Switch from using flash_area_erase to flash_area_flatten. The later function uses write on devices that do not have erase by design, to scramble data. Signed-off-by: Dominik Ermel --- boot/bootutil/src/bootutil_public.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/boot/bootutil/src/bootutil_public.c b/boot/bootutil/src/bootutil_public.c index afa601cac..f9c117225 100644 --- a/boot/bootutil/src/bootutil_public.c +++ b/boot/bootutil/src/bootutil_public.c @@ -548,7 +548,11 @@ boot_set_next(const struct flash_area *fa, bool active, bool confirm) /* The image slot is corrupt. There is no way to recover, so erase the * slot to allow future upgrades. */ +#if !defined(CONFIG_MCUBOOT) && defined(__ZEPHYR__) + flash_area_flatten(fa, 0, flash_area_get_size(fa)); +#else flash_area_erase(fa, 0, flash_area_get_size(fa)); +#endif rc = BOOT_EBADIMAGE; } break;