Begin implementation of ot_keymgr for Earlgrey
#183
Conversation
This file contains hidden or 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
rivos-eblot
previously requested changes
Sep 15, 2025
There was a problem hiding this comment.
I do not know the keymgr, maybe @loiclefort has better knowledge on this EarlGrey version.
"assert.h" is not used as we use QEMU's `g_assert` instead. "stdint.h" should not be included as QEMU's "osdep.h" takes care of all system-wide header files for us. Signed-off-by: Alex Jones <[email protected]>
This adds a very basic stubbed keymgr implementation with all of the different registers and fields (and some constant pkg values) defined. This is entirely stubbed at the moment - all writes and reads to registers are unimplemented, as are register reset values and any of the internal logic. The intention is to build upon the keymgr implementation in further commits. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg-keymgr-beginning
branch
from
9fd5c56 to
178a799
Compare
The OT keymgr HJSON features a parameter "RndCnstCdi" containing the compile-time random bits to use for the generation seed when no CDI is selected. This is like "RndCnstNoneSeed", but used when there is no CDI selected instead of no destination selected. Unlike the other compile-time random seeds, this is not suffixed with "Seed" and thus is not extracted by the existing regex. This commit adds a special case for this in `cfggen.py` to extract this attribute, and appropriately rename it to a `cdi_seed` property instead of `cdi` so that it converges with the other seed config parameters. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg-keymgr-beginning
branch
from
178a799 to
9442674
Compare
rivos-eblot
reviewed
Sep 15, 2025
AlexJones0
dismissed
rivos-eblot’s stale review
Leaving for others to review.
|
Progresses lowRISC/opentitan#27894 |
Add properties for loading compile-time generation seeds for the keymgr from the OpenTitan configuration file. This includes defining the different seeds that are present in the keymgr, which diverge from those present in the keymgr_dpe due to the differences in the design. Signed-off-by: Alex Jones <[email protected]>
Signed-off-by: Alex Jones <[email protected]>
In the keymgr, alerts from the ALERT_TEST register and recoverable operation error alerts are transient (they signal once and then de-assert), whilst fatal fault alerts are latched (they stay signaled). This logic is implemented in the `update_alerts` function. Signed-off-by: Alex Jones <[email protected]>
Replace the default unimplemented keymgr device in the Earlgrey build with the stubbed keymgr device, which will be iteratively developed. Now that the stubbed keymgr has its seed properties and IRQ & alert updates it should be at least as useful as the unimplemented device. Signed-off-by: Alex Jones <[email protected]>
Implements the logic for keymgr register reads and writes, mostly limited to write masking, RW0C/RW1C/RO/WO behaviour, REGWENs and shadowed registers. A small bit of additional logic is added to hook up the interrupt/alert state/test registers to propagate IRQs and alert signals. Some logic is also implemented to load the salt and sealing keys into separate byte arrays, since these will be needed for later parts of the implementation. The RC (read-clear) software share outputs are left completely unimplemented for now as these will be added later along with their implementation. Signed-off-by: Alex Jones <[email protected]>
AlexJones0
force-pushed
the
eg-keymgr-beginning
branch
from
9442674 to
06d83da
Compare
This test occasionally fails in CI due to crypto test timing checks being quite tight. This is difficult to reproduce locally, so it may even be due to differences in the active loads of the CI runners etc. For now, mark this test as flaky to prevent issues in QEMU CI. Signed-off-by: Alex Jones <[email protected]>
loiclefort
approved these changes
Sep 16, 2025
| @@ -83,6 +83,8 @@ static_assert(KEYMGR_GEN_DATA_BYTES <= KEYMGR_KDF_BUFFER_BYTES, | |||
| #define KEYMGR_ENTROPY_WIDTH (KEYMGR_LFSR_WIDTH / 2u) | |||
| #define KEYMGR_ENTROPY_ROUNDS (KEYMGR_KEY_WIDTH / KEYMGR_ENTROPY_WIDTH) | |||
|
|
|||
| #define KEYMGR_LFSR_SEED_BYTES ((KEYMGR_LFSR_WIDTH) / 8u) | |||
There was a problem hiding this comment.
maybe add a static assert to make sure that KEYMGR_LFSR_SEED_BYTES <= KEYMGR_SEED_BYTES? (because seeds array is allocated with string of length KEYMGR_SEED_BYTES)
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 is the first of a series of 4 PRs to implement OpenTitan's
keymgrIP, as used in Earlgrey, split to ease review. You can see all the relevantkeymgrcommits (including this PR) here, which is functionally complete, though note that small details in future commits may still be WIP.The
keymgris quite similar to the existingot_keymgr_dpe, meaning that a lot of the logic is similar and the design of the block follows a similar structure. Despite this, their design is different -keymgrhas unique states for each key stage and multiple CDIs, whereaskeymgr_dpehas no CDIs but many slots with different boot stages, and only 1 generic FSM key state. There has also been a fair amount of RTL divergence between the two IPs, meaning that many specifics of the implementation are unique to Earlgrey. Thus, these PRs create thekeymgrfrom scratch, attempting to note what is the same as / changed fromkeymgr_dpein the commit messages where relevant for review.One note: for now I have opted to use separate
ot_keymgrtraces from the existingot_keymgr_dpetraces, because in future PRs the blocks diverge a bit further and use different traces for different things (andkeymgr_dpecould perhaps diverge further in the future). However, I think both blocks could probably just use genericot_keymgrtraces with redundant traces left unused for each block.This PR adds a lot of the initial description for the
keymgr, and sets up initial functionality like register reads/writes, IRQs, alerts. This stubbed keymgr is then connected to Earlgrey. Future PRs will implement the main FSM andkeymgroperation, but they are separated from this PR to ease review.See the commit messages for more details about each change.