Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'bugfix/spiflash_read_psram_v4.1' into 'release/v4.1'

Browse source 
spi_flash: Fix over-allocation and OOM crash when reading from SPI flash to PSRAM buffers (v4.1)

See merge request espressif/esp-idf!7877
This commit is contained in:
Michael (XIAO Xufeng) 2020-03-09 11:13:29 +08:00
parent c99eac2f0b 425486223e
commit a559d55379
5 changed files with 185 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
components/spi_flash/esp_flash_api.c
View file

@ -497,12 +497,28 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
bool direct_read = chip->host->supports_direct_read(chip->host, buffer);
uint8_t* temp_buffer = NULL;
//each time, we at most read this length
//after that, we release the lock to allow some other operations
size_t read_chunk_size = MIN(MAX_READ_CHUNK, length);
if (!direct_read) {
uint32_t length_to_allocate = MAX(MAX_READ_CHUNK, length);
length_to_allocate = (length_to_allocate+3)&(~3);
temp_buffer = heap_caps_malloc(length_to_allocate, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
ESP_LOGV(TAG, "allocate temp buffer: %p", temp_buffer);
if (temp_buffer == NULL) return ESP_ERR_NO_MEM;
/* Allocate temporary internal buffer to use for the actual read. If the preferred size
doesn't fit in free internal memory, allocate the largest available free block.
(May need to shrink read_chunk_size and retry due to race conditions with other tasks
also allocating from the heap.)
*/
unsigned retries = 5;
while(temp_buffer == NULL && retries--) {
read_chunk_size = MIN(read_chunk_size, heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT));
read_chunk_size = (read_chunk_size + 3) & ~3;
temp_buffer = heap_caps_malloc(read_chunk_size, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
}
ESP_LOGV(TAG, "allocate temp buffer: %p (%d)", temp_buffer, read_chunk_size);
if (temp_buffer == NULL) {
return ESP_ERR_NO_MEM;
}
}
esp_err_t err = ESP_OK;
@ -514,9 +530,9 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
}
//if required (dma buffer allocated), read to the buffer instead of the original buffer
uint8_t* buffer_to_read = (temp_buffer)? temp_buffer : buffer;
//each time, we at most read this length
//after that, we release the lock to allow some other operations
uint32_t length_to_read = MIN(MAX_READ_CHUNK, length);
// Length we will read this iteration is either the chunk size or the remaining length, whichever is smaller
size_t length_to_read = MIN(read_chunk_size, length);
if (err == ESP_OK) {
err = chip->chip_drv->read(chip, buffer_to_read, address, length_to_read);
@ -682,27 +698,32 @@ esp_err_t esp_flash_app_disable_protect(bool disable)
#ifndef CONFIG_SPI_FLASH_USE_LEGACY_IMPL
/* Translate any ESP_ERR_FLASH_xxx error code (new API) to a generic ESP_ERR_xyz error code
*/
static IRAM_ATTR esp_err_t spi_flash_translate_rc(esp_err_t err)
{
switch (err) {
case ESP_OK:
return ESP_OK;
case ESP_ERR_INVALID_ARG:
return ESP_ERR_INVALID_ARG;
case ESP_ERR_NO_MEM:
return err;
case ESP_ERR_FLASH_NOT_INITIALISED:
case ESP_ERR_FLASH_PROTECTED:
return ESP_ERR_INVALID_STATE;
case ESP_ERR_NOT_FOUND:
case ESP_ERR_FLASH_UNSUPPORTED_HOST:
case ESP_ERR_FLASH_UNSUPPORTED_CHIP:
return ESP_ERR_NOT_SUPPORTED;
case ESP_ERR_FLASH_NO_RESPONSE:
return ESP_ERR_INVALID_RESPONSE;
default:
ESP_EARLY_LOGE(TAG, "unexpected spi flash error code: %x", err);
ESP_EARLY_LOGE(TAG, "unexpected spi flash error code: 0x%x", err);
abort();
}
return ESP_OK;
}
esp_err_t IRAM_ATTR spi_flash_erase_range(uint32_t start_addr, uint32_t size)

3
components/spi_flash/include/esp_flash.h
View file

@ -230,8 +230,7 @@ esp_err_t esp_flash_set_protected_region(esp_flash_t *chip, const esp_flash_regi
*
* @return
* - ESP_OK: success
* - ESP_ERR_NO_MEM: the buffer is not valid, however failed to malloc on
* the heap.
* - ESP_ERR_NO_MEM: Buffer is in external PSRAM which cannot be concurrently accessed, and a temporary internal buffer could not be allocated.
* - or a flash error code if operation failed.
*/
esp_err_t esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint32_t length);

91
components/spi_flash/test/test_esp_flash.c
View file

@ -68,17 +68,25 @@ static uint8_t sector_buf[4096];
#define VSPI_PIN_NUM_CS FSPI_PIN_NUM_CS
#endif
#define ALL_TEST_NUM (sizeof(config_list)/sizeof(flashtest_config_t))
#define TEST_CONFIG_NUM (sizeof(config_list)/sizeof(flashtest_config_t))
typedef void (*flash_test_func_t)(esp_flash_t* chip);
/* Use FLASH_TEST_CASE for SPI flash tests that only use the main SPI flash chip
*/
#define FLASH_TEST_CASE(STR, FUNC_TO_RUN) \
TEST_CASE(STR, "[esp_flash]") {flash_test_func(FUNC_TO_RUN, 1);}
TEST_CASE(STR, "[esp_flash]") {flash_test_func(FUNC_TO_RUN, 1 /* first index reserved for main flash */ );}
/* Use FLASH_TEST_CASE_3 for tests which also run on external flash, which sits in the place of PSRAM
(these tests are incompatible with PSRAM)
These tests run for all the flash chip configs shown in config_list, below (internal and external).
*/
#if defined(CONFIG_SPIRAM_SUPPORT) || TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2BETA)
// These tests needs external flash, right on the place of psram
#define FLASH_TEST_CASE_3(STR, FUNCT_TO_RUN)
#else
#define FLASH_TEST_CASE_3(STR, FUNC_TO_RUN) \
TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") {flash_test_func(FUNC_TO_RUN, ALL_TEST_NUM);}
TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);}
#endif
//currently all the configs are the same with esp_flash_spi_device_config_t, no more information required
@ -271,12 +279,14 @@ void teardown_test_chip(esp_flash_t* chip, spi_host_device_t host)
static void flash_test_func(flash_test_func_t func, int test_num)
{
for (int i = 0; i < test_num; i++) {
ESP_LOGI(TAG, "Testing config %d/%d", i, test_num);
flashtest_config_t* config = &config_list[i];
esp_flash_t* chip;
setup_new_chip(config, &chip);
(*func)(chip);
teardown_test_chip(chip, config->host_id);
}
ESP_LOGI(TAG, "Completed %d configs", test_num);
}
/* ---------- Test code start ------------*/
@ -615,7 +625,7 @@ TEST_CASE("SPI flash test reading with all speed/mode permutations", "[esp_flash
#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2BETA)
TEST_CASE("SPI flash test reading with all speed/mode permutations, 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]")
{
for (int i = 0; i < ALL_TEST_NUM; i++) {
for (int i = 0; i < TEST_CONFIG_NUM; i++) {
test_permutations(&config_list[i]);
}
}
@ -685,4 +695,73 @@ static void test_write_large_buffer(esp_flash_t *chip, const uint8_t *source, si
write_large_buffer(chip, part, source, length);
read_and_check(chip, part, source, length);
}
}
#ifdef CONFIG_SPIRAM_USE_MALLOC
/* Utility: Read into a small internal RAM buffer using esp_flash_read() and compare what
we read with 'buffer' */
static void s_test_compare_flash_contents_small_reads(esp_flash_t *chip, const uint8_t *buffer, size_t offs, size_t len)
{
const size_t INTERNAL_BUF_SZ = 1024; // Should fit in internal RAM
uint8_t *ibuf = heap_caps_malloc(INTERNAL_BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_INTERNAL);
TEST_ASSERT_NOT_NULL(ibuf);
for (int i = 0; i < len; i += INTERNAL_BUF_SZ) {
size_t to_read = MIN(INTERNAL_BUF_SZ, len - i);
ESP_ERROR_CHECK( esp_flash_read(chip, ibuf, offs + i, to_read) );
TEST_ASSERT_EQUAL_HEX8_ARRAY(buffer + i, ibuf, to_read);
}
free(ibuf);
}
static void test_flash_read_large_psram_buffer(esp_flash_t *chip)
{
const size_t BUF_SZ = 256 * 1024; // Too large for internal RAM
const size_t TEST_OFFS = 0x1000; // Can be any offset, really
uint8_t *buf = heap_caps_malloc(BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_SPIRAM);
TEST_ASSERT_NOT_NULL(buf);
ESP_ERROR_CHECK( esp_flash_read(chip, buf, TEST_OFFS, BUF_SZ) );
// Read back the same into smaller internal memory buffer and check it all matches
s_test_compare_flash_contents_small_reads(chip, buf, TEST_OFFS, BUF_SZ);
free(buf);
}
FLASH_TEST_CASE("esp_flash_read large PSRAM buffer", test_flash_read_large_psram_buffer);
/* similar to above test, but perform it under memory pressure */
static void test_flash_read_large_psram_buffer_low_internal_mem(esp_flash_t *chip)
{
const size_t BUF_SZ = 256 * 1024; // Too large for internal RAM
const size_t REMAINING_INTERNAL = 1024; // Exhaust internal memory until maximum free block is less than this
const size_t TEST_OFFS = 0x8000;
/* Exhaust the available free internal memory */
test_utils_exhaust_memory_rec erec = test_utils_exhaust_memory(MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT, REMAINING_INTERNAL);
uint8_t *buf = heap_caps_malloc(BUF_SZ, MALLOC_CAP_8BIT|MALLOC_CAP_SPIRAM);
TEST_ASSERT_NOT_NULL(buf);
/* Calling esp_flash_read() here will need to allocate a small internal buffer,
so check it works. */
ESP_ERROR_CHECK( esp_flash_read(chip, buf, TEST_OFFS, BUF_SZ) );
test_utils_free_exhausted_memory(erec);
// Read back the same into smaller internal memory buffer and check it all matches
s_test_compare_flash_contents_small_reads(chip, buf, TEST_OFFS, BUF_SZ);
free(buf);
}
FLASH_TEST_CASE("esp_flash_read large PSRAM buffer low memory", test_flash_read_large_psram_buffer_low_internal_mem);
#endif

28
tools/unit-test-app/components/test_utils/include/test_utils.h
View file

@ -257,3 +257,31 @@ esp_err_t test_utils_set_leak_level(size_t leak_level, esp_type_leak_t type, esp
* return Leak level
*/
size_t test_utils_get_leak_level(esp_type_leak_t type, esp_comp_leak_t component);
typedef struct test_utils_exhaust_memory_record_s *test_utils_exhaust_memory_rec;
/**
* Limit the largest free block of memory with a particular capability set to
* 'limit' bytes (meaning an allocation of 'limit' should succeed at least once,
* but any allocation of more bytes will fail.)
*
* Returns a record pointer which needs to be passed back in to test_utils_free_exhausted_memory
* before the test completes, to avoid a major memory leak.
*
* @param caps Capabilities of memory to exhause
* @param limit The size to limit largest free block to
* @return Record pointer to pass to test_utils_free_exhausted_memory() once done
*/
test_utils_exhaust_memory_rec test_utils_exhaust_memory(uint32_t caps, size_t limit);
/**
* Call to free memory which was taken up by test_utils_exhaust_memory() call
*
* @param rec Result previously returned from test_utils_exhaust_memory()
*/
void test_utils_free_exhausted_memory(test_utils_exhaust_memory_rec rec);

38
tools/unit-test-app/components/test_utils/test_utils.c
View file

@ -141,3 +141,41 @@ size_t test_utils_get_leak_level(esp_type_leak_t type_of_leak, esp_comp_leak_t c
}
return leak_level;
}
#define EXHAUST_MEMORY_ENTRIES 100
struct test_utils_exhaust_memory_record_s {
int *entries[EXHAUST_MEMORY_ENTRIES];
};
test_utils_exhaust_memory_rec test_utils_exhaust_memory(uint32_t caps, size_t limit)
{
int idx = 0;
test_utils_exhaust_memory_rec rec = calloc(1, sizeof(struct test_utils_exhaust_memory_record_s));
TEST_ASSERT_NOT_NULL_MESSAGE(rec, "test_utils_exhaust_memory: not enough free memory to allocate record structure!");
while (idx < EXHAUST_MEMORY_ENTRIES) {
size_t free_caps = heap_caps_get_largest_free_block(caps);
if (free_caps <= limit) {
return rec; // done!
}
rec->entries[idx] = heap_caps_malloc(free_caps - limit, caps);
TEST_ASSERT_NOT_NULL_MESSAGE(rec->entries[idx],
"test_utils_exhaust_memory: something went wrong while freeing up memory, is another task using heap?");
heap_caps_check_integrity_all(true);
idx++;
}
TEST_FAIL_MESSAGE("test_utils_exhaust_memory: The heap with the requested caps is too fragmented, increase EXHAUST_MEMORY_ENTRIES or defrag the heap!");
abort();
}
void test_utils_free_exhausted_memory(test_utils_exhaust_memory_rec rec)
{
for (int i = 0; i < EXHAUST_MEMORY_ENTRIES; i++) {
free(rec->entries[i]);
}
free(rec);
}