[hmac] D3 Signoff

[rnongie]

Description

No response

[andrea-caforio]

This procedure covers all commits that introduced RTL/software changes in HMAC over the period between
the D2(S) signoff d748311 and the latest state of the upstream master branch f698217.

Checklist

NEW_FEATURES_D3

Any approved new features since D2 have been documented and reviewed with DV/SW/FPGA

Most of the changes since D2 concerned bugfixes around the save/restore and secure wiping capabilities,
notable changes that added new features to the IP block are as follows:

• [prim_sha2/hmac,rtl/dv] HMAC/SHA-2 secure wiping fix #22852: New key_none value in the key_length field of the CFG register and an updated order of the values in the digest_size field of the CFG register.
• [hmac,regs/rtl] Add key_swap field and expose Idle status to SW #23242: New key_swap and hmac_idle fields into CFG and STATUS registers respectively.

Both features have been properly documented and appear in the DV workflow.

TODO_COMPLETE

All TODOs are resolved. Deferred TODOs have been marked as “iceboxed” in the code and have been attached an issue as follows: // ICEBOX(#issue-nr).

The HMAC contains an unresolved TODO at rtl/hmac.sv+171:

DoneAwaitMessageComplete: begin
  if (digest_on_blk) begin
    // Once the digest is being computed for the complete message block, wait for the hash to
    // complete.
    // TODO (issue #21710): handle incomplete message size and check against 512 or 1024
    done_state_d = DoneAwaitHashComplete;
  end
end

The mentioned issue #21710 has been closed as unplanned.

LINT_COMPLETE

The lint checking flow is clean. Any lint waiver files have been reviewed and signed off by the technical steering committee.

Both veriblelint and ascentlint do not report any issues which is contrasted by the verilator output upon execution of the command:

$ ./util/dvsim/dvsim.py hw/top_earlgrey/lint/top_earlgrey_lint_cfgs.hjson --tool verilator --local --purge --select-cfgs hmac

ERROR: %Warning-WIDTH: ../src/lowrisc_ip_hmac_0.1/rtl/hmac_core.sv:296:24: Operator ASSIGNDLY expects 4 bits on the Assign RHS, but Assign RHS's CONST '3'h0' generates 3 bits.
%Warning-WIDTH: ../src/lowrisc_ip_hmac_0.1/rtl/hmac_core.sv:298:24: Operator ASSIGNDLY expects 4 bits on the Assign RHS, but Assign RHS's CONST '3'h0' generates 3 bits.
%Warning-WIDTH: ../src/lowrisc_ip_hmac_0.1/rtl/hmac.sv:553:57: Operator SHIFTR expects 32 or 7 bits on the LHS, but LHS's VARREF 'hmac_fifo_wdata_sel' generates 4 bits.

CDC_COMPLETE/RDC_COMPLETE

The CDC checking flow is clean. CDC waiver files have been reviewed and understood. If there is no CDC flow at the block-level, this requirement can be waived. The RDC checking flow is clean. RDC waiver files have been reviewed and understood. If there is no RDC flow at the block-level, this requirement can be waived.

Not relevant here.

REVIEW_RTL

A simple design review has been conducted by an independent designer.

I went through the RTL and tried to digest the design. Nothing to report here.

REVIEW_DELETED_FF

Any deleted flops have been reviewed and signed off. If there is no synthesis flow at the block-level, this requirement can be waived.

After a top-level Vivado synthesis no trimmed flip-flops were reported in the HMAC block. There is a basic
synthesis flow defined for DC for HMAC, maybe someone else with access to the tool can take a look at it.

REVIEW_SW_CHANGE

Any software-visible design changes have been reviewed by the software team.

The two new features described above induce software changes in the cryptolib drive for HMAC that have been handled in #23510.
Note that the changes are not reflected (alongside varying key sizes) in the DIF as per this open issue #22332.

REVIEW_SW_ERRATA

All known “Won’t Fix” bugs and “Errata” have been reviewed by the software team.

Nothing to report.

Commits since the D2(S) Signoff

Below we list all the commits (most recent first) that touched upon the hw/ip/hmac directory since the D2(S) signoff at d748311. Those
highlighted in yellow are commits relevant to this D3 signoff. To recreate this list run:

git log d748311..HEAD --oneline hw/ip/hmac

• 70c425d [hmac, rtl] Increase coverage through minor RTL restructure
  • PR: [hmac, rtl] Increase coverage through minor RTL restructure #25766
  • Description: Minor RTL restructures with no functional impact to increase coverage.
• b15c989 [hmac,dv] Rename intr_test to intr_test_rename
• 9d19f92 [hmac,dv] Allow reset w/o CSR accesses complete
• 2b174a1 [hmac,dv] Reset S&R skip context
• 0b1da4b [hmac,dv] Fix SCB fifo_full_detected signal
• 2d46210 [bazel,hw] exclude DV files from hw/ip/*/BUILD globs
• 734d9c1 [hmac,rtl] Use fifo_full instead of fifo_wready
  • PR: [hmac,rtl] Use fifo_full instead of fifo_wready #25844
  • Description: Fixes a spurious fifo_full signal that is asserted during a reset.
• dacadc0 [hmac,dv] Reset intr_test signal
• 1c06620 [hmac,dv] Reduce long_msg test duration
• 4f55782 [hmac,dv] Disable S&R for stress_reset test
• 5f89d80 [hmac,dv] Increase time before reset
• 37a4c97 [hmac,dv] Fix fifo_empty interrupt
• eccd638 Remove old header targets
• e90f9f2 [bazel] Use new rules to describe IPs
• 6aea90d [hmac,dv] Skip msg_length check when reset
• 3ff8ddc [hmac,dv] Wipe secret assertions
  • PR: [hmac] Wipe secret assertions and spec update #25674
  • Description: Only adds an assertion that makes sure secret values are wiped correctly after an wipe_secret command.
• 2a6843b [hmac,doc] Wipe secret doc update
• 39ca65f [hmac,dv] Move IDLE check from vseq to scb
• 4b3d2e6 [hmac, sw] Revert hash stop hang workaround
• 89a83ed [hmac,doc] S&R reg changes when IDLE
• a65355f [hmac,dv] Add a delay before issuing stop command
• a06c37c [hmac] Bump version to 2.0.1
• 393248c [hmac] Clean some TODO for V3
• 9152bef [hmac,dv] Add new test to cover FSM transitions
• 20e80d9 [hmac, rtl] Remove obsolete TODO
• 81012ea [hmac, rtl] Resolve Verilator lint warnings
  • PR: [hmac, rtl] Resolve lint warnings and TODOs for the V3 signoff #25519
  • Description: These two commits resolve the obsolete TODO and the Verilator lint warnings mentioned above. They have no functional impact.
• 92dd265 [hw,tlul_adapter_sram,rtl] Expose reserved user bits
  • PR: [hw,tlul_adapter_sram,rtl] Expose reserved user bits #25373
  • Description: Expose the reserved user bits field in the TLUL adapter.
• 5d1b525 [hmac,doc] Update for arbitrary key length
• 7f32449 [hmac,doc] Update Save and Restore SW guide
• 35f9a82 [hmac,dv] Adpot extern method declarations
• 99ef469 [hmac,doc] Align documentation with HW changes
• ad39ed2 [hmac,dv] Increase timeouts for some tests
• f02c6d9 [hmac,dv] Verify idle status register
• 9f9cf18 [hmac,dv] Extend timeouts
• 4fec3e5 [hmac,doc] Move into V2S stage
• 3fd461f [hmac,dv] Additional UNR exclusions
• c9a3203 [hmac/dv] Map tests to fifo_empty_status_interrupt testpoint
• aa17608 [hmac,dv] Fix clearing of wipe_secret flag
• e7be8f2 [hmac,dv] UNR exclusions list
• 5133650 [hmac,dv] Split S&R into sub-tasks
• ceee346 [hmac,dv] Test msg_length_upper register
• 2765d30 [hmac,dv] Fix msg_length coverage
• ecf3010 [hmac,dv] Fix testplan for hmac_test_vector names
• 77b419f [hmac,dv] Extend timeout on outstanding acceses
• 0539d58 [hmac,dv] Optimize regression duration
• 32a1a99 [hmac,dv] Fix secret wiping issues
• 1be44e2 [hmac,dv] Refactor reset handling
• 4eb3239 [hmac,dv] FIFO empty interrupt and test mode pins
• 8b560b0 [hmac,dv] Add key_swap coverage
• 2ec1df5 [hmac,dv] Verify key_swap
• 1b56f19 [hmac,dv] Increase SHA2 test vector timeout
• 2f6b4a4 [hmac,dv] Improve feature coverage
• 6409aa6 [hmac,rtl] FIFO Empty intr after FIFO seen full
  • PR: [hmac,rtl] FIFO Empty intr after FIFO seen full #23739
  • Issue: [hmac,rtl] FIFO Empty interrupt raised after FIFO seen full #23738
  • Description: Resolve an erroneous FIFO empty signal.
• 9cfd621 [hmac,dv] Add HMAC NIST test vectors
• 9fc3638 [hmac,dv] Fix rare issue with wipe_secret
• 9b45a70 [hmac,dv] SHA test vec uses arg/rand digest value
• 9ba26fc [hmac,dv] SCB digest_idx issue in SHA2_384
• a7f8506 [hmac,dv] Add block-level test vectors testing
• 8ea3710 [hmac,dv] Improve slightly fcov
• 80a16da [hmac,dv] Adopt gmv macro for clarification
• d7d34e2 [hmac,dv] Remove unnecessary comparison
• ee8b623 [hmac,dv] Break if conditions to ease debug
• e011274 [hmac,dv] Rename msg_part to msg_part_q
• 72d53c1 [hmac,dv] Disable S&R for some tests
• 72ef9b7 [hmac,dv] Add S&R coverage changes
• 7c988ea [hmac,dv] Add 'StopAndContinue' case
• 4546065 [hmac/dv] S&R with context changes
• 9759a35 [hmac/dv] SAR with wider digest/key sizes
• d508525 [hmac/dv] Test vectors SHA and cleans
• e00f490 [hmac/dv] Fix msg bursts with S&R
• 53bf2f2 [hmac/dv] S&R with same context
• 764cffc [hmac/doc] Update docs for V2 signoff
• d851a38 [hmac] Avoid inference of latch
  • PR: [hmac] Avoid inference of latch #23674
  • Description: Prevent latch inference of a combinatorial signal.
• 85baec4 [hmac,dv] Minor DV assertion fix
  • PR: [hmac,dv] Minor DV assertion fix #23504
  • Description: Fix a faulty assertion.
• 763bf42 [dv] Reduce number of seeds for stress_all_with_rand_reset tests
• d79a3bb [hmac,rtl] Coding cosmetics fixes
  • PR: [hmac,rtl] Coding style fixes #23549
  • Description: Only coding style fixes.
• 37bcf1e [hmac/lint] Remove unused localparam
  • PR: [hmac/lint] Remove unused localparam #23535
  • Description: Remove unused localparams.
• 32d52b8 [hmac,rtl] Fix Save & Restore FSM bug
  • PR: [hmac,rtl] Fix Save & Restore HMAC FSM bug #23538
  • Issue: [hmac,rtl] Save & Restore FSM bug #23530
  • Description: Resolve an edge case where the hmac_done signal is never set.
• 99ce062 [hmac, dv] Fix ValidHmacEnConditionAssert
  • PR: [hmac, dv] Fix ValidHmacEnConditionAssert #23456
  • Issue: [hmac, dv] initiated remains high during context switches #23348
  • Description: Prevent assertion from being triggered.
• 01a2089 [hmac,rtl/dv] DV synchronization and error handling fixes
  • Description: Despite the tag, there are no RTL changes here.
• f489ff7 [hmac,rtl] HMAC FSM RTL fixes to handle SAR
  • PR: [hmac,rtl] HMAC FSM fixes for SAR #23432
  • Issue: [hmac, rtl] Reset hmac_core FSM to Idle after stop #23352
  • Description: Fix an issue where two streams are interleaved in a certain manner causing a hang.
• 7d8ace2 [dv] Remove default value for wait_to_issue_reset argument
• 6c51bc0 [hmac,dv] DV fixes for block-level tests
• ee5f4a1 [sram_ctrl] Add readback feature
  • PR: [sram_ctrl] Add readback feature #23212
  • Description: New readback SRAM mode changes the TLUL/SRAM adapter module instantiation in HMAC.
• f9f6fe5 [hmac,csrs/rtl] Add key_swap field and expose hmac idle status
  • PR: [hmac,regs/rtl] Add key_swap field and expose Idle status to SW #23242
  • Issue: [hmac] Add endianness swap option to KEY registers #23158, [hmac, rtl] Expose Idle status of HMAC #22916
  • Description: New key_swap and hmac_idle fields into CFG and STATUS registers respectively.
• aace3c0 [hmac,rtl/dv] RTL and DV fixes for digest loading and error handling
  • PR: [hmac,rtl/dv] Fix digest loading/error wiping and error handling #23236
  • Issue: [hmac, rtl] Fix switching between different digest-sized streams #23181
  • Description: Rearrange digest loading order.
• 76b1c74 [hmac,dv] DV synchronization fixes for do_read_check
• c1fa322 [hmac,rtl] Minor HMAC fixes for secret key wiping
  • PR: [hmac,dv/rtl] Fix HMAC DV scoreboard synchronization #23220
  • Description: Wipe all 32 key registers and not just 8.
• 5a74a15 [hmac/rtl] Fix idle_o when told to stop
  • PR: [hmac/rtl] Fix idle_o when told to stop #23233
  • Issue: [hmac/spec] Output pin doesn't go to Idle in case of HMAC stop invoked #23036
  • Description: Only switch to idle state after a stop command when the full block has been processed.
• e71da1c [hmac/rtl] Wait for digest of complete block when stopping
• dbe6169 [prim_sha2_pad,rtl] Go to idle (without padding) when told to stop
  • PR: [prim_sha2_pad,hmac/rtl] Multiple fixes for save & restore #23224
  • Issue: [hmac/rtl] Save and Restore digest mismatch when SHA digest update is too close to a block boundary #23163
  • Description: Two commits relating to a bug where incomplete blocks interfered with the HMAC stop command.
• e01609e [hmac,doc] Documentation fixes
• 128d609 [hmac,dv] Scoreboard checks for different digest sizes
• e120a95 [hmac/prim_2,rtl] Do not clear redundant digest values
  • PR: [hmac,dv/rtl] Implement DV checks for new digest sizes/key lengths #23108
  • Issue: [test-triage] hmac_wipe_secret started failing #23053
  • Description: Replicates the wiping value across the digest register instead of setting it to 0.
• ed04dc9 [hmac, doc] Update HMAC context saving guide
• f189ae8 [hmac, doc] Update programmers guide for streaming
• 5bc6e0a [hmac, rtl] Fix indexing bug/typo
  • PR: [hmac, rtl] Fix digest reloading bug #23018
  • Issue: [hmac, rtl] Loading SHA-256 back thru DIGEST registers. #23014
  • Description: A typo caused an incorrect byte ordering after reloading the HMAC state.
• ab878b5 [hmac/dv] Cleanups and coverage fixes/improvements
• e2eff20 [hmac,dv] Extend DV to test invalid configurations
• c859a39 [hmac/prim_sha2,rtl] Implement SW error for invalid HMAC config
  • PR: [hmac/rtl,dv] Signal SW error at invalid HMAC configuration #22580
  • Issue: [hmac,rtl] Flag error for invalid digest size/key length configuration #22312
  • Description: New key_none value in the key_length field of the CFG register and an updated order of the values in the digest_size field of the CFG register.
• 4bc1570 [prim_sha2,rtl/dv] Fix secret value wiping
  • PR: [prim_sha2/hmac,rtl/dv] HMAC/SHA-2 secure wiping fix #22852
  • Issue: [hmac/rtl] Overwrite internal values with WIPE_SECRET instead of XOR'ing with it #22555
  • Description: Internal SHA2 variables are now overwritten with the secret wiping value instead of XORed. This makes the wiping value ports in the HMAC core obsolete.
• 0d0ce1a [hmac/dv] Cleanups without functional impact
• dc848b3 [sram_ctrl,prim,rtl] Fix tlul_we -> ready timing path.
  • PR: [sram_ctrl/rtl] Timing fixes #22497
  • Issue: [top_earlgrey] Timing optimizations for production #22462
  • Description: New output port in the TLUL/SRAM adapter which is also used in HMAC to fix a timing issue.
• 8295903 [bazel,autogen_hjson] Split C and rust header generation
• 30d7e78 Add the project name to the copyright header
  • PR: Add the project name to the copyright header #20823
  • Description: Only modifies copyright headers.
• d01864e Fix or waive Python lint errors and warnings
• f4c2bb9 Remove trailing whitespaces
• 5b9f3a9 [hmac] Sign off at V1
• d562315 [hmac/dv] Add testplan items for recently added features
• 7fa9df0 [hmac] Sign off at D2S
• 90aba7a [hmac,doc] Documentation updates
• 5084b38 [hmac] Coding style and minor fixes
  • PR: [hmac] Coding style and minor fixes #22247
  • Issue: [hmac/rtl] Fix suffixes of hmac_core's port names #21666
  • Description: Mostly style fixes. Addresses some issues raised in [hmac] Implementation of wider digest & configurable key length #21604.

Open Bug/Feature Issues

• [hmac] HMAC_CMD_HASH_STOP_BIT and HMAC_CMD_HASH_PROCESS commands causing hang conditions #24767
• [prim_sha2_pad, hmac] Don't allow skipping the padding after stopping a hash operation #23936
• [hmac,difs] Update DIFs and DIF-based TLTs  #22332

[rswarbrick]

Thanks for doing this carefully. Tracking through the text, I think I can see a few action items:

• Remove the TODO at rtl/hmac.sv:171.
• Can we just fix the Verilator lint warnings? At a glance, getting the widths right should be pretty trivial.
• Issues [hmac] HMAC_CMD_HASH_STOP_BIT and HMAC_CMD_HASH_PROCESS commands causing hang conditions #24767 and [prim_sha2_pad, hmac] Don't allow skipping the padding after stopping a hash operation #23936 seem to describe RTL issues that need fixing.

I believe that all three need addressing before D3 signoff is complete for this block, but (on the plus side) they hopefully give an upper bound on the changes required.

[andrea-caforio]

@rswarbrick @martin-velay

Update January 24

The open action items of the report on December 4, 2024 have been fixed. The list of commits has been amended.

[martin-velay]

LGTM, thanks Andrea !

[vogelpi]

Thanks for updating this issue @andrea-caforio . Would you mind preparing the D3 sign-off PR please? For inspiration, please check the V3 sign-off PR #26262

[vogelpi]

The PR is here: #26294

[rswarbrick]

Signoff commit merged. Congratulations, @andrea-caforio !