From 701197defe4f6f204445a9e15c3e7a26d7440c81 Mon Sep 17 00:00:00 2001 From: Drzony Date: Fri, 7 Aug 2020 17:40:15 +0200 Subject: [PATCH 01/19] Do not write more data than requested on PUYA flashes --- cores/esp8266/Esp.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 80d969e40c..0d8f474df9 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -698,7 +698,7 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si bytesLeft = 0; } size_t bytesAligned = (bytesNow + 3) & ~3; - rc = spi_flash_read(pos, flash_write_puya_buf, bytesAligned); + rc = spi_flash_read(pos, flash_write_puya_buf, bytesNow); if (rc != SPI_FLASH_RESULT_OK) { return rc; } @@ -706,7 +706,7 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si flash_write_puya_buf[i] &= *ptr; ++ptr; } - rc = spi_flash_write(pos, flash_write_puya_buf, bytesAligned); + rc = spi_flash_write(pos, flash_write_puya_buf, bytesNow); pos += bytesNow; } return rc; From 02adea2579ac7a171a8a2f25f8764c7352e5d5a7 Mon Sep 17 00:00:00 2001 From: Drzony Date: Mon, 10 Aug 2020 10:12:20 +0200 Subject: [PATCH 02/19] Always align flash reads/writes to 4 bytes --- cores/esp8266/Esp.cpp | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 0d8f474df9..8ed67aab04 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -668,11 +668,18 @@ bool EspClass::flashEraseSector(uint32_t sector) { return rc == 0; } +namespace { #if PUYA_SUPPORT +#if PUYA_BUFFER_SIZE % 256 != 0 +#error PUYA_BUFFER_SIZE is not 256 byte aligned +#endif static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, size_t size) { if (data == nullptr) { return SPI_FLASH_RESULT_ERR; } + if (size % 4 != 0) { + return SPI_FLASH_RESULT_ERR; + } // PUYA flash chips need to read existing data, update in memory and write modified data again. static uint32_t *flash_write_puya_buf = nullptr; @@ -697,12 +704,11 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } else { bytesLeft = 0; } - size_t bytesAligned = (bytesNow + 3) & ~3; rc = spi_flash_read(pos, flash_write_puya_buf, bytesNow); if (rc != SPI_FLASH_RESULT_OK) { return rc; } - for (size_t i = 0; i < bytesAligned / 4; ++i) { + for (size_t i = 0; i < bytesNow / 4; ++i) { flash_write_puya_buf[i] &= *ptr; ++ptr; } @@ -713,22 +719,47 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } #endif +static SpiFlashOpResult spi_flash_write_unaligned(uint32_t offset, uint32_t data, size_t remainder) { + uint32_t tempData; + SpiFlashOpResult rc = spi_flash_read(offset, &tempData, 4); + if (rc != SPI_FLASH_RESULT_OK) { + return rc; + } + for (int i = 4 - remainder; i < 4; i++) { + ((uint8_t *)&data)[i] = 0xFF; + } + tempData &= data; + rc = spi_flash_write(offset, &tempData, 4); + return rc; +} +} // namespace + bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { SpiFlashOpResult rc = SPI_FLASH_RESULT_OK; + size_t sizeAligned = size & ~3; #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - rc = spi_flash_write_puya(offset, data, size); + rc = spi_flash_write_puya(offset, data, sizeAligned); } else #endif { - rc = spi_flash_write(offset, data, size); + rc = spi_flash_write(offset, data, sizeAligned); + } + if (sizeAligned < size) { + rc = spi_flash_write_unaligned(offset + sizeAligned, data[sizeAligned / 4], size - sizeAligned); } return rc == SPI_FLASH_RESULT_OK; } bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { - auto rc = spi_flash_read(offset, (uint32_t*) data, size); + size_t sizeAligned = size & ~3; + auto rc = spi_flash_read(offset, data, sizeAligned); + if (sizeAligned < size) { + uint32_t tempData; + rc = spi_flash_read(offset + sizeAligned, &tempData, 4); + memcpy((uint8_t *)data + sizeAligned, &tempData, size - sizeAligned); + } return rc == SPI_FLASH_RESULT_OK; } From f079f00a0c65d8b31282a2a6942d74004233c74e Mon Sep 17 00:00:00 2001 From: Drzony Date: Tue, 11 Aug 2020 01:28:57 +0200 Subject: [PATCH 03/19] fixup! Always align flash reads/writes to 4 bytes This commit simplifies the code a bit and fixes a bug that caused wrong number of bytes to be written --- cores/esp8266/Esp.cpp | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 8ed67aab04..c1b9b38f3d 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -668,7 +668,6 @@ bool EspClass::flashEraseSector(uint32_t sector) { return rc == 0; } -namespace { #if PUYA_SUPPORT #if PUYA_BUFFER_SIZE % 256 != 0 #error PUYA_BUFFER_SIZE is not 256 byte aligned @@ -719,21 +718,6 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } #endif -static SpiFlashOpResult spi_flash_write_unaligned(uint32_t offset, uint32_t data, size_t remainder) { - uint32_t tempData; - SpiFlashOpResult rc = spi_flash_read(offset, &tempData, 4); - if (rc != SPI_FLASH_RESULT_OK) { - return rc; - } - for (int i = 4 - remainder; i < 4; i++) { - ((uint8_t *)&data)[i] = 0xFF; - } - tempData &= data; - rc = spi_flash_write(offset, &tempData, 4); - return rc; -} -} // namespace - bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { SpiFlashOpResult rc = SPI_FLASH_RESULT_OK; size_t sizeAligned = size & ~3; @@ -747,7 +731,23 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { rc = spi_flash_write(offset, data, sizeAligned); } if (sizeAligned < size) { - rc = spi_flash_write_unaligned(offset + sizeAligned, data[sizeAligned / 4], size - sizeAligned); + uint32_t tempData; + SpiFlashOpResult rc = spi_flash_read(offset, &tempData, 4); + if (rc == SPI_FLASH_RESULT_OK) { + memcpy(&tempData, (uint8_t *)data + sizeAligned, size - sizeAligned); + rc = spi_flash_write(offset, &tempData, 4); + } + } + return rc == SPI_FLASH_RESULT_OK; +} + +bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { + size_t sizeAligned = size & ~3; + auto rc = spi_flash_read(offset, data, sizeAligned); + if (sizeAligned < size) { + uint32_t tempData; + rc = spi_flash_read(offset + sizeAligned, &tempData, 4); + memcpy((uint8_t *)data + sizeAligned, &tempData, size - sizeAligned); } return rc == SPI_FLASH_RESULT_OK; } From e1cd190189b07e16a6ca6370d2b382285fd3240b Mon Sep 17 00:00:00 2001 From: Drzony Date: Tue, 11 Aug 2020 01:48:06 +0200 Subject: [PATCH 04/19] fixup! Always align flash reads/writes to 4 bytes --- cores/esp8266/Esp.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index c1b9b38f3d..30bd44adf3 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -752,17 +752,6 @@ bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { return rc == SPI_FLASH_RESULT_OK; } -bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { - size_t sizeAligned = size & ~3; - auto rc = spi_flash_read(offset, data, sizeAligned); - if (sizeAligned < size) { - uint32_t tempData; - rc = spi_flash_read(offset + sizeAligned, &tempData, 4); - memcpy((uint8_t *)data + sizeAligned, &tempData, size - sizeAligned); - } - return rc == SPI_FLASH_RESULT_OK; -} - String EspClass::getSketchMD5() { static String result; From 6cc5f1d85e3c7bcbb09b7bfa00a816e34e8c1210 Mon Sep 17 00:00:00 2001 From: Drzony Date: Tue, 11 Aug 2020 09:22:03 +0200 Subject: [PATCH 05/19] fixup! Always align flash reads/writes to 4 bytes --- cores/esp8266/Esp.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 30bd44adf3..8dc72602ab 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -732,10 +732,10 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { } if (sizeAligned < size) { uint32_t tempData; - SpiFlashOpResult rc = spi_flash_read(offset, &tempData, 4); + SpiFlashOpResult rc = spi_flash_read(offset + sizeAligned, &tempData, 4); if (rc == SPI_FLASH_RESULT_OK) { memcpy(&tempData, (uint8_t *)data + sizeAligned, size - sizeAligned); - rc = spi_flash_write(offset, &tempData, 4); + rc = spi_flash_write(offset + sizeAligned, &tempData, 4); } } return rc == SPI_FLASH_RESULT_OK; From d941932666208438a01423d19389552548b795d4 Mon Sep 17 00:00:00 2001 From: Drzony Date: Tue, 6 Oct 2020 09:16:24 +0200 Subject: [PATCH 06/19] Check for result before additional read/write --- cores/esp8266/Esp.cpp | 4 ++-- cores/esp8266/Esp.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 51569da155..5e26317a72 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -723,7 +723,7 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { { rc = spi_flash_write(offset, data, sizeAligned); } - if (sizeAligned < size) { + if (sizeAligned < size && rc == SPI_FLASH_RESULT_OK) { uint32_t tempData; SpiFlashOpResult rc = spi_flash_read(offset + sizeAligned, &tempData, 4); if (rc == SPI_FLASH_RESULT_OK) { @@ -737,7 +737,7 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { size_t sizeAligned = size & ~3; auto rc = spi_flash_read(offset, data, sizeAligned); - if (sizeAligned < size) { + if (sizeAligned < size && rc == SPI_FLASH_RESULT_OK) { uint32_t tempData; rc = spi_flash_read(offset + sizeAligned, &tempData, 4); memcpy((uint8_t *)data + sizeAligned, &tempData, size - sizeAligned); diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index c321db3823..7d38f48293 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -150,6 +150,7 @@ class EspClass { bool flashEraseSector(uint32_t sector); bool flashWrite(uint32_t offset, uint32_t *data, size_t size); + bool flashRead(uint32_t offset, uint8_t *data, size_t size); bool flashRead(uint32_t offset, uint32_t *data, size_t size); uint32_t getSketchSize(); From 2bd885f7ab4b2b255db42c708236e055bfb7ff21 Mon Sep 17 00:00:00 2001 From: Drzony Date: Thu, 8 Oct 2020 15:32:47 +0200 Subject: [PATCH 07/19] Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 122 ++++++++++++++++++++++++++----- cores/esp8266/Esp.h | 5 +- cores/esp8266/Updater.cpp | 8 +- cores/esp8266/flash_hal.cpp | 133 +++------------------------------- tests/host/common/MockEsp.cpp | 16 ++++ 5 files changed, 137 insertions(+), 147 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 24ee432c5c..830561beda 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -713,38 +713,124 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } #endif -bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) { +bool EspClass::flashWrite(uint32_t offset, const uint32_t *data, size_t size) { SpiFlashOpResult rc = SPI_FLASH_RESULT_OK; - size_t sizeAligned = size & ~3; + if ((uintptr_t)data % 4 != 0 || size % 4 != 0) { + return false; + } #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - rc = spi_flash_write_puya(offset, data, sizeAligned); + rc = spi_flash_write_puya(offset, data, size); } else -#endif +#endif // PUYA_SUPPORT { - rc = spi_flash_write(offset, data, sizeAligned); + rc = spi_flash_write(offset, const_cast(data), size); } - if (sizeAligned < size && rc == SPI_FLASH_RESULT_OK) { - uint32_t tempData; - SpiFlashOpResult rc = spi_flash_read(offset + sizeAligned, &tempData, 4); - if (rc == SPI_FLASH_RESULT_OK) { - memcpy(&tempData, (uint8_t *)data + sizeAligned, size - sizeAligned); - rc = spi_flash_write(offset + sizeAligned, &tempData, 4); + return rc == SPI_FLASH_RESULT_OK; +} + +static const int UNALIGNED_WRITE_BUFFER_SIZE = 512; + +bool EspClass::flashWrite(uint32_t offset, const uint8_t *data, size_t size) { + size_t sizeAligned = size & ~3; + size_t currentOffset = 0; + if ((uintptr_t)data % 4 != 0) { + // Memory is unaligned, so we need to copy it to an aligned buffer + uint32_t alignedData[UNALIGNED_WRITE_BUFFER_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); + size_t sizeLeft = sizeAligned; + + while (sizeLeft) { + size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); + memcpy(alignedData, data + currentOffset, willCopy); + // We now have data and size aligned to 4 bytes, so we can use aligned write + if (!flashWrite(offset + currentOffset, alignedData, willCopy)) + { + return false; + } + sizeLeft -= willCopy; + currentOffset += willCopy; } + } else { + if (!flashWrite(offset, data, sizeAligned)) { + return false; + } + currentOffset = sizeAligned; } - return rc == SPI_FLASH_RESULT_OK; + if (currentOffset < size) { + // Size was not aligned, but writes must be 4-byte aligned +#if PUYA_SUPPORT + if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { + uint32_t tempData; + if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + for (int i = 0; i < size - currentOffset; i++) { + tempData[i] &= ((uint8_t *)data)[currentOffset + i]; + } + if (spi_flash_write(currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + } + else +#endif // PUYA_SUPPORT + { + uint32_t tempData; + if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + memcpy(&tempData, (uint8_t *)data + currentOffset, size - currentOffset); + if (spi_flash_write(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + } + } + + return true; } -bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { +bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { size_t sizeAligned = size & ~3; - auto rc = spi_flash_read(offset, data, sizeAligned); - if (sizeAligned < size && rc == SPI_FLASH_RESULT_OK) { + size_t currentOffset = 0; + + if ((uintptr_t)data % 4 != 0) { + uint32_t alignedData[UNALIGNED_WRITE_BUFFER_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); + size_t sizeLeft = sizeAligned; + + while (sizeLeft) { + size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); + // We read to our aligned buffer and then copy to data + if (!flashRead(offset + currentOffset, alignedData, willCopy)) + { + return false; + } + memcpy(data + currentOffset, alignedData, willCopy); + sizeLeft -= willCopy; + currentOffset += willCopy; + } + } else { + if (!flashRead(offset, data, sizeAligned)) { + return false; + } + currentOffset = sizeAligned; + } + + if (currentOffset < size) { uint32_t tempData; - rc = spi_flash_read(offset + sizeAligned, &tempData, 4); - memcpy((uint8_t *)data + sizeAligned, &tempData, size - sizeAligned); + if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + memcpy((uint8_t *)data + currentOffset, &tempData, size - currentOffset); } - return rc == SPI_FLASH_RESULT_OK; + + return true; +} + +bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { + if ((uintptr_t)data % 4 != 0 || size % 4 != 0) { + return false; + } + return (spi_flash_read(offset, data, size) == SPI_FLASH_RESULT_OK); } String EspClass::getSketchMD5() diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index cd988c52bd..9bc6592a76 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -150,9 +150,10 @@ class EspClass { bool checkFlashCRC(); bool flashEraseSector(uint32_t sector); - bool flashWrite(uint32_t offset, uint32_t *data, size_t size); - bool flashRead(uint32_t offset, uint8_t *data, size_t size); + bool flashWrite(uint32_t offset, const uint32_t *data, size_t size); + bool flashWrite(uint32_t offset, const uint8_t *data, size_t size); bool flashRead(uint32_t offset, uint32_t *data, size_t size); + bool flashRead(uint32_t offset, uint8_t *data, size_t size); uint32_t getSketchSize(); String getSketchMD5(); diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp index fd6a7d2d49..23917fb0d6 100644 --- a/cores/esp8266/Updater.cpp +++ b/cores/esp8266/Updater.cpp @@ -236,7 +236,7 @@ bool UpdaterClass::end(bool evenIfRemaining){ DEBUG_UPDATER.printf_P(PSTR("[Updater] Adjusted binsize: %d\n"), binSize); #endif // Calculate the MD5 and hash using proper size - uint8_t buff[128]; + uint8_t buff[128] __attribute__((aligned(4)); for(int i = 0; i < binSize; i += sizeof(buff)) { ESP.flashRead(_startAddress + i, (uint32_t *)buff, sizeof(buff)); size_t read = std::min((int)sizeof(buff), binSize - i); @@ -255,7 +255,7 @@ bool UpdaterClass::end(bool evenIfRemaining){ _reset(); return false; } - ESP.flashRead(_startAddress + binSize, (uint32_t *)sig, sigLen); + ESP.flashRead(_startAddress + binSize, sig, sigLen); #ifdef DEBUG_UPDATER DEBUG_UPDATER.printf_P(PSTR("[Updater] Received Signature:")); for (size_t i=0; i alignedEnd) { - uint32_t nb = addr + size - alignedEnd; - uint32_t tmp; - if (!ESP.flashRead(alignedEnd, &tmp, 4)) { - DEBUGV("_spif_read(%d) addr=%x size=%x ab=%x ae=%x\r\n", - __LINE__, addr, size, alignedBegin, alignedEnd); - return FLASH_HAL_READ_ERROR; - } - - memcpy(dst + size - nb, &tmp, nb); - } - - return result; } -/* - Like spi_flash_read, spi_flash_write has a requirement for flash address to be - aligned. However it also requires RAM address to be aligned as it reads data - in 32-bit words. Flash address (mis-)alignment is handled much the same way - as for reads, but for RAM alignment we have to copy data into a temporary - buffer. The size of this buffer is a tradeoff between number of writes required - and amount of stack required. This is chosen to be 512 bytes here, but might - be adjusted in the future if there are good reasons to do so. -*/ - -static const int UNALIGNED_WRITE_BUFFER_SIZE = 512; - int32_t flash_hal_write(uint32_t addr, uint32_t size, const uint8_t *src) { optimistic_yield(10000); - uint32_t alignedBegin = (addr + 3) & (~3); - uint32_t alignedEnd = (addr + size) & (~3); - if (alignedEnd < alignedBegin) { - alignedEnd = alignedBegin; + // We use flashWrite overload that handles proper alignment + if (ESP.flashWrite(addr, src, size)) { + return FLASH_HAL_OK; + } else { + return FLASH_HAL_WRITE_ERROR; } - - if (addr < alignedBegin) { - uint32_t ofs = alignedBegin - addr; - uint32_t nb = (size < ofs) ? size : ofs; - uint8_t tmp[4] __attribute__((aligned(4))) = {0xff, 0xff, 0xff, 0xff}; - memcpy(tmp + 4 - ofs, src, nb); - if (!ESP.flashWrite(alignedBegin - 4, (uint32_t*) tmp, 4)) { - DEBUGV("_spif_write(%d) addr=%x size=%x ab=%x ae=%x\r\n", - __LINE__, addr, size, alignedBegin, alignedEnd); - return FLASH_HAL_WRITE_ERROR; - } - } - - if (alignedEnd != alignedBegin) { - uint32_t* srcLeftover = (uint32_t*) (src + alignedBegin - addr); - uint32_t srcAlign = ((uint32_t) srcLeftover) & 3; - if (!srcAlign) { - if (!ESP.flashWrite(alignedBegin, (uint32_t*) srcLeftover, - alignedEnd - alignedBegin)) { - DEBUGV("_spif_write(%d) addr=%x size=%x ab=%x ae=%x\r\n", - __LINE__, addr, size, alignedBegin, alignedEnd); - return FLASH_HAL_WRITE_ERROR; - } - } - else { - uint8_t buf[UNALIGNED_WRITE_BUFFER_SIZE]; - for (uint32_t sizeLeft = alignedEnd - alignedBegin; sizeLeft; ) { - size_t willCopy = std::min(sizeLeft, sizeof(buf)); - memcpy(buf, srcLeftover, willCopy); - - if (!ESP.flashWrite(alignedBegin, (uint32_t*) buf, willCopy)) { - DEBUGV("_spif_write(%d) addr=%x size=%x ab=%x ae=%x\r\n", - __LINE__, addr, size, alignedBegin, alignedEnd); - return FLASH_HAL_WRITE_ERROR; - } - - sizeLeft -= willCopy; - srcLeftover += willCopy; - alignedBegin += willCopy; - } - } - } - - if (addr + size > alignedEnd) { - uint32_t nb = addr + size - alignedEnd; - uint32_t tmp = 0xffffffff; - memcpy(&tmp, src + size - nb, nb); - - if (!ESP.flashWrite(alignedEnd, &tmp, 4)) { - DEBUGV("_spif_write(%d) addr=%x size=%x ab=%x ae=%x\r\n", - __LINE__, addr, size, alignedBegin, alignedEnd); - return FLASH_HAL_WRITE_ERROR; - } - } - - return FLASH_HAL_OK; } int32_t flash_hal_erase(uint32_t addr, uint32_t size) { diff --git a/tests/host/common/MockEsp.cpp b/tests/host/common/MockEsp.cpp index 2045523e82..d379ca05ff 100644 --- a/tests/host/common/MockEsp.cpp +++ b/tests/host/common/MockEsp.cpp @@ -167,6 +167,14 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) return true; } +bool EspClass::flashWrite(uint32_t offset, uint8_t *data, size_t size) +{ + (void)offset; + (void)data; + (void)size; + return true; +} + bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { (void)offset; @@ -175,6 +183,14 @@ bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) return true; } +bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) +{ + (void)offset; + (void)data; + (void)size; + return true; +} + uint32_t EspClass::magicFlashChipSize(uint8_t byte) { switch(byte & 0x0F) { case 0x0: // 4 Mbit (512KB) From d850f65d74d3df5a7db2e26ff0b2c08d71b2a3f2 Mon Sep 17 00:00:00 2001 From: Drzony Date: Thu, 8 Oct 2020 15:41:48 +0200 Subject: [PATCH 08/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 830561beda..65eb02d633 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -720,7 +720,7 @@ bool EspClass::flashWrite(uint32_t offset, const uint32_t *data, size_t size) { } #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - rc = spi_flash_write_puya(offset, data, size); + rc = spi_flash_write_puya(offset, const_cast(data), size); } else #endif // PUYA_SUPPORT @@ -765,8 +765,8 @@ bool EspClass::flashWrite(uint32_t offset, const uint8_t *data, size_t size) { if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { return false; } - for (int i = 0; i < size - currentOffset; i++) { - tempData[i] &= ((uint8_t *)data)[currentOffset + i]; + for (size_t i = 0; i < size - currentOffset; i++) { + ((uint8_t *)tempData)[i] &= ((uint8_t *)data)[currentOffset + i]; } if (spi_flash_write(currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { return false; From 5175867456f6e6b8cd820e836b4515afee6ef43b Mon Sep 17 00:00:00 2001 From: Drzony Date: Thu, 8 Oct 2020 17:00:51 +0200 Subject: [PATCH 09/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Updater.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp index 23917fb0d6..b2e92978eb 100644 --- a/cores/esp8266/Updater.cpp +++ b/cores/esp8266/Updater.cpp @@ -236,7 +236,7 @@ bool UpdaterClass::end(bool evenIfRemaining){ DEBUG_UPDATER.printf_P(PSTR("[Updater] Adjusted binsize: %d\n"), binSize); #endif // Calculate the MD5 and hash using proper size - uint8_t buff[128] __attribute__((aligned(4)); + uint8_t buff[128] __attribute__((aligned(4))); for(int i = 0; i < binSize; i += sizeof(buff)) { ESP.flashRead(_startAddress + i, (uint32_t *)buff, sizeof(buff)); size_t read = std::min((int)sizeof(buff), binSize - i); @@ -435,7 +435,7 @@ bool UpdaterClass::_verifyHeader(uint8_t data) { bool UpdaterClass::_verifyEnd() { if(_command == U_FLASH) { - uint8_t buf[4] __attribute__((aligned(4)); + uint8_t buf[4] __attribute__((aligned(4))); if(!ESP.flashRead(_startAddress, (uint32_t *) &buf[0], 4)) { _currentAddress = (_startAddress); _setError(UPDATE_ERROR_READ); From 94c2df378f980388ed79f9c0698d628310d364f7 Mon Sep 17 00:00:00 2001 From: Drzony Date: Thu, 8 Oct 2020 17:03:58 +0200 Subject: [PATCH 10/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/flash_hal.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/cores/esp8266/flash_hal.cpp b/cores/esp8266/flash_hal.cpp index 402f19102f..986d3745d6 100644 --- a/cores/esp8266/flash_hal.cpp +++ b/cores/esp8266/flash_hal.cpp @@ -32,7 +32,6 @@ extern "C" { int32_t flash_hal_read(uint32_t addr, uint32_t size, uint8_t *dst) { optimistic_yield(10000); - uint32_t result = FLASH_HAL_OK; // We use flashRead overload that handles proper alignment if (ESP.flashRead(addr, dst, size)) { return FLASH_HAL_OK; From 64ceb928e462612fd3d88450152cc151b7613d9a Mon Sep 17 00:00:00 2001 From: Drzony Date: Thu, 8 Oct 2020 17:07:34 +0200 Subject: [PATCH 11/19] fixup! Add overloads for unaligned reads/writes --- tests/host/common/MockEsp.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/host/common/MockEsp.cpp b/tests/host/common/MockEsp.cpp index d379ca05ff..4394c41625 100644 --- a/tests/host/common/MockEsp.cpp +++ b/tests/host/common/MockEsp.cpp @@ -159,7 +159,7 @@ FlashMode_t EspClass::magicFlashChipMode(uint8_t byte) return FM_DOUT; } -bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) +bool EspClass::flashWrite(uint32_t offset, const uint32_t *data, size_t size) { (void)offset; (void)data; @@ -167,7 +167,7 @@ bool EspClass::flashWrite(uint32_t offset, uint32_t *data, size_t size) return true; } -bool EspClass::flashWrite(uint32_t offset, uint8_t *data, size_t size) +bool EspClass::flashWrite(uint32_t offset, const uint8_t *data, size_t size) { (void)offset; (void)data; From 279a293d349db6c28a6f5a85368b7872fb8119e5 Mon Sep 17 00:00:00 2001 From: Drzony Date: Fri, 9 Oct 2020 17:14:13 +0200 Subject: [PATCH 12/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 65eb02d633..20b4d58df3 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -752,7 +752,8 @@ bool EspClass::flashWrite(uint32_t offset, const uint8_t *data, size_t size) { currentOffset += willCopy; } } else { - if (!flashWrite(offset, data, sizeAligned)) { + // Pointer is properly aligned, so use aligned write + if (!flashWrite(offset, (uint32_t *)data, sizeAligned)) { return false; } currentOffset = sizeAligned; @@ -809,7 +810,8 @@ bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { currentOffset += willCopy; } } else { - if (!flashRead(offset, data, sizeAligned)) { + // Pointer is properly aligned, so use aligned read + if (!flashRead(offset, (uint32_t *)data, sizeAligned)) { return false; } currentOffset = sizeAligned; From c0d0257513302463190aa062ebfd4e90ed972919 Mon Sep 17 00:00:00 2001 From: Drzony Date: Sun, 11 Oct 2020 15:14:09 +0200 Subject: [PATCH 13/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 240 ++++++++++++++++++++++++++++++------------ cores/esp8266/Esp.h | 80 +++++++++++++- 2 files changed, 249 insertions(+), 71 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index 20b4d58df3..a1b5de5190 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -42,11 +42,6 @@ extern struct rst_info resetInfo; #ifndef PUYA_SUPPORT #define PUYA_SUPPORT 1 #endif -#ifndef PUYA_BUFFER_SIZE - // Good alternative for buffer size is: SPI_FLASH_SEC_SIZE (= 4k) - // Always use a multiple of flash page size (256 bytes) - #define PUYA_BUFFER_SIZE 256 -#endif /** * User-defined Literals @@ -664,9 +659,6 @@ bool EspClass::flashEraseSector(uint32_t sector) { } #if PUYA_SUPPORT -#if PUYA_BUFFER_SIZE % 256 != 0 -#error PUYA_BUFFER_SIZE is not 256 byte aligned -#endif static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, size_t size) { if (data == nullptr) { return SPI_FLASH_RESULT_ERR; @@ -678,7 +670,7 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si static uint32_t *flash_write_puya_buf = nullptr; if (flash_write_puya_buf == nullptr) { - flash_write_puya_buf = (uint32_t*) malloc(PUYA_BUFFER_SIZE); + flash_write_puya_buf = (uint32_t*) malloc(FLASH_PAGE_SIZE); // No need to ever free this, since the flash chip will never change at runtime. if (flash_write_puya_buf == nullptr) { // Memory could not be allocated. @@ -692,9 +684,9 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si uint32_t pos = offset; while (bytesLeft > 0 && rc == SPI_FLASH_RESULT_OK) { size_t bytesNow = bytesLeft; - if (bytesNow > PUYA_BUFFER_SIZE) { - bytesNow = PUYA_BUFFER_SIZE; - bytesLeft -= PUYA_BUFFER_SIZE; + if (bytesNow > FLASH_PAGE_SIZE) { + bytesNow = FLASH_PAGE_SIZE; + bytesLeft -= FLASH_PAGE_SIZE; } else { bytesLeft = 0; } @@ -713,95 +705,209 @@ static SpiFlashOpResult spi_flash_write_puya(uint32_t offset, uint32_t *data, si } #endif -bool EspClass::flashWrite(uint32_t offset, const uint32_t *data, size_t size) { - SpiFlashOpResult rc = SPI_FLASH_RESULT_OK; - if ((uintptr_t)data % 4 != 0 || size % 4 != 0) { +bool EspClass::flashReplaceBlock(uint32_t address, const uint8_t *value, uint32_t byteCount) { + uint32_t alignedAddress = (address & ~3); + uint32_t alignmentOffset = address - alignedAddress; + + if (alignedAddress != ((address + byteCount - 1) & ~3)) { + // Only one 4 byte block is supported return false; } #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - rc = spi_flash_write_puya(offset, const_cast(data), size); + uint32_t tempData; + if (spi_flash_read(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } + for (size_t i = 0; i < byteCount; i++) { + ((uint8_t *)tempData)[i + alignmentOffset] &= value[i]; + } + if (spi_flash_write(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { + return false; + } } else #endif // PUYA_SUPPORT { - rc = spi_flash_write(offset, const_cast(data), size); + 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 rc == SPI_FLASH_RESULT_OK; + return true; } -static const int UNALIGNED_WRITE_BUFFER_SIZE = 512; - -bool EspClass::flashWrite(uint32_t offset, const uint8_t *data, size_t size) { - size_t sizeAligned = size & ~3; +size_t EspClass::flashWriteUnalignedMemory(uint32_t address, const uint8_t *data, size_t size) { + size_t sizeLeft = (size & ~3); size_t currentOffset = 0; - if ((uintptr_t)data % 4 != 0) { - // Memory is unaligned, so we need to copy it to an aligned buffer - uint32_t alignedData[UNALIGNED_WRITE_BUFFER_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); - size_t sizeLeft = sizeAligned; + // 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) / FLASH_PAGE_SIZE)); - while (sizeLeft) { - size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); - memcpy(alignedData, data + currentOffset, willCopy); - // We now have data and size aligned to 4 bytes, so we can use aligned write - if (!flashWrite(offset + currentOffset, alignedData, willCopy)) - { - return false; - } - sizeLeft -= willCopy; - currentOffset += willCopy; + if (pageBreak) { + size_t byteCount = 4 - (address % 4); + + if (!flashReplaceBlock(address, data, byteCount)) { + return 0; } - } else { - // Pointer is properly aligned, so use aligned write - if (!flashWrite(offset, (uint32_t *)data, sizeAligned)) { - return false; + // 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)) + { + return 0; } - currentOffset = sizeAligned; + sizeLeft -= willCopy; + currentOffset += willCopy; + } + + return currentOffset; +} + +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 (!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; + } + return true; +} + +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) / FLASH_PAGE_SIZE)); + + if ((uintptr_t)data % 4 != 0 || size % 4 != 0 || pageBreak) { + return false; } - if (currentOffset < size) { - // Size was not aligned, but writes must be 4-byte aligned #if PUYA_SUPPORT - if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - uint32_t tempData; - if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { + rc = spi_flash_write_puya(address, const_cast(data), size); + } + else +#endif // PUYA_SUPPORT + { + rc = spi_flash_write(address, const_cast(data), size); + } + return rc == 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; } - for (size_t i = 0; i < size - currentOffset; i++) { - ((uint8_t *)tempData)[i] &= ((uint8_t *)data)[currentOffset + i]; - } - if (spi_flash_write(currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; + currentOffset += written; + sizeLeft -= written; + } else { + bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft) / 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; } } - else -#endif // PUYA_SUPPORT - { - uint32_t tempData; - if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { - return false; - } - memcpy(&tempData, (uint8_t *)data + currentOffset, size - currentOffset); - if (spi_flash_write(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + } + 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) / 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); } } return true; } -bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { +bool EspClass::flashRead(uint32_t address, uint8_t *data, size_t size) { size_t sizeAligned = size & ~3; size_t currentOffset = 0; if ((uintptr_t)data % 4 != 0) { - uint32_t alignedData[UNALIGNED_WRITE_BUFFER_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); + uint32_t alignedData[FLASH_PAGE_SIZE / sizeof(uint32_t)] __attribute__((aligned(4))); size_t sizeLeft = sizeAligned; while (sizeLeft) { size_t willCopy = std::min(sizeLeft, sizeof(alignedData)); // We read to our aligned buffer and then copy to data - if (!flashRead(offset + currentOffset, alignedData, willCopy)) + if (!flashRead(address + currentOffset, alignedData, willCopy)) { return false; } @@ -811,7 +917,7 @@ bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { } } else { // Pointer is properly aligned, so use aligned read - if (!flashRead(offset, (uint32_t *)data, sizeAligned)) { + if (!flashRead(address, (uint32_t *)data, sizeAligned)) { return false; } currentOffset = sizeAligned; @@ -819,7 +925,7 @@ bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { if (currentOffset < size) { uint32_t tempData; - if (spi_flash_read(offset + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { + if (spi_flash_read(address + currentOffset, &tempData, 4) != SPI_FLASH_RESULT_OK) { return false; } memcpy((uint8_t *)data + currentOffset, &tempData, size - currentOffset); @@ -828,11 +934,11 @@ bool EspClass::flashRead(uint32_t offset, uint8_t *data, size_t size) { return true; } -bool EspClass::flashRead(uint32_t offset, uint32_t *data, size_t size) { +bool EspClass::flashRead(uint32_t address, uint32_t *data, size_t size) { if ((uintptr_t)data % 4 != 0 || size % 4 != 0) { return false; } - return (spi_flash_read(offset, data, size) == SPI_FLASH_RESULT_OK); + return (spi_flash_read(address, data, size) == SPI_FLASH_RESULT_OK); } String EspClass::getSketchMD5() diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index 9bc6592a76..54f8d21752 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -150,10 +150,48 @@ class EspClass { bool checkFlashCRC(); bool flashEraseSector(uint32_t sector); - bool flashWrite(uint32_t offset, const uint32_t *data, size_t size); - bool flashWrite(uint32_t offset, const uint8_t *data, size_t size); - bool flashRead(uint32_t offset, uint32_t *data, size_t size); - bool flashRead(uint32_t offset, uint8_t *data, size_t size); + /** + * @brief Write @a size bytes from @a data to flash at @a address + * This overload requires @a data and @a size to be always 4 byte aligned and + * @a address to be 4 byte aligned if the write crossess page boundary, + * but guarantees no overhead (except on PUYA flashes) + * @param address address on flash where write should start, 4 byte alignment is conditional + * @param data input buffer, must be 4-byte aligned + * @param size amount of data, must be a multiple of 4 + * @return bool result of operation + * @retval true success + * @retval false failure to write to flash or incorrect alignment of params + */ + bool flashWrite(uint32_t address, const uint32_t *data, size_t size); + /** + * @brief Write @a size bytes from @a data to flash at @a address + * This overload handles all misalignment cases + * @param address address on flash where write should start + * @param data input buffer, passing unaligned memory will cause significant stack usage + * @param size amount of data, passing not multiple of 4 will cause additional reads and writes + * @return bool result of operation + */ + bool flashWrite(uint32_t address, const uint8_t *data, size_t size); + /** + * @brief Read @a size bytes to @a data to flash at @a address + * This overload requires @a data and @a size to be 4 byte aligned + * @param address address on flash where read should start + * @param data input buffer, must be 4-byte aligned + * @param size amount of data, must be a multiple of 4 + * @return bool result of operation + * @retval true success + * @retval false failure to read from flash or incorrect alignment of params + */ + bool flashRead(uint32_t address, uint32_t *data, size_t size); + /** + * @brief Read @a size bytes to @a data to flash at @a address + * This overload handles all misalignment cases + * @param address address on flash where read should start + * @param data input buffer, passing unaligned memory will cause significant stack usage + * @param size amount of data, passing not multiple of 4 will cause additional read + * @return bool result of operation + */ + bool flashRead(uint32_t address, uint8_t *data, size_t size); uint32_t getSketchSize(); String getSketchMD5(); @@ -177,6 +215,40 @@ class EspClass { #else uint32_t getCycleCount(); #endif // !defined(CORE_MOCK) + 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 + */ + 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 acces 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 + * @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 + */ + 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 + */ + bool flashWritePageBreak(uint32_t address, const uint8_t *data, size_t size); }; extern EspClass ESP; From b3b3957b3356d085dd2aab98bc2da254c3242fd8 Mon Sep 17 00:00:00 2001 From: Drzony Date: Sun, 11 Oct 2020 15:19:08 +0200 Subject: [PATCH 14/19] fixup! Add overloads for unaligned reads/writes --- tools/sdk/include/spi_flash_geometry.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/sdk/include/spi_flash_geometry.h b/tools/sdk/include/spi_flash_geometry.h index bb8c0ea22f..d6d7cf16d7 100644 --- a/tools/sdk/include/spi_flash_geometry.h +++ b/tools/sdk/include/spi_flash_geometry.h @@ -7,6 +7,7 @@ #define FLASH_SECTOR_SIZE 0x1000 #define FLASH_BLOCK_SIZE 0x10000 +#define FLASH_PAGE_SIZE 0x100 #define APP_START_OFFSET 0x1000 //pulled this define from spi_flash.h for reuse in the Arduino core without pulling in a bunch of other stuff From f2a6e0e112314d69757a3f77e7f9907c02986dce Mon Sep 17 00:00:00 2001 From: Drzony Date: Sun, 11 Oct 2020 23:22:31 +0200 Subject: [PATCH 15/19] Add tests for flashRead/flashWrite --- .../device/test_spi_flash/test_spi_flash.ino | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 tests/device/test_spi_flash/test_spi_flash.ino diff --git a/tests/device/test_spi_flash/test_spi_flash.ino b/tests/device/test_spi_flash/test_spi_flash.ino new file mode 100644 index 0000000000..a0cc1c5835 --- /dev/null +++ b/tests/device/test_spi_flash/test_spi_flash.ino @@ -0,0 +1,171 @@ +#include +#include + +void preinit() { + // (no C++ in function) + // disable wifi + ESP8266WiFiClass::preinitWiFiOff(); +} + +void setup() +{ + Serial.begin(115200); + BS_RUN(Serial); +} + +bool pretest() +{ + return true; +} + +void dumpBuffer(uint8_t *buffer, size_t len) +{ + for (size_t i = 0; i < len; i++) + { + Serial.printf("%02x ", buffer[i]); + } + Serial.println(); +} + +bool testFlash(uint32_t start_offset, uint8_t data_offset, size_t amount) +{ + static uint32_t *write_buffer = (uint32_t *)malloc(4096); + static uint32_t *read_buffer = (uint32_t *)malloc(4096); + + for (uint32_t i = 0; i < 1024; i++) + { + write_buffer[i] = (i + 100) * 33; + read_buffer[i] = 0xAAAAAAAA; + } + Serial.println("---------------------------------------------------"); + ESP.flashEraseSector(start_offset / 0x1000); + Serial.printf("Testing %d bytes @ %08x + %d\n", amount, start_offset, data_offset); + unsigned long start = micros(); + + if (!ESP.flashWrite(start_offset, (uint8_t *)write_buffer + data_offset, amount)) + { + Serial.printf("Write fail\n"); + return false; + } + if (!ESP.flashRead(start_offset, (uint8_t *)read_buffer + data_offset, amount)) + { + Serial.printf("Read fail\n"); + return false; + } + if (memcmp((uint8_t *)write_buffer + data_offset, (uint8_t *)read_buffer + data_offset, amount) != 0) + { + Serial.printf("Compare fail\n"); + dumpBuffer((uint8_t *)write_buffer, 8); + dumpBuffer((uint8_t *)read_buffer, 8); + return false; + } + Serial.printf("Write took %lu us\n", DL::Timing::diff(start, micros())); + return true; +} + +// Columns in test case names are as following: +// 1. Offset -> +o (4 byte aligned), -o (unaligned) +// 2. Memory pointer -> +m (4 byte aligned), -m (unaligned) +// 3. Size -> +s (4 byte ), -s (unaligned) +// 4. Number of pages crossed -> np + +// Aligned offset +// Aligned memory +// Aligned size +TEST_CASE("|+o|+m|+s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 0, 100)); +} +TEST_CASE("|+o|+m|+s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 0, 512)); +} +// Unaligned size +TEST_CASE("|+o|+m|-s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 0, 101)); +} +TEST_CASE("|+o|+m|-s|2p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 0, 515)); +} +// Unaligned memory +// Aligned size +TEST_CASE("|+o|-m|+s|0|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 1, 100)); +} +TEST_CASE("|+o|-m|+s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 3, 512)); +} +// Unaligned size +TEST_CASE("|+o|-m|-s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 2, 101)); +} +TEST_CASE("|+o|-m|-s|2p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0000, 1, 515)); +} +// Unaligned offset +// Aligned memory +// Aligned size +TEST_CASE("|-o|+m|+s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 0, 100)); +} +TEST_CASE("|-o|+m|+s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 0, 260)); +} +// Unaligned size +TEST_CASE("|-o|+m|-s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 0, 105)); +} +TEST_CASE("|-o|+m|-s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 0, 271)); +} +// Unaligned memory +// Aligned size +TEST_CASE("|-o|-m|+s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 1, 100)); +} +TEST_CASE("|-o|-m|+s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 2, 260)); +} +// Unaligned size +TEST_CASE("|-o|-m|-s|0p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 3, 105)); +} +TEST_CASE("|-o|-m|-s|1p|", "[spi_flash]") +{ + CHECK(testFlash(0xa0001, 1, 271)); +} + +TEST_CASE("Last bytes of page", "[spi_flash]") +{ + CHECK(testFlash(0xa0000 + 255, 0, 1)); + CHECK(testFlash(0xa0000 + 255, 1, 1)); + CHECK(testFlash(0xa0000 + 254, 0, 2)); + CHECK(testFlash(0xa0000 + 254, 1, 2)); + CHECK(testFlash(0xa0000 + 253, 0, 3)); + CHECK(testFlash(0xa0000 + 253, 1, 3)); +} + +TEST_CASE("Unaligned page cross only", "[spi_flash]") +{ + CHECK(testFlash(0xa0000 + 254, 0, 3)); + CHECK(testFlash(0xa0000 + 254, 1, 3)); + CHECK(testFlash(0xa0000 + 255, 0, 2)); + CHECK(testFlash(0xa0000 + 255, 1, 2)); +} + +void loop () +{ +} From eeb96fc8cdcb7b9ff3aef572f5ec83985ebfa5cf Mon Sep 17 00:00:00 2001 From: Drzony Date: Mon, 12 Oct 2020 09:35:02 +0200 Subject: [PATCH 16/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index a1b5de5190..fdb49ba1ab 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -747,7 +747,7 @@ size_t EspClass::flashWriteUnalignedMemory(uint32_t address, const uint8_t *data // 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) / FLASH_PAGE_SIZE)); + bool pageBreak = ((address % 4) != 0) && ((address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); if (pageBreak) { size_t byteCount = 4 - (address % 4); @@ -801,7 +801,7 @@ bool EspClass::flashWritePageBreak(uint32_t address, const uint8_t *data, size_t 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) / FLASH_PAGE_SIZE)); + 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; @@ -835,7 +835,7 @@ bool EspClass::flashWrite(uint32_t address, const uint8_t *data, size_t size) { currentOffset += written; sizeLeft -= written; } else { - bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft) / FLASH_PAGE_SIZE)); + bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); if (pageBreak) { while (sizeLeft) { @@ -880,7 +880,7 @@ bool EspClass::flashWrite(uint32_t address, const uint8_t *data, size_t size) { 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) / FLASH_PAGE_SIZE)); + bool pageBreak = ((address % 4) != 0 && (address / FLASH_PAGE_SIZE) != ((address + sizeLeft - 1) / FLASH_PAGE_SIZE)); if (pageBreak) { // Cross the page boundary From 7905a298ed4001ee0ffb48c34cfa39c66e88f9dc Mon Sep 17 00:00:00 2001 From: Drzony Date: Mon, 12 Oct 2020 09:35:13 +0200 Subject: [PATCH 17/19] fixup! Add tests for flashRead/flashWrite --- tests/device/test_spi_flash/test_spi_flash.ino | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/device/test_spi_flash/test_spi_flash.ino b/tests/device/test_spi_flash/test_spi_flash.ino index a0cc1c5835..33746d10b6 100644 --- a/tests/device/test_spi_flash/test_spi_flash.ino +++ b/tests/device/test_spi_flash/test_spi_flash.ino @@ -1,6 +1,8 @@ #include #include +BS_ENV_DECLARE(); + void preinit() { // (no C++ in function) // disable wifi @@ -59,7 +61,7 @@ bool testFlash(uint32_t start_offset, uint8_t data_offset, size_t amount) dumpBuffer((uint8_t *)read_buffer, 8); return false; } - Serial.printf("Write took %lu us\n", DL::Timing::diff(start, micros())); + Serial.printf("Write took %lu us\n", micros() - start); return true; } From 323ffc0912530098745f348b979b6abe0793a566 Mon Sep 17 00:00:00 2001 From: Drzony Date: Mon, 12 Oct 2020 14:24:48 +0200 Subject: [PATCH 18/19] fixup! Add tests for flashRead/flashWrite --- .../device/test_spi_flash/test_spi_flash.ino | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/device/test_spi_flash/test_spi_flash.ino b/tests/device/test_spi_flash/test_spi_flash.ino index 33746d10b6..fa7ea767b8 100644 --- a/tests/device/test_spi_flash/test_spi_flash.ino +++ b/tests/device/test_spi_flash/test_spi_flash.ino @@ -20,13 +20,30 @@ bool pretest() return true; } -void dumpBuffer(uint8_t *buffer, size_t len) +bool compareBuffers(uint32_t *first, uint32_t *second, size_t offset, size_t len) { - for (size_t i = 0; i < len; i++) + uint8_t *firstBytes = (uint8_t *)first; + uint8_t *secondBytes = (uint8_t *)second; + + for (size_t i = offset; i < offset + len; i++) { - Serial.printf("%02x ", buffer[i]); + if (firstBytes[i] != secondBytes[i]) + { + Serial.printf("Compare fail @ %u\n", i); + for (size_t j = i & ~3; j < (i & ~3) + 4; j++) + { + Serial.printf("%02x ", firstBytes[j]); + } + Serial.println(); + for (size_t j = i & ~3; j < (i & ~3) + 4; j++) + { + Serial.printf("%02x ", secondBytes[j]); + } + Serial.println(); + return false; + } } - Serial.println(); + return true; } bool testFlash(uint32_t start_offset, uint8_t data_offset, size_t amount) @@ -54,11 +71,8 @@ bool testFlash(uint32_t start_offset, uint8_t data_offset, size_t amount) Serial.printf("Read fail\n"); return false; } - if (memcmp((uint8_t *)write_buffer + data_offset, (uint8_t *)read_buffer + data_offset, amount) != 0) + if (!compareBuffers(write_buffer, read_buffer, data_offset, amount)) { - Serial.printf("Compare fail\n"); - dumpBuffer((uint8_t *)write_buffer, 8); - dumpBuffer((uint8_t *)read_buffer, 8); return false; } Serial.printf("Write took %lu us\n", micros() - start); From b919faab608ff6073eec386ef11ce832d400c95d Mon Sep 17 00:00:00 2001 From: Drzony Date: Mon, 12 Oct 2020 14:25:14 +0200 Subject: [PATCH 19/19] fixup! Add overloads for unaligned reads/writes --- cores/esp8266/Esp.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp index fdb49ba1ab..60990103f1 100644 --- a/cores/esp8266/Esp.cpp +++ b/cores/esp8266/Esp.cpp @@ -715,14 +715,14 @@ bool EspClass::flashReplaceBlock(uint32_t address, const uint8_t *value, uint32_ } #if PUYA_SUPPORT if (getFlashChipVendorId() == SPI_FLASH_VENDOR_PUYA) { - uint32_t tempData; - if (spi_flash_read(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { + 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++) { - ((uint8_t *)tempData)[i + alignmentOffset] &= value[i]; + tempData[i + alignmentOffset] &= value[i]; } - if (spi_flash_write(alignedAddress, &tempData, 4) != SPI_FLASH_RESULT_OK) { + if (spi_flash_write(alignedAddress, (uint32_t *)tempData, 4) != SPI_FLASH_RESULT_OK) { return false; } }