Repository: incubator-mynewt-core Updated Branches: refs/heads/develop f13d0dece -> e136d0896
Revert "boot - Don't read full sector layout upfront." This reverts commit e7fc231a687dabf70b53be004c18289b40fd876b The original change was made to accommodate split image. By not caching sector information, the second app no longer had to malloc a large chunk of memory. It turns out there is an easier fix: the second app doesn't read sector information in the first place. This change will be made in a subsequent commit. The original design that caches sector information is more straightforward for the boot loader, so the no-cache change is being reverted. 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/8be888ac Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/8be888ac Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/8be888ac Branch: refs/heads/develop Commit: 8be888ac32fc13779d4c3522722a8db925b50c1e Parents: f13d0de Author: Christopher Collins <[email protected]> Authored: Mon Feb 6 14:39:01 2017 -0800 Committer: Christopher Collins <[email protected]> Committed: Mon Feb 6 14:39:01 2017 -0800 ---------------------------------------------------------------------- boot/bootutil/src/loader.c | 150 +++++++------------ sys/config/src/config_init.c | 30 ++-- sys/flash_map/include/flash_map/flash_map.h | 13 +- sys/flash_map/src/flash_map.c | 45 ++---- .../test/src/testcases/flash_map_test_case_1.c | 2 +- .../test/src/testcases/flash_map_test_case_2.c | 2 +- 6 files changed, 83 insertions(+), 159 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/boot/bootutil/src/loader.c ---------------------------------------------------------------------- diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c index e219ff7..e4c278c 100644 --- a/boot/bootutil/src/loader.c +++ b/boot/bootutil/src/loader.c @@ -40,16 +40,15 @@ /** Number of image slots in flash; currently limited to two. */ #define BOOT_NUM_SLOTS 2 -struct boot_slot { - struct image_header hdr; - struct flash_area area; - int first_sector_idx; - int num_sectors; -}; - static struct { - struct boot_slot slots[BOOT_NUM_SLOTS]; - struct flash_area scratch_area; + struct { + struct image_header hdr; + struct flash_area *sectors; + int num_sectors; + } imgs[BOOT_NUM_SLOTS]; + + struct flash_area scratch_sector; + uint8_t write_sz; } boot_data; @@ -217,21 +216,6 @@ boot_previous_swap_type(void) return BOOT_SWAP_TYPE_REVERT; } -static void -boot_read_sector(int slot_idx, int sector_idx, struct flash_area *out_sect) -{ - const struct boot_slot *slot; - int rc; - - assert(slot_idx >= 0 && slot_idx < BOOT_NUM_SLOTS); - slot = boot_data.slots + slot_idx; - - rc = flash_read_sector(slot->area.fa_device_id, - slot->first_sector_idx + sector_idx, - out_sect); - assert(rc == 0); -} - static int boot_read_image_header(int slot, struct image_header *out_hdr) { @@ -266,7 +250,7 @@ boot_read_image_headers(void) int i; for (i = 0; i < BOOT_NUM_SLOTS; i++) { - rc = boot_read_image_header(i, &boot_data.slots[i].hdr); + rc = boot_read_image_header(i, &boot_data.imgs[i].hdr); if (rc != 0) { /* If at least one header was read successfully, then the boot * loader can attempt a boot. Failure to read any headers is a @@ -293,8 +277,8 @@ boot_write_sz(void) * on what the minimum write size is for scratch area, active image slot. * We need to use the bigger of those 2 values. */ - elem_sz = hal_flash_align(boot_data.slots[0].area.fa_device_id); - align = hal_flash_align(boot_data.scratch_area.fa_device_id); + elem_sz = hal_flash_align(boot_data.imgs[0].sectors[0].fa_device_id); + align = hal_flash_align(boot_data.scratch_sector.fa_device_id); if (align > elem_sz) { elem_sz = align; } @@ -305,19 +289,18 @@ boot_write_sz(void) static int boot_slots_compatible(void) { - struct flash_area sector0; - struct flash_area sector1; + const struct flash_area *sector0; + const struct flash_area *sector1; int i; /* Ensure both image slots have identical sector layouts. */ - if (boot_data.slots[0].num_sectors != boot_data.slots[1].num_sectors) { + if (boot_data.imgs[0].num_sectors != boot_data.imgs[1].num_sectors) { return 0; } - for (i = 0; i < boot_data.slots[0].num_sectors; i++) { - boot_read_sector(0, i, §or0); - boot_read_sector(1, i, §or1); - - if (sector0.fa_size != sector1.fa_size) { + for (i = 0; i < boot_data.imgs[0].num_sectors; i++) { + sector0 = boot_data.imgs[0].sectors + i; + sector1 = boot_data.imgs[1].sectors + i; + if (sector0->fa_size != sector1->fa_size) { return 0; } } @@ -335,59 +318,32 @@ static int boot_read_sectors(void) { const struct flash_area *scratch; - const struct flash_area *slot0; - const struct flash_area *slot1; + int num_sectors_slot0; + int num_sectors_slot1; int rc; - /*** Slot 0. */ - - rc = flash_area_open(FLASH_AREA_IMAGE_0, &slot0); + num_sectors_slot0 = BOOT_MAX_IMG_SECTORS; + rc = flash_area_to_sectors(FLASH_AREA_IMAGE_0, &num_sectors_slot0, + boot_data.imgs[0].sectors); if (rc != 0) { return BOOT_EFLASH; } + boot_data.imgs[0].num_sectors = num_sectors_slot0; - /* Remember flash area properties. */ - boot_data.slots[0].area = *slot0; - - /* Determine sector count. */ - rc = flash_area_to_sectors(FLASH_AREA_IMAGE_0, - &boot_data.slots[0].first_sector_idx, - &boot_data.slots[0].num_sectors, - NULL); + num_sectors_slot1 = BOOT_MAX_IMG_SECTORS; + rc = flash_area_to_sectors(FLASH_AREA_IMAGE_1, &num_sectors_slot1, + boot_data.imgs[1].sectors); if (rc != 0) { return BOOT_EFLASH; } - - /*** Slot 1. */ - - rc = flash_area_open(FLASH_AREA_IMAGE_1, &slot1); - if (rc != 0) { - return BOOT_EFLASH; - } - - /* Remember flash area properties. */ - boot_data.slots[1].area = *slot1; - - /* Determine sector count. */ - rc = flash_area_to_sectors(FLASH_AREA_IMAGE_1, - &boot_data.slots[1].first_sector_idx, - &boot_data.slots[1].num_sectors, - NULL); - if (rc != 0) { - return BOOT_EFLASH; - } - - /*** Scratch. */ + boot_data.imgs[1].num_sectors = num_sectors_slot1; rc = flash_area_open(FLASH_AREA_IMAGE_SCRATCH, &scratch); if (rc != 0) { return BOOT_EFLASH; } + boot_data.scratch_sector = *scratch; - /* Remember flash area properties. */ - boot_data.scratch_area = *scratch; - - /* Determine flash hardware's minimum write size. */ boot_data.write_sz = boot_write_sz(); return 0; @@ -492,7 +448,7 @@ boot_read_status(struct boot_status *bs) } /** - * Writes the supplied boot status to the boot vector. The boot status + * Writes the supplied boot status to the flash file system. The boot status * contains the current state of an in-progress image copy operation. * * @param bs The boot status to write. @@ -593,8 +549,8 @@ boot_validate_slot1(void) const struct flash_area *fap; int rc; - if (boot_data.slots[1].hdr.ih_magic == 0xffffffff || - boot_data.slots[1].hdr.ih_flags & IMAGE_F_NON_BOOTABLE) { + if (boot_data.imgs[1].hdr.ih_magic == 0xffffffff || + boot_data.imgs[1].hdr.ih_flags & IMAGE_F_NON_BOOTABLE) { /* No bootable image in slot 1; continue booting from slot 0. */ return -1; @@ -608,8 +564,8 @@ boot_validate_slot1(void) return BOOT_EFLASH; } - if (boot_data.slots[1].hdr.ih_magic != IMAGE_MAGIC || - boot_image_check(&boot_data.slots[1].hdr, fap) != 0) { + if (boot_data.imgs[1].hdr.ih_magic != IMAGE_MAGIC || + boot_image_check(&boot_data.imgs[1].hdr, fap) != 0) { /* Image in slot 1 is invalid. Erase the image and continue booting * from slot 0. @@ -669,7 +625,6 @@ boot_validated_swap_type(void) static uint32_t boot_copy_sz(int last_sector_idx, int *out_first_sector_idx) { - struct flash_area sector; uint32_t new_sz; uint32_t sz; int i; @@ -677,9 +632,8 @@ boot_copy_sz(int last_sector_idx, int *out_first_sector_idx) sz = 0; for (i = last_sector_idx; i >= 0; i--) { - boot_read_sector(0, i, §or); - new_sz = sz + sector.fa_size; - if (new_sz > boot_data.scratch_area.fa_size) { + new_sz = sz + boot_data.imgs[0].sectors[i].fa_size; + if (new_sz > boot_data.scratch_sector.fa_size) { break; } sz = new_sz; @@ -814,15 +768,13 @@ done: static int boot_swap_sectors(int idx, uint32_t sz, struct boot_status *bs) { - struct flash_area sector; uint32_t copy_sz; uint32_t img_off; int rc; - boot_read_sector(0, idx, §or); - /* Calculate offset from start of image area. */ - img_off = sector.fa_off - boot_data.slots[0].area.fa_off; + img_off = boot_data.imgs[0].sectors[idx].fa_off - + boot_data.imgs[0].sectors[0].fa_off; if (bs->state == 0) { rc = boot_erase_sector(FLASH_AREA_IMAGE_SCRATCH, 0, sz); @@ -846,7 +798,9 @@ boot_swap_sectors(int idx, uint32_t sz, struct boot_status *bs) } copy_sz = sz; - if (img_off + sz >= boot_data.slots[0].area.fa_size) { + if (boot_data.imgs[0].sectors[idx].fa_off + sz >= + boot_data.imgs[1].sectors[0].fa_off) { + /* This is the end of the area. Don't copy the image state into * slot 1. */ @@ -903,7 +857,7 @@ boot_copy_image(struct boot_status *bs) int swap_idx; swap_idx = 0; - last_sector_idx = boot_data.slots[0].num_sectors - 1; + last_sector_idx = boot_data.imgs[0].num_sectors - 1; while (last_sector_idx >= 0) { sz = boot_copy_sz(last_sector_idx, &first_sector_idx); if (swap_idx >= bs->idx) { @@ -1048,6 +1002,16 @@ boot_go(struct boot_rsp *rsp) int slot; int rc; + /* The array of slot sectors are defined here (as opposed to file scope) so + * that they don't get allocated for non-boot-loader apps. This is + * necessary because the gcc option "-fdata-sections" doesn't seem to have + * any effect in older gcc versions (e.g., 4.8.4). + */ + static struct flash_area slot0_sectors[BOOT_MAX_IMG_SECTORS]; + static struct flash_area slot1_sectors[BOOT_MAX_IMG_SECTORS]; + boot_data.imgs[0].sectors = slot0_sectors; + boot_data.imgs[1].sectors = slot1_sectors; + /* Determine the sector layout of the image slots and scratch area. */ rc = boot_read_sectors(); if (rc != 0) { @@ -1104,9 +1068,9 @@ boot_go(struct boot_rsp *rsp) } /* Always boot from the primary slot. */ - rsp->br_flash_id = boot_data.slots[0].area.fa_device_id; - rsp->br_image_addr = boot_data.slots[0].area.fa_off; - rsp->br_hdr = &boot_data.slots[slot].hdr; + rsp->br_flash_id = boot_data.imgs[0].sectors[0].fa_device_id; + rsp->br_image_addr = boot_data.imgs[0].sectors[0].fa_off; + rsp->br_hdr = &boot_data.imgs[slot].hdr; return 0; } @@ -1154,9 +1118,9 @@ split_go(int loader_slot, int split_slot, void **entry) * bootable or non-bootable image. Just validate that the image check * passes which is distinct from the normal check. */ - rc = split_image_check(&boot_data.slots[split_slot].hdr, + rc = split_image_check(&boot_data.imgs[split_slot].hdr, app_fap, - &boot_data.slots[loader_slot].hdr, + &boot_data.imgs[loader_slot].hdr, loader_fap); if (rc != 0) { rc = SPLIT_GO_NON_MATCHING; http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/sys/config/src/config_init.c ---------------------------------------------------------------------- diff --git a/sys/config/src/config_init.c b/sys/config/src/config_init.c index 6b429bd..53a5f25 100644 --- a/sys/config/src/config_init.c +++ b/sys/config/src/config_init.c @@ -51,42 +51,40 @@ config_init_fs(void) #include "fcb/fcb.h" #include "config/config_fcb.h" -static struct flash_area conf_fcb_sectors[NFFS_AREA_MAX + 1]; +static struct flash_area conf_fcb_area[NFFS_AREA_MAX + 1]; static struct conf_fcb config_init_conf_fcb = { .cf_fcb.f_magic = MYNEWT_VAL(CONFIG_FCB_MAGIC), - .cf_fcb.f_sectors = conf_fcb_sectors, + .cf_fcb.f_sectors = conf_fcb_area, }; static void config_init_fcb(void) { - const struct flash_area *fap; int cnt; int rc; - rc = flash_area_to_sectors(MYNEWT_VAL(CONFIG_FCB_FLASH_AREA), NULL, - &cnt, NULL); + rc = flash_area_to_sectors(MYNEWT_VAL(CONFIG_FCB_FLASH_AREA), &cnt, NULL); SYSINIT_PANIC_ASSERT(rc == 0); SYSINIT_PANIC_ASSERT( - cnt <= sizeof(conf_fcb_sectors) / sizeof(conf_fcb_sectors[0])); + cnt <= sizeof(conf_fcb_area) / sizeof(conf_fcb_area[0])); flash_area_to_sectors( - MYNEWT_VAL(CONFIG_FCB_FLASH_AREA), NULL, &cnt, conf_fcb_sectors); + MYNEWT_VAL(CONFIG_FCB_FLASH_AREA), &cnt, conf_fcb_area); config_init_conf_fcb.cf_fcb.f_sector_cnt = cnt; - rc = flash_area_open(MYNEWT_VAL(CONFIG_FCB_FLASH_AREA), &fap); - SYSINIT_PANIC_ASSERT(rc == 0); - rc = conf_fcb_src(&config_init_conf_fcb); - if (rc != 0) { - rc = flash_area_erase(fap, 0, fap->fa_size); - SYSINIT_PANIC_ASSERT(rc == 0); - + if (rc) { + for (cnt = 0; + cnt < config_init_conf_fcb.cf_fcb.f_sector_cnt; + cnt++) { + + flash_area_erase(&conf_fcb_area[cnt], 0, + conf_fcb_area[cnt].fa_size); + } rc = conf_fcb_src(&config_init_conf_fcb); - SYSINIT_PANIC_ASSERT(rc == 0); } - + SYSINIT_PANIC_ASSERT(rc == 0); rc = conf_fcb_dst(&config_init_conf_fcb); SYSINIT_PANIC_ASSERT(rc == 0); } http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/sys/flash_map/include/flash_map/flash_map.h ---------------------------------------------------------------------- diff --git a/sys/flash_map/include/flash_map/flash_map.h b/sys/flash_map/include/flash_map/flash_map.h index 00401cd..c0c9489 100644 --- a/sys/flash_map/include/flash_map/flash_map.h +++ b/sys/flash_map/include/flash_map/flash_map.h @@ -43,11 +43,6 @@ extern "C" { */ #include <inttypes.h> -/** - * Represents a named region of flash. - * - * NOTE: Also used to represent an individual sector. - */ struct flash_area { uint8_t fa_id; uint8_t fa_device_id; @@ -89,13 +84,7 @@ uint8_t flash_area_align(const struct flash_area *); /* * Given flash map index, return info about sectors within the area. */ -int flash_area_to_sectors(int id, - int *out_first_sector_idx, - int *out_num_sectors, - struct flash_area *out_sectors); - -int flash_read_sector(int area_id, int sector_idx, - struct flash_area *out_sect); +int flash_area_to_sectors(int idx, int *cnt, struct flash_area *ret); int flash_area_id_from_image_slot(int slot); int flash_area_id_to_image_slot(int area_id); http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/sys/flash_map/src/flash_map.c ---------------------------------------------------------------------- diff --git a/sys/flash_map/src/flash_map.c b/sys/flash_map/src/flash_map.c index c3248c0..1470fcf 100644 --- a/sys/flash_map/src/flash_map.c +++ b/sys/flash_map/src/flash_map.c @@ -62,34 +62,12 @@ flash_area_close(const struct flash_area *fa) } int -flash_read_sector(int device_id, int sector_idx, struct flash_area *out_sect) -{ - const struct hal_flash *hf; - uint32_t start; - uint32_t size; - - hf = hal_bsp_flash_dev(device_id); - hf->hf_itf->hff_sector_info(hf, sector_idx, &start, &size); - if (out_sect != NULL) { - out_sect->fa_device_id = device_id; - out_sect->fa_off = start; - out_sect->fa_size = size; - } - - return 0; -} - -int -flash_area_to_sectors(int id, - int *out_first_sector_idx, - int *out_num_sectors, - struct flash_area *out_sectors) +flash_area_to_sectors(int id, int *cnt, struct flash_area *ret) { const struct flash_area *fa; const struct hal_flash *hf; uint32_t start; uint32_t size; - int first; int rc; int i; @@ -98,25 +76,20 @@ flash_area_to_sectors(int id, return rc; } - first = 1; - *out_num_sectors = 0; + *cnt = 0; hf = hal_bsp_flash_dev(fa->fa_device_id); for (i = 0; i < hf->hf_sector_cnt; i++) { hf->hf_itf->hff_sector_info(hf, i, &start, &size); if (start >= fa->fa_off && start < fa->fa_off + fa->fa_size) { - if (first && out_first_sector_idx != NULL) { - *out_first_sector_idx = i; - } - if (out_sectors != NULL) { - out_sectors->fa_id = id; - out_sectors->fa_device_id = fa->fa_device_id; - out_sectors->fa_off = start; - out_sectors->fa_size = size; - out_sectors++; + if (ret) { + ret->fa_id = id; + ret->fa_device_id = fa->fa_device_id; + ret->fa_off = start; + ret->fa_size = size; + ret++; } - first = 0; - (*out_num_sectors)++; + (*cnt)++; } } return 0; http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/sys/flash_map/test/src/testcases/flash_map_test_case_1.c ---------------------------------------------------------------------- diff --git a/sys/flash_map/test/src/testcases/flash_map_test_case_1.c b/sys/flash_map/test/src/testcases/flash_map_test_case_1.c index c797d62..b4a7f79 100644 --- a/sys/flash_map/test/src/testcases/flash_map_test_case_1.c +++ b/sys/flash_map/test/src/testcases/flash_map_test_case_1.c @@ -46,7 +46,7 @@ TEST_CASE(flash_map_test_case_1) hf = hal_bsp_flash_dev(fa->fa_device_id); TEST_ASSERT_FATAL(hf != NULL, "hal_bsp_flash_dev"); - rc = flash_area_to_sectors(i, NULL, §_cnt, fa_sectors); + rc = flash_area_to_sectors(i, §_cnt, fa_sectors); TEST_ASSERT_FATAL(rc == 0, "flash_area_to_sectors failed"); end = fa->fa_off; http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8be888ac/sys/flash_map/test/src/testcases/flash_map_test_case_2.c ---------------------------------------------------------------------- diff --git a/sys/flash_map/test/src/testcases/flash_map_test_case_2.c b/sys/flash_map/test/src/testcases/flash_map_test_case_2.c index 4adb545..412c320 100644 --- a/sys/flash_map/test/src/testcases/flash_map_test_case_2.c +++ b/sys/flash_map/test/src/testcases/flash_map_test_case_2.c @@ -40,7 +40,7 @@ TEST_CASE(flash_map_test_case_2) rc = flash_area_open(FLASH_AREA_IMAGE_0, &fa); TEST_ASSERT_FATAL(rc == 0, "flash_area_open() fail"); - rc = flash_area_to_sectors(FLASH_AREA_IMAGE_0, NULL, &sec_cnt, fa_sectors); + rc = flash_area_to_sectors(FLASH_AREA_IMAGE_0, &sec_cnt, fa_sectors); TEST_ASSERT_FATAL(rc == 0, "flash_area_to_sectors failed"); /*
