Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 0fa458469 -> f95f30afd


boot loader - Fix reset during revert.

Prior to this fix:

    If the boot loader resets while it is in the middle of a revert (user
    tests a new image, reboots, then reboots again), the behavior is
    incorrect.  The image under test becomes confirmed - it should be
    the original image that is confirmed.  The problem is that it is not
    currently possible to determine the status of the previous swap
    operation under these conditions.

Now:

    The boot loader checks slot 0 for status of an interrupted image
    revert.  An image revert is identified by an unwritten image trailer
    in slot 0.  There are two conditions under which slot 0's trailer
    is unwritten:

        * No swaps ever performed, and therefore no status bytes have
          been written.

        * Boot loader was in mid-revert when it got reset.

    When a revert swap completes, the boot loader writes a trailer to
    slot 0, making it look like a test image that just got confirmed.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/f95f30af
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/f95f30af
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/f95f30af

Branch: refs/heads/develop
Commit: f95f30afd64bb2a89ad9ca05082a378ba4aff9a2
Parents: 0fa4584
Author: Christopher Collins <ccoll...@apache.org>
Authored: Mon Oct 17 08:32:47 2016 -0700
Committer: Christopher Collins <ccoll...@apache.org>
Committed: Mon Oct 17 08:37:28 2016 -0700

----------------------------------------------------------------------
 boot/bootutil/src/bootutil_misc.c  | 67 +++++++++++++++++++++++----------
 boot/bootutil/src/bootutil_priv.h  |  1 +
 boot/bootutil/src/loader.c         |  1 +
 boot/bootutil/test/src/boot_test.c | 11 +++---
 4 files changed, 56 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/f95f30af/boot/bootutil/src/bootutil_misc.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/src/bootutil_misc.c 
b/boot/bootutil/src/bootutil_misc.c
index 87e17e8..675308f 100644
--- a/boot/bootutil/src/bootutil_misc.c
+++ b/boot/bootutil/src/bootutil_misc.c
@@ -50,21 +50,6 @@ static const struct boot_status_table boot_status_tables[] = 
{
     {
         /*           | slot-0     | scratch    |
          * ----------+------------+------------|
-         *     magic | 0xffffffff | 0xffffffff |
-         * copy-done | 0x**       | N/A        |
-         * ----------+------------+------------'
-         * status: none                        |
-         * ------------------------------------'
-         */
-        .bst_magic_slot0 =      0xffffffff,
-        .bst_magic_scratch =    0xffffffff,
-        .bst_copy_done_slot0 =  0,
-        .bst_status_source =    BOOT_STATUS_SOURCE_NONE,
-    },
-
-    {
-        /*           | slot-0     | scratch    |
-         * ----------+------------+------------|
          *     magic | 0x12344321 | 0x******** |
          * copy-done | 0x01       | N/A        |
          * ----------+------------+------------'
@@ -106,6 +91,25 @@ static const struct boot_status_table boot_status_tables[] 
= {
         .bst_copy_done_slot0 =  0,
         .bst_status_source =    BOOT_STATUS_SOURCE_SCRATCH,
     },
+
+    {
+        /*           | slot-0     | scratch    |
+         * ----------+------------+------------|
+         *     magic | 0xffffffff | 0xffffffff |
+         * copy-done | 0xff       | N/A        |
+         * ----------+------------+------------|
+         * status: slot0                       |
+         * 
------------------------------------+-------------------------------+
+         * This represents one of two cases:                                   
|
+         * o No swaps ever (no status to read anyway, so no harm in checking). 
|
+         * o Mid-revert; status in slot 0.                                     
|
+         * 
--------------------------------------------------------------------'
+         */
+        .bst_magic_slot0 =      0xffffffff,
+        .bst_magic_scratch =    0,
+        .bst_copy_done_slot0 =  0xff,
+        .bst_status_source =    BOOT_STATUS_SOURCE_SLOT0,
+    },
 };
 
 #define BOOT_STATUS_TABLES_COUNT \
@@ -591,22 +595,24 @@ boot_read_status(struct boot_status *bs)
 
     switch (status_loc) {
     case BOOT_STATUS_SOURCE_NONE:
-        return 0;
+        break;
 
     case BOOT_STATUS_SOURCE_SCRATCH:
         boot_scratch_loc(&flash_id, &off);
         boot_read_status_bytes(bs, flash_id, off);
-        return 1;
+        break;
 
     case BOOT_STATUS_SOURCE_SLOT0:
         boot_magic_loc(0, &flash_id, &off);
         boot_read_status_bytes(bs, flash_id, off);
-        return 1;
+        break;
 
     default:
         assert(0);
-        return 0;
+        break;
     }
+
+    return bs->idx != 0 || bs->state != 0;
 }
 
 
@@ -659,6 +665,29 @@ boot_finalize_test_swap(void)
     return rc;
 }
 
+/**
+ * Marks a reverted image in slot 0 as confirmed.  This is necessary to ensure
+ * the status bytes from the image revert operation don't get processed on a
+ * subsequent boot.
+ */
+int
+boot_finalize_revert_swap(void)
+{
+    struct boot_img_trailer bit;
+    uint32_t off;
+    uint8_t flash_id;
+    int rc;
+
+    boot_magic_loc(0, &flash_id, &off);
+
+    bit.bit_copy_start = BOOT_IMG_MAGIC;
+    bit.bit_copy_done = 1;
+    bit.bit_img_ok = 1;
+    rc = hal_flash_write(flash_id, off, &bit, sizeof bit);
+
+    return rc;
+}
+
 void
 boot_set_image_slot_split(void)
 {

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/f95f30af/boot/bootutil/src/bootutil_priv.h
----------------------------------------------------------------------
diff --git a/boot/bootutil/src/bootutil_priv.h 
b/boot/bootutil/src/bootutil_priv.h
index 3a40abc..1279e41 100644
--- a/boot/bootutil/src/bootutil_priv.h
+++ b/boot/bootutil/src/bootutil_priv.h
@@ -69,6 +69,7 @@ int boot_write_status(struct boot_status *bs);
 int boot_read_status(struct boot_status *bs);
 int boot_schedule_test_swap(void);
 int boot_finalize_test_swap(void);
+int boot_finalize_revert_swap(void);
 
 void boot_magic_loc(int slot_num, uint8_t *flash_id, uint32_t *off);
 void boot_scratch_loc(uint8_t *flash_id, uint32_t *off);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/f95f30af/boot/bootutil/src/loader.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index c8dac3e..db8b73a 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -591,6 +591,7 @@ boot_go(const struct boot_req *req, struct boot_rsp *rsp)
 
     case BOOT_SWAP_TYPE_REVERT:
         slot = 1;
+        boot_finalize_revert_swap();
         break;
 
     default:

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/f95f30af/boot/bootutil/test/src/boot_test.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/test/src/boot_test.c 
b/boot/bootutil/test/src/boot_test.c
index 6184e9e..205993c 100644
--- a/boot/bootutil/test/src/boot_test.c
+++ b/boot/bootutil/test/src/boot_test.c
@@ -286,6 +286,11 @@ boot_test_util_write_bit(int flash_area_id, struct 
boot_img_trailer *bit)
     const struct flash_area *fap;
     int rc;
 
+    /* XXX: This function only works by chance.  It requires that the boot
+     * loader have have been run once already so that its global state has been
+     * populated.
+     */
+
     rc = flash_area_open(flash_area_id, &fap);
     TEST_ASSERT_FATAL(rc == 0);
 
@@ -1177,11 +1182,7 @@ TEST_SUITE(boot_test_main)
     boot_test_no_flag_has_hash();
     boot_test_invalid_hash();
     boot_test_revert();
-
-    /* XXX: This test fails due to a known issue.  We do not currently recover
-     * correctly when the device reboots during a revert swap.
-     */
-    //boot_test_revert_continue();
+    boot_test_revert_continue();
 }
 
 int

Reply via email to