From ffb1b3a336a1a49cd97681720a7bf4b23821b3ca Mon Sep 17 00:00:00 2001 From: Mike Date: Thu, 14 Mar 2024 14:55:56 +0000 Subject: [PATCH] PartitionStream requires blockErase, other OTA-related improvements (#2729) Leaving `blockErase` parameter at `false` when constructing a `PartitionStream` will result in corruption unless partition has already been erased. This is not always immediately apparent so can be difficult to diagnose. - **Breaking change** Replacing `blockErase` with an enumerated `mode` parameter enforces read-only behaviour by default (new feature and a good thing), with write access more explicitly defined. It's a simple change to existing code and will ensure it gets manually checked to ensure correct behaviour. - Update the `Basic_Ota` sample to check that an OTA update is not being attempted on the active running partition. This can happen with esp8266, for example, when running in temporary boot mode. (NB. Other checks may include active filing system partitions but that is application-specific.) - Update notes on migrating from Sming 4.2 to include updating boot sector. (Learning points froim #2727.) - Remove redundant flashsize calculation from rboot and update version string to `Sming v1.5` (from v1.4.2). Note this should have been done in #2258. - Fix rboot README regarding use of slot 2, should correspond with subtype `ota_2` not `ota_1`. --- .../Esp8266/Components/esp8266/startup.cpp | 11 ++++- .../Esp8266/Components/esp_no_wifi/README.rst | 6 +++ Sming/Components/Storage/ota-migration.rst | 49 +++++++++++++++---- .../Storage/src/PartitionStream.cpp | 6 ++- .../src/include/Storage/PartitionStream.h | 47 ++++++++++++++++-- Sming/Components/rboot/README.rst | 2 +- Sming/Components/rboot/rboot | 2 +- docs/source/information/flash.rst | 17 +++---- docs/source/upgrading/5.1-5.2.rst | 13 +++++ samples/Basic_Ota/app/application.cpp | 13 ++++- 10 files changed, 137 insertions(+), 29 deletions(-) create mode 100644 docs/source/upgrading/5.1-5.2.rst diff --git a/Sming/Arch/Esp8266/Components/esp8266/startup.cpp b/Sming/Arch/Esp8266/Components/esp8266/startup.cpp index 79d1d9794b..f8f6aa807d 100644 --- a/Sming/Arch/Esp8266/Components/esp8266/startup.cpp +++ b/Sming/Arch/Esp8266/Components/esp8266/startup.cpp @@ -18,6 +18,7 @@ extern void init(); extern void cpp_core_initialize(); +// Normal entry point for user application code from SDK extern "C" void user_init(void) { // Initialise hardware timers @@ -39,12 +40,20 @@ extern "C" void user_init(void) gdb_init(); + /* + * Load partition information. + * Normally this is done in user_pre_init() but if building without WiFi + * (via esp_no_wifi Component) then user_pre_init() is not called as none of the + * SDK-related partitions are required. + * Calling this a second time is a no-op. + */ Storage::initialize(); init(); // User code init } -extern "C" void ICACHE_FLASH_ATTR WEAK_ATTR user_pre_init(void) +// SDK 3+ calls this method to configure partitions +extern "C" void user_pre_init(void) { Storage::initialize(); diff --git a/Sming/Arch/Esp8266/Components/esp_no_wifi/README.rst b/Sming/Arch/Esp8266/Components/esp_no_wifi/README.rst index a81ca6fc75..2127075d74 100644 --- a/Sming/Arch/Esp8266/Components/esp_no_wifi/README.rst +++ b/Sming/Arch/Esp8266/Components/esp_no_wifi/README.rst @@ -24,6 +24,12 @@ The SDKnoWiFi implements the startup code and some system functions but contains which is provided by the SDK and in other parts of the Sming framework. We need to provide replacement functions to interoperate correctly with the remaining SDK code. +Advantages +---------- + +- Reduces code image size when networking/WiFi is not required +- Eliminates need for SDK-specific partitions (rf_cal, phy_init, sys_param) + Process ------- diff --git a/Sming/Components/Storage/ota-migration.rst b/Sming/Components/Storage/ota-migration.rst index bcd3044997..f16b951bc8 100644 --- a/Sming/Components/Storage/ota-migration.rst +++ b/Sming/Components/Storage/ota-migration.rst @@ -14,8 +14,11 @@ This function is documented in the NON-OS-SDK guide and was introduced in versio It is called by the SDK before user_init() so nothing else in the framework has yet been initialised, including any C++ static initialisers. -Sming uses this function to read the partition table into memory. -Applications may override this function to perform any custom upgrade operations. +Sming uses this function to read the partition table and pass the required information to the SDK +(via :c:func:`partition_table_regist`). + +By overriding (``wrapping``) this function, applications have the opportunity to perform any +additional checks or upgrades on the partition table before proceeding. Add to application's `component.mk`: @@ -33,20 +36,43 @@ Add this to your application:: { // Note: This file won't exist on initial build! IMPORT_FSTR(partitionTableData, PROJECT_DIR "/out/Esp8266/debug/firmware/partitions.bin") + + // Optionally include updated bootloader + // IMPORT_FSTR(rbootData, PROJECT_DIR "/out/Esp8266/debug/firmware/rboot.bin") } + // Called by SDK extern "C" void __wrap_user_pre_init(void) { + // Make sure code is compiled with expected partition table offset - adjust as required static_assert(PARTITION_TABLE_OFFSET == 0x3fa000, "Bad PTO"); + + // Attempt to load the partition table Storage::initialize(); - auto& flash = *Storage::spiFlash; + + auto& flash = *Storage::spiFlash; if(!flash.partitions()) { - LOAD_FSTR(data, partitionTableData) - flash.erase_range(PARTITION_TABLE_OFFSET, flash.getBlockSize()); - flash.write(PARTITION_TABLE_OFFSET, data, partitionTableData.size()); + /* + * No partitions were found, which implies this device was previously + * running with a pre Sming 4.3 firmware. + */ + // Write our partition table data to flash + { + LOAD_FSTR(data, partitionTableData) + flash.erase_range(PARTITION_TABLE_OFFSET, flash.getBlockSize()); + flash.write(PARTITION_TABLE_OFFSET, data, partitionTableData.size()); + } + + // Load the newly written partition information flash.loadPartitions(PARTITION_TABLE_OFFSET); + + // Optionally update bootloader + // LOAD_FSTR(data, rbootData) + // flash.erase_range(0, flash.getBlockSize()); + // flash.write(0, data, rbootData.size()); } + // Pass control back to Sming extern void __real_user_pre_init(void); __real_user_pre_init(); } @@ -64,6 +90,7 @@ An alternative method is to build the partition table layout in code, so there a // Support updating legacy devices without partition tables (Sming 4.2 and earlier) #ifdef ARCH_ESP8266 + // Need low-level definition for the on-flash `esp_partition_info_t` structure #include extern "C" void __wrap_user_pre_init(void) @@ -77,14 +104,15 @@ An alternative method is to build the partition table layout in code, so there a using FullType = Storage::Partition::FullType; using SubType = Storage::Partition::SubType; #define PT_ENTRY(name, fulltype, offset, size) \ - { ESP_PARTITION_MAGIC, FullType(fulltype).type, FullType(fulltype).subtype, offset, size, name, 0 } + { Storage::ESP_PARTITION_MAGIC, FullType(fulltype).type, FullType(fulltype).subtype, offset, size, name, 0 } + // Amend this layout as required so it corresponds with your existing device static constexpr Storage::esp_partition_info_t partitionTableData[] PROGMEM{ PT_ENTRY("spiFlash", Storage::Device::Type::flash, 0, 0x400000), PT_ENTRY("rom0", SubType::App::ota0, 0x2000, 0xf8000), - PT_ENTRY("rom1", SubType::App::ota1, 0x102000, 0xf8000), - PT_ENTRY("spiffs0", SubType::Data::spiffs, 0x200000, 0xc0000), - PT_ENTRY("spiffs1", SubType::Data::spiffs, 0x2c0000, 0xc0000), + PT_ENTRY("spiffs0", SubType::Data::spiffs, 0x100000, 0xc0000), + PT_ENTRY("rom1", SubType::App::ota1, 0x202000, 0xf8000), + PT_ENTRY("spiffs1", SubType::Data::spiffs, 0x300000, 0xc0000), PT_ENTRY("rf_cal", SubType::Data::rfCal, 0x3fb000, 0x1000), PT_ENTRY("phy_init", SubType::Data::phy, 0x3fc000, 0x1000), PT_ENTRY("sys_param", SubType::Data::sysParam, 0x3fd000, 0x3000), @@ -103,5 +131,6 @@ An alternative method is to build the partition table layout in code, so there a #endif // ARCH_ESP8266 +This example is based on a typical Sming 4.0 4MByte flash layout as for the ``Basic_rBoot`` sample application. The above examples are provided as templates and should be modified as required and tested thoroughly! diff --git a/Sming/Components/Storage/src/PartitionStream.cpp b/Sming/Components/Storage/src/PartitionStream.cpp index beec49dd7d..07980de01b 100644 --- a/Sming/Components/Storage/src/PartitionStream.cpp +++ b/Sming/Components/Storage/src/PartitionStream.cpp @@ -46,12 +46,16 @@ int PartitionStream::seekFrom(int offset, SeekOrigin origin) size_t PartitionStream::write(const uint8_t* data, size_t length) { + if(mode < Mode::Write) { + return 0; + } + auto len = std::min(size_t(size - writePos), length); if(len == 0) { return 0; } - if(blockErase) { + if(mode == Mode::BlockErase) { auto endPos = writePos + len; if(endPos > erasePos) { size_t blockSize = partition.getBlockSize(); diff --git a/Sming/Components/Storage/src/include/Storage/PartitionStream.h b/Sming/Components/Storage/src/include/Storage/PartitionStream.h index af0b02ddd5..41b5cd9070 100644 --- a/Sming/Components/Storage/src/include/Storage/PartitionStream.h +++ b/Sming/Components/Storage/src/include/Storage/PartitionStream.h @@ -15,6 +15,12 @@ namespace Storage { +enum class Mode { + ReadOnly, + Write, ///< Write but do not erase, region should be pre-erased + BlockErase, ///< Erase blocks as required before writing +}; + /** * @brief Stream operating directory on a Storage partition * @@ -33,9 +39,11 @@ class PartitionStream : public ReadWriteStream * @param blockErase Set to true to erase blocks before writing * * If blockErase is false then region must be pre-erased before writing. + * + * @deprecated Use `mode` parameter instead of `blockErase` */ - PartitionStream(Partition partition, storage_size_t offset, size_t size, bool blockErase = false) - : partition(partition), startOffset(offset), size(size), blockErase(blockErase) + SMING_DEPRECATED PartitionStream(Partition partition, storage_size_t offset, size_t size, bool blockErase) + : PartitionStream(partition, offset, size, blockErase ? Mode::BlockErase : Mode::ReadOnly) { } @@ -45,9 +53,38 @@ class PartitionStream : public ReadWriteStream * @param blockErase Set to true to erase blocks before writing * * If blockErase is false then partition must be pre-erased before writing. + * + * @deprecated Use `mode` parameter instead of `blockErase` + */ + SMING_DEPRECATED PartitionStream(Partition partition, bool blockErase) + : PartitionStream(partition, blockErase ? Mode::BlockErase : Mode::ReadOnly) + { + } + + /** + * @brief Access part of a partition using a stream + * @param partition + * @param offset Limit access to this starting offset + * @param size Limit access to this number of bytes from starting offset + * @param mode + * @note When writing in Mode::BlockErase, block erasure is only performed at the + * start of each block. Therefore if `offset` is not a block boundary then the corresponding + * block will *not* be erased first. + */ + PartitionStream(Partition partition, storage_size_t offset, size_t size, Mode mode = Mode::ReadOnly) + : partition(partition), startOffset(offset), size(size), mode(mode) + { + } + + /** + * @brief Access entire partition using stream + * @param partition + * @param mode + * + * If blockErase is false then partition must be pre-erased before writing. */ - PartitionStream(Partition partition, bool blockErase = false) - : partition(partition), startOffset{0}, size(partition.size()), blockErase(blockErase) + PartitionStream(Partition partition, Mode mode = Mode::ReadOnly) + : partition(partition), startOffset{0}, size(partition.size()), mode(mode) { } @@ -74,7 +111,7 @@ class PartitionStream : public ReadWriteStream uint32_t writePos{0}; uint32_t readPos{0}; uint32_t erasePos{0}; - bool blockErase; + Mode mode; }; } // namespace Storage diff --git a/Sming/Components/rboot/README.rst b/Sming/Components/rboot/README.rst index 95ddeb0203..7ce6c629ef 100644 --- a/Sming/Components/rboot/README.rst +++ b/Sming/Components/rboot/README.rst @@ -104,7 +104,7 @@ To enable slot 2, set these values: "address": "0x100000", "size": "512K", "type": "app", - "subtype": "ota_1" + "subtype": "ota_2" } } } diff --git a/Sming/Components/rboot/rboot b/Sming/Components/rboot/rboot index cc00071f59..4159458fe2 160000 --- a/Sming/Components/rboot/rboot +++ b/Sming/Components/rboot/rboot @@ -1 +1 @@ -Subproject commit cc00071f593ef7d55c0c0a7b97828173f9cda89b +Subproject commit 4159458fe25f93399a204e73d99331c37536f288 diff --git a/docs/source/information/flash.rst b/docs/source/information/flash.rst index 884da9ccc4..e999b304ac 100644 --- a/docs/source/information/flash.rst +++ b/docs/source/information/flash.rst @@ -21,16 +21,15 @@ A typical layout for a 4MByte device might look like this: (hex) (if any) (KB) (if applicable) ======= =============== ==== ========================= =================================================== 000000 1 rboot.bin Boot loader - 001000 4 rBoot configuration - 002000 4 Partition table - 003000 4 esp_init_data_default.bin PHY configuration data - 004000 12 blank.bin System parameter area - 006000 4 blank.bin RF Calibration data (Initialised to FFh) - 006000 4 Reserved - 008000 ROM_0_ADDR rom0.bin First ROM image - 100000 RBOOT_SPIFFS_0 - 208000 ROM_1_ADDR rom1.bin Second ROM image + 001000 4 rBoot configuration + 002000 ROM_0_ADDR rom0.bin First ROM image + 102000 ROM_1_ADDR rom1.bin Second ROM image + 200000 RBOOT_SPIFFS_0 300000 RBOOT_SPIFFS_1 + 3FA000 4 Partition table + 3FB000 4 blank.bin RF Calibration data (Initialised to FFh) + 3FC000 4 esp_init_data_default.bin PHY configuration data + 3FD000 12 blank.bin System parameter area ======= =============== ==== ========================= =================================================== diff --git a/docs/source/upgrading/5.1-5.2.rst b/docs/source/upgrading/5.1-5.2.rst new file mode 100644 index 0000000000..0dcbfa40f3 --- /dev/null +++ b/docs/source/upgrading/5.1-5.2.rst @@ -0,0 +1,13 @@ +From v5.1 to v5.2 +================= + +.. highlight:: c++ + +**Breaking change** + +The :cpp:class:`Storage::PartitionStream` constructors with ``blockErase`` parameter have been deprecated. +The intended default behaviour is read-only, however previously this also allowed writing without block erase. +This can result in corrupted flash contents where the flash has not been explicitly erased beforehand. + +The new constructors instead use a :cpp:enum:`Storage::Mode` so behaviour is more explicit. +The default is read-only and writes will now be failed. diff --git a/samples/Basic_Ota/app/application.cpp b/samples/Basic_Ota/app/application.cpp index 8b446c70b6..2663252230 100644 --- a/samples/Basic_Ota/app/application.cpp +++ b/samples/Basic_Ota/app/application.cpp @@ -54,6 +54,16 @@ void doUpgrade() // select rom slot to flash auto part = ota.getNextBootPartition(); + /* + * Applications should always include a sanity check to ensure partitions being updated are + * not in use. This should always included the application partition but should also consider + * filing system partitions, etc. which may be actively in use. + */ + if(part == ota.getRunningPartition()) { + Serial << F("May be running in temporary mode. Please reboot and try again.") << endl; + return; + } + #ifndef RBOOT_TWO_ROMS // flash rom to position indicated in the rBoot config rom table otaUpdater->addItem(ROM_0_URL, part); @@ -67,7 +77,8 @@ void doUpgrade() auto spiffsPart = findSpiffsPartition(part); if(spiffsPart) { // use user supplied values (defaults for 4mb flash in hardware config) - otaUpdater->addItem(SPIFFS_URL, spiffsPart, new Storage::PartitionStream(spiffsPart)); + otaUpdater->addItem(SPIFFS_URL, spiffsPart, + new Storage::PartitionStream(spiffsPart, Storage::Mode::BlockErase)); } // request switch and reboot on success