Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add command to retrieve IDevID CSR from persistent storage. #1732

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Add command to retrieve IDevID CSR from persistent storage. #1732

merged 1 commit into from
Nov 12, 2024

Conversation

clundin25
Copy link
Contributor

@clundin25 clundin25 commented Oct 23, 2024 •
edited
Loading

ROM Backwards compatibility

If the runtime is used with a ROM that does not support CSR generation, RUNTIME_GET_IDEV_ID_UNSUPPORTED_ROM is returned as an error.

This was tested manually by pulling in the ROM checked into https://github.com/chipsalliance/caliptra-sw/releases/tag/release_v20240719_1 and running the get_idev_csr test suite.

clundin25
clundin25 commented Oct 23, 2024
View reviewed changes
runtime/README.md Outdated Show resolved Hide resolved
@clundin25
Copy link
Contributor Author

@jhand2 I have a runtime implementation as well, but I need to refactor the runtime integration tests to allow setting the GENERATE_CSR flag.

mhatrevi
mhatrevi requested changes Oct 23, 2024
View reviewed changes
Copy link
Collaborator

@mhatrevi mhatrevi left a comment

Choose a reason for hiding this comment

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

Discussion pending on whether a ROM change is necessary.

@jhand2
Copy link
Collaborator

jhand2 commented Oct 23, 2024

Discussion pending on whether a ROM change is necessary.

#1687 can't be done without a ROM change because no other code can wield IDevID to sign the CSR. But anyway, we can discuss the use case and if anyone is uncomfortable with the solution.

jhand2
jhand2 reviewed Nov 5, 2024
View reviewed changes
Copy link
Collaborator

@jhand2 jhand2 left a comment

Choose a reason for hiding this comment

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

Make it about half way throught. Will take another look later today or tomorrow.

As a general comment, I think moving the log stuff to a separate PR would help make this one a lot smaller.

api/src/mailbox.rs Outdated Show resolved Hide resolved
drivers/src/idevid_csr.rs Outdated Show resolved Hide resolved
drivers/src/idevid_csr.rs Outdated Show resolved Hide resolved
drivers/src/idevid_csr.rs Outdated Show resolved Hide resolved
hw-model/src/lib.rs Show resolved Hide resolved
rom/dev/src/fht.rs Outdated Show resolved Hide resolved
rom/dev/src/flow/cold_reset/fw_processor.rs Show resolved Hide resolved
rom/dev/src/flow/cold_reset/x509.rs Show resolved Hide resolved
This was referenced Nov 5, 2024
Merged
Merged
@clundin25 clundin25 force-pushed the idevid-csr branch 4 times, most recently from bad1ed3 to 9118048 Compare November 7, 2024 19:19
jhand2
jhand2 reviewed Nov 7, 2024
View reviewed changes
drivers/src/persistent.rs Outdated Show resolved Hide resolved
error/src/lib.rs Show resolved Hide resolved
rom/dev/src/flow/cold_reset/fw_processor.rs Outdated Show resolved Hide resolved
rom/dev/src/flow/cold_reset/idev_id.rs Outdated Show resolved Hide resolved
runtime/src/get_idv_csr.rs Outdated Show resolved Hide resolved
runtime/src/lib.rs Outdated Show resolved Hide resolved
runtime/tests/runtime_integration_tests/test_get_idev_csr.rs Show resolved Hide resolved
jhand2
jhand2 previously approved these changes Nov 8, 2024
View reviewed changes
@clundin25 clundin25 force-pushed the idevid-csr branch 2 times, most recently from 360a5c6 to f6e36e7 Compare November 8, 2024 01:17
jhand2
jhand2 previously approved these changes Nov 8, 2024
View reviewed changes
@clundin25 clundin25 requested a review from mhatrevi November 8, 2024 04:54
@jhand2 jhand2 merged commit 6cd0ebe into chipsalliance:main Nov 12, 2024
11 checks passed
@clundin25 clundin25 mentioned this pull request Nov 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhatrevi mhatrevi mhatrevi approved these changes

@jhand2 jhand2 jhand2 approved these changes

@fdamato fdamato Awaiting requested review from fdamato fdamato is a code owner

@rusty1968 rusty1968 Awaiting requested review from rusty1968 rusty1968 is a code owner

@bluegate010 bluegate010 Awaiting requested review from bluegate010 bluegate010 is a code owner

@swenson swenson Awaiting requested review from swenson swenson is a code owner

@vsonims vsonims Awaiting requested review from vsonims vsonims is a code owner

@ajisaxena ajisaxena Awaiting requested review from ajisaxena ajisaxena is a code owner

@korran korran Awaiting requested review from korran korran is a code owner

@JohnTraverAmd JohnTraverAmd Awaiting requested review from JohnTraverAmd JohnTraverAmd is a code owner

@jlmahowa-amd jlmahowa-amd Awaiting requested review from jlmahowa-amd jlmahowa-amd is a code owner

Assignees

@jhand2 jhand2

@mhatrevi mhatrevi

Labels
Caliptra v1.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RFC: If requested, persist IDevID CSR for later retrieval
4 participants
@clundin25 @jhand2 @mhatrevi @nquarton