Add ReadRot message for CMPA / CFPA #116
Conversation
| @@ -70,15 +70,15 @@ fn host_request() { | |||
| let request = | |||
| MgsRequest::ReadSensor(SensorRequest { kind, id: 0x1234 }); | |||
| let mut expected = vec
mkeeter
force-pushed
the
read-rot-message
branch
from
3e35abd
to
fa8957f
Compare
gateway-sp-comms/src/single_sp.rs
Outdated
| @@ -814,6 +816,24 @@ impl SingleSp { | |||
| }), | |||
| } | |||
| } | |||
|
|
|||
| pub async fn read_rot_cmpa(&self) -> Result<[u8; 512]> { | |||
There was a problem hiding this comment.
One nit and one question:
- Could we add a constant (or possibly two, for naming clarity? I'm ambivalent about this bit) for the CMPA / CFPA size? As written I think clients of this crate will also need to create some
[u8; 512]s, and constants would probably be friendlier. - How is a client (MGS, or possibly the client of MGS) supposed to interpret the returned blob? Will hubtools take this as input for some kind of "display what we know about this" or "compare against a known list" operation?
There was a problem hiding this comment.
I think the interpretation depends on what problems we are actually trying to solve. We can use this with the verify operation in lpc55_sign to validate a particular Hubris image will work with a particular CMPA/CFPA combination.
The constant is the LPC55 flash page size. We have that defined in a driver but I don't know the best place to put it for other not-LPC55 software. We could also cheat and use the block_size call to get the same answer.
gateway-messages/src/lib.rs
Outdated
| /// Specific CFPA page | ||
| #[derive( | ||
| Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize, | ||
| )] | ||
| pub enum CfpaPage { | ||
| /// Currently active page | ||
| Active, | ||
| /// Page that will become active on reset | ||
| Pending, | ||
| // We could add variants for ping/pong/scratch pages here if needed | ||
| } |
There was a problem hiding this comment.
I think we should be more explicit to cover all cases
enum CfpaPage {
Active, // HIghest version between ping/pong
Alternate, // Lower version between ping/pong
Scratch, // Data to be applied on next reboot
}
Given the difficulty in getting data out of the RoT, I'd like to guarantee we can actually get all possible data we want and then leave interpretation of that elsewhere. My fear is that the RoT tries to calculate what will happen with "pending", something goes wrong and then we're stuck with confusing data when it doesn't get applied.
There was a problem hiding this comment.
Sure, I can make those changes. Do you want Active / Alternate, or should we get even more manual and report Ping / Pong (leaving the host to interpret which one is active)?
There was a problem hiding this comment.
I think Active vs Inactive is better defined than ping and pong and it's easy to add that logic.
There was a problem hiding this comment.
Okay, switched to Active / Inactive / Scratch in 39455d6
mkeeter
force-pushed
the
read-rot-message
branch
from
2d543f8
to
8ae20d8
Compare
This PR adds a new
ReadRottop-level message, for all of our RoT reading needs (right now, just CMPA / CFPA).The CMPA / CFPA results are a fixed size of 512 bytes, but I'm still putting them in the trailing data buffer to avoid blowing up
SpResponse::MAX_SIZE.This PR should be left open until the Hubris side is also done, in case we discover the need for further changes (e.g. new error variants)