Skip to content

[keymgr_dpe] Replace keymgr in EG - PR8 crypto driver - #30677

Merged
andreaskurth merged 3 commits into
lowRISC:masterfrom
rroth-lowrisc:keymgr_dpe_PR8_cryptolib
Aug 3, 2026
Merged

[keymgr_dpe] Replace keymgr in EG - PR8 crypto driver#30677
andreaskurth merged 3 commits into
lowRISC:masterfrom
rroth-lowrisc:keymgr_dpe_PR8_cryptolib

Conversation

@rroth-lowrisc

@rroth-lowrisc rroth-lowrisc commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR8 - crypto driver

This PR is the eight in a series which will replace the keymgr with the kemgr_dpe in earlgrey. The replacement was approved by this RFC.

Merge-Dependencies

Relevant Commits

Last three two commit

Description

This PR will add a first version of the crypto lib driver for the keymgr_dpe. It adds the basic functionality to interact with the keymgr_dpe. A first draft version of the keymgr_dpe_test is already present, however some commits needs to be backported from the master branch first.

It is not yet included in the bazel build system as several dependencies are missing until the top integration is finished.

@rroth-lowrisc

rroth-lowrisc commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

DO NOT MERGE THIS PR

This PR cannot be merged until the following PR passes the CI:

The introduced keymgr_dpe drivers are not yet referenced in any build system until the TOP integration PR. Therefore none of the CI checks really verifies anything in this PR. The only reason for this PR is to break down the review effort into several smaller chunks of work along "logical" lines of code spaces.

@nasahlpa nasahlpa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Raphael, I had a look into the last three commits.
The first commit I already have reviewed in another repo.
The other two commits LGTM!

Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c Outdated
@@ -0,0 +1,212 @@
// Copyright lowRISC contributors (OpenTitan project).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to already add this test to the BUILD file but just mark it as broken such that it is not build and executed.

@nasahlpa nasahlpa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Raphael - I had a look into the last three commits:

  • The first commit I already have reviewed in another repo
  • The last two commits look good to me, they streamline the cryptolib drivers with upstream as well as adding a new test

@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch from e167c75 to 910b63b Compare July 7, 2026 14:59
@rroth-lowrisc rroth-lowrisc added the CI:Rerun Rerun failed CI jobs label Jul 8, 2026
@github-actions github-actions Bot removed the CI:Rerun Rerun failed CI jobs label Jul 8, 2026
@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch from 910b63b to a8dc659 Compare July 13, 2026 07:21
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe_test.c
@rroth-lowrisc
rroth-lowrisc marked this pull request as ready for review July 16, 2026 12:45
@rroth-lowrisc
rroth-lowrisc requested review from a team as code owners July 16, 2026 12:45
@rroth-lowrisc
rroth-lowrisc requested review from alees24, h-filali and rswarbrick and removed request for a team, alees24 and rswarbrick July 16, 2026 12:45
@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch from a8dc659 to 586a894 Compare July 24, 2026 10:23

@h-filali h-filali left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this. A couple of nits and questions but this looks very good in general!

Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c Outdated
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c Outdated
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c Outdated
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c
Comment thread sw/device/lib/crypto/drivers/keymgr_dpe.c Outdated
@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch 2 times, most recently from 612651c to 456888c Compare July 29, 2026 13:02
@rroth-lowrisc rroth-lowrisc added the CI:Rerun Rerun failed CI jobs label Jul 30, 2026
@github-actions github-actions Bot removed the CI:Rerun Rerun failed CI jobs label Jul 30, 2026
@rroth-lowrisc rroth-lowrisc added the CI:Rerun Rerun failed CI jobs label Jul 30, 2026
@github-actions github-actions Bot removed the CI:Rerun Rerun failed CI jobs label Jul 30, 2026
@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch 3 times, most recently from db2982a to 9ba31df Compare July 31, 2026 16:25
To ensure backwards compatibility, the file has been split into two
sections due to the different boot processes. The first part covers
the boot process for darjeeling and the second part covers the boot process
for earlgrey. This `if define ...` should either be replaced with two
testutils files for the `keymgr_dpe` (linked by Bazel according to the top)
or with a finer granularity of defines (e.g. function-wise)

Signed-off-by: Raphael Roth <rroth@lowrisc.org>
This commit introduces a first version of the keymgr_dpe sw in
the crypto lib. It covers the basic keymgr_dpe features.

Add source / header file **without** linking them in the
appropriate BUILD file as some dependencies do not exist yet!

This commit contains the backported changes from the following
git commit hashes (changes done in `keymgr` but relevant for
`keymgr_dpe` too):

- 9dcbb05 [sw] Fix missing switch default labels
- 4afe7cd [crypto] Move entropy_init and check
- ab8c166 [coverage/drivers] Add comments to unreached lines
- b10d2fe [crypto] Add laundered otcrypto ok status
- e587303 [crypto] Remove coverage remarks
- 068c37e [crypto] Fix usage of launder32()

Signed-off-by: Raphael Roth <rroth@lowrisc.org>
This commit introduces the test for the `keymgr_dpe` driver inside
the crypto lib. It covers the basic `keymgr_dpe` features.

Add source / header file **without** linking them in the
appropriate BUILD file as some dependencies do not exist yet!

Signed-off-by: Raphael Roth <rroth@lowrisc.org>
@rroth-lowrisc
rroth-lowrisc force-pushed the keymgr_dpe_PR8_cryptolib branch from 9ba31df to b2559c9 Compare August 3, 2026 07:23
@rroth-lowrisc

Copy link
Copy Markdown
Contributor Author

This PR also includes PR7 (#30670) which is also approved.

@andreaskurth
andreaskurth added this pull request to the merge queue Aug 3, 2026
Merged via the queue into lowRISC:master with commit 2991a73 Aug 3, 2026
101 of 106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants