diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index d517f6a496..8cddd13353 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -673,7 +673,46 @@ bool EspClass::flashEraseSector(uint32_t sector) { return rc == 0; } +// Adapted from the old version of `flash_hal_write()` (before 3.0.0), which was used for SPIFFS to allow +// writing from both unaligned u8 buffers and to an unaligned offset on flash. +// Updated version re-uses some of the code from RTOS, replacing individual methods for block & page +// writes with just a single one +// https://github.com/espressif/ESP8266_RTOS_SDK/blob/master/components/spi_flash/src/spi_flash.c +// (if necessary, we could follow the esp-idf code and introduce flash chip drivers controling more than just writing methods?) + +// This is a generic writer that does not cross page boundaries. +// Offset, data address and size *must* be 4byte aligned. +static SpiFlashOpResult spi_flash_write_page_break(uint32_t offset, uint32_t *data, size_t size) { + static constexpr uint32_t PageSize { FLASH_PAGE_SIZE }; + size_t size_page_aligned = PageSize - (offset % PageSize); + + // most common case, we don't cross a page and simply write the data + if (size < size_page_aligned) { + return spi_flash_write(offset, data, size); + } + + // otherwise, write the initial part and continue writing breaking each page interval + SpiFlashOpResult result = SPI_FLASH_RESULT_ERR; + if ((result = spi_flash_write(offset, data, size_page_aligned)) != SPI_FLASH_RESULT_OK) { + return result; + } + + const auto last_page = (size - size_page_aligned) / PageSize; + for (uint32_t page = 0; page < last_page; ++page) { + if ((result = spi_flash_write(offset + size_page_aligned, data + (size_page_aligned >> 2), PageSize)) != SPI_FLASH_RESULT_OK) { + return result; + } + + size_page_aligned += PageSize; + } + + // finally, the remaining data + return spi_flash_write(offset + size_page_aligned, data + (size_page_aligned >> 2), size - size_page_aligned); +} + #if PUYA_SUPPORT +// Special wrapper for spi_flash_write *only for PUYA flash chips* +// Already handles paging, could be used as a `spi_flash_write_page_break` replacement static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, size_t size) { if (data == nullptr) { return SPI_FLASH_RESULT_ERR; @@ -720,195 +759,129 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } #endif -bool EspClass::flashReplaceBlock(uint32_t address, const uint8_t *value, uint32_t byteCount) { - uint32_t alignedAddress = (address & ~3); - uint32_t alignmentOffset = address - alignedAddress; +static constexpr size_t Alignment { 4 }; - if (alignedAddress != ((address + byteCount - 1) & ~3)) { - // Only one 4 byte block is supported - return false; - } -#if PUYA_SUPPORT - if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - uint8_t tempData[4] __attribute__((aligned(4))); - if (spi_flash_read(alignedAddress, (uint32_t *)tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; - } - for (size_t i = 0; i < byteCount; i++) { - tempData[i + alignmentOffset] &= value[i]; - } - if (spi_flash_write(alignedAddress, (uint32_t *)tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; - } - } - else -#endif // PUYA_SUPPORT - { - uint32_t tempData; - if (spi_flash_read(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; - } - memcpy((uint8_t *)&tempData + alignmentOffset, value, byteCount); - if (spi_flash_write(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; - } - } - return true; +template +static T aligned(T value) { + static constexpr auto Mask = Alignment - 1; + return (value + Mask) & ~Mask; +} + +template +static T alignBefore(T value) { + return aligned(value) - Alignment; +} + +static bool isAlignedAddress(uint32_t address) { + return (address & (Alignment - 1)) == 0; +} + +static bool isAlignedSize(size_t size) { + return (size & (Alignment - 1)) == 0; +} + +static bool isAlignedPointer(const uint8_t *ptr) { + return isAlignedAddress(reinterpret_cast(ptr)); } + size_t EspClass::flashWriteUnalignedMemory(uint32_t address, const uint8_t *data, size_t size) { - size_t sizeLeft = (size & ~3); - size_t currentOffset = 0; - // Memory is unaligned, so we need to copy it to an aligned buffer - uint32_t alignedData[FLASH_PAGE_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); - // Handle page boundary - bool pageBreak = ((address % 4) != 0) && ((address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); + auto flash_write = [](uint32_t address, uint8_t *data, size_t size) { + return spi_flash_write(address, reinterpret_cast(data), size) == SPI_FLASH_RESULT_OK; + }; + + auto flash_read = [](uint32_t address, uint8_t *data, size_t size) { + return spi_flash_read(address, reinterpret_cast(data), size) == SPI_FLASH_RESULT_OK; + }; - if (pageBreak) { - size_t byteCount = 4 - (address % 4); + constexpr size_t BufferSize { FLASH_PAGE_SIZE }; + alignas(alignof(uint32_t)) uint8_t buf[BufferSize]; - if (!flashReplaceBlock(address, data, byteCount)) { + size_t written = 0; + + if (!isAlignedAddress(address)) { + auto before_address = alignBefore(address); + auto offset = address - before_address; + auto wlen = std::min(Alignment - offset, size); + + if (!flash_read(before_address, &buf[0], Alignment)) { return 0; } - // We will now have aligned address, so we can cross page boundaries - currentOffset += byteCount; - // Realign size to 4 - sizeLeft = (size - byteCount) & ~3; - } - while (sizeLeft) { - size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); - memcpy(alignedData, data + currentOffset, willCopy); - // We now have address, data and size aligned to 4 bytes, so we can use aligned write - if (!flashWrite(address + currentOffset, alignedData, willCopy)) - { +#if PUYA_SUPPORT + if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { + for (size_t i = 0; i < wlen ; ++i) { + buf[offset + i] &= data[i]; + } + } else { +#endif + memcpy(&buf[offset], data, wlen); +#if PUYA_SUPPORT + } +#endif + + if (!flash_write(before_address, &buf[0], Alignment)) { return 0; } - sizeLeft -= willCopy; - currentOffset += willCopy; + + address += wlen; + data += wlen; + written += wlen; + size -= wlen; } - return currentOffset; -} + while (size > 0) { + auto len = std::min(size, BufferSize); + auto wlen = aligned(len); -bool EspClass::flashWritePageBreak(uint32_t address, const uint8_t *data, size_t size) { - if (size > 4) { - return false; - } - size_t pageLeft = FLASH_PAGE_SIZE - (address % FLASH_PAGE_SIZE); - size_t offset = 0; - size_t sizeLeft = size; - if (pageLeft > 3) { - return false; - } + if (wlen != len) { + auto partial = wlen - Alignment; + if (!flash_read(address + partial, &buf[partial], Alignment)) { + return written; + } + } - if (!flashReplaceBlock(address, data, pageLeft)) { - return false; - } - offset += pageLeft; - sizeLeft -= pageLeft; - // We replaced last 4-byte block of the page, now we write the remainder in next page - if (!flashReplaceBlock(address + offset, data + offset, sizeLeft)) { - return false; + memcpy(&buf[0], data, len); + if (!flashWrite(address, reinterpret_cast(&buf[0]), wlen)) { + return written; + } + + address += len; + data += len; + written += len; + size -= len; } - return true; + + return written; } bool EspClass::flashWrite(uint32_t address, const uint32_t *data, size_t size) { - SpiFlashOpResult rc = SPI_FLASH_RESULT_OK; - bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + size - 1) / FLASH_PAGE_SIZE)); - - if ((uintptr_t)data % 4 != 0 || size % 4 != 0 || pageBreak) { - return false; - } + SpiFlashOpResult result; #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - rc = spi_flash_write_puya(address, const_cast(data), size); + result = spi_flash_write_puya(address, const_cast(data), size); } else -#endif // PUYA_SUPPORT +#endif { - rc = spi_flash_write(address, const_cast(data), size); + result = spi_flash_write_page_break(address, const_cast(data), size); } - return rc == SPI_FLASH_RESULT_OK; + return result == SPI_FLASH_RESULT_OK; } bool EspClass::flashWrite(uint32_t address, const uint8_t *data, size_t size) { - if (size == 0) { - return true; - } - - size_t sizeLeft = size & ~3; - size_t currentOffset = 0; - - if (sizeLeft) { - if ((uintptr_t)data % 4 != 0) { - size_t written = flashWriteUnalignedMemory(address, data, size); - if (!written) { - return false; - } - currentOffset += written; - sizeLeft -= written; - } else { - bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); - - if (pageBreak) { - while (sizeLeft) { - // We cannot cross page boundary, but the write must be 4 byte aligned, - // so this is the maximum amount we can write - size_t pageBoundary = (FLASH_PAGE_SIZE - ((address + currentOffset) % FLASH_PAGE_SIZE)) & ~3; - - if (sizeLeft > pageBoundary) { - // Aligned write up to page boundary - if (!flashWrite(address + currentOffset, (uint32_t *)(data + currentOffset), pageBoundary)) { - return false; - } - currentOffset += pageBoundary; - sizeLeft -= pageBoundary; - // Cross the page boundary - if (!flashWritePageBreak(address + currentOffset, data + currentOffset, 4)) { - return false; - } - currentOffset += 4; - sizeLeft -= 4; - } else { - // We do not cross page boundary - if (!flashWrite(address + currentOffset, (uint32_t *)(data + currentOffset), sizeLeft)) { - return false; - } - currentOffset += sizeLeft; - sizeLeft = 0; - } - } - } else { - // Pointer is properly aligned and write does not cross page boundary, - // so use aligned write - if (!flashWrite(address, (uint32_t *)data, sizeLeft)) { - return false; - } - currentOffset = sizeLeft; - sizeLeft = 0; - } - } - } - sizeLeft = size - currentOffset; - if (sizeLeft > 0) { - // Size was not aligned, so we have some bytes left to write, we also need to recheck for - // page boundary crossing - bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); - - if (pageBreak) { - // Cross the page boundary - if (!flashWritePageBreak(address + currentOffset, data + currentOffset, sizeLeft)) { - return false; - } - } else { - // Just write partial block - flashReplaceBlock(address + currentOffset, data + currentOffset, sizeLeft); + if (data && size) { + if (!isAlignedAddress(address) + || !isAlignedPointer(data) + || !isAlignedSize(size)) + { + return flashWriteUnalignedMemory(address, data, size) == size; } + + return flashWrite(address, reinterpret_cast(data), size); } - return true; + return false; } bool EspClass::flashRead(uint32_t address, uint8_t *data, size_t size) { @@ -916,23 +889,24 @@ bool EspClass::flashRead(uint32_t address, uint8_t *data, size_t size) { size_t currentOffset = 0; if ((uintptr_t)data % 4 != 0) { - uint32_t alignedData[FLASH_PAGE_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); + constexpr size_t BufferSize { FLASH_PAGE_SIZE / sizeof(uint32_t) }; + alignas(alignof(uint32_t)) uint32_t buf[BufferSize]; size_t sizeLeft = sizeAligned; while (sizeLeft) { - size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); + size_t willCopy = std::min(sizeLeft, BufferSize); // We read to our aligned buffer and then copy to data - if (!flashRead(address + currentOffset, alignedData, willCopy)) + if (!flashRead(address + currentOffset, &buf[0], willCopy)) { return false; } - memcpy(data + currentOffset, alignedData, willCopy); + memcpy(data + currentOffset, &buf[0], willCopy); sizeLeft -= willCopy; currentOffset += willCopy; } } else { // Pointer is properly aligned, so use aligned read - if (!flashRead(address, (uint32_t *)data, sizeAligned)) { + if (!flashRead(address, reinterpret_cast(data), sizeAligned)) { return false; } currentOffset = sizeAligned; @@ -940,10 +914,10 @@ bool EspClass::flashRead(uint32_t address, uint8_t *data, size_t size) { if (currentOffset < size) { uint32_t tempData; - if (spi_flash_read(address + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + if (spi_flash_read(address + currentOffset, &tempData, sizeof(tempData)) != SPI_FLASH_RESULT_OK) { return false; } - memcpy((uint8_t *)data + currentOffset, &tempData, size - currentOffset); + memcpy(data + currentOffset, &tempData, size - currentOffset); } return true; @@ -973,7 +947,7 @@ String EspClass::getSketchMD5() md5.begin(); while( lengthLeft > 0) { size_t readBytes = (lengthLeft < bufSize) ? lengthLeft : bufSize; - if (!flashRead(offset, reinterpret_cast(buf.get()), (readBytes + 3) & ~3)) { + if (!flashRead(offset, reinterpret_cast(buf.get()), (readBytes + 3) & ~3)) { return emptyString; } md5.add(buf.get(), readBytes); diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index 773d3cd192..1bb793ef7f 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -103,11 +103,11 @@ class EspClass { static void reset(); static void restart(); - /** - * @brief When calling this method the ESP8266 reboots into the UART download mode without - * the need of any external wiring. This is the same mode which can also be entered by - * pulling GPIO0=low, GPIO2=high, GPIO15=low and resetting the ESP8266. - */ + /** + * @brief When calling this method the ESP8266 reboots into the UART download mode without + * the need of any external wiring. This is the same mode which can also be entered by + * pulling GPIO0=low, GPIO2=high, GPIO15=low and resetting the ESP8266. + */ [[noreturn]] static void rebootIntoUartDownloadMode(); static uint16_t getVcc(); @@ -252,39 +252,18 @@ class EspClass { */ static void resetHeap(); private: - /** - * @brief Replaces @a byteCount bytes of a 4 byte block on flash - * - * @param address flash address - * @param value buffer with data - * @param byteCount number of bytes to replace - * @return bool result of operation - * @retval true success - * @retval false failed to read/write or invalid args - */ - static bool flashReplaceBlock(uint32_t address, const uint8_t *value, uint32_t byteCount); /** * @brief Write up to @a size bytes from @a data to flash at @a address - * This function takes case of unaligned memory access by copying @a data to a temporary buffer, - * it also takes care of page boundary crossing see @a flashWritePageBreak as to why it's done. - * Less than @a size bytes may be written, due to 4 byte alignment requirement of spi_flash_write + * This function handles all cases of unaligned memory acccess; when either + * address is not aligned, data pointer is not aligned or size is not a multiple of 4. + * User of this function should note that @a data will be copied into a buffer allocated on stack. + * * @param address address on flash where write should start * @param data input buffer * @param size amount of data * @return size_t amount of data written, 0 on failure */ static size_t flashWriteUnalignedMemory(uint32_t address, const uint8_t *data, size_t size); - /** - * @brief Splits up to 4 bytes into 4 byte blocks and writes them to flash - * We need this since spi_flash_write cannot handle writing over a page boundary with unaligned offset - * i.e. spi_flash_write(254, data, 4) will fail, also we cannot write less bytes as in - * spi_flash_write(254, data, 2) since it will be extended internally to 4 bytes and fail - * @param address start of write - * @param data data to be written - * @param size amount of data, must be < 4 - * @return bool result of operation - */ - static bool flashWritePageBreak(uint32_t address, const uint8_t *data, size_t size); }; extern EspClass ESP; diff --git a/tests/device/test_spi_flash/test_spi_flash.ino b/tests/device/test_spi_flash/test_spi_flash.ino index b1e18241f5..16fa4a9750 100644 --- a/tests/device/test_spi_flash/test_spi_flash.ino +++ b/tests/device/test_spi_flash/test_spi_flash.ino @@ -1,5 +1,4 @@ #include -#include BS_ENV_DECLARE(); @@ -176,6 +175,11 @@ TEST_CASE("Unaligned page cross only", "[spi_flash]") CHECK(testFlash(0xa0000 + 255, 1, 2)); } +TEST_CASE("Unaligned page cross with unaligned size (#8372, #8588, #8605)", "[spi_flash]") +{ + CHECK(testFlash(0xa00b, 0, 202)); +} + void loop () { }