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

[rtl] Expose ICacheScrNumPrinceRoundsHalf parameter #2164

Merged
merged 1 commit into from
May 2, 2024

Conversation

vogelpi
Copy link
Contributor

@vogelpi vogelpi commented May 2, 2024

This parameter allows integrators controlling the number of PRINCE half rounds in the scrambled ICache SRAM primitives, e.g., to balance timing impact and security guarantees.

@vogelpi
Copy link
Contributor Author

vogelpi commented May 2, 2024

FYI: This is needed for OpenTitan where the ICache scrambling is acceptable to be slightly weaker compared to the scrambling of the main SRAM.

Copy link
Contributor

@rswarbrick rswarbrick 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 this is changing behaviour because the currently checked-in version of the prim has a default of 2 (in the Ibex repository).

Is the change intentional? If so, would you mind doing it as two commits? The first could be a no-op and pass the default value, which would mean all the line noise from the reformatting was for the no-op. Then the second commit would be a 1-line 2->3 change.

@vogelpi
Copy link
Contributor Author

vogelpi commented May 2, 2024

It's correct that the currently checked in version of the prim uses 2 half rounds. In upstream OpenTitan it's 3 already and we need to move back to 2 for just the ICache. If we re-vendored in the prims from OT, it would be 3 as a default here too.

@vogelpi
Copy link
Contributor Author

vogelpi commented May 2, 2024

Would you prefer me switching to 2 here?

@rswarbrick
Copy link
Contributor

I think I'm suggesting switching it to 2 here to keep things unchanged locally. Then presumably rv_core_ibex can be instantiated with the parameter set to 3 in OpenTitan, right?

@rswarbrick
Copy link
Contributor

Oh! Sorry! Now I understand (having read both messages...) I guess I'd suggest leaving things so that we have the values from the prim that is currently vendored into Ibex. If not, I'd definitely split the change out to a separate commit in the same PR.

This parameter allows integrators controlling the number of PRINCE
half rounds in the scrambled ICache SRAM primitives, e.g., to balance
timing impact and security guarantees.

Signed-off-by: Pirmin Vogel <[email protected]>
@vogelpi vogelpi force-pushed the expose-num-prince-rounds-param branch from 3018169 to 5e004e1 Compare May 2, 2024 15:50
@vogelpi
Copy link
Contributor Author

vogelpi commented May 2, 2024

Alright, I've switched it to 2 in this PR.

@vogelpi vogelpi requested a review from rswarbrick May 2, 2024 16:24
@vogelpi vogelpi added this pull request to the merge queue May 2, 2024
Merged via the queue into lowRISC:master with commit eea2bf0 May 2, 2024
11 checks passed
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.

2 participants