Update Earlgrey's OTP for parity with Darjeeling, add generic scrambling key support + misc. fixes #167
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
AlexJones0
force-pushed
the
eg-otp
branch
3 times, most recently
from
f2e17f7 to
99a4cb3
Compare
AlexJones0
requested review from
engdoreis,
jwnrt,
loiclefort and
rivos-eblot
|
Wow, this is a massive PR, I will need some time to digest it :-) |
I have more planned PRs coming up soon for EG's keymgr, more flash_ctrl support and updates to a couple of other blocks - I'm getting to the point of booting into ROM_EXT on Earlgrey :) Apologies in advance for the large PRs, don't worry about taking time to digest them 😅 |
rivos-eblot
requested changes
Sep 3, 2025
Great!! |
|
Thanks for the review @rivos-eblot! I realize that this PR may be too big - I can try and split it off into more PRs if it would make it easier for you? I could for example make the initial Darjeeling OTP changes one PR, then another PR to add Earlgrey's OTP, and maybe a third and fourth PR for the scrambling key changes and the OTP BE delay changes? Though as you can see, the commit recreating the Earlgrey's OTP based on Darjeeling's will always be huge, hence I recommend reviewing by diffing the two 😅 |
Do not bother, it is ok the way it is.
Sure :) |
rivos-eblot
requested changes
Sep 4, 2025
rivos-eblot
requested changes
Sep 4, 2025
Thanks for catching these, I do this quite often. Is there a reason that we have |
I'm not sure if this is still needed. I think at some point we use to have 2 separate lines in specific cases. Might be worth to change it to 1, run |
Due to limited support for other HW peripherals, Earlgrey is missing all of its life cycle broadcast signals. Most notably, the outdated OTP controller and the flash_ctrl uses a lot of these broadcast signals. Where possible, update the Earlgrey implementation to add the LC broadcast signals to achieve parity with the Darjeeling implementation. To do this, implement IRQ splitters for certain LC signals, connect up the LC_CTRL with the ibex_wrapper and pwrmgr, and most importantly, leave TODOs noting the signals that are not connected and why. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
For Earlgrey's OTP, there is a constant value used that is one nibble smaller than the full constant width. To avoid special casing in the Earlgrey OTP controller, add additional logic to handle this in the python scripts used by cfggen, which will automatically left-pad OTP constants with 0s to match the width specified by the SystemVerilog type. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Fix a few typos, add an additional static assertion for the OTP address width for safety, and make sure temporary #DEFINEs are properly undefined and cleaned up after use. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This field does not exist in Darjeeling's OTP on OpenTitan `master`. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The second alert update intended to handle the transience of the alert_test updates was tracing on every alert value update, regardless of whether the value changed or not. Remove some needless tracing noise by only conditionally emitting this trace, as in the initial alert update loop. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
For IRQs and alerts, the levels were being masked (but not shifted), and then cast to a bool and back to an int. The result in `level` was *then* shifted, meaning that value updates for IRQs and alerts after the first IRQ and the first alert were being lost, and not actually sending relevant signals. Fix (and simply) the masking/shifting and conversion logic so that IRQs and alerts are now handled correctly in the OTP. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
When fetching entropy for ephemeral scrambling key generation, we checked that `SRAM_NONCE_WORDS` were available but then iterate over `SRAM_KEY_WORDS`. This does not cause problems because these are the same size (4 words), but the correct definition should be used to prevent potentially introducing issues in the future. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Currently the generated SRAM scrambling key is not written back to the key->seed field correctly: for each scrambling block (2 for the SRAM key), we are instead just taking the LSB 8 bytes of the generated 64bit seed and writing it in that index. This means that the majority of the key will still contain the initialized values, and 56 bits of each generated key is ignored. Correct the writeback logic, iterating over each byte in the scrambling block and individually writing each byte. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
…lementation Earlgrey's OTP (`ot_otp_eg`) has long since become outdated compared to Darjeeling's OTP (`ot_otp_dj`), which has seen a lot of active development and is now much more feature-complete. This commit aims to bring EG's OTP to parity with DJ's OTP, creating a new Earlgrey OTP implementation based on the current Darjeeling OTP. That means that the contents of `ot_otp_eg` and `ot_otp_dj` are now broadly the same, with the following exceptions: - EG has 11 less partitions, and the relevant logic is updated to match. In particular, EG has no `SECRET3` partition. - EG has no notion of `iskeymgr_owner` partitions, but this property is kept to keep the EG & DJ implementations as close as possible. - Additional `entropy_cfg` state exists in EG loaded from the HW_CFG1 partition, which does not exist in DJ. By default we do not prevent CSRNG app reads if no OTP configuration is loaded. - The EG HW_CFG1 OTP contains fields that are single bytes in size, which causes some issues with the existing register addressing. This is stubbed as a TODO for this commit. - EG does not have a CREATOR_SEED or OWNER_SEED secret stored in its OTP SECRET partitions, unlike Darjeeling. - EG has no `SOC_DBG_STATE` field in its HW_CFG1 partition unlike DJ. - The EG code is re-ordered to match the DJ implementation, to improve maintainability and potentially facilitate a future improved combination of the two implementations into a single source, As part of this work I have also fully updated the Earlgrey register mappings to fix any issues / missing fields etc. The Earlgrey machine definition has been updated to add the connection between the OTP Controller and pwrmgr to allow OpenTitan to continue to boot after this commit. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that Earlgrey's OTP is updated to match Darjeeling and can support broadcast signals from the lifecycle controller, hook up the three broadcast signals that are relevant on Earlgrey (OWNER_SEED is not stored in OTP on EG, unlike DJ, and so there is no need for connections with the OTP here). Also connects the escalation signal, though this has limited support in the otp_ctrl at present. The `OT_LC_CREATOR_SEED_SW_RW_EN` also needs to be connected to the OTP. Even though it sounds like this should only sound the Software read / writability of the Creator Seed (stored in flash on Earlgrey), it actually controls any keymgr_creator partition, which includes the SECRET2 partition containing the Creator Root Key. So this is maybe more aptly a "CREATOR_SECRET_SW_RW_EN" seed, as it pertains to many creator secrets and not just the Creator Seed. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Slightly refactor OTP constant loading to reduce repetition as part of this change, so that it now has more generic logic to load and rename (prefix) the loaded digest constants. Now also only loads digest pairs if they exist, which is needed because Darjeeling does not have flash data or flash address key OTP digest constants. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
…neration Previously, both Earlgrey and Darjeeling's OTP controllers only supported getting the SRAM scrambling key, despite the fact that both tops have more than one scrambling key stored in their OTP (both have an SRAM and an OTBN scrambling key). Replace the `...generate_otp_sram_key` function with a more generic `generate_scrambling_key` which more closely emulates the HW OTP scrambling interface. This can then be given appropriate parameters for each key, including IVs and finalization constants as well as options for whether to fill the key nonce with entropy and/or create ephemeral scrambling keys instead of static scrambling keys. This also moves the entropy faking inside this generic function, to avoid duplication across different scrambling key generation calls that might each individually need entropy. This commit only performs the conversion, and does not add any additional scrambling key logic. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that cfggen.py can export config files with the flash address key and flash data key IV and finalization constant values for generating the scrambling keys, add these to the Earlgrey OTP model in the same way as is already done for the SRAM constants. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
… OTP Now that the OTP scrambling logic has been made generic based on the scrambling request bundle interface used in the RTL, we can fairly easily add the option to generate/get the OTBN scrambling key by applying the matching parameters. Do this for both Earlgrey and Darjeeling's OTP, since both store the OTBN scrambling key in the SECRET1 OTP partition. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that the OTP scrambling logic has been made generic, we can fairly easily add the option to generate/get the flash address and flash data scrambling keys by applying the matching parameters. This is only applied on Earlgrey, because Darjeeling does not have flash address/data scrambling keys stored in its OTP. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Earlgrey's OTP has some SWCFG values that are sub-word (bytes, tightly packed). The existing `..._swcfg_reg_name` function uses the register (word-address) offset to determine the swcfg name, so change this to use the byte address accordingly to fix this issue. Also apply this patch to Darjeeling for consistency, though this makes no difference as Darjeeling has no sub-word SWCFG OTP values. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
There was a "clang-format on" with a missing "clang-format off" so the majority of this file wasn't being checked for formatting. Add the missing format string and run clang-format. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
… properties This will allow different OpenTitan machines to use OTP backends with different timing configurations as is needed. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The read and write times for the OTP backend appear to be somewhat arbitrary values. Actual Earlgrey tests fail to pass OTP timing expectations under these default values, so it must be that actual OTP tends to be a bit faster than these emulated timings. To match the speed expected by some Earlgrey tests, halve the write time to take 25 us per cell write instead of 50 us. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Now that the OTP changes are backported from Darjeeling, the device status should match what is reported in Darjeeling's documentation. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
…laky This passes 99 out of 100 times for me, but had 1 failure with the following error: E00004 profile.c:15] CHECK-fail: cycles <= 4294967295U As such, this seems to be timing related, and the test should be marked as flaky on QEMU for now. Long-term, the test should conditionally increase its max cycle bound in the QEMU environment, as there is still value in running the test. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This was referenced Sep 9, 2025
|
Progresses lowRISC/opentitan#28122 |
This was referenced Sep 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 updates Earlgrey's OTP to match the current state's of Darjeeling's OTP. It also replaces the existing SRAM scrambling key computation with a generic scrambling key computation that can handle both ephemeral and static scrambling keys, and implements the OTBN / flash data / flash address scrambling key generation. There are also a variety of miscellaneous fixes. I would highly recommend reviewing commit-by-commit.
Aside: The commit recreating the EG OTP shows a horrible diff - to review, I would recommend checking out that commit and diffing the
hw/opentitan/ot_otp_eg.candhw/opentitan/ot_otp_dj.cto see the changes that were made specifically for Earlgrey.@rivos-eblot / @loiclefort Could you double check the Darjeeling OTP changes look correct? I might have missed something in one of my changes and I don't know how to easily check Darjeeling at the moment.