Merge branch 'bugfix/esp_flash_erase_0' into 'master'

esp_flash: fix esp_flash_erase_region over-erase with 0 length (Github PR)

Closes IDFGH-5592

See merge request espressif/esp-idf!14498
This commit is contained in:
Michael (XIAO Xufeng) 2021-07-28 15:09:08 +00:00
commit 58022f8599
8 changed files with 52 additions and 13 deletions

View File

@ -20,3 +20,4 @@
#define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM
#define ESP_ROM_USB_SERIAL_DEVICE_NUM (3) // UART uses USB_SERIAL_JTAG port in ROM.
#define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking
#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug

View File

@ -260,7 +260,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400002fc );
PROVIDE( esp_flash_read_id = 0x40000300 );
PROVIDE( esp_flash_get_size = 0x40000304 );
PROVIDE( esp_flash_erase_chip = 0x40000308 );
PROVIDE( esp_flash_erase_region = 0x4000030c );
PROVIDE( rom_esp_flash_erase_region = 0x4000030c );
PROVIDE( esp_flash_get_chip_write_protect = 0x40000310 );
PROVIDE( esp_flash_set_chip_write_protect = 0x40000314 );
PROVIDE( esp_flash_get_protectable_regions = 0x40000318 );

View File

@ -20,3 +20,4 @@
#define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM
#define ESP_ROM_USB_SERIAL_DEVICE_NUM (3) // UART uses USB_SERIAL_JTAG port in ROM.
#define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking
#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug

View File

@ -267,7 +267,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400002f8 );
PROVIDE( esp_flash_read_id = 0x400002fc );
PROVIDE( esp_flash_get_size = 0x40000300 );
PROVIDE( esp_flash_erase_chip = 0x40000304 );
PROVIDE( esp_flash_erase_region = 0x40000308 );
PROVIDE( rom_esp_flash_erase_region = 0x40000308 );
PROVIDE( esp_flash_get_chip_write_protect = 0x4000030c );
PROVIDE( esp_flash_set_chip_write_protect = 0x40000310 );
PROVIDE( esp_flash_get_protectable_regions = 0x40000314 );

View File

@ -21,3 +21,4 @@
#define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM
#define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking
#define ESP_ROM_USB_SERIAL_DEVICE_NUM (4) // The serial port ID (UART, USB, ...) of USB_SERIAL_JTAG in the ROM.
#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug

View File

@ -334,7 +334,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400010bc );
PROVIDE( esp_flash_read_id = 0x400010c8 );
PROVIDE( esp_flash_get_size = 0x400010d4 );
PROVIDE( esp_flash_erase_chip = 0x400010e0 );
PROVIDE( esp_flash_erase_region = 0x400010ec );
PROVIDE( rom_esp_flash_erase_region = 0x400010ec );
PROVIDE( esp_flash_get_chip_write_protect = 0x400010f8 );
PROVIDE( esp_flash_set_chip_write_protect = 0x40001104 );
PROVIDE( esp_flash_get_protectable_regions = 0x40001110 );

View File

@ -23,6 +23,7 @@
#include "sdkconfig.h"
#include "esp_flash_internal.h"
#include "spi_flash_defs.h"
#include "esp_rom_caps.h"
#if CONFIG_IDF_TARGET_ESP32S2
#include "esp_crypto_lock.h" // for locking flash encryption peripheral
#endif //CONFIG_IDF_TARGET_ESP32S2
@ -460,6 +461,9 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui
// Can only erase multiples of the sector size, starting at sector boundary
return ESP_ERR_INVALID_ARG;
}
if (len == 0) {
return ESP_OK;
}
err = ESP_OK;
// Check for write protected regions overlapping the erase region
@ -522,6 +526,8 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui
len_remain -= sector_size;
}
assert(len_remain < len);
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
@ -540,6 +546,28 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui
return rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, start, len);
}
#endif // !CONFIG_SPI_FLASH_ROM_IMPL
#if defined(CONFIG_SPI_FLASH_ROM_IMPL) && ESP_ROM_HAS_ERASE_0_REGION_BUG
/* ROM esp_flash_erase_region implementation doesn't handle 0 erase size correctly.
* Check the size and call ROM function instead of overriding it completely.
* The behavior is slightly different from esp_flash_erase_region above, thought:
* here the check for 0 size is done first, but in esp_flash_erase_region the check is
* done after the other arguments are checked.
*/
extern esp_err_t rom_esp_flash_erase_region(esp_flash_t *chip, uint32_t start, uint32_t len);
esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, uint32_t len)
{
if (len == 0) {
return ESP_OK;
}
return rom_esp_flash_erase_region(chip, start, len);
}
#endif // defined(CONFIG_SPI_FLASH_ROM_IMPL) && ESP_ROM_HAS_ERASE_0_REGION_BUG
#ifndef CONFIG_SPI_FLASH_ROM_IMPL
esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *out_write_protected)
{
esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);
@ -668,14 +696,14 @@ esp_err_t IRAM_ATTR esp_flash_set_protected_region(esp_flash_t *chip, const esp_
esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint32_t length)
{
if (length == 0) {
return ESP_OK;
}
esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);
VERIFY_CHIP_OP(read);
if (buffer == NULL || address > chip->size || address+length > chip->size) {
return ESP_ERR_INVALID_ARG;
}
if (length == 0) {
return ESP_OK;
}
//when the cache is disabled, only the DRAM can be read, check whether we need to receive in another buffer in DRAM.
bool direct_read = chip->host->driver->supports_direct_read(chip->host, buffer);
@ -735,15 +763,15 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length)
{
if (length == 0) {
return ESP_OK;
}
esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);
VERIFY_CHIP_OP(write);
CHECK_WRITE_ADDRESS(chip, address, length);
if (buffer == NULL || address > chip->size || address+length > chip->size) {
return ESP_ERR_INVALID_ARG;
}
if (length == 0) {
return ESP_OK;
}
//when the cache is disabled, only the DRAM can be read, check whether we need to copy the data first
bool direct_write = chip->host->driver->supports_direct_write(chip->host, buffer);
@ -786,6 +814,7 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3
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
@ -810,10 +839,6 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3
esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t address, const void *buffer, uint32_t length)
{
if (length == 0) {
return ESP_OK;
}
esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);
// Flash encryption only support on main flash.
if (chip != esp_flash_default_chip) {
@ -829,6 +854,10 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres
return ESP_ERR_INVALID_ARG;
}
if (length == 0) {
return ESP_OK;
}
if ((length % 16) != 0) {
ESP_EARLY_LOGE(TAG, "flash encrypted write length must be multiple of 16");
return ESP_ERR_INVALID_SIZE;

View File

@ -615,6 +615,13 @@ void test_erase_large_region(const esp_partition_t *part)
TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address, 4));
TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data));
/* Erase zero bytes, check that nothing got erased */
TEST_ASSERT_EQUAL(ESP_OK, esp_flash_erase_region(chip, part->address, 0));
TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address + part->size - 5, 4));
TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data));
TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address, 4));
TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data));
/* Erase whole region */
TEST_ASSERT_EQUAL(ESP_OK, esp_flash_erase_region(chip, part->address, part->size));