Merge branch 'bugfix/flash_enc_write_ram_v5.1' into 'release/v5.1'

fix(spi_flash): Fix that internal RAM has no enough space to put all stuff inside (backport v5.1)

See merge request espressif/esp-idf!29657

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
 
@@ -1218,7 +1233,7 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
     }
 
     if ((address % 16) != 0) {
-        ESP_EARLY_LOGE(TAG, "flash encrypted write address must be 16 bytes aligned");
+        ESP_DRAM_LOGE(TAG, "flash encrypted write address must be 16 bytes aligned");
         return ESP_ERR_INVALID_ARG;
     }
 
@@ -1227,14 +1242,22 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
     }
 
     if ((length % 16) != 0) {
-        ESP_EARLY_LOGE(TAG, "flash encrypted write length must be multiple of 16");
+        ESP_DRAM_LOGE(TAG, "flash encrypted write length must be multiple of 16");
         return ESP_ERR_INVALID_SIZE;
     }
 
     bool bus_acquired = false;
 
+    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();
 
     /* On ESP32, write_encrypted encrypts data in RAM as it writes,
@@ -1252,6 +1275,27 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
     */
     uint8_t encrypt_buf[64] __attribute__((aligned(4)));
     uint32_t row_size_length;
+#if CONFIG_IDF_TARGET_ESP32
+    uint8_t pre_buf[16] = {0};
+    uint8_t post_buf[16] = {0};
+
+    if((address % 32) != 0) {
+        esp_flash_read_encrypted(chip, address - 16, pre_buf, 16);
+    }
+    if(((address + length) % 32) != 0) {
+        esp_flash_read_encrypted(chip, address + length, post_buf, 16);
+    }
+#endif
+
+    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;
+    }
+
     for (size_t i = 0; i < length; i += row_size_length) {
         uint32_t row_addr = address + i;
         uint8_t row_size;
@@ -1264,14 +1308,14 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
             /* copy to second block in buffer */
             memcpy(encrypt_buf + 16, ssrc + i, row_size);
             /* decrypt the first block from flash, will reencrypt to same bytes */
-            esp_flash_read_encrypted(chip, row_addr, encrypt_buf, 16);
+            memcpy(encrypt_buf, pre_buf, 16);
         } else if (length - i == 16) {
             /* 16 bytes left, is first block of a 32 byte row */
             row_size = 16;
             /* copy to first block in buffer */
             memcpy(encrypt_buf, ssrc + i, row_size);
             /* decrypt the second block from flash, will reencrypt to same bytes */
-            esp_flash_read_encrypted(chip, row_addr + 16, encrypt_buf + 16, 16);
+            memcpy(encrypt_buf + 16, post_buf, 16);
         } else {
             /* Writing a full 32 byte row (2 blocks) */
             row_size = 32;
@@ -1297,46 +1341,39 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
 #if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
         err = s_check_setting_zero_to_one(chip, row_addr, encrypt_byte, NULL, 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  //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
 
-#if CONFIG_IDF_TARGET_ESP32S2
+        if (lock_once == false) {
-        esp_crypto_dma_lock_acquire();
+            err = s_encryption_write_lock(chip);
-#endif //CONFIG_IDF_TARGET_ESP32S2
+            if (err != ESP_OK) {
-        err = rom_spiflash_api_funcs->start(chip);
+                goto restore_cache;
-
+            }
-        if (err != ESP_OK) {
+            bus_acquired = true;
-#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;
         }
-        bus_acquired = true;
 
         err = chip->chip_drv->write_encrypted(chip, (uint32_t *)encrypt_buf, row_addr, encrypt_byte);
         if (err!= 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;
         }
-        err = rom_spiflash_api_funcs->end(chip, ESP_OK);
+        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_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;
-        }
-        bus_acquired = false;
 
 #if CONFIG_SPI_FLASH_VERIFY_WRITE
         err = s_verify_write(chip, row_addr, encrypt_byte, (uint32_t *)encrypt_buf, is_encrypted);
@@ -1347,12 +1384,25 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
 #endif //CONFIG_SPI_FLASH_VERIFY_WRITE
     }
 
+    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;
+
     COUNTER_STOP(write);
     err = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length);
 
     return err;
 
 restore_cache:
+    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) {

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)

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")

components/spi_flash/test_apps/flash_encryption/main/idf_component.yml

@@ -0,0 +1,2 @@
+dependencies:
+  ccomp_timer: "^1.0.0"

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);
 }

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