Merge pull request #15135 from bergzand/pr/riotboot/flashwrite_raw

riotboot/flashwrite: add flashpage_raw capabilities

sys/Makefile.dep

@@ -891,6 +891,7 @@ endif
 ifneq (,$(filter riotboot_flashwrite, $(USEMODULE)))
   USEMODULE += riotboot_slot
   FEATURES_REQUIRED += periph_flashpage
+  FEATURES_OPTIONAL += periph_flashpage_raw
 endif
 
 ifneq (,$(filter riotboot_slot, $(USEMODULE)))

sys/include/riotboot/flashwrite.h

@@ -41,6 +41,15 @@
  * 2. write image starting at second block
  * 3. write first block
  *
+ * When using periph_flashpage_raw with this module, the need to buffer a full
+ * flashpage page is removed, instead it must buffer two times the
+ * FLASHPAGE_RAW_BLOCKSIZE. One is used to buffer the current write block,
+ * the other buffers the first chunk (offset zero, page zero). This first
+ * chunk is written when finalizing the flash operation. The minimal size for
+ * RIOTBOOT_FLASHPAGE_BUFFER_SIZE is 4, at least the riotboot magic number must
+ * fit into this and FLASHPAGE_SIZE must be a multiple of
+ * RIOTBOOT_FLASHPAGE_BUFFER_SIZE
+ *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
  * @author      Koen Zandberg <koen@bergzand.net>
  *
@@ -57,6 +66,36 @@ extern "C" {
 #include "riotboot/slot.h"
 #include "periph/flashpage.h"
 
+/**
+ * @brief Enable/disable raw writes to flash
+ */
+#ifndef CONFIG_RIOTBOOT_FLASHWRITE_RAW
+#define CONFIG_RIOTBOOT_FLASHWRITE_RAW  IS_ACTIVE(MODULE_PERIPH_FLASHPAGE_RAW)
+#endif
+
+/**
+ * @brief Intermediate buffer size for firmware image data
+ */
+#if CONFIG_RIOTBOOT_FLASHWRITE_RAW
+
+#if (FLASHPAGE_RAW_BLOCKSIZE < 4)
+#define RIOTBOOT_FLASHPAGE_BUFFER_SIZE 4
+#else
+#define RIOTBOOT_FLASHPAGE_BUFFER_SIZE FLASHPAGE_RAW_BLOCKSIZE
+#endif
+
+#else /* CONFIG_RIOTBOOT_FLASHWRITE_RAW */
+
+#define RIOTBOOT_FLASHPAGE_BUFFER_SIZE FLASHPAGE_SIZE
+
+#endif /* !CONFIG_RIOTBOOT_FLASHWRITE_RAW */
+
+/**
+ * @brief Extra attributes required for the firmware intermediate buffer
+ */
+#define RIOTBOOT_FLASHPAGE_BUFFER_ATTRS \
+    __attribute__((aligned(FLASHPAGE_RAW_ALIGNMENT)))
+
 /**
  * @brief   firmware update state structure
  *
@@ -67,7 +106,20 @@ typedef struct {
     int target_slot;                        /**< update targets this slot     */
     size_t offset;                          /**< update is at this position   */
     unsigned flashpage;                     /**< update is at this flashpage  */
-    uint8_t flashpage_buf[FLASHPAGE_SIZE];  /**< flash writing buffer         */
+
+    /**
+     * @brief flash writing buffer
+     */
+    uint8_t RIOTBOOT_FLASHPAGE_BUFFER_ATTRS
+        flashpage_buf[RIOTBOOT_FLASHPAGE_BUFFER_SIZE];
+#if CONFIG_RIOTBOOT_FLASHWRITE_RAW || DOXYGEN
+    /**
+     * @brief Buffer for the first chunk containing the checksum when using
+     *        FLASHWRITE_RAW
+     */
+    uint8_t RIOTBOOT_FLASHPAGE_BUFFER_ATTRS
+        firstblock_buf[RIOTBOOT_FLASHPAGE_BUFFER_SIZE];
+#endif
 } riotboot_flashwrite_t;
 
 /**

sys/riotboot/flashwrite.c

@@ -44,6 +44,8 @@ int riotboot_flashwrite_init_raw(riotboot_flashwrite_t *state, int target_slot,
                              size_t offset)
 {
     assert(offset <= FLASHPAGE_SIZE);
+    /* the flashpage size must be a multiple of the riotboot flashpage buffer */
+    static_assert(!(FLASHPAGE_SIZE % RIOTBOOT_FLASHPAGE_BUFFER_SIZE));
 
     LOG_INFO(LOG_PREFIX "initializing update to target slot %i\n",
              target_slot);
@@ -54,14 +56,37 @@ int riotboot_flashwrite_init_raw(riotboot_flashwrite_t *state, int target_slot,
     state->target_slot = target_slot;
     state->flashpage = flashpage_page((void *)riotboot_slot_get_hdr(target_slot));
 
+    if (CONFIG_RIOTBOOT_FLASHWRITE_RAW && offset) {
+        /* Erase the first page only if the offset (!=0) specifies that there is
+         * a checksum or other mechanism at the start of the page. */
+        flashpage_write(state->flashpage, NULL);
+    }
+
     return 0;
 }
 
 int riotboot_flashwrite_flush(riotboot_flashwrite_t *state)
 {
-    if (flashpage_write_and_verify(state->flashpage, state->flashpage_buf) != FLASHPAGE_OK) {
+    if (CONFIG_RIOTBOOT_FLASHWRITE_RAW) {
-        LOG_WARNING(LOG_PREFIX "error writing flashpage %u!\n", state->flashpage);
+        /* Check if there is leftover data in the buffer */
-        return -1;
+        size_t flashwrite_buffer_pos = state->offset % RIOTBOOT_FLASHPAGE_BUFFER_SIZE;
+        if (flashwrite_buffer_pos == 0) {
+            return 0;
+        }
+        uint8_t* slot_start =
+            (uint8_t*)riotboot_slot_get_hdr(state->target_slot);
+        /* Get the offset of the remaining chunk */
+        size_t flashpage_pos = state->offset - flashwrite_buffer_pos;
+        /* Write remaining chunk */
+        flashpage_write_raw(slot_start + flashpage_pos,
+                            state->flashpage_buf,
+                            RIOTBOOT_FLASHPAGE_BUFFER_SIZE);
+    }
+    else {
+        if (flashpage_write_and_verify(state->flashpage, state->flashpage_buf) != FLASHPAGE_OK) {
+            LOG_WARNING(LOG_PREFIX "error writing flashpage %u!\n", state->flashpage);
+            return -1;
+        }
     }
     return 0;
 }
@@ -73,22 +98,52 @@ int riotboot_flashwrite_putbytes(riotboot_flashwrite_t *state,
 
     while (len) {
         size_t flashpage_pos = state->offset % FLASHPAGE_SIZE;
-        size_t flashpage_avail = FLASHPAGE_SIZE - flashpage_pos;
+        size_t flashwrite_buffer_pos = state->offset % RIOTBOOT_FLASHPAGE_BUFFER_SIZE;
+        size_t flashpage_avail = RIOTBOOT_FLASHPAGE_BUFFER_SIZE - flashwrite_buffer_pos;
 
         size_t to_copy = min(flashpage_avail, len);
 
-        memcpy(state->flashpage_buf + flashpage_pos, bytes, to_copy);
+        if (CONFIG_RIOTBOOT_FLASHWRITE_RAW && flashpage_pos == 0) {
+            /* Erase the next page */
+            state->flashpage++;
+            flashpage_write(state->flashpage, NULL);
+        }
+        if (CONFIG_RIOTBOOT_FLASHWRITE_RAW &&
+                flashwrite_buffer_pos == 0) {
+            memset(state->flashpage_buf, 0, RIOTBOOT_FLASHPAGE_BUFFER_SIZE);
+        };
+
+        memcpy(state->flashpage_buf + flashwrite_buffer_pos, bytes, to_copy);
         flashpage_avail -= to_copy;
 
+
         state->offset += to_copy;
         bytes += to_copy;
         len -= to_copy;
         if ((!flashpage_avail) || (!more)) {
-            if (flashpage_write_and_verify(state->flashpage, state->flashpage_buf) != FLASHPAGE_OK) {
+#if CONFIG_RIOTBOOT_FLASHWRITE_RAW  /* Guards access to state::firstblock_buf */
+            void * addr = flashpage_addr(state->flashpage);
+            if (addr == riotboot_slot_get_hdr(state->target_slot) &&
+                    state->offset == RIOTBOOT_FLASHPAGE_BUFFER_SIZE) {
+                /* Skip flashing the first block, store it for later to flash it
+                 * during the flashwrite_finish function */
+                memcpy(state->firstblock_buf,
+                       state->flashpage_buf, RIOTBOOT_FLASHPAGE_BUFFER_SIZE);
+            }
+            else {
+                flashpage_write_raw((uint8_t*)addr + flashpage_pos,
+                                    state->flashpage_buf,
+                                    RIOTBOOT_FLASHPAGE_BUFFER_SIZE);
+            }
+#else
+            int res = flashpage_write_and_verify(state->flashpage,
+                                                 state->flashpage_buf);
+            if (res != FLASHPAGE_OK) {
                 LOG_WARNING(LOG_PREFIX "error writing flashpage %u!\n", state->flashpage);
                 return -1;
             }
             state->flashpage++;
+#endif
         }
     }
 
@@ -104,6 +159,10 @@ int riotboot_flashwrite_finish_raw(riotboot_flashwrite_t *state,
 
     uint8_t *slot_start = (uint8_t *)riotboot_slot_get_hdr(state->target_slot);
 
+#if CONFIG_RIOTBOOT_FLASHWRITE_RAW
+    memcpy(state->firstblock_buf, bytes, len);
+    flashpage_write_raw(slot_start, state->firstblock_buf, RIOTBOOT_FLASHPAGE_BUFFER_SIZE);
+#else
     uint8_t *firstpage;
 
     if (len < FLASHPAGE_SIZE) {
@@ -118,14 +177,13 @@ int riotboot_flashwrite_finish_raw(riotboot_flashwrite_t *state,
     }
 
     int flashpage = flashpage_page((void *)slot_start);
-    if (flashpage_write_and_verify(flashpage, firstpage) != FLASHPAGE_OK) {
+    if (flashpage_write_and_verify(flashpage, firstpage) == FLASHPAGE_OK) {
-        LOG_WARNING(LOG_PREFIX "re-flashing first block failed!\n");
+        LOG_INFO(LOG_PREFIX "riotboot flashing completed successfully\n");
-        goto out;
+        res = 0;
     }
-
+    else {
-    LOG_INFO(LOG_PREFIX "riotboot flashing completed successfully\n");
+        LOG_WARNING(LOG_PREFIX "re-flashing first block failed!\n");
-    res = 0;
+    }
-
+#endif /* !CONFIG_RIOTBOOT_FLASHWRITE_RAW */
-out:
     return res;
 }

sys/suit/storage/flashwrite.c

@@ -37,6 +37,9 @@ static inline suit_storage_flashwrite_t *_get_fw(suit_storage_t *storage)
 static int _flashwrite_init(suit_storage_t *storage)
 {
     (void)storage;
+
+    LOG_INFO("Storage size %u\n", (unsigned)sizeof(suit_storage_flashwrite_t));
+
     return 0;
 }
 
@@ -100,7 +103,7 @@ static int _flashwrite_install(suit_storage_t *storage,
 static int _flashwrite_read(suit_storage_t *storage, uint8_t *buf,
                             size_t offset, size_t len)
 {
-    (void)storage;
+    suit_storage_flashwrite_t *fw = _get_fw(storage);
 
     static const char _prefix[] = "RIOT";
     static const size_t _prefix_len = sizeof(_prefix) - 1;
@@ -116,6 +119,28 @@ static int _flashwrite_read(suit_storage_t *storage, uint8_t *buf,
         buf += prefix_to_copy;
 
     }
+
+#if CONFIG_RIOTBOOT_FLASHWRITE_RAW
+    /* Insert the first chunk from the separate buffer here, there are cases
+     * where the chunk size is 4 bytes and we can skip this because it only
+     * contains the magic number already copied above. */
+    if (offset < RIOTBOOT_FLASHPAGE_BUFFER_SIZE) {
+        const size_t chunk_remaining =
+            RIOTBOOT_FLASHPAGE_BUFFER_SIZE - _prefix_len;
+        /* How much of the first page must be copied */
+        size_t firstpage_to_copy = len > chunk_remaining ?
+            (chunk_remaining) : len;
+        /* Copy the first buffer */
+        memcpy(buf, fw->writer.firstblock_buf + offset, firstpage_to_copy);
+
+        offset += firstpage_to_copy;
+        buf += firstpage_to_copy;
+        len -= firstpage_to_copy;
+    }
+#else
+    (void)fw;
+#endif /* CONFIG_RIOTBOOT_FLASHWRITE_RAW */
+
     if (offset + len > slot_size) {
         return -1;
     }