xtask: Include expected measurement for hubris images in build archive. #1491
Conversation
build/xtask/src/dist.rs
Outdated
| let mut file = File::create(&cfg.img_file("final.fwid", image_name))?; | ||
| writeln!(file, "{}", hex::encode(&digest))?; | ||
|
|
||
| archive.add_file("img/final.fwid", digest.as_bytes())?; |
There was a problem hiding this comment.
I'm not quite sure why we're creating the file twice, is the hubris one not sufficient? Aren't the two files different since one is hex encoded and the other is the raw bytes?
There was a problem hiding this comment.
Ah that's a good catch: I had intended for the contents of these two files to be identical. I'll fix that up. As to why I create a file on disk and one in the archive: the final.bin is manipulated / caboosed in the archive after creation.
There was a problem hiding this comment.
Just pushed a change to make the final.fwid files on disk and in the archive identical: both now contain the hex encoded digest.
flihp
force-pushed
the
measure-sp-at-build
branch
2 times, most recently
from
614898a
to
176c94d
Compare
flihp
force-pushed
the
measure-sp-at-build
branch
from
176c94d
to
5f8aa00
Compare
build/xtask/src/dist.rs
Outdated
| // can we use lpc55-romapi | ||
| const LPC55_FLASH_PAGE_SIZE: usize = 512; |
There was a problem hiding this comment.
Hmmmm, I think what we actually want is like lpc55-rom-constants or something similar (currently running into this with some of the CMPA/CFPA reading as well). Probably not worth blocking for it.
There was a problem hiding this comment.
I've added a new commit addressing this feedback. Only the constant used here by xtask was moved to the new lpc55-rom-data crate. Others can migrate on demand. I'll squash all of this down once the PR is ready. LMK if this wasn't what you were after.
| #[serde(default)] | ||
| fwid: bool, |
There was a problem hiding this comment.
Would it be easier to make this no_fwid so only boards that we probably aren't going to care about can opt out? I was trying to go through the mental list to make sure we hit all the boards we care about.
There was a problem hiding this comment.
I went back and forth on this a few times. The current approach, as you point out, is error prone & requires we update more toml files. Doing this with negative logic would require fewer toml changes but might end up on the wrong side of the law of least surprise when folks add new boards and they get build errors until they figure out that they need to add fwid = false. There are good reasons for going either way. I'm happy to defer to your judgement.
There was a problem hiding this comment.
Eh, let's keep it as is for now.
| // after we've appended a newline fwid is immutable | ||
| let mut fwid = hex::encode(&digest); | ||
| writeln!(fwid, "").context("appending newline to FWID")?; | ||
| let fwid = fwid; |
There was a problem hiding this comment.
Is the newline just for ease of reading or is there some deeper meaning?
There was a problem hiding this comment.
No deeper meaning, just gives better output when the file is dumped to the console.
This gives us a place to put data relevant to the LPC55 ROM API that we need in software built for the host.
Include the FWID / expected measurement for hubris images in the build archive 'img/final.fwid'.
flihp
force-pushed
the
measure-sp-at-build
branch
from
a3d9e30
to
e73df4f
Compare
This implementation is currently limited to stm32h7 images for the SP. This was tested on a
gimletletby comparing the FWID value calculated byxtaskwith the value calculated by thesp_measurebuild.rs.