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

[otbn,doc] Document the bus-accessible part of DMEM #8890

Merged
merged 1 commit into from Oct 29, 2021

Conversation

rswarbrick
Copy link
Contributor

@rswarbrick rswarbrick commented Oct 26, 2021

This doesn't yet change the implementation, nor does it change DV or
tooling code.

Fixes #8720.

(@GregAC, @imphil: Any ideas on other good reviewers for doc/spec changes like this once Philipp has left?)

@rswarbrick rswarbrick added Component:Doc Type:Enhancement Component:OTBN labels Oct 26, 2021
@rswarbrick rswarbrick requested review from GregAC and imphil Oct 26, 2021
@rswarbrick rswarbrick added this to In review in OTBN Development via automation Oct 26, 2021
@imphil imphil requested review from cdgori and felixmiller Oct 26, 2021
@imphil
Copy link
Contributor

@imphil imphil commented Oct 26, 2021

I really dislike the "lower" vs "upper" naming, as it always confuses me: when I think of a memory layout, I start at the top with address 0, and it then grows downwards. So the "upper half" is address 0 to 2k-1 for me, and the "lower half" is address 2k to 4k-1. I'd just make it explicit which addresses we mean.

Felix, Chris: Can you have a look? This feature is intended to give OTBN software a scratchpad/private memory, which cannot easily be read out from the bus. Intermediate values, keys, etc. should be stored there to give them a bit of additional protection from read-out attacks.

@rswarbrick
Copy link
Contributor Author

@rswarbrick rswarbrick commented Oct 26, 2021

@imphil: I'd forgotten you disliked that terminology. Sorry! Will re-word (and similarly in the tooling / DV patches I've just pushed).

@rswarbrick
Copy link
Contributor Author

@rswarbrick rswarbrick commented Oct 26, 2021

Done. Honestly, I think it's more clunky, but apparently not everyone has the same mental model of memory as me :-)

hw/ip/otbn/doc/_index.md Outdated Show resolved Hide resolved
GregAC
GregAC approved these changes Oct 27, 2021
Copy link
Contributor

@felixmiller felixmiller left a comment

LGTM apart from the remaining "upper" if that wasn't intended.

Out of curiosity: Would other window sizes be possible via a design parameter?

@@ -500,6 +500,9 @@ These accesses are ignored if OTBN is busy.
A host processor can check whether OTBN is busy by reading the {{< regref "STATUS">}} register.
All memory accesses through the register interface must be word-aligned 32b word accesses.

While DMEM is 4kiB, only first 2kiB (at addresses `0x0` to `0x7ff`) is visible through the register interface.
This is to allow OTBN applications to store sensitive information in the upper half, making it harder for that information to leak back to Ibex.
Copy link
Contributor

@felixmiller felixmiller Oct 27, 2021

Choose a reason for hiding this comment

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

Did you miss the "upper" here when switching to the first/second terminology.

Copy link
Contributor Author

@rswarbrick rswarbrick Oct 28, 2021

Choose a reason for hiding this comment

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

Gah! Yes, I did: thanks. For other window sizes: we could do that easily enough. The only awkward bit is that there are some (rather stupid) bits of tooling that would need to know both sizes: at the moment, my pending patches hardwire the "divide by 2" but there's no real reason we couldn't do better.

@rswarbrick rswarbrick force-pushed the otbn-2kb-dmem-doc branch 2 times, most recently from 218b252 to 6cd0338 Compare Oct 28, 2021
This doesn't yet change the implementation, nor does it change DV or
tooling code.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
@rswarbrick rswarbrick merged commit 0edab91 into lowRISC:master Oct 29, 2021
1 check passed
OTBN Development automation moved this from In review to Done Oct 29, 2021
@rswarbrick rswarbrick deleted the otbn-2kb-dmem-doc branch Oct 29, 2021
Copy link

@cdgori cdgori left a comment

Post-merge (sorry for delay in looking at this, I somehow didn't realize it was doc-only) - this LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:Doc Type:Enhancement
Projects
Development

Successfully merging this pull request may close these issues.

5 participants