Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: RFC: Preliminary work on flash_area open once. #1569

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

WIP: RFC: Preliminary work on flash_area open once. #1569

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aef9e80
flash_map_backend: Remove flash_area_id_to_multi_image_slot
de-nordic Jan 3, 2023
fa01b2f
zephyr: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
45865bf
nuttx: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
e69fc07
mbed: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
1856529
espressif: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
eb70599
cypress: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
92e0e1e
sim: Add implementation of flash_area_get_sectors_fa
de-nordic Jan 5, 2023
9dfeac3
boot: RFC: WIP: Preliminary work on flash_area open-once
de-nordic Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions boot/bootutil/include/bootutil/bootutil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,26 @@ struct image_trailer {
uint8_t magic[BOOT_MAGIC_SZ];
};

#if defined(MCUBOOT_SINGLE_APPLICATION_SLOT)
#define FLASH_AREA_IMAGES 1
#elif MCUBOOT_IMAGE_NUMBER == 1
#define FLASH_AREA_IMAGES 2
#elif MCUBOOT_IMAGE_NUMBER == 2
#define FLASH_AREA_IMAGES 4
#endif

#if !defined(MCUBOOT_SWAP_USING_SCRATCH)
#define FLASH_AREA_OBJECTS FLASH_AREA_IMAGES
#else
#define FLASH_AREA_OBJECTS (FLASH_AREA_IMAGES + 1)
#define SCRATCH_FA flash_area_objects[FLASH_AREA_OBJECTS - 1]
#endif

extern const struct flash_area *flash_area_objects[FLASH_AREA_OBJECTS];

#define PRIMARY_IMAGE_FA(x) flash_area_objects[(x) + 0]
#define SECONDARY_IMAGE_FA(x) flash_area_objects[(x) + 1]

/* you must have pre-allocated all the entries within this structure */
fih_int boot_go(struct boot_rsp *rsp);
fih_int boot_go_for_image_id(struct boot_rsp *rsp, uint32_t image_id);
Expand Down
3 changes: 2 additions & 1 deletion boot/bootutil/src/bootutil_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ struct flash_area;

#if (defined(MCUBOOT_OVERWRITE_ONLY) + \
defined(MCUBOOT_SWAP_USING_MOVE) + \
defined(MCUBOOT_SWAP_USING_SCRATCH) + \
defined(MCUBOOT_DIRECT_XIP) + \
defined(MCUBOOT_RAM_LOAD)) > 1
#error "Please enable only one of MCUBOOT_OVERWRITE_ONLY, MCUBOOT_SWAP_USING_MOVE, MCUBOOT_DIRECT_XIP or MCUBOOT_RAM_LOAD"
#error "Please enable only one of MCUBOOT_OVERWRITE_ONLY, MCUBOOT_SWAP_USING_MOVE, MCUBOOT_SWAP_USING_SCRATCH, MCUBOOT_DIRECT_XIP or MCUBOOT_RAM_LOAD"
#endif

#if !defined(MCUBOOT_OVERWRITE_ONLY) && \
Expand Down
8 changes: 4 additions & 4 deletions boot/bootutil/src/bootutil_public.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,15 +432,15 @@ boot_swap_type_multi(int image_index)
BOOT_HOOK_REGULAR, image_index, &primary_slot);
if (rc == BOOT_HOOK_REGULAR)
{
rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_PRIMARY(image_index),
&primary_slot);
rc = boot_read_swap_state(PRIMARY_IMAGE_FA(image_index),
&primary_slot);
}
if (rc) {
return BOOT_SWAP_TYPE_PANIC;
}

rc = boot_read_swap_state_by_id(FLASH_AREA_IMAGE_SECONDARY(image_index),
&secondary_slot);
rc = boot_read_swap_state(SECONDARY_IMAGE_FA(image_index),
&secondary_slot);
if (rc == BOOT_EFLASH) {
BOOT_LOG_INF("Secondary image of image pair (%d.) "
"is unreachable. Treat it as empty", image_index);
Expand Down
6 changes: 3 additions & 3 deletions boot/bootutil/src/encrypted.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ boot_enc_load(struct enc_key_data *enc_state, int image_index,
uint8_t slot;
int rc;

rc = flash_area_id_to_multi_image_slot(image_index, flash_area_get_id(fap));
rc = flash_area_multi_image_slot(image_index, fap);
if (rc < 0) {
return rc;
}
Expand Down Expand Up @@ -745,7 +745,7 @@ boot_enc_valid(struct enc_key_data *enc_state, int image_index,
{
int rc;

rc = flash_area_id_to_multi_image_slot(image_index, flash_area_get_id(fap));
rc = flash_area_to_multi_image_slot(image_index, fap);
if (rc < 0) {
/* can't get proper slot number - skip encryption, */
/* postpone the error for a upper layer */
Expand Down Expand Up @@ -777,7 +777,7 @@ boot_encrypt(struct enc_key_data *enc_state, int image_index,
nonce[14] = (uint8_t)(off >> 8);
nonce[15] = (uint8_t)off;

rc = flash_area_id_to_multi_image_slot(image_index, flash_area_get_id(fap));
rc = flash_area_to_multi_image_slot(image_index, fap);
if (rc < 0) {
assert(0);
return;
Expand Down
98 changes: 24 additions & 74 deletions boot/bootutil/src/loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <inttypes.h>
#include <stdlib.h>
#include <string.h>
#include "mcuboot_config/mcuboot_config.h"
#include "bootutil/bootutil.h"
#include "bootutil/bootutil_public.h"
#include "bootutil/image.h"
Expand All @@ -56,7 +57,6 @@
#include <os/os_malloc.h>
#endif

#include "mcuboot_config/mcuboot_config.h"

BOOT_LOG_MODULE_DECLARE(mcuboot);

Expand Down Expand Up @@ -240,12 +240,7 @@ boot_read_image_size(struct boot_loader_state *state, int slot, uint32_t *size)
(void)state;
#endif

area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot);
rc = flash_area_open(area_id, &fap);
if (rc != 0) {
rc = BOOT_EFLASH;
goto done;
}
fap = flash_area_from_multi_image_slot(BOOT_CURR_IMG(state), slot);

off = BOOT_TLV_OFF(boot_img_hdr(state, slot));

Comment on lines +243 to 246
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would need to check if there is any way that this function (from any call) could be passed a different value, e.g. by fault injection, because if so then the bootloader operation becomes undefined

Expand Down Expand Up @@ -309,7 +304,7 @@ boot_write_sz(struct boot_loader_state *state)
}

static int
boot_initialize_area(struct boot_loader_state *state, int flash_area)
boot_initialize_area(struct boot_loader_state *state, const struct flash_area *fa)
{
uint32_t num_sectors = BOOT_MAX_IMG_SECTORS;
boot_sector_t *out_sectors;
Expand All @@ -318,14 +313,14 @@ boot_initialize_area(struct boot_loader_state *state, int flash_area)

num_sectors = BOOT_MAX_IMG_SECTORS;

if (flash_area == FLASH_AREA_IMAGE_PRIMARY(BOOT_CURR_IMG(state))) {
if (fa == PRIMARY_IMAGE_FA(BOOT_CURR_IMG(state))) {
out_sectors = BOOT_IMG(state, BOOT_PRIMARY_SLOT).sectors;
out_num_sectors = &BOOT_IMG(state, BOOT_PRIMARY_SLOT).num_sectors;
} else if (flash_area == FLASH_AREA_IMAGE_SECONDARY(BOOT_CURR_IMG(state))) {
} else if (fa == SECONDARY_IMAGE_FA(BOOT_CURR_IMG(state))) {
out_sectors = BOOT_IMG(state, BOOT_SECONDARY_SLOT).sectors;
out_num_sectors = &BOOT_IMG(state, BOOT_SECONDARY_SLOT).num_sectors;
#if MCUBOOT_SWAP_USING_SCRATCH
} else if (flash_area == FLASH_AREA_IMAGE_SCRATCH) {
} else if (fa == SCRATCH_FA) {
out_sectors = state->scratch.sectors;
out_num_sectors = &state->scratch.num_sectors;
#endif
Expand All @@ -334,8 +329,12 @@ boot_initialize_area(struct boot_loader_state *state, int flash_area)
}

#ifdef MCUBOOT_USE_FLASH_AREA_GET_SECTORS
rc = flash_area_get_sectors(flash_area, &num_sectors, out_sectors);
rc = flash_area_get_sectors_fa(fa, &num_sectors, out_sectors);
#else
/* TODO: This is only used by mynewt; note that flash_area_to_sectors
* actually does flash_area_open/flash_area_close pair but this safe for now,
* as these function do no locking or reference counting. */
int flash_area = flash_area_get_id(fa);
_Static_assert(sizeof(int) <= sizeof(uint32_t), "Fix needed");
rc = flash_area_to_sectors(flash_area, (int *)&num_sectors, out_sectors);
#endif /* defined(MCUBOOT_USE_FLASH_AREA_GET_SECTORS) */
Expand All @@ -360,12 +359,12 @@ boot_read_sectors(struct boot_loader_state *state)

image_index = BOOT_CURR_IMG(state);

rc = boot_initialize_area(state, FLASH_AREA_IMAGE_PRIMARY(image_index));
rc = boot_initialize_area(state, PRIMARY_IMAGE_FA(image_index));
if (rc != 0) {
return BOOT_EFLASH;
}

rc = boot_initialize_area(state, FLASH_AREA_IMAGE_SECONDARY(image_index));
rc = boot_initialize_area(state, SECONDARY_IMAGE_FA(image_index));
if (rc != 0) {
/* We need to differentiate from the primary image issue */
return BOOT_EFLASH_SEC;
Expand Down Expand Up @@ -439,20 +438,15 @@ boot_write_status(const struct boot_loader_state *state, struct boot_status *bs)
#if MCUBOOT_SWAP_USING_SCRATCH
if (bs->use_scratch) {
/* Write to scratch. */
area_id = FLASH_AREA_IMAGE_SCRATCH;
fap = SCRATCH_FA;
} else {
#endif
/* Write to the primary slot. */
area_id = FLASH_AREA_IMAGE_PRIMARY(BOOT_CURR_IMG(state));
fap = PRIMARY_IMAGE_FA(BOOT_CURR_IMG(state));
#if MCUBOOT_SWAP_USING_SCRATCH
}
#endif

rc = flash_area_open(area_id, &fap);
if (rc != 0) {
return BOOT_EFLASH;
}

off = boot_status_off(fap) +
boot_status_internal_off(bs, BOOT_WRITE_SZ(state));
align = flash_area_align(fap);
Expand All @@ -465,8 +459,6 @@ boot_write_status(const struct boot_loader_state *state, struct boot_status *bs)
rc = BOOT_EFLASH;
}

flash_area_close(fap);

return rc;
}
#endif /* !MCUBOOT_RAM_LOAD */
Expand Down Expand Up @@ -596,14 +588,9 @@ boot_check_header_erased(struct boot_loader_state *state, int slot)
int area_id;
int rc;

area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot);
rc = flash_area_open(area_id, &fap);
if (rc != 0) {
return -1;
}
fap = flash_area_from_multi_image_slot(BOOT_CURR_IMG(state), slot);

erased_val = flash_area_erased_val(fap);
flash_area_close(fap);

hdr = boot_img_hdr(state, slot);
if (!boot_data_is_set_to(erased_val, &hdr->ih_magic, sizeof(hdr->ih_magic))) {
Expand Down Expand Up @@ -712,11 +699,7 @@ boot_validate_slot(struct boot_loader_state *state, int slot,
fih_int fih_rc = FIH_FAILURE;
int rc;

area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot);
rc = flash_area_open(area_id, &fap);
if (rc != 0) {
FIH_RET(fih_rc);
}
fap = flash_area_from_multi_image_slot(BOOT_CURR_IMG(state), slot);

hdr = boot_img_hdr(state, slot);
if (boot_check_header_erased(state, slot) == 0 ||
Expand Down Expand Up @@ -817,8 +800,6 @@ boot_validate_slot(struct boot_loader_state *state, int slot,
#endif

out:
flash_area_close(fap);

FIH_RET(fih_rc);
}

Expand All @@ -844,12 +825,7 @@ boot_update_security_counter(uint8_t image_index, int slot,
uint32_t img_security_cnt;
int rc;

rc = flash_area_open(flash_area_id_from_multi_image_slot(image_index, slot),
&fap);
if (rc != 0) {
rc = BOOT_EFLASH;
goto done;
}
fap = flash_area_from_multi_image_slot(image_index, slot);

rc = bootutil_get_img_security_cnt(hdr, fap, &img_security_cnt);
if (rc != 0) {
Expand All @@ -862,7 +838,6 @@ boot_update_security_counter(uint8_t image_index, int slot,
}

done:
flash_area_close(fap);
return rc;
}
#endif /* MCUBOOT_HW_ROLLBACK_PROT */
Expand Down Expand Up @@ -1410,12 +1385,7 @@ boot_verify_slot_dependencies(struct boot_loader_state *state, uint32_t slot)
int area_id;
int rc;

area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot);
rc = flash_area_open(area_id, &fap);
if (rc != 0) {
rc = BOOT_EFLASH;
goto done;
}
fap = flash_area_from_multi_image_slot(BOOT_CURR_IMG(state), slot);

rc = bootutil_tlv_iter_begin(&it, boot_img_hdr(state, slot), fap,
IMAGE_TLV_DEPENDENCY, true);
Expand Down Expand Up @@ -1457,7 +1427,6 @@ boot_verify_slot_dependencies(struct boot_loader_state *state, uint32_t slot)
}

done:
flash_area_close(fap);
return rc;
}

Expand Down Expand Up @@ -1966,7 +1935,6 @@ context_boot_go(struct boot_loader_state *state, struct boot_rsp *rsp)
struct boot_status bs;
int rc = -1;
fih_int fih_rc = FIH_FAILURE;
int fa_id;
int image_index;
bool has_upgrade;

Expand Down Expand Up @@ -2019,14 +1987,10 @@ context_boot_go(struct boot_loader_state *state, struct boot_rsp *rsp)
* of this call.
*/
for (slot = 0; slot < BOOT_NUM_SLOTS; slot++) {
fa_id = flash_area_id_from_multi_image_slot(image_index, slot);
rc = flash_area_open(fa_id, &BOOT_IMG_AREA(state, slot));
assert(rc == 0);
BOOT_IMG_AREA(state, slot) = flash_area_from_multi_image_slot(image_index, slot);
}
#if MCUBOOT_SWAP_USING_SCRATCH
rc = flash_area_open(FLASH_AREA_IMAGE_SCRATCH,
&BOOT_SCRATCH_AREA(state));
assert(rc == 0);
BOOT_SCRATCH_AREA(state) = SCRATCH_FA;
#endif

/* Determine swap type and complete swap if it has been aborted. */
Expand Down Expand Up @@ -2213,8 +2177,6 @@ split_go(int loader_slot, int split_slot, void **entry)
{
boot_sector_t *sectors;
uintptr_t entry_val;
int loader_flash_id;
int split_flash_id;
int rc;
fih_int fih_rc = FIH_FAILURE;

Expand All @@ -2225,14 +2187,8 @@ split_go(int loader_slot, int split_slot, void **entry)
BOOT_IMG(&boot_data, loader_slot).sectors = sectors + 0;
BOOT_IMG(&boot_data, split_slot).sectors = sectors + BOOT_MAX_IMG_SECTORS;

loader_flash_id = flash_area_id_from_image_slot(loader_slot);
rc = flash_area_open(loader_flash_id,
&BOOT_IMG_AREA(&boot_data, loader_slot));
assert(rc == 0);
split_flash_id = flash_area_id_from_image_slot(split_slot);
rc = flash_area_open(split_flash_id,
&BOOT_IMG_AREA(&boot_data, split_slot));
assert(rc == 0);
BOOT_IMG_AREA(&boot_data, loader_slot) = flash_area_from_image_slot(loader_slot);
BOOT_IMG_AREA(&boot_data, split_slot) = flash_area_from_image_slot(split_slot);

/* Determine the sector layout of the image slots and scratch area. */
rc = boot_read_sectors(&boot_data);
Expand Down Expand Up @@ -2577,11 +2533,7 @@ boot_decrypt_and_copy_image_to_sram(struct boot_loader_state *state,
uint8_t * ram_dst = (void *)(IMAGE_RAM_BASE + img_dst);

image_index = BOOT_CURR_IMG(state);
area_id = flash_area_id_from_multi_image_slot(BOOT_CURR_IMG(state), slot);
rc = flash_area_open(area_id, &fap_src);
if (rc != 0){
return BOOT_EFLASH;
}
fap_src = flash_area_from_multi_image_slot(BOOT_CURR_IMG(state), slot);

tlv_off = BOOT_TLV_OFF(hdr);

Expand Down Expand Up @@ -2633,8 +2585,6 @@ boot_decrypt_and_copy_image_to_sram(struct boot_loader_state *state,
rc = 0;

done:
flash_area_close(fap_src);

return rc;
}

Expand Down
Loading