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.

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

[entropy complex blocks] D2S reviews completed #10486

Merged
merged 2 commits into from
Feb 7, 2022

Conversation

mwbranstad
Copy link
Contributor

Approval of this PR will close all related review issues and signify
that entropy_src, csrng, and edn blocks have completed D2S checkpoints.
Closes #10095.
Closes #10096.
Closes #10262.

Signed-off-by: Mark Branstad [email protected]

@cdgori
Copy link

cdgori commented Feb 4, 2022

I added the RTL and security council roles from all 3 D2S reviews (ES, CSRNG, EDN) as reviewers to this PR. But, this is marked as draft still (I believe pending #10621 merging and resolution of AES countermeasure labeling from #10132).

If those 2 are done, this looks good from my side.

@msfschaffner msfschaffner marked this pull request as ready for review February 4, 2022 22:21
@msfschaffner
Copy link
Contributor

msfschaffner commented Feb 4, 2022

I'll mark this as ready for review with the understanding that it should not be merged until the other PRs you mentioned have landed.

Approval of this PR will close all related review issues and signify
that entropy_src, csrng, and edn blocks have completed D2S checkpoints.
Closes lowRISC#10095.
Closes lowRISC#10096.
Closes lowRISC#10262.

Signed-off-by: Mark Branstad <[email protected]>
Signed-off-by: Michael Schaffner <[email protected]>
@msfschaffner
Copy link
Contributor

@mwbranstad I realized that not all D2S checklist items have been checked off.
I've added a commit in this PR to do that (rebased so that it comes before the .prj.hjson update commit).

Copy link

@cdgori cdgori left a comment

Choose a reason for hiding this comment

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

I think #10132 updates are in (I see the AES block countermeasure tags in the hjson), so approving this but it's not to be merged before #10621

@msfschaffner
Copy link
Contributor

Alright, all updates should be in by now.

If we all agree that #9853 does not gate D2S, we can move this in and declare D2S for the entropy complex.

Copy link
Contributor

@msfschaffner msfschaffner left a comment

Choose a reason for hiding this comment

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

LGTM from my side, but would be good to get another vote.

@cdgori
Copy link

cdgori commented Feb 7, 2022

I see #10621 is merged now, and I also agree that #9853 is functional (or 99% functional) and shouldn't gate D2S. I think we are good to merge this and move all 3 entropy blocks to D2S!

@msfschaffner
Copy link
Contributor

Thanks for checking @cdgori. I think we can proceed then. If anyone spots something that slipped through the cracks, please reach out or flag it via an issue.

@msfschaffner msfschaffner merged commit cde1eb9 into lowRISC:master Feb 7, 2022
@mwbranstad mwbranstad deleted the ec_d2sdone branch February 8, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EDN D2S Review AIs Entropy_Src D2S Review AIs CSRNG D2S Review AIs
3 participants