Conversation
| } | ||
| if matches!(slots[s].role, Some(SlotRole::Recovery)) { | ||
| recovery_slots += 1; | ||
| assert!(slots[s].bootable, "a recovery-role slot must be bootable"); |
There was a problem hiding this comment.
To prevent invalid slot definitions with role Recovery and bootable false we could create a simple constructor that prohibits this and throws an error.
type SlotDescError ...
impl SlotDesc {
pub fn new(id: u8, writebale: bool, bootable: bool, policy: Option<SlotRole>) -> Result<SlotDescError, Self> {
...
}
}There was a problem hiding this comment.
a constructor is a good idea, done
There was a problem hiding this comment.
Where is this implemented @chrysh, I can't seem to find it in this patch.
There was a problem hiding this comment.
I changed it, now golden has its own role.
| /// is plain A/B, A/B + golden, or single + golden purely by what the table | ||
| /// declares — no layout shape is named anywhere. | ||
| #[derive(Debug, Clone, Copy)] | ||
| pub struct SlotDesc { |
There was a problem hiding this comment.
Generally, we should discuss whether a SlotDesc should be mutable or not. If not, we can make validation a method for the SlotDesc and run it on creation to make the API safer for the users, such that the user cannot 'forget' to validate the SlotDesc.
There was a problem hiding this comment.
A constructor is a good idea, done
5e323b6 to
e7a45b4
Compare
| /// A layout without rungs — e.g. empty, for a device that owns its | ||
| /// boot selection internally (the PLDM archetype) — leaves escalation | ||
| /// as the only step. | ||
| pub slots: &'static [Slot], |
There was a problem hiding this comment.
A board can declare more recovery slots than the orchestrator is allowed to try!
How far recovery falls back is set by max_retry (in the orchestrator), but the ladder depth is set here (in the slot table), and nothing checks that they agree.
If max_retry is smaller than the number of slots, the sm locks up before it ever reaches the golden slot — so the last-resort image silently never runs.
Since this is a schema-only PR, either add a build-time check that max_retry covers the deepest ladder or note it as a required follow-up before anything starts using these slots.
Otherwise a board can ship with an unreachable golden slot and no one finds out.
There was a problem hiding this comment.
Why do we have max_retry ? Shouldn't it be dictated by the number of configured slots? The sm just tries the next configured slot until no more slots left to try?
b45d353 to
12709d3
Compare
479c440 to
69f43db
Compare
|
Closed until @FerralCoder returns the config file and gives us more instructions. |
69f43db to
0042237
Compare
chrysh
changed the title
Board tables now say where a device's images live: the slots the eRoT writes, each a byte range in the device's firmware partition, plus the golden image recovery falls back to last. ImageLayout::new rejects duplicate slot ids and overlapping regions at build time. Golden is its own type instead of a flag on a slot, so code that walks the slot list to pick a write target, or to check the SVN floor, never sees it. The floor has to skip it. A golden image's security version is fixed when the board is made, so once the floor moves past that version, checking the floor would make the last image that still boots unbootable. The layout is optional. A device that takes its own updates over PLDM and picks what it boots declares none, and the eRoT never names a byte range for it.
0042237 to
b803af5
Compare
3 participants
Redesigned since the last review round: SlotDesc, RecoveryPolicy and the
per-slot flags are gone, so the earlier comments no longer apply.
Board tables now say where each device's images live.
ImageLayoutholds theslots the eRoT writes plus the golden image; a
Slotis an id and aRegion, abyte range counted from the start of that device's firmware partition.
ImageLayout::newrejects duplicate slot ids and overlapping regions at buildtime.
Golden is its own type, not a flag on a slot, so code that picks a write target
or checks the SVN floor never sees it. The floor has to skip it: a golden
image's security version is fixed when the board is made, so enforcing the floor
would eventually make the last image that still boots unbootable.
The layout is optional. A device that takes its own updates over PLDM and picks
what it boots declares none.
assert_retry_reaches_every_imagecloses the gap rusty1968 raised: boards callit from a const fence, so a retry budget too small to boot every image fails the
build. The floor is one more than the image count, because the last restore the
budget allows is never booted.
Part of 9elements#1.