Add TBYB safe rom_pick_ab_partition function #1850
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kilograham
reviewed
Aug 22, 2024
| @@ -7,6 +7,9 @@ | |||
| #include "pico/bootrom.h" | |||
| #include "boot/picoboot.h" | |||
| #include "boot/picobin.h" | |||
| #if !PICO_RP2040 | |||
There was a problem hiding this comment.
strictly more idiomatic to use #if HAS_RP2040_RTC
There was a problem hiding this comment.
This is the RCP not the RTC - given the functions in this file are also guarded by !PICO_RP2040 I think this if fine to use here too?
kilograham
reviewed
Aug 22, 2024
| @@ -0,0 +1,211 @@ | |||
| #include "boot/picobin.h" | |||
There was a problem hiding this comment.
I'm really not a fan of exposing this header of structs. It implies that this is a public API, which it isn't
Can we try with just using memory offsets, and then we need to guard with the rom_version also.
lurch
reviewed
Oct 3, 2024
| /*! \brief Pick A/B partition for a separate partition | ||
| * \ingroup pico_bootrom | ||
| * | ||
| * This will perform extra checks to prevent disrupting a main image TBYB, and return errors |
There was a problem hiding this comment.
Might want to reword this slightly? "This will ... return errors" 😉
will-v-pi
force-pushed
the
rom_pick_ab_update_partition
branch
from
c3de287
to
9260cf7
Compare
|
Superseded by #1969 - that adds this function plus a use of it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an extra wrapper around
rom_pick_ab_partition, which makes it safer when called during a TBYB boot, and includes a call to get the flash_update_baseCurrently, when doing a flash update boot which requires a flash erase, if you call
pick_ab_partitionbefore callingexplicit_buy(eg to pick a firmware partition or similar) then it will clearalways->zero_init.version_downgrade_erase_flash_addr. This means the subsequentexplicit_buycall will not perform the required flash erase, which means the flash update boot will not succeed.This adds saving and restoring the
erase_flash_addrto prevent this happening, while still allowing theerase_flash_addrto be set if the firmware partition is being updated and requires a flash eraseThis also adds setting up
always->zero_init.tbyb_flag_flash_addrto allow for TBYB executables in the firmware partition to have the address set up correctlyThe function returns an error if too many things are updating at once - either if both the boot and firmware images are TBYB, or if the boot and firmware images both require a flash erase
It also returns an error if the chosen image was not verified, which acts as a signature check on the firmware image