Simplify status management
This reworks much of the code, as well as tables, handling swap
state to make them simpler. Only states that require an actual
swap to be performed, perm/test/revert are checked for and acted
upon. Other possible states try to default to no operation
performed.
One extra state, BOOT_SWAP_TYPE_PANIC, was added to differentiate
between "soft" errors and unrecoverable ones (as flash read/write
errors).
Non well defined state changes after swap failures, as described
in MCUB-59 were also clean up.
This should also fix situations as described in MCUB-63, where
images generated using imgtool (magic + image_ok set) are written
to slot 0 and cause an incorrect "revert".
Signed-off-by: Fabio Utzig <utzig@apache.org>
diff --git a/boot/bootutil/src/bootutil_misc.c b/boot/bootutil/src/bootutil_misc.c
index 5c04c2f..ed879b8 100644
--- a/boot/bootutil/src/bootutil_misc.c
+++ b/boot/bootutil/src/bootutil_misc.c
@@ -48,97 +48,45 @@
struct boot_swap_table {
/** * For each field, a value of 0 means "any". */
- uint8_t bsw_magic_slot0;
- uint8_t bsw_magic_slot1;
- uint8_t bsw_image_ok_slot0;
- uint8_t bsw_image_ok_slot1;
+ uint8_t magic_slot0;
+ uint8_t magic_slot1;
+ uint8_t image_ok_slot0;
+ uint8_t image_ok_slot1;
- uint8_t bsw_swap_type;
+ uint8_t swap_type;
};
/**
* This set of tables maps image trailer contents to swap operation type.
* When searching for a match, these tables must be iterated sequentially.
+ *
+ * NOTE: the table order is very important. The settings in Slot 1 always
+ * are priority to Slot 0 and should be located earlier in the table.
+ *
+ * The table lists only states where there is action needs to be taken by
+ * the bootloader, as in starting/finishing a swap operation.
*/
static const struct boot_swap_table boot_swap_tables[] = {
{
- /* | slot-0 | slot-1 |
- *----------+------------+------------|
- * magic | Unset | Unset |
- * image-ok | Any | Any |
- * ---------+------------+------------'
- * swap: none |
- * -----------------------------------'
- */
- .bsw_magic_slot0 = BOOT_MAGIC_UNSET,
- .bsw_magic_slot1 = BOOT_MAGIC_UNSET,
- .bsw_image_ok_slot0 = 0,
- .bsw_image_ok_slot1 = 0,
- .bsw_swap_type = BOOT_SWAP_TYPE_NONE,
+ .magic_slot0 = 0,
+ .magic_slot1 = BOOT_MAGIC_GOOD,
+ .image_ok_slot0 = 0,
+ .image_ok_slot1 = 0xff,
+ .swap_type = BOOT_SWAP_TYPE_TEST,
},
-
{
- /* | slot-0 | slot-1 |
- *----------+------------+------------|
- * magic | Any | Good |
- * image-ok | Any | Unset |
- * ---------+------------+------------`
- * swap: test |
- * -----------------------------------'
- */
- .bsw_magic_slot0 = 0,
- .bsw_magic_slot1 = BOOT_MAGIC_GOOD,
- .bsw_image_ok_slot0 = 0,
- .bsw_image_ok_slot1 = 0xff,
- .bsw_swap_type = BOOT_SWAP_TYPE_TEST,
+ .magic_slot0 = 0,
+ .magic_slot1 = BOOT_MAGIC_GOOD,
+ .image_ok_slot0 = 0,
+ .image_ok_slot1 = 0x01,
+ .swap_type = BOOT_SWAP_TYPE_PERM,
},
-
{
- /* | slot-0 | slot-1 |
- *----------+------------+------------|
- * magic | Any | Good |
- * image-ok | Any | 0x01 |
- * ---------+------------+------------`
- * swap: permanent |
- * -----------------------------------'
- */
- .bsw_magic_slot0 = 0,
- .bsw_magic_slot1 = BOOT_MAGIC_GOOD,
- .bsw_image_ok_slot0 = 0,
- .bsw_image_ok_slot1 = 0x01,
- .bsw_swap_type = BOOT_SWAP_TYPE_PERM,
- },
-
- {
- /* | slot-0 | slot-1 |
- *----------+------------+------------|
- * magic | Good | Unset |
- * image-ok | 0xff | Any |
- * ---------+------------+------------'
- * swap: revert (test image running) |
- * -----------------------------------'
- */
- .bsw_magic_slot0 = BOOT_MAGIC_GOOD,
- .bsw_magic_slot1 = BOOT_MAGIC_UNSET,
- .bsw_image_ok_slot0 = 0xff,
- .bsw_image_ok_slot1 = 0,
- .bsw_swap_type = BOOT_SWAP_TYPE_REVERT,
- },
-
- {
- /* | slot-0 | slot-1 |
- *----------+------------+------------|
- * magic | Good | Unset |
- * image-ok | 0x01 | Any |
- * ---------+------------+------------'
- * swap: none (confirmed test image) |
- * -----------------------------------'
- */
- .bsw_magic_slot0 = BOOT_MAGIC_GOOD,
- .bsw_magic_slot1 = BOOT_MAGIC_UNSET,
- .bsw_image_ok_slot0 = 0x01,
- .bsw_image_ok_slot1 = 0,
- .bsw_swap_type = BOOT_SWAP_TYPE_NONE,
+ .magic_slot0 = BOOT_MAGIC_GOOD,
+ .magic_slot1 = BOOT_MAGIC_UNSET,
+ .image_ok_slot0 = 0xff,
+ .image_ok_slot1 = 0,
+ .swap_type = BOOT_SWAP_TYPE_REVERT,
},
};
@@ -357,40 +305,41 @@
boot_swap_type(void)
{
const struct boot_swap_table *table;
- struct boot_swap_state state_slot0;
- struct boot_swap_state state_slot1;
+ struct boot_swap_state slot0;
+ struct boot_swap_state slot1;
int rc;
int i;
- rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_0, &state_slot0);
- assert(rc == 0);
+ rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_0, &slot0);
+ if (rc) {
+ return BOOT_SWAP_TYPE_PANIC;
+ }
- rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_1, &state_slot1);
- assert(rc == 0);
+ rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_1, &slot1);
+ if (rc) {
+ return BOOT_SWAP_TYPE_PANIC;
+ }
for (i = 0; i < BOOT_SWAP_TABLES_COUNT; i++) {
table = boot_swap_tables + i;
- if ((table->bsw_magic_slot0 == 0 ||
- table->bsw_magic_slot0 == state_slot0.magic) &&
- (table->bsw_magic_slot1 == 0 ||
- table->bsw_magic_slot1 == state_slot1.magic) &&
- (table->bsw_image_ok_slot0 == 0 ||
- table->bsw_image_ok_slot0 == state_slot0.image_ok) &&
- (table->bsw_image_ok_slot1 == 0 ||
- table->bsw_image_ok_slot1 == state_slot1.image_ok)) {
+ if ((!table->magic_slot0 || table->magic_slot0 == slot0.magic) &&
+ (!table->magic_slot1 || table->magic_slot1 == slot1.magic) &&
+ (!table->image_ok_slot0 || table->image_ok_slot0 == slot0.image_ok) &&
+ (!table->image_ok_slot1 || table->image_ok_slot1 == slot1.image_ok)) {
BOOT_LOG_INF("Swap type: %s",
- table->bsw_swap_type == BOOT_SWAP_TYPE_NONE ? "none" :
- table->bsw_swap_type == BOOT_SWAP_TYPE_TEST ? "test" :
- table->bsw_swap_type == BOOT_SWAP_TYPE_PERM ? "perm" :
- table->bsw_swap_type == BOOT_SWAP_TYPE_REVERT ? "revert" :
- table->bsw_swap_type == BOOT_SWAP_TYPE_FAIL ? "fail" :
- "BUG; can't happen");
- return table->bsw_swap_type;
+ table->swap_type == BOOT_SWAP_TYPE_TEST ? "test" :
+ table->swap_type == BOOT_SWAP_TYPE_PERM ? "perm" :
+ table->swap_type == BOOT_SWAP_TYPE_REVERT ? "revert" :
+ "BUG; can't happen");
+ assert(table->swap_type == BOOT_SWAP_TYPE_TEST ||
+ table->swap_type == BOOT_SWAP_TYPE_PERM ||
+ table->swap_type == BOOT_SWAP_TYPE_REVERT);
+ return table->swap_type;
}
}
- assert(0);
+ BOOT_LOG_INF("Swap type: none");
return BOOT_SWAP_TYPE_NONE;
}
diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index 178a9c7..205422d 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -126,21 +126,6 @@
#define BOOT_STATUS_TABLES_COUNT \
(sizeof boot_status_tables / sizeof boot_status_tables[0])
-/**
- * This table indicates the next swap type that should be performed. The first
- * column contains the current swap type. The second column contains the swap
- * type that should be effected after the first completes.
- */
-static const uint8_t boot_swap_trans_table[][2] = {
- /* From To */
- { BOOT_SWAP_TYPE_REVERT, BOOT_SWAP_TYPE_NONE },
- { BOOT_SWAP_TYPE_PERM, BOOT_SWAP_TYPE_NONE },
- { BOOT_SWAP_TYPE_TEST, BOOT_SWAP_TYPE_REVERT },
-};
-
-#define BOOT_SWAP_TRANS_TABLE_SIZE \
- (sizeof boot_swap_trans_table / sizeof boot_swap_trans_table[0])
-
#define BOOT_LOG_SWAP_STATE(area, state) \
BOOT_LOG_INF("%s: magic=%s, copy_done=0x%x, image_ok=0x%x", \
(area), \
@@ -201,25 +186,24 @@
/**
* Calculates the type of swap that just completed.
+ *
+ * This is used when a swap is interrupted by an external event. After
+ * finishing the swap operation determines what the initial request was.
*/
static int
boot_previous_swap_type(void)
{
int post_swap_type;
- int i;
post_swap_type = boot_swap_type();
- for (i = 0; i < BOOT_SWAP_TRANS_TABLE_SIZE; i++){
- if (boot_swap_trans_table[i][1] == post_swap_type) {
- return boot_swap_trans_table[i][0];
- }
+ switch (post_swap_type) {
+ case BOOT_SWAP_TYPE_NONE : return BOOT_SWAP_TYPE_PERM;
+ case BOOT_SWAP_TYPE_REVERT : return BOOT_SWAP_TYPE_TEST;
+ case BOOT_SWAP_TYPE_PANIC : return BOOT_SWAP_TYPE_PANIC;
}
- /* XXX: Temporary assert. */
- assert(0);
-
- return BOOT_SWAP_TYPE_REVERT;
+ return BOOT_SWAP_TYPE_FAIL;
}
static int
@@ -605,18 +589,16 @@
boot_validated_swap_type(void)
{
int swap_type;
- int rc;
swap_type = boot_swap_type();
- if (swap_type == BOOT_SWAP_TYPE_NONE) {
- /* Continue using slot 0. */
- return BOOT_SWAP_TYPE_NONE;
- }
-
- /* Boot loader wants to switch to slot 1. Ensure image is valid. */
- rc = boot_validate_slot(1);
- if (rc != 0) {
- return BOOT_SWAP_TYPE_FAIL;
+ switch (swap_type) {
+ case BOOT_SWAP_TYPE_TEST:
+ case BOOT_SWAP_TYPE_PERM:
+ case BOOT_SWAP_TYPE_REVERT:
+ /* Boot loader wants to switch to slot 1. Ensure image is valid. */
+ if (boot_validate_slot(1) != 0) {
+ swap_type = BOOT_SWAP_TYPE_FAIL;
+ }
}
return swap_type;
@@ -1191,6 +1173,12 @@
rc = boot_copy_image(&bs);
assert(rc == 0);
+ /* NOTE: here we have finished a swap resume. The initial request
+ * was either a TEST or PERM swap, which now after the completed
+ * swap will be determined to be respectively REVERT (was TEST)
+ * or NONE (was PERM).
+ */
+
/* Extrapolate the type of the partial swap. We need this
* information to know how to mark the swap complete in flash.
*/
@@ -1301,6 +1289,13 @@
reload_headers = true;
break;
+ case BOOT_SWAP_TYPE_PANIC:
+ /* TODO: what to do it a fatal error like flash read/write error
+ * happened?
+ */
+ assert(0);
+ break;
+
default:
assert(0);
slot = 0;