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.

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

bootutil: Fix compatible sector check #1883

Merged
merged 3 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions boot/bootutil/src/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,10 @@ boot_write_status(const struct boot_loader_state *state, struct boot_status *bs)
memset(buf, erased_val, BOOT_MAX_ALIGN);
buf[0] = bs->state;

BOOT_LOG_DBG("writing swap status; fa_id=%d off=0x%lx (0x%lx)",
flash_area_get_id(fap), (unsigned long)off,
(unsigned long)flash_area_get_off(fap) + off);

rc = flash_area_write(fap, off, buf, align);
if (rc != 0) {
rc = BOOT_EFLASH;
Expand Down
56 changes: 38 additions & 18 deletions boot/bootutil/src/swap_move.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,28 @@ boot_status_internal_off(const struct boot_status *bs, int elem_sz)
return off;
}

static int app_max_sectors(struct boot_loader_state *state)
{
uint32_t sz = 0;
uint32_t sector_sz;
uint32_t trailer_sz;
uint32_t first_trailer_idx;

sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
first_trailer_idx = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - 1;

while (1) {
sz += sector_sz;
if (sz >= trailer_sz) {
break;
}
first_trailer_idx--;
}
Comment on lines +246 to +254
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is size or division optimization for

first_trailer_idx = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) -  (trailer_sz + sector_sz - 1) / sector_sz;

?
I know that this was here previously, just wondering why.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was added 5 years ago like that apparently, could be changed to the summarised form if wanted

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nah. Lets fix it first, than we can go through these things and do some refactoring.


return first_trailer_idx;
}

int
boot_slots_compatible(struct boot_loader_state *state)
{
Expand All @@ -242,20 +264,31 @@ boot_slots_compatible(struct boot_loader_state *state)
size_t sector_sz_pri = 0;
size_t sector_sz_sec = 0;
size_t i;
size_t num_usable_sectors_pri;

num_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT);
num_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT);
num_usable_sectors_pri = app_max_sectors(state);

if ((num_sectors_pri != num_sectors_sec) &&
(num_sectors_pri != (num_sectors_sec + 1))) {
(num_sectors_pri != (num_sectors_sec + 1)) &&
(num_usable_sectors_pri != (num_sectors_sec + 1))) {
BOOT_LOG_WRN("Cannot upgrade: not a compatible amount of sectors");
BOOT_LOG_DBG("slot0 sectors: %d, slot1 sectors: %d, usable slot0 sectors: %d",
(int)num_sectors_pri, (int)num_sectors_sec,
(int)(num_usable_sectors_pri - 1));
return 0;
}

if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
} else if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
BOOT_LOG_WRN("Cannot upgrade: more sectors than allowed");
return 0;
}

if (num_usable_sectors_pri != (num_sectors_sec + 1)) {
BOOT_LOG_DBG("Non-optimal sector distribution, slot0 has %d usable sectors (%d assigned) "
"but slot1 has %d assigned", (int)(num_usable_sectors_pri - 1),
(int)num_sectors_pri, (int)num_sectors_sec);
}

for (i = 0; i < num_sectors_sec; i++) {
sector_sz_pri = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, i);
sector_sz_sec = boot_img_sector_size(state, BOOT_SECONDARY_SLOT, i);
Expand Down Expand Up @@ -544,24 +577,11 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,

int app_max_size(struct boot_loader_state *state)
{
uint32_t sz = 0;
uint32_t sector_sz;
uint32_t trailer_sz;
uint32_t first_trailer_idx;

sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
first_trailer_idx = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - 1;

while (1) {
sz += sector_sz;
if (sz >= trailer_sz) {
break;
}
first_trailer_idx--;
}

return (first_trailer_idx * sector_sz);
return (app_max_sectors(state) * sector_sz);
}

#endif
7 changes: 7 additions & 0 deletions docs/release-notes.d/bootutil-sector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- bootutil: Fixed issue with comparing sector sizes for
compatibility, this now also checks against the number of usable
sectors (which is the slot size minus the swap status and moved
up by one sector).
- bootutil: Added debug logging to show write location of swap status
and details on sectors including if slot sizes are not optimal for
a given board.