opentitan icon indicating copy to clipboard operation
opentitan copied to clipboard

[hmac] Initial implementation of save & restore

Open andreaskurth opened this issue 2 years ago • 5 comments

This is an initial implementation of save & restore for HMAC (issue #49), which should allow SW to switch between different parallel message streams. In short, SW usage follows a pattern like this:

  1. Start processing message stream A by configuring HMAC and then setting the CMD.hash_start bit.
  2. Write an arbitrary number of message blocks to HMAC's MSG_FIFO.
  3. Stop HMAC by setting the CMD.hash_stop bit and wait for the hmac_done interrupt (or poll the interrupt status register).
  4. Save the context by reading the DIGEST_0..7 and MSG_LENGTH_{LOWER,UPPER} registers. (The values in the CFG register must also be preserved, but that is purely SW-controlled so doesn't need to be read from HW.)
  5. Repeat steps 1-4 for message stream B.
  6. Restore the context of message stream A by writing the CFG, DIGEST_0..7, and MSG_LENGTH_{LOWER,UPPER} registers.
  7. Continue processing message stream A by setting the CMD.hash_continue bit.
  8. Write an arbitrary number of message blocks to HMAC's MSG_FIFO.
  9. Continue this with as many message blocks and parallel message streams as needed. The final hash for any message stream can be obtained at any time (no need for complete blocks) as before by setting CMD.hash_process and waiting for the hmac_done interrupt / status bit, finally reading the digest from the DIGEST registers.

A noteworthy limitation of this implementation is that context can only be switched on complete message blocks (512 bit for SHA-256). This is so that no additional internal state (i.e., working variables and round counter) needs to be exposed to and controlled from SW. As noted in issue #49, this shouldn't be a major limitation for SW, though.

Although this PR adds preliminary DV support for the changes, save & restore does not yet get checked by the tests (the save_and_restore_pct randomization knob is currently 0 for all tests). I have manually verified that save & restore works under the basic operating conditions for HMAC and SHA but this needs to be tested much more thoroughly, especially the corner cases. When one enables save & restores in the tests, one will get errors from DV. My first assessment is that these are DV limitations, not RTL bugs, though.

I ran a full regression on HMAC to check that this doesn't break existing functionality or DV, and I got the same pass rates as reported in nightlies -- so no breakage of existing RTL & DV expected.

Opens before this PR can be merged:

  • [x] Update documentation (mainly Programmer's Guide and Theory of Operation).

Opens that can be resolved in a follow-up PR:

  • [ ] Extend testplan and coverplan to test and cover save & restore feature.
  • [ ] Enable save & restores in tests and make necessary extensions to the scoreboard as well as burst_wr_msg so this gets correctly checked and fully covered.
  • [ ] Add additional directed test that covers context switching between multiple message streams (not sure if this has to be an UVM test, maybe a SW test would suffice?).

andreaskurth avatar Feb 12 '24 15:02 andreaskurth

@gdessouky As discussed, here's a first implementation of save & restore. I would be super happy to get your feedback on this. Unless you think this is fundamentally broken, I think it would be good to get this merged soon, so that SW folks can write a small test that exercises this to check that it meets their needs.

andreaskurth avatar Feb 12 '24 15:02 andreaskurth

kokoro reported lint errors that I've fixed by now; the remaining flow errors seem to be spurious.

andreaskurth avatar Feb 13 '24 10:02 andreaskurth

Thanks for doing this (and splitting up the work into multiple easy-to-review commits)! Overall this looks good to me, but it'd be great to get an ACK from some of the cryptolib devs to make sure this interface fulfills their requirements.

msfschaffner avatar Feb 15 '24 23:02 msfschaffner

Thanks @andreaskurth, this looks good to me too, but I still hope to go through this in detail later today. Sorry to not have gotten to it yet, but been struggling with DV issues for the extended digest feature at my end :(

gdessouky avatar Feb 16 '24 16:02 gdessouky

Force-pushed to address @rswarbrick's feedback. Most notably for users and other reviewers, the hash_restart command bit is now called hash_continue.

andreaskurth avatar Feb 20 '24 13:02 andreaskurth

Awesome, thanks for getting this merged!

msfschaffner avatar Feb 27 '24 23:02 msfschaffner