fixup! sys/riotboot: riotboot head v2

Author: fabian18

bootloaders/riotboot/main.c

@@ -49,7 +49,7 @@ void kernel_init(void)
             continue;
         }
         state = riotboot_hdr_get_img_state(riot_hdr);
-        if (state == RIOTBOOT_HDR_IMG_STATE_INSTALLED ||
+        if (state == RIOTBOOT_HDR_IMG_STATE_DEACTIVATED ||
             state == RIOTBOOT_HDR_IMG_STATE_DISMISSED) {
             /* skip image which previously failed to boot or is not yet activated */
             continue;

dist/tools/riotboot_gen_hdr/genhdr.c

@@ -50,8 +50,6 @@ static size_t populate_hdr_v2(riotboot_hdr_t *hdr, uint32_t ver, uint32_t addr)
     hdr->v2.version = ver;
     hdr->v2.start_addr = addr;
     hdr->v2.flags = 0xffffffff;
-    hdr->v2.flags &= ~RIOTBOOT_HDR_IMAGE_STATE_MASK;
-    hdr->v2.flags |= (RIOTBOOT_HDR_IMG_STATE_ACTIVATED << RIOTBOOT_HDR_IMAGE_STATE_SHIFT);
     /* calculate header checksum */
     hdr->v2.chksum = riotboot_hdr_checksum(hdr);
     return sizeof(hdr->v2);

examples/advanced/suit_update/main.c

@@ -25,6 +25,7 @@
 
 #include "suit/transport/coap.h"
 #ifdef MODULE_SUIT_STORAGE_FLASHWRITE
+#include "periph/flashpage.h"
 #include "riotboot/slot.h"
 #endif
 
@@ -69,19 +70,47 @@ static int cmd_print_riotboot_hdr(int argc, char **argv)
     (void)argc;
     (void)argv;
 
-    int current_slot = riotboot_slot_current();
-    if (current_slot != -1) {
+    if (riotboot_slot_current() == -1) {
+        printf("[FAILED] You're not running riotboot\n");
+        return -1;
+    }
+    if (argc == 1) {
+        printf("Usage: %s [<slot> [activate|deactivate|confirm|dismiss]]\n", argv[0]);
+        return 0;
+    }
+    int slot = atoi(argv[1]);
+    if (argc == 2) {
         /* Sometimes, udhcp output messes up the following printfs.  That
-         * confuses the test script. As a workaround, just disable interrupts
-         * for a while.
-         */
+        * confuses the test script. As a workaround, just disable interrupts
+        * for a while.
+        */
         unsigned state = irq_disable();
-        riotboot_slot_print_hdr(current_slot);
+        riotboot_slot_print_hdr(slot);
         irq_restore(state);
+        return 0;
     }
-    else {
-        printf("[FAILED] You're not running riotboot\n");
+    riotboot_hdr_t hdr = *riotboot_slot_get_hdr(slot);
+    if (riotboot_hdr_is_v2(&hdr)) {
+        if (!strcmp(argv[2], "activate")) {
+            riotboot_hdr_set_img_state(&hdr, RIOTBOOT_HDR_IMG_STATE_ACTIVATED);
+        }
+        else if (!strcmp(argv[2], "deactivate")) {
+            riotboot_hdr_set_img_state(&hdr, RIOTBOOT_HDR_IMG_STATE_DEACTIVATED);
+        }
+        else if (!strcmp(argv[2], "confirm")) {
+            riotboot_hdr_set_img_state(&hdr, RIOTBOOT_HDR_IMG_STATE_CONFIRMED);
+        }
+        else if (!strcmp(argv[2], "dismiss")) {
+            riotboot_hdr_set_img_state(&hdr, RIOTBOOT_HDR_IMG_STATE_DISMISSED);
+        }
+        else {
+            printf("Unknown command %s\n", argv[2]);
+            return -1;
+        }
+        flashpage_write(flashpage_addr(flashpage_page(riotboot_slot_get_hdr(slot))),
+                        &hdr.v2, sizeof(hdr.v2));
     }
+
     return 0;
 }
 

sys/include/riotboot/hdr.h

@@ -90,10 +90,11 @@ static_assert(CONFIG_RIOTBOOT_MAX_ATTEMPTS <= 4,
  * @brief   Image state enumeration
  */
 typedef enum riotboot_hdr_img_state {
-    RIOTBOOT_HDR_IMG_STATE_INSTALLED = 0x0f,    /**< Image is installed (1111) */
-    RIOTBOOT_HDR_IMG_STATE_ACTIVATED = 0x0e,    /**< Image is activated (1110) */
-    RIOTBOOT_HDR_IMG_STATE_CONFIRMED = 0x0c,    /**< Image is confirmed (1100) */
-    RIOTBOOT_HDR_IMG_STATE_DISMISSED = 0x08,    /**< Image is dismissed (1000) */
+    RIOTBOOT_HDR_IMG_STATE_INSTALLED   = 0x0f,      /**< Image is installed (1111) */
+    RIOTBOOT_HDR_IMG_STATE_DEACTIVATED = 0x0e,      /**< Image is deactivated (1110) */
+    RIOTBOOT_HDR_IMG_STATE_ACTIVATED   = 0x0c,      /**< Image is activated (1100) */
+    RIOTBOOT_HDR_IMG_STATE_CONFIRMED   = 0x08,      /**< Image is confirmed (1000) */
+    RIOTBOOT_HDR_IMG_STATE_DISMISSED   = 0x00,      /**< Image is dismissed (0000) */
 } riotboot_hdr_img_state_t;
 
 /**

sys/include/riotboot/slot.h

@@ -128,6 +128,18 @@ void riotboot_slot_dump_addrs(void);
  */
 void riotboot_slot_confirm(void);
 
+/**
+ * @brief  Activate or deactivate the other slot
+ *
+ * This function sets the image state to ACTIVATED or DEACTIVATED  in the header of the
+ * non-running image slot.
+ *
+ * A deactivated slot can be activated but an activated slot cannot be deactivated.
+ *
+ * @param[in]   active  if true, set to ACTIVATED, else set to INACTIVE
+ */
+void riotboot_slot_activate_other(bool active);
+
 /**
  * @brief  Get the size of a slot
  *

sys/riotboot/hdr.c

@@ -186,7 +186,7 @@ int riotboot_hdr_set_img_state(riotboot_hdr_t *riotboot_hdr,
 {
     if (riotboot_hdr_is_v2(riotboot_hdr)) {
         int current = riotboot_hdr_get_img_state(riotboot_hdr);
-        if (current == RIOTBOOT_HDR_IMG_STATE_INSTALLED &&
+        if (current == RIOTBOOT_HDR_IMG_STATE_DEACTIVATED &&
             state != RIOTBOOT_HDR_IMG_STATE_ACTIVATED) {
             return -1;
         }

sys/riotboot/slot.c

@@ -121,3 +121,20 @@ void riotboot_slot_confirm(void)
         }
     }
 }
+
+void riotboot_slot_activate_other(bool active)
+{
+    int slot = riotboot_slot_other();
+    riotboot_hdr_t riot_hdr = *riotboot_slot_get_hdr(slot);
+    if (riotboot_hdr_is_v2(&riot_hdr)) {
+        riotboot_hdr_set_img_state(&riot_hdr,
+                                   active ? RIOTBOOT_HDR_IMG_STATE_ACTIVATED
+                                          : RIOTBOOT_HDR_IMG_STATE_DEACTIVATED);
+        if (memcmp(&riot_hdr, riotboot_slot_get_hdr(slot), sizeof(riot_hdr.v2))) {
+#ifdef MODULE_PERIPH_FLASHPAGE
+            flashpage_write(flashpage_addr(flashpage_page(riotboot_slot_get_hdr(slot))),
+                            &riot_hdr.v2, sizeof(riot_hdr.v2));
+#endif
+        }
+    }
+}

sys/suit/storage/flashwrite.c

@@ -106,14 +106,6 @@ static int _flashwrite_install(suit_storage_t *storage,
     if (fin < 0) {
         return SUIT_ERR_STORAGE;
     }
-#if IS_USED(MODULE_RIOTBOOT_HDR_AUTO_ACTIVATE)
-    riotboot_hdr_t hdr = *riotboot_slot_get_hdr(fw->writer.target_slot);
-    if (riotboot_hdr_is_v2(&hdr)) {
-        riotboot_hdr_set_img_state(&hdr, RIOTBOOT_HDR_IMG_STATE_ACTIVATED);
-        flashpage_write(flashpage_addr(flashpage_page(riotboot_slot_get_hdr(fw->writer.target_slot))),
-                        &hdr.v2, sizeof(hdr.v2));
-    }
-#endif
     return fin;
 }
 

sys/suit/transport/worker.c

@@ -142,14 +142,21 @@ int suit_handle_manifest_buf(const uint8_t *buffer, size_t size)
 
     if ((res = riotboot_hdr_validate(hdr)) < 0) {
         LOG_INFO("suit_worker: target slot header invalid\n");
-        res = -EINVAL;
+        return -EINVAL;
     }
-    else if (riotboot_hdr_get_flags(hdr) != UINT32_MAX) {
+    if (riotboot_hdr_get_flags(hdr) != UINT32_MAX) {
         LOG_INFO("suit_worker: target slot header flags invalid\n");
-        res = -EINVAL;
+        return -EINVAL;
+    }
+    if (IS_USED(MODULE_RIOTBOOT_HDR_AUTO_ACTIVATE)) {
+        LOG_INFO("suit_worker: activating target slot\n");
+        riotboot_slot_activate_other(true);
+    }
+    else {
+        LOG_INFO("suit_worker: deactivating target slot\n");
+        riotboot_slot_activate_other(false);
     }
 #endif
-
     return res;
 }