[otbn] D2S Signoff

[msfschaffner]

Description

Ensure D2S signoff criteria are fulfilled after focus area changes have landed.

[GregAC]

Review Hash: d6f2de9

Commits since Earlgrey-ES tapeout

0856354 [doc] Fix typo & wrong links in OTBN
3938983 [otbn] Add missing CM annotation
5228dfc [otbn] Fix interpretation of lc_rma_req signal values
5b9cbee [warnings,otbn,chip_level] Clean up some VCS warnings
66472e2 [pre_syn] Include csrng_pkg.sv to re-enable Yosys synthesis
5639924 Revert "[edn] Move prim_edn_req out of prim"
c0bb145 [otbn] Slightly weaken some loop stack assertions
cc86151 [otbn,dv] Wait for longer for RF use in otbn_rf_base_intg_err_vseq
fe702d3 [otbn,dv] Sync comments with integrated branch
ee913c5 [otbn,dv] Fail properly if a wait times out
9294ad3 [otbn,dv] Wait a bit longer for secure wipe at start of vseq
f390f06 [otbn/lint] Line length fix
fea9bd0 [otbn, rtl] Comment tweaks
039ab4b [otbn,rtl] Refactor loop control so commit doesn't factor into prefetch
c721c51 [rtl, prim] Add 'commit' functionality to prim_count
5200790 [otbn,rtl] Simplify logic driving state_reset
6297660 [otbn,rtl] Rework imem fetch logic for branches
69cbddb [otbn,rtl] Fix timing on instruction memory request
6bac489 [otbn, rtl] Fix timing on base RF data outputs
61a237e [util/reggen] reverse order of substruct generation
fc84846 [reggen,hw] Create index parameter for registers windows
3b4e36e [edn] Move prim_edn_req out of prim
de31bdf [reggen] Remove the devmode input
803d9ae [prim, rom_ctrl] Remove S&P layer from data scrambling
364b304 [crypto] Update benchmarks for ECDSA-P256 and RSA.
963a500 [doc] Minor tweak to md sanitisation code
68d9716 [doc] Fix CSR and WSR tables in OTBN README
5be278b [aes, kmac, otbn] Perform final clean -purge step in Yosys synthesis
efe8ada [otbn,sw] Tweak assembly to use named OTBN CSRs
625a69f [otbn]: Add ISR support to otbn_as and otbn_objdump
4002b28 [otbn,python] Tweak code to avoid some flake8 errors
4497d19 [otbn,python] Fix incorrectly indented line in insn_yaml.py
b3147f5 [otbn,doc] Extract list of ISRs into YAML files
ca2b62b [dv, testplan] Replace descr by desc for consistency
1c6ddfa [doc] Add an OTBN introduction page.
d23672d [otbn,dv] Make "dirty" flag more selective in otbnsim ext_regs.py
cbde2e4 [otbn,sival] Extend otbn testplan with SiVal tests
7b89440 [dv,otbn] Fix raw hierarchical reference in a sequence
6564b8f [dv,otbn] Fix otbn_env.core missing dependency
564627c [otbn,dv] Be more explicit about not tracking URND changes in wsr.py
7c1f177 [otbn,dv] Simplify types for URND value/next_value in wsr.py
ba50371 [otbn,dv] Remove some do-nothing 'return' lines in wsr.py
c758fe9 [otbn,dv] Remove bogus abort() for URND in wsr.py
222db28 [otbn,dv] Make update of URND _state field slightly clearer
95ccc4e [otbn,dv] Get rid of "_out" field in wsr.py
50216ca [otbn,dv] Add underscores to some internal fields in wsr.py
1b16ca2 [reggen] Add mubi support SWAccess that sets/clears a reg
a793023 [otbn,dv] Make types slightly cleaner in ext_regs.py
31cbfd6 [otbn, doc] Document requirements on lc_ctrl connections
a6bbefa [otbn] Fix interpretation of lc_escalate_en signal values
d9b9221 [otbn,dv] Rewrite a line more efficiently in wsr.py
1ae6c74 [doc] Fix broken link in ISA Guide
4f61c3c [otbn,dv] Strengthen otbn_ctrl_redun_vseq check
99db027 [otbn,dv] Correct HDL access in otbn_ctrl_redun_vseq.sv
20f0ba7 [otbn,dv] Add a "safe" mode to RIG; use it in otbn_ctrl_redun test
59f8142 [doc] Moved badges over to using hosted images
b348d94 [doc] Fixed links to the getting started section.
a53d500 [doc] otbn registers and interfaces now use CMDGEN
f56cb90 [otbn,dv] Delay "start" signal for OTBN in pre-DV code by a cycle
a405062 [otbn,dv] Wrap some secure wipe assertions in a named block
7688e71 [reggen] Add initial support for version and cip_id hjson fields
70a2e02 Revert "[reggen] Add initial support for version and feature regs"
fbd888e Revert "[reggen] Add CIP_IDs and bump all major versions"
9e6df0d [otbn,dv] Wait more carefully for secure wipes in vseqs
14835e5 [otbn,dv] Wait longer for model status change after escalation
2d1e2a4 [otbn,dv] Wait longer for model status change in otbn_scoreboard.sv
d50bbe8 [otbn,dv] Disable RTL assertions in otbn_zero_state_err_urnd test
4929482 [otbn,dv] Wait long enough for RF read in otbn_rf_base_intg_err_vseq
0ba10b3 [reggen] Add CIP_IDs and bump all major versions
d2044b6 [reggen] Add initial support for version and feature regs
fc0d332 [otbn,dv] Fix missing connections when binding in otbn_loop_if
cd28cee [otbn,dv] Explicitly check some calls to uvm_hdl_read
c408661 [otbn,dv] Factor out some UVM spin-wait logic
9ab302c [otbn,dv] Add some missing "protected" keywords to vseqs
b1daa11 [otbn,dv] Avoid Xcelium lint warning in otbn_trace_if
01c5995 [otbn,dv] Wait longer for secure wipe in otbn_intg_err_vseq
a19d366 [otbn,dv] Fix logic when waiting in start_running_otbn
2c73763 [otbn,dv] Fail properly in otbn_rf_base_intg_err_vseq if RF not used
0982be6 [otbn,dv] Be more explicit in yields in insn.py:open
d440d94 [otbn,dv] Tweak length of time to wait for reset
fc0cde7 [otbn,dv] Properly notice mismatches in insn_cnt in otbn_top_sim.sv
a67d90b [otbn,dv] Don't apply loop warps in Verilator when no longer running
e47df29 [misc] Use lc_tx_t testing functions at endpoints
a557c07 [otbn,dv] Fix trivial Python lint bugs in otbnsim
c6dc166 [otbn,dv] Fix indentation in otbn_escalate_if.sv

Of these commits, the follow effect RTL functionality:

5228dfc [otbn] Fix interpretation of lc_rma_req signal values
a6bbefa [otbn] Fix interpretation of lc_escalate_en signal values

• Security related fixes, ensuring lc signals are being correctly used (signals other than ON must be interpreted as OFF for non-escalation LC signals). This is needed for PROD, see #19050.

039ab4b [otbn,rtl] Refactor loop control so commit doesn't factor into prefetch
c721c51 [rtl, prim] Add 'commit' functionality to prim_count
5200790 [otbn,rtl] Simplify logic driving state_reset
6297660 [otbn,rtl] Rework imem fetch logic for branches
69cbddb [otbn,rtl] Fix timing on instruction memory request
6bac489 [otbn, rtl] Fix timing on base RF data outputs

• Timing fixes from integrated

803d9ae [prim, rom_ctrl] Remove S&P layer from data scrambling

• Alters use of scrambling in OTBN, improves resilience against fault injection, see RFC: Removing the S&P Layer in Scrambling for Top Earl Grey #20788 and [OTBN/SRAM/Icache] Investigate interaction of scrambling and bus integrity #10976

Closed Issues

These issues have all been reviewed, comment has been made on issues of particular interest/importance

• [entropy_src, OTBN] Consider renaming or removing the FIPS signal #20949
  • Not directly OTBN RTL related, though in particular has favoured a SW controlled FIPS bit from the entropy source and hasn't removed the OTBN error on de-asserted FIPS bit, worth creating an issue to track removing/refactoring this for a future release (e.g. SW configurable reg with a protected write-enable that configs if it's a register)
• [dv] Add countermeasure annotations in RTL #20887
• [otbn] Carry over timing optimizations from Darjeeling #20827
• [dv,otbn] Fix build failures #19815
• [dv/prim_one_hot_check] Fix coverage hole due to AddrCheck set to zero #19404
• [flash_ctrl/otbn/lc_ctrl] Consider routing the RMA signals back to LC in parallel #19136
• [otbn] Fix OTBN usage of life cycle control signals #19050
• [rtl] Are design guidlines for use of prim_lc_sync correct? #19044
• [otbn/rtl] Consider separate reset scrambling key and nonce for imem and dmem #18267
• [otbn] Overwrite DMEM contents using new scrambling key before RMA entry #18114
• [dv/cdc] Enable CDC instrumentation in all ips #16689
  • Note this issue is closed but the issue tracking OTBN cdc specifically is open [otbn,dv] CDC randomisation test failures #18856
• Aligning behavior on local caching of entropy for secure wipe #15473
• [top_earlgrey, rtl] Instantiate EDN1 with NumEndPoints set to 1 #15393
• [otbn] DMEM and IMEM read blanking during secure wipe #14066
• [otbn] Check seed_valid bit of OTP scrambling interface #13201
• [sram_ctrl, flash_ctrl, icache, otbn] Protecting scrambling keys at rest #12111
  • Superseded by issue [otbn] Protect scrambling keys at rest #20162 (still open)
• [OTBN/SRAM/Icache] Investigate interaction of scrambling and bus integrity #10976
  • Fixed with changes to data scrambling
• [otbn] Potentially refactor otbn_start_stop_control into otbn_controller #8689
• [otbn] Hardening BN.CMP and BN.CMPB #6483
• [otbn] Add vector-vector Rotate and Add SIMD instructions #5058
• [otbn] Support WSR/CSR names in assembly #3323
• [otbn] Produce multiply-routine generation script #3269
• [otbn] Evaluate performance of modular arithmetic subroutines for NIST_P384 #2856

Open Issues

• [otbn] The otbn_multi_err test currently fails with an assertion error #21658
  • Should investigate before closing D2
• [lc_ctrl/dv] Improve coverage for RMA ACK signals #21268
  • DV improvement, no D2 impact
• [otbn] V1 Signoff  #21021
  • V1 sign-off issue
• [otbn] Correct model for handling staggered rma_req_i transitions #20835
  • RTL aspect of this issue is fixed ([otbn] Fix interpretation of lc_rma_req signal values #21272) issue remains open to track model changes which are lower priority
• [otbn,darjeeling] Loop stack push/pop at same time #20792
  • Should investigate before closing D2
• [otbn/doc] CSR and WSR rendering #20515
  • Documentation improvement
• [otbn] Protect scrambling keys at rest #20162
  • Security improvement to consider for a future release (agreed by the security working group)[otbn/dv] Improve pass rate of otbn_escalate #18124
• [HW, OTBN] multi-driver to wr_error_o #19583
  • Signal is FPGA register file has multiple drivers, no silicon impact
• [otbn, dv] OTBN regression failure triage with CDC instrumentation #19045
  • Triage of DV failures with CDC instrumentation enabled. Triage shows these are all DV environment issues other than one indicating an RTL issue which is fixed.
• [otbn,dv] Complete coverage and tests for the various checks that use pre-decode #19043
  • V3 related work, no D2 impact
• [otbn,dv] Check that we really do clear the call stack on wipe #19041
  • Testbench improvement, no D2 impact
• [dv] Remaining OTBN DV TODOs #19034
  • Testbench improvements, no D2 impact
• [otbn,dv] CDC randomisation test failures #18856
  • Related to [otbn, dv] OTBN regression failure triage with CDC instrumentation #19045 CDC issues has been triaged and one RTL problem fixed. The remaining issues are all dv environment related.
• [otbn] Tests to V2(S) levels  #18417
  • V2S related work, no D2 impact
• [doc] Signals between blocks can be very hard to read #18308
  • Documentation improvement
• [otbn/rtl] Holes in end to end integrity implementation #18266
  • Security improvement for future release (discussed in security working group)
• [otbn/dv] Improve pass rate of otbn_escalate #18124
  • DV improvement, no D2 impact
• [dvsim] Riviera/Questa support is broken - around otbn_memutil #18039
  • DV environment issues with Questa & Riviera
• [otbn] Harden req/ack handshake of scrambling key interface #17772
  • Security improvement intended to be included for PROD, but no large PD impact, reasonable to close D2 without it
• [otbn/dv] Investigate coverage holes around errors #17662
  • Improving coverage for errors, no D2 impact
• [top_darjeeling] Memory scrambling and ECC | undetected faults #17661
  • Addressed by [top_darjeeling] Memory scrambling and ECC | undetected faults #17661 (PR already merged)
• [otbn] Add assertions to check if local escalation can trigger fatal alerts #17652
  • DV improvement, no D2 impact
• [otbn] OTBN autogenerated headers do not have value defines for CMD, STATUS #16754
  • SW/tooling related clean-up
• [dv/sec_cm] Add assertions to check if local escalation can trigger fatal alerts #16269
  • DV improvement, no D2 impact
• [otbn,dv] Model mismatch for URND in LOCKED state #15710
  • DV environment issue, no D2 impact
• Primitives to hold errors high until reset #15644
  • Potential general security improvement, highlighted by fix to OTBN issue. To be considered for future release
• [otbn,dv] Parameterize otbn_intg_err base class  #15527
  • Potential DV improvement, no D2 impact
• [chip, dv, cov] Address low coverage of alert_dump and crashdump signals #15427
  • Coverage closure work, no D2 impact
• [otbn,dv] Modelled RMA request behaviour is not matching vs RTL #14974
  • DV environment issue, no D2 impact
• [dif] Add missing dif to access fault CSRs #14518
  • SW issue
• [otbn,rtl] Multi-bit encode secure wipe signals #12061
  • Security improvement, to be completed for M3
• [req/ack] Examine req/ack interfaces between IP blocks and enhance their security #11172
  • Also tracked by [otbn] Harden req/ack handshake of scrambling key interface #17772 security improvement to be completed for M3
• [otbn] Dmem window enhancement #11171
  • Security enhancement, not being considered for PROD
• Brainstorming future OTBN ISA extensions #10750
  • Issue capturing ideas for a next generation OTBN
• [otbn] Refactor decode to merge alu and non alu control decode #9522
  • RTL clean-up with no functional impact
• [otbn] Add functions to classify err_bits #9480
  • RTL clean-up with no functional impact
• [otbn,dv] Cover all points in insn_jal_cg and insn_jalr_cg #8090
  • Coverage improvements, no D2 impact
• [otbn,dv] Generate extremes for offset immediate for BN.*ID #8089
  • Coverage improvements, no D2 impact
• [otbn,dv] We don't hit extreme values of immediates in enc_i_cg #8080
  • Coverage improvements, no D2 impact
• [otbn,dv] We don't hit extreme values for offset in enc_s_cg #8079
  • Coverage improvements, no D2 impact
• [otbn,dv] We struggle to hit flags_11_cross in enc_bnai_cg #8076
  • Coverage improvements, no D2 impact
• [otbn,dv] Cover the "branch to self" coverpoint #8074
  • Coverage improvements, no D2 impact
• [otbn] Issue a warning in tooling when using BN.ADDI _, _, 0 #7320
  • Tooling improvement
• [otbn] Specify recommended software countermeasures #6729
  • Documentation improvement
• [dv] Backdoor loading of scrambled memories from a Verilator simulation #6665
  • DV environment improvement, no D2 impact
• [otbn] Add CSR/WSR definitions to yaml and refactor to use them #5841
  • Tooling improvement, no D2 impact
• [otbn] Add named "enums" for flag indices in assembly #4473
  • Tooling improvement, no D2 impact
• [otbn] Duplicated error code list #4450
  • Tooling/documentation improvement, no D2 impact
• [otbn] Tidy up RTL signal naming #4166
  • RTL clean-up with no functional impact
• [otbn] Instruction encoding quick reference #3438
  • Tooling/documentation improvement
• [otbn] Add support for pseudo-instructions in the assembler #2967
  • Tooling improvement

Of these open issues the following need some work before D2 sign-off:

• [otbn] The otbn_multi_err test currently fails with an assertion error #21658
• [otbn,darjeeling] Loop stack push/pop at same time #20792

Both of these don't raise any large concerns but should have some initial investigation to be on the safe side. This is not anticipated to be a significant effort.

Also of note are the open issues that require RTL work that we want done for PROD. They are all security related so should be done for M3:

• [otbn,rtl] Multi-bit encode secure wipe signals #12061
• [req/ack] Examine req/ack interfaces between IP blocks and enhance their security #11172
• [otbn] Harden req/ack handshake of scrambling key interface #17772

When #21658 and #20792 have had some initial investigation I am happy for OTBN to retain its D2 status.

[GregAC]

@rswarbrick @vogelpi @andreaskurth @msfschaffner PTAL

[GregAC]

I've now addressed the open issues #21658 and #20792, they both remain open as they do involve some RTL fixes, however none of them are essential. Rather than addressing actual bugs they're more about making the behaviour of OTBN 'tidier' in a few ways.

I don't think we need to address whether we'll merge these RTL fixes for D2 so this is ready to sign-off once reviewed.

[rswarbrick]

I've just taken a quick look through this and I think I agree with your analysis. I've also looked through the signoff checklist for D2: looks good to me.

From my side: I'm happy to close this issue as complete.

[andreaskurth]

Great, thanks @GregAC and @rswarbrick!

[vogelpi]

After cross checking with @GregAC , I've now changed the issue title to D2S. OTBN was at D2S before (and the hjson still is), where were only a very limited set of RTL changes without a version number increase (no new features or API changes). These few RTL changes were done by people knowing the design and it's security countermeasures very well (or that have designed them) and the changes were reviewed with care. I believe it's thus justified to sign off OTBN at D2S rather than D2.