Skip to content

[rtl] Add common security features of regfiles to separate module #2266

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aklsh
Copy link

@aklsh aklsh commented Apr 19, 2025

Resolves #2217

@aklsh aklsh force-pushed the refactor-regfile-impls branch from 7f7f613 to 8466e42 Compare April 19, 2025 17:18
@aklsh aklsh marked this pull request as ready for review April 20, 2025 03:51
@aklsh aklsh marked this pull request as draft April 20, 2025 15:11
@aklsh aklsh marked this pull request as ready for review April 20, 2025 16:32
@aklsh
Copy link
Author

aklsh commented Apr 21, 2025

can someone with access to the private ci let me know what the fails are?

Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in the Ibex repository. In its current state this PR is a bit hard to review. First, please avoid making lots of white space changes. And if you do want to make a whole load of white space changes please separate them from the commits that have functional changes. Would you mind adding a comment in the common register file to say what functionality you've pulled out. And it would also be nice if you could put a short description in the pull request itself to describe what you've done and how what you've done still has the same functionality as before.

@aklsh aklsh marked this pull request as draft April 25, 2025 14:51
@aklsh aklsh force-pushed the refactor-regfile-impls branch from 2d1bb01 to 2d0414f Compare April 25, 2025 16:04
@aklsh aklsh marked this pull request as ready for review April 25, 2025 16:08
@aklsh
Copy link
Author

aklsh commented Apr 25, 2025

I've tried resolving all the whitespace changes I had introduced unintentionally @marnovandermaas

And it would also be nice if you could put a short description in the pull request itself to describe what you've done and how what you've done still has the same functionality as before.

#2217 was opened to bring together common logic present in each of the three regfile implementations into one common module which can then be instantiated in each of the three implementations.

This PR mainly combines the security hardening logic that checks for onehot encoding in various signals for glitches in addresses and write enable(s). It tries to maintain the same logic as before wrt parameterization of these checks.

@aklsh aklsh force-pushed the refactor-regfile-impls branch from 2d0414f to e3db76b Compare April 25, 2025 16:20
@aklsh aklsh force-pushed the refactor-regfile-impls branch from e3db76b to c4a9389 Compare April 25, 2025 16:26
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.

[rtl] Refactor register files to bring common logic together
2 participants