From 110853517a6197c71c5492134a81dd835fc882a5 Mon Sep 17 00:00:00 2001 From: Armando Date: Thu, 2 Mar 2023 16:10:02 +0800 Subject: [PATCH] spi_flash: support write verify feature on esp_flash_write API --- components/spi_flash/Kconfig | 1 + components/spi_flash/esp_flash_api.c | 133 +++++++++++++++--- components/spi_flash/include/esp_flash.h | 1 + .../test_apps/esp_flash/pytest_esp_flash.py | 1 + .../esp_flash/sdkconfig.ci.flash_qio | 2 - .../test_apps/esp_flash/sdkconfig.ci.release | 2 - .../test_apps/esp_flash/sdkconfig.ci.rom_impl | 2 - .../test_apps/esp_flash/sdkconfig.ci.verify | 4 +- 8 files changed, 119 insertions(+), 27 deletions(-) diff --git a/components/spi_flash/Kconfig b/components/spi_flash/Kconfig index f3f88ce8f5..0560ed937e 100644 --- a/components/spi_flash/Kconfig +++ b/components/spi_flash/Kconfig @@ -3,6 +3,7 @@ menu "SPI Flash driver" config SPI_FLASH_VERIFY_WRITE bool "Verify SPI flash writes" + depends on !SPI_FLASH_ROM_IMPL default n help If this option is enabled, any time SPI flash is written then the data will be read diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 09248e0d65..9bb8a2c990 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -23,7 +23,7 @@ #include "esp_crypto_lock.h" // for locking flash encryption peripheral #endif //CONFIG_IDF_TARGET_ESP32S2 -static const char TAG[] = "spi_flash"; +DRAM_ATTR static const char TAG[] = "spi_flash"; #ifdef CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE #define MAX_WRITE_CHUNK CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE /* write in chunks */ @@ -32,6 +32,7 @@ static const char TAG[] = "spi_flash"; #endif // CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE #define MAX_READ_CHUNK 16384 +#define VERIFY_BUF_LEN 64 #ifdef CONFIG_SPI_FLASH_DANGEROUS_WRITE_ABORTS @@ -847,10 +848,89 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add return err; } +#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE +static esp_err_t IRAM_ATTR s_check_setting_zero_to_one(esp_flash_t *chip, uint32_t verify_address, uint32_t remain_verify_len, const uint32_t *to_write_buf, bool is_encrypted) +{ + esp_err_t err = ESP_FAIL; + uint8_t verify_buffer[VERIFY_BUF_LEN]; + uint32_t *val_in_flash = (uint32_t *)verify_buffer; + + while (remain_verify_len) { + uint32_t this_len = MIN(remain_verify_len, VERIFY_BUF_LEN); + + err = chip->chip_drv->read(chip, verify_buffer, verify_address, this_len); + if (err != ESP_OK) { + ESP_DRAM_LOGE(TAG, "failed to read flash to verify if setting zero to one, err: 0x%x", err); + return err; + } + + for (int r = 0; r < this_len / sizeof(uint32_t); r++) { + if (is_encrypted) { + (void)to_write_buf; + if (val_in_flash[r] != 0xFFFFFFFF) { + ESP_DRAM_LOGW(TAG, "Write at offset 0x%x but not erased (0x%08x)", + verify_address + r, val_in_flash[r]); + } + } else { + if ((val_in_flash[r] & to_write_buf[r]) != to_write_buf[r]) { + ESP_DRAM_LOGW(TAG, "Write at offset 0x%x requests 0x%08x but will write 0x%08x -> 0x%08x", + verify_address + r, to_write_buf[r], val_in_flash[r], (val_in_flash[r] & to_write_buf[r])); + } + } + } + + remain_verify_len -= this_len; + verify_address += this_len; + } + + return ESP_OK; +} +#endif //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE + +#if CONFIG_SPI_FLASH_VERIFY_WRITE +static esp_err_t IRAM_ATTR s_verify_write(esp_flash_t *chip, uint32_t verify_address, uint32_t remain_verify_len, const uint32_t *expected_buf, bool is_encrypted) +{ + esp_err_t err = ESP_FAIL; + uint8_t verify_buffer[VERIFY_BUF_LEN]; + uint32_t *val_in_flash = (uint32_t *)verify_buffer; + + while (remain_verify_len) { + uint32_t this_len = MIN(remain_verify_len, VERIFY_BUF_LEN); + + if (is_encrypted) { + err = esp_flash_read_encrypted(chip, verify_address, verify_buffer, this_len); + } else { + err = chip->chip_drv->read(chip, verify_buffer, verify_address, this_len); + } + if (err != ESP_OK) { + ESP_DRAM_LOGE(TAG, "failed to read flash to verify previous write, err: 0x%x", err); + return err; + } + + for (int r = 0; r < this_len / sizeof(uint32_t); r++) { + if (val_in_flash[r] != expected_buf[r]) { +#if CONFIG_SPI_FLASH_LOG_FAILED_WRITE + ESP_DRAM_LOGE(TAG, "Bad write at %d offset: 0x%x, expected: 0x%08x, readback: 0x%08x", r, verify_address + r, expected_buf[r], val_in_flash[r]); +#endif //#if CONFIG_SPI_FLASH_LOG_FAILED_WRITE + return ESP_FAIL; + } + } + + expected_buf = (uint32_t *)((void *)expected_buf + this_len); + remain_verify_len -= this_len; + verify_address += this_len; + } + + return ESP_OK; +} +#endif //#if CONFIG_SPI_FLASH_VERIFY_WRITE + esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length) { + esp_err_t ret = ESP_FAIL; #if CONFIG_SPI_FLASH_VERIFY_WRITE - const uint32_t *except_buf = buffer; + //used for verify write + bool is_encrypted = false; #endif //CONFIG_SPI_FLASH_VERIFY_WRITE esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip); @@ -902,25 +982,45 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 err = rom_spiflash_api_funcs->start(chip); if (err != ESP_OK) { - break; + goto restore_cache; } bus_acquired = true; +#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE + err = s_check_setting_zero_to_one(chip, write_addr, write_len, write_buf, is_encrypted); + if (err != ESP_OK) { + //Error happens, we end flash operation. Re-enable cache and flush it + goto restore_cache; + } +#endif //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE + err = chip->chip_drv->write(chip, write_buf, write_addr, write_len); len_remain -= write_len; assert(len_remain < length); - if (err != ESP_OK || len_remain == 0) { - // On ESP32, the cache re-enable is in the end() function, while flush_cache should - // happen when the cache is still disabled on ESP32. Break before the end() function and - // do end() later + if (err != ESP_OK) { + //Error happens, we end flash operation. Re-enable cache and flush it assert(bus_acquired); + goto restore_cache; + } + +#if CONFIG_SPI_FLASH_VERIFY_WRITE + err = s_verify_write(chip, write_addr, write_len, write_buf, is_encrypted); + if (err != ESP_OK) { + //Error happens, we end flash operation. Re-enable cache and flush it + goto restore_cache; + } +#endif //#if CONFIG_SPI_FLASH_VERIFY_WRITE + + + if (len_remain == 0) { + //Flash operation done break; } err = rom_spiflash_api_funcs->end(chip, err); if (err != ESP_OK) { - break; + goto restore_cache; } bus_acquired = false; @@ -930,20 +1030,15 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 err = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length); -#if CONFIG_SPI_FLASH_VERIFY_WRITE - uint32_t *actual_buf = malloc(length);; - esp_flash_read(chip, actual_buf, address, length); + return err; - for (int r = 0; r < length / sizeof(uint32_t); r++) { - if (actual_buf[r] != except_buf[r]) { - ESP_LOGE(TAG, "Bad write at %d offset: 0x%x, expected: 0x%08x, readback: 0x%08x",r, address + r, except_buf[r], actual_buf[r]); - err = ESP_FAIL; - } +restore_cache: + + ret = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length); + if (ret != ESP_OK) { + ESP_DRAM_LOGE(TAG, "restore cache fail\n"); } - free(actual_buf); -#endif //CONFIG_SPI_FLASH_VERIFY_WRITE - return err; } diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index 634a6c4783..8a893e17cd 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -317,6 +317,7 @@ esp_err_t esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint * * @return * - ESP_OK on success, + * - ESP_FAIL, bad write, this will be detected only when CONFIG_SPI_FLASH_VERIFY_WRITE is enabled * - ESP_ERR_NOT_SUPPORTED if the chip is not able to perform the operation. This is indicated by WREN = 1 after the command is sent. * - Other flash error code if operation failed. */ diff --git a/components/spi_flash/test_apps/esp_flash/pytest_esp_flash.py b/components/spi_flash/test_apps/esp_flash/pytest_esp_flash.py index 5b3836f2af..179b424b56 100644 --- a/components/spi_flash/test_apps/esp_flash/pytest_esp_flash.py +++ b/components/spi_flash/test_apps/esp_flash/pytest_esp_flash.py @@ -16,6 +16,7 @@ from pytest_embedded import Dut [ 'release', 'flash_qio', + 'verify' ], indirect=True, ) diff --git a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.flash_qio b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.flash_qio index 45ef9b6097..9e24ff5cf4 100644 --- a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.flash_qio +++ b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.flash_qio @@ -1,4 +1,2 @@ CONFIG_ESP_TASK_WDT_EN=n -CONFIG_PARTITION_TABLE_CUSTOM=y -CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv" CONFIG_ESPTOOLPY_FLASHMODE_QIO=y diff --git a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.release b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.release index ceab55e4bb..7eb62a2852 100644 --- a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.release +++ b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.release @@ -3,5 +3,3 @@ CONFIG_FREERTOS_USE_TICKLESS_IDLE=y CONFIG_COMPILER_OPTIMIZATION_SIZE=y CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE=y CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y -CONFIG_PARTITION_TABLE_CUSTOM=y -CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv" diff --git a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.rom_impl b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.rom_impl index 1ad7ae28f9..d878b890f7 100644 --- a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.rom_impl +++ b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.rom_impl @@ -1,4 +1,2 @@ CONFIG_ESP_TASK_WDT_EN=n -CONFIG_PARTITION_TABLE_CUSTOM=y -CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv" CONFIG_SPI_FLASH_ROM_IMPL=y diff --git a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.verify b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.verify index f7ae153c3f..61d3f59eff 100644 --- a/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.verify +++ b/components/spi_flash/test_apps/esp_flash/sdkconfig.ci.verify @@ -1,4 +1,4 @@ CONFIG_ESP_TASK_WDT=n -CONFIG_PARTITION_TABLE_CUSTOM=y -CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv" CONFIG_SPI_FLASH_VERIFY_WRITE=y +CONFIG_SPI_FLASH_LOG_FAILED_WRITE=y +CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE=y