From 723519cf67194359d1e71b01f660fe0c5c638297 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Tue, 28 Nov 2023 14:51:54 +0800 Subject: [PATCH] fix(spi_flash): Fix that internal RAM has no enough space to put all stuff inside --- components/spi_flash/esp_flash_api.c | 99 +++++++++++-------- .../test_apps/flash_encryption/CMakeLists.txt | 5 + .../flash_encryption/main/CMakeLists.txt | 1 + .../flash_encryption/main/idf_component.yml | 2 + .../flash_encryption/main/test_app_main.c | 10 +- .../main/test_flash_encryption.c | 59 ++++++++--- 6 files changed, 121 insertions(+), 55 deletions(-) create mode 100644 components/spi_flash/test_apps/flash_encryption/main/idf_component.yml diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 4c9702b49f..ac2fdf8f34 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -1194,6 +1194,21 @@ IRAM_ATTR esp_err_t esp_flash_set_io_mode(esp_flash_t* chip, bool qe) } #endif //CONFIG_SPI_FLASH_ROM_IMPL +FORCE_INLINE_ATTR esp_err_t s_encryption_write_lock(esp_flash_t *chip) { +#if CONFIG_IDF_TARGET_ESP32S2 + esp_crypto_dma_lock_acquire(); +#endif //CONFIG_IDF_TARGET_ESP32S2 + return rom_spiflash_api_funcs->start(chip); +} + +FORCE_INLINE_ATTR esp_err_t s_encryption_write_unlock(esp_flash_t *chip) { +#if CONFIG_IDF_TARGET_ESP32S2 + esp_crypto_dma_lock_release(); +#endif //CONFIG_IDF_TARGET_ESP32S2 + return rom_spiflash_api_funcs->end(chip, ESP_OK); +} + + #if !CONFIG_SPI_FLASH_ROM_IMPL || ESP_ROM_HAS_ENCRYPTED_WRITES_USING_LEGACY_DRV // use `esp_flash_write_encrypted` ROM version not in C3 and S3 @@ -1233,13 +1248,15 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres bool bus_acquired = false; - // Copy buffer to IRAM. - uint8_t *ssrc = (uint8_t*)heap_caps_calloc(1, length, MALLOC_CAP_INTERNAL); - if (ssrc == NULL) { - ESP_DRAM_LOGE(TAG, "No extra memory for encryption flash write"); - return ESP_ERR_NO_MEM; - } - memcpy(ssrc, buffer, length); + bool lock_once = true; + const uint8_t *ssrc = (const uint8_t *)buffer; + + /* For buffer in internal RAM already, we only need to lock only once. + While for buffer in flash, we need to copy data from flash to internal RAM before + encrypted write every time. That means we need to lock/unlock before/after encrypted + write every time. + */ + lock_once = esp_ptr_in_dram(buffer); COUNTER_START(); @@ -1270,15 +1287,15 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres } #endif -#if CONFIG_IDF_TARGET_ESP32S2 - esp_crypto_dma_lock_acquire(); -#endif //CONFIG_IDF_TARGET_ESP32S2 - - err = rom_spiflash_api_funcs->start(chip); - if (err != ESP_OK) { - goto restore_cache; + if (lock_once == true) { + err = s_encryption_write_lock(chip); + if (err != ESP_OK) { + ESP_DRAM_LOGE(TAG, "flash acquire lock failed"); + return err; + } + bus_acquired = true; } - bus_acquired = true; + for (size_t i = 0; i < length; i += row_size_length) { uint32_t row_addr = address + i; uint8_t row_size; @@ -1333,47 +1350,50 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres } #endif //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE + if (lock_once == false) { + err = s_encryption_write_lock(chip); + if (err != ESP_OK) { + goto restore_cache; + } + bus_acquired = true; + } + err = chip->chip_drv->write_encrypted(chip, (uint32_t *)encrypt_buf, row_addr, encrypt_byte); if (err!= ESP_OK) { - rom_spiflash_api_funcs->end(chip, ESP_OK); -#if CONFIG_IDF_TARGET_ESP32S2 - esp_crypto_dma_lock_release(); -#endif //CONFIG_IDF_TARGET_ESP32S2 - bus_acquired = false; - assert(bus_acquired); //Error happens, we end flash operation. Re-enable cache and flush it goto restore_cache; } + if (lock_once == false) { + err = s_encryption_write_unlock(chip); + if (err != ESP_OK) { + bus_acquired = false; + //Error happens, we end flash operation. Re-enable cache and flush it + goto restore_cache; + } + bus_acquired = false; + } COUNTER_ADD_BYTES(write, encrypt_byte); #if CONFIG_SPI_FLASH_VERIFY_WRITE err = s_verify_write(chip, row_addr, encrypt_byte, (uint32_t *)encrypt_buf, is_encrypted); if (err != ESP_OK) { - rom_spiflash_api_funcs->end(chip, ESP_OK); -#if CONFIG_IDF_TARGET_ESP32S2 - esp_crypto_dma_lock_release(); -#endif //CONFIG_IDF_TARGET_ESP32S2 //Error happens, we end flash operation. Re-enable cache and flush it goto restore_cache; } #endif //CONFIG_SPI_FLASH_VERIFY_WRITE } - err = rom_spiflash_api_funcs->end(chip, ESP_OK); -#if CONFIG_IDF_TARGET_ESP32S2 - esp_crypto_dma_lock_release(); -#endif //CONFIG_IDF_TARGET_ESP32S2 - if (err != ESP_OK) { - bus_acquired = false; - //Error happens, we end flash operation. Re-enable cache and flush it - goto restore_cache; + if (lock_once == true) { + err = s_encryption_write_unlock(chip); + if (err != ESP_OK) { + bus_acquired = false; + //Error happens, we end flash operation. Re-enable cache and flush it + goto restore_cache; + } } - bus_acquired = false; - if(ssrc) { - free(ssrc); - } + bus_acquired = false; COUNTER_STOP(write); err = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length); @@ -1381,9 +1401,8 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres return err; restore_cache: - if(ssrc) { - free(ssrc); - } + s_encryption_write_unlock(chip); + bus_acquired = false; COUNTER_STOP(write); ret = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length); if (ret != ESP_OK) { diff --git a/components/spi_flash/test_apps/flash_encryption/CMakeLists.txt b/components/spi_flash/test_apps/flash_encryption/CMakeLists.txt index 6f1f017893..debb41982c 100644 --- a/components/spi_flash/test_apps/flash_encryption/CMakeLists.txt +++ b/components/spi_flash/test_apps/flash_encryption/CMakeLists.txt @@ -2,4 +2,9 @@ cmake_minimum_required(VERSION 3.16) include($ENV{IDF_PATH}/tools/cmake/project.cmake) +set(EXTRA_COMPONENT_DIRS + "$ENV{IDF_PATH}/tools/unit-test-app/components" + "$ENV{IDF_PATH}/components/spi_flash/test_apps/components" +) + project(test_flash_encryption) diff --git a/components/spi_flash/test_apps/flash_encryption/main/CMakeLists.txt b/components/spi_flash/test_apps/flash_encryption/main/CMakeLists.txt index df989b7274..54646b5d4b 100644 --- a/components/spi_flash/test_apps/flash_encryption/main/CMakeLists.txt +++ b/components/spi_flash/test_apps/flash_encryption/main/CMakeLists.txt @@ -2,5 +2,6 @@ set(srcs "test_app_main.c" "test_flash_encryption.c") idf_component_register(SRCS ${srcs} + PRIV_REQUIRES unity spi_flash bootloader_support esp_partition test_utils test_flash_utils WHOLE_ARCHIVE) target_compile_options(${COMPONENT_LIB} PRIVATE "-Wno-format") diff --git a/components/spi_flash/test_apps/flash_encryption/main/idf_component.yml b/components/spi_flash/test_apps/flash_encryption/main/idf_component.yml new file mode 100644 index 0000000000..2ae836a935 --- /dev/null +++ b/components/spi_flash/test_apps/flash_encryption/main/idf_component.yml @@ -0,0 +1,2 @@ +dependencies: + ccomp_timer: "^1.0.0" diff --git a/components/spi_flash/test_apps/flash_encryption/main/test_app_main.c b/components/spi_flash/test_apps/flash_encryption/main/test_app_main.c index c8d6ded5bb..e3bcad0ebd 100644 --- a/components/spi_flash/test_apps/flash_encryption/main/test_app_main.c +++ b/components/spi_flash/test_apps/flash_encryption/main/test_app_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,6 +7,8 @@ #include "unity.h" #include "unity_test_runner.h" #include "esp_heap_caps.h" +#include "esp_partition.h" +#include "memory_checks.h" // Some resources are lazy allocated in flash encryption, the threadhold is left for that case #define TEST_MEMORY_LEAK_THRESHOLD (-600) @@ -23,6 +25,12 @@ static void check_leak(size_t before_free, size_t after_free, const char *type) void setUp(void) { + // Calling esp_partition_find_first ensures that the paritions have been loaded + // and subsequent calls to esp_partition_find_first from the tests would not + // load partitions which otherwise gets considered as a memory leak. + esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL); + + test_utils_record_free_mem(); before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT); before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT); } diff --git a/components/spi_flash/test_apps/flash_encryption/main/test_flash_encryption.c b/components/spi_flash/test_apps/flash_encryption/main/test_flash_encryption.c index 52cb317144..f285f5ddb2 100644 --- a/components/spi_flash/test_apps/flash_encryption/main/test_flash_encryption.c +++ b/components/spi_flash/test_apps/flash_encryption/main/test_flash_encryption.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -15,6 +15,10 @@ #include "esp_log.h" #include "esp_partition.h" #include "esp_heap_caps.h" +#include "esp_cpu.h" +#include "test_utils.h" +#include "ccomp_timer.h" +#include "test_flash_utils.h" /*-------------------- For running this test, some configurations are necessary -------------------*/ /* ESP32 | CONFIG_SECURE_FLASH_ENC_ENABLED | SET */ @@ -32,15 +36,6 @@ static void verify_erased_flash(size_t offset, size_t length); static size_t start; -const esp_partition_t *get_test_data_partition(void) -{ - /* This finds "flash_test" partition defined in partition_table_unit_test_app.csv */ - const esp_partition_t *result = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, - ESP_PARTITION_SUBTYPE_ANY, "flash_test"); - TEST_ASSERT_NOT_NULL(result); /* means partition table set wrong */ - return result; -} - static void setup_tests(void) { const esp_partition_t *part = get_test_data_partition(); @@ -260,7 +255,7 @@ TEST_CASE("test read & write encrypted data(32 bytes alianed address)", "[flash_ start = (start + 31) & (~31); // round up to 32 byte boundary ESP_LOG_BUFFER_HEXDUMP(TAG, plainttext_data, sizeof(plainttext_data), ESP_LOG_INFO); - printf("Encrypteed writting......\n"); + printf("Encrypted writing......\n"); TEST_ESP_OK(esp_flash_write_encrypted(NULL, start, plainttext_data, sizeof(plainttext_data))); uint8_t *cmp_encrypt_buf = heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_32BIT | MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); @@ -292,7 +287,7 @@ TEST_CASE("test read & write encrypted data(16 bytes alianed but 32 bytes unalig printf("Write data partition @ 0x%x\n", start); ESP_LOG_BUFFER_HEXDUMP(TAG, plainttext_data, sizeof(plainttext_data), ESP_LOG_INFO); - printf("Encrypteed writting......\n"); + printf("Encrypted writing......\n"); TEST_ESP_OK(esp_flash_write_encrypted(NULL, start, plainttext_data, sizeof(plainttext_data))); uint8_t *cmp_encrypt_buf = heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_32BIT | MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); @@ -329,14 +324,50 @@ TEST_CASE("test read & write encrypted data with large buffer(n*64+32+16)", "[fl // The tested buffer should be n*64(or n*32)+16 bytes. setup_tests(); TEST_ESP_OK(esp_flash_erase_region(NULL, start, 5 * 4096)); - printf("Encrypteed writting......\n"); + printf("Encrypted writing......\n"); + TEST_ESP_OK(ccomp_timer_start()); TEST_ESP_OK(esp_flash_write_encrypted(NULL, start, large_const_buffer, sizeof(large_const_buffer))); - uint8_t *buf = (uint8_t*)heap_caps_malloc(sizeof(large_const_buffer), MALLOC_CAP_32BIT | MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); + int64_t write_time = ccomp_timer_stop(); + IDF_LOG_PERFORMANCE(TAG, "Writing speed: %.2f us/KB", (double)(write_time/sizeof(large_const_buffer))*1024); + + uint8_t *buf = (uint8_t*)heap_caps_malloc(sizeof(large_const_buffer), MALLOC_CAP_8BIT); TEST_ESP_OK(esp_flash_read_encrypted(NULL, start, buf, sizeof(large_const_buffer))); TEST_ASSERT_EQUAL_HEX8_ARRAY(buf, large_const_buffer, sizeof(large_const_buffer)); free(buf); } +static DRAM_ATTR const uint8_t large_const_buffer_dram[16432] = { + 203, // first byte + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, + 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, + [50 ... 99] = 2, + [108 ... 1520] = 0x9b, + [1600 ... 2000] = 0x3d, + [8000 ... 9000] = 0xf7, + [15000 ... 16398] = 0xe8, + 43, 0x7f, + [16401 ... 16430] = 0xd1, + 202, // last byte +}; + +TEST_CASE("test read & write encrypted data with large buffer in ram", "[flash_encryption]") +{ + // The tested buffer should be n*64(or n*32)+16 bytes. + setup_tests(); + TEST_ESP_OK(esp_flash_erase_region(NULL, start, 5 * 4096)); + printf("Encrypted writing......\n"); + + TEST_ESP_OK(ccomp_timer_start()); + TEST_ESP_OK(esp_flash_write_encrypted(NULL, start, large_const_buffer_dram, sizeof(large_const_buffer_dram))); + int64_t write_time = ccomp_timer_stop(); + IDF_LOG_PERFORMANCE(TAG, "Writing speed: %.2f us/KB", (double)(write_time/sizeof(large_const_buffer_dram))*1024); + uint8_t *buf = (uint8_t*)heap_caps_malloc(sizeof(large_const_buffer_dram), MALLOC_CAP_32BIT | MALLOC_CAP_8BIT); + + TEST_ESP_OK(esp_flash_read_encrypted(NULL, start, buf, sizeof(large_const_buffer_dram))); + TEST_ASSERT_EQUAL_HEX8_ARRAY(buf, large_const_buffer_dram, sizeof(large_const_buffer_dram)); + free(buf); +} + #endif // CONFIG_SECURE_FLASH_ENC_ENABLED