Initial Build system - #16
Conversation
There was a problem hiding this comment.
Thanks, this seems good to me - but with the caveat that I am not familiar with the ot-sku code.
For the final commit, maybe you should instead merge / pull in #15?
| ":signed", | ||
| ], | ||
| ecdsa_key = { | ||
| "//open/keys/root:keyset": "ot00-earlgrey-a2-root-ecdsa-prod-0", |
There was a problem hiding this comment.
Nit: this is hardcoding the SKU which was generic in skus/open/perso/BUILD: is there a reason we know this must be ot00, or is it worth keeping this generic?
(repeated below for the SPHINCS+ key)
There was a problem hiding this comment.
I copied the commit from Chris but I agree that this is not ideal. In fact we ideally shouldn't even set the sku at all and instead have one target per sku? @cfrantz what do you think?
813cb28 to
eaa15bf
Compare
|
|
||
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| SKU = "ot00" |
There was a problem hiding this comment.
I have currently duplicated the approach of perso but I think we should generalize this. I am proposing to postpone this to an issue.
There was a problem hiding this comment.
There was a problem hiding this comment.
Agree, this is fine to merge as is and can easily be extended upon in an issue / a separate PR after further discussion.
Create the initial bazel targets for build and release of personalization binaries and ROM_EXT binaries. This PR depends on opentitan#30146. Addresses #5. Signed-off-by: Chris Frantz <cfrantz@google.com>
Signed-off-by: Amaury Pouly <amaury.pouly@gmail.com>
116f1c8 to
4447ce6
Compare
AlexJones0
left a comment
AlexJones0
left a comment
There was a problem hiding this comment.
LGTM, though I'm not sure if the FPGA test CI failures warrant some further investigation before we can merge.
|
There was a failure due to a download, I rerun them (https://github.com/lowRISC/opentitan/actions/runs/34236431239/job/102102157219) and the result seems better. I will merge, we can investigate specific failures later to see if they need further fixing. |
3 participants
This is a adapted version of #6 to the new infrastructure. We should have merged this ages ago but apparently we forgot it during the release flow creation. Also include a change due to lowRISC/opentitan#30887