[hmac] Initial implementation of save & restore #21307

andreaskurth · 2024-02-12T15:14:01Z

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:

Start processing message stream A by configuring HMAC and then setting the CMD.hash_start bit.
Write an arbitrary number of message blocks to HMAC's MSG_FIFO.
Stop HMAC by setting the CMD.hash_stop bit and wait for the hmac_done interrupt (or poll the interrupt status register).
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.)
Repeat steps 1-4 for message stream B.
Restore the context of message stream A by writing the CFG, DIGEST_0..7, and MSG_LENGTH_{LOWER,UPPER} registers.
Continue processing message stream A by setting the CMD.hash_continue bit.
Write an arbitrary number of message blocks to HMAC's MSG_FIFO.
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:

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?).

--> tracked in #21708

andreaskurth · 2024-02-12T15:22:18Z

@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 · 2024-02-13T10:04:27Z

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

rswarbrick

Some nitty comments about the first few commits. I've got as far as "Make MSG_LENGTH CSRs SW-writable". There are a few commits to go yet...

hw/ip/prim/rtl/prim_sha2_32.sv

hw/ip/hmac/dv/env/seq_lib/hmac_base_vseq.sv

rswarbrick

Lots of nitty comments, but this looks sensible in general.

In future, I'd definitely suggest splitting up giant PRs into chains that depend on one another. It becomes a bit easier to review and you can "merge the first chunk" rather quicker :-)

hw/ip/prim/rtl/prim_sha2.sv

hw/ip/prim/rtl/prim_sha2_32.sv

hw/ip/hmac/data/hmac.hjson

hw/ip/hmac/dv/env/seq_lib/hmac_base_vseq.sv

hw/ip/prim/rtl/prim_sha2.sv

hw/ip/hmac/dv/tests/hmac_base_test.sv

hw/ip/hmac/dv/env/hmac_env_cfg.sv

hw/ip/hmac/dv/env/seq_lib/hmac_base_vseq.sv

hw/ip/hmac/doc/programmers_guide.md

msfschaffner · 2024-02-15T19:29:11Z

hw/ip/hmac/data/hmac.hjson

@@ -195,6 +201,24 @@
                based on currently received message.
                '''
        }
+        { bits: "2",
+          name: "hash_stop",


@moidx @jadephilipoom @ballifatih can you take a look at the API changes to double check this is fulfills the requirements of the cryptolib?

msfschaffner · 2024-02-15T23:56:03Z

hw/ip/hmac/doc/programmers_guide.md

+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_restart` 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) by setting `CMD.hash_process` and waiting for the `hmac_done` interrupt / status bit, finally reading the digest from the `DIGEST` registers.


@moidx @jadephilipoom @ballifatih FYI

msfschaffner · 2024-02-15T23:58:58Z

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.

jadephilipoom

This looks good to me! I can confirm that the need to only stop at full message blocks shouldn't be an issue for SW (we can just buffer the trailing partial message block and store it along with the hash state until we're ready to resume).

gdessouky · 2024-02-16T16:01:56Z

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 :(

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

The components of the `event_intr` signal can directly be fed into the `prim_intr_hw` modules, so there's no need for this intermediary signal. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

This signals whether the hash computation is actively running -- as opposed to `idle_o`, which also is also false (thus 'busy') when waiting for a FIFO input. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

Prior to this commit, *done* would only be signaled when done with processing a full message. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

The chances for doing this save & restore are controlled by the knob introduced in the previous commit. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

With the addition of *save & restore* (PR lowRISC#21307) and *wider digest and configurable key length* (PR lowRISC#21604), HMAC will see major changes. For this reason, this commit gives HMAC a major version bump to 2.0.0 and resets its development stages to L1 (Development), D1 (Functional), V0 (*Initial Work* because the DV environment may not be completely functional for some time), and S0 (*Initial Work* because the DIF may not be completely functional for some time). The previous version 1.0.0 got assigned a commit ID for future reference even though it had not reached L2, V3, and S3. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

With the addition of *save & restore* (PR #21307) and *wider digest and configurable key length* (PR #21604), HMAC will see major changes. For this reason, this commit gives HMAC a major version bump to 2.0.0 and resets its development stages to L1 (Development), D1 (Functional), V0 (*Initial Work* because the DV environment may not be completely functional for some time), and S0 (*Initial Work* because the DIF may not be completely functional for some time). The previous version 1.0.0 got assigned a commit ID for future reference even though it had not reached L2, V3, and S3. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

msfschaffner · 2024-02-27T23:06:36Z

Awesome, thanks for getting this merged!

cdgori

Catching up to SHA2/HMAC changes in reverse chronological order. This LGTM.

andreaskurth added Component:DV DV issue: testbench, test case, etc. Component:RTL IP:hmac labels Feb 12, 2024

andreaskurth assigned andreaskurth and gdessouky Feb 12, 2024

andreaskurth requested a review from a team as a code owner February 12, 2024 15:14

andreaskurth requested review from hcallahan-lowrisc, rswarbrick, gdessouky, vogelpi and msfschaffner and removed request for a team February 12, 2024 15:14

andreaskurth force-pushed the hmac-save-and-restore branch 2 times, most recently from a130858 to dd8b50f Compare February 13, 2024 09:47

andreaskurth force-pushed the hmac-save-and-restore branch from 996a787 to 9b15e15 Compare February 13, 2024 14:57

rswarbrick reviewed Feb 14, 2024

View reviewed changes

hw/ip/prim/rtl/prim_sha2_32.sv Show resolved Hide resolved

hw/ip/hmac/dv/env/seq_lib/hmac_base_vseq.sv Outdated Show resolved Hide resolved

rswarbrick reviewed Feb 14, 2024

View reviewed changes

msfschaffner added the kokoro:rebuild label Feb 15, 2024

opentitan-github-bot removed the kokoro:rebuild label Feb 15, 2024

msfschaffner reviewed Feb 15, 2024

View reviewed changes

msfschaffner requested review from moidx, jadephilipoom, ballifatih and msfschaffner February 15, 2024 19:29

msfschaffner reviewed Feb 15, 2024

View reviewed changes

jadephilipoom approved these changes Feb 16, 2024

View reviewed changes

andreaskurth added 14 commits February 26, 2024 21:10

[hmac/dv] Scoreboard changes for making MSG_LENGTH CSR writable

cf0a2c4

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac/dv] Add task to write the MSG_LENGTH CSR

c1d756c

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[prim_sha2] Add hash_continue_i

a1a2741

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Add *stop* and *continue* bits to CMD CSR

63c808e

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac/dv] Add functions to stop and continue

60d8e37

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Implement hash continuing in hmac_core

c070178

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Implement *hash continue* command

3b819d9

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Remove event_intr signal

54a29b5

The components of the `event_intr` signal can directly be fed into the `prim_intr_hw` modules, so there's no need for this intermediary signal. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[prim_sha2] Add hash_running_o

991905c

This signals whether the hash computation is actively running -- as opposed to `idle_o`, which also is also false (thus 'busy') when waiting for a FIFO input. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Implement *hash stop* command

e114038

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac] Signal *done* also when told to stop and hash is complete

46a85b3

Prior to this commit, *done* would only be signaled when done with processing a full message. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac/dv] Add randomization knob to exercise save & restore

fc3dd10

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac/dv] Randomly save & restore in wr_msg task

9063ed2

The chances for doing this save & restore are controlled by the knob introduced in the previous commit. Signed-off-by: Andreas Kurth <adk@lowrisc.org>

[hmac/doc] Document context save & restore

f7bc865

Signed-off-by: Andreas Kurth <adk@lowrisc.org>

andreaskurth force-pushed the hmac-save-and-restore branch from ad6c1ba to f7bc865 Compare February 26, 2024 21:10

andreaskurth added the kokoro:rebuild label Feb 27, 2024

opentitan-github-bot removed the kokoro:rebuild label Feb 27, 2024

andreaskurth mentioned this pull request Feb 27, 2024

[hmac] Bump version to 2.0.0 and reset development stages #21703

Merged

andreaskurth merged commit 0ebe13d into lowRISC:master Feb 27, 2024
33 checks passed

andreaskurth deleted the hmac-save-and-restore branch February 27, 2024 10:19

andreaskurth mentioned this pull request Feb 27, 2024

[hmac] Add support for SHA / HMAC context switch #49

Closed

This was referenced Feb 27, 2024

[hmac,rtl] Handle incomplete message size after cmd.hash_stop #21710

Closed

[hmac,doc] Consolidate documentation changes #21711

Closed

This was referenced Mar 28, 2024

[hmac] D2(S) Signoff #20996

Closed

[hmac] V1 Signoff #21031

Closed

cdgori reviewed May 2, 2024

View reviewed changes

martin-velay mentioned this pull request Jun 6, 2024

[hmac] V2(S) Signoff #22471

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[hmac] Initial implementation of save & restore #21307

[hmac] Initial implementation of save & restore #21307

andreaskurth commented Feb 12, 2024 •

edited

Loading

andreaskurth commented Feb 12, 2024

andreaskurth commented Feb 13, 2024

rswarbrick left a comment

rswarbrick left a comment

msfschaffner Feb 15, 2024 •

edited

Loading

msfschaffner Feb 15, 2024

msfschaffner commented Feb 15, 2024

jadephilipoom left a comment

gdessouky commented Feb 16, 2024

msfschaffner commented Feb 27, 2024

cdgori left a comment

[hmac] Initial implementation of save & restore #21307

[hmac] Initial implementation of save & restore #21307

Conversation

andreaskurth commented Feb 12, 2024 • edited Loading

andreaskurth commented Feb 12, 2024

andreaskurth commented Feb 13, 2024

rswarbrick left a comment

Choose a reason for hiding this comment

rswarbrick left a comment

Choose a reason for hiding this comment

msfschaffner Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

msfschaffner Feb 15, 2024

Choose a reason for hiding this comment

msfschaffner commented Feb 15, 2024

jadephilipoom left a comment

Choose a reason for hiding this comment

gdessouky commented Feb 16, 2024

msfschaffner commented Feb 27, 2024

cdgori left a comment

Choose a reason for hiding this comment

andreaskurth commented Feb 12, 2024 •

edited

Loading

msfschaffner Feb 15, 2024 •

edited

Loading