[dv] Remaining OTBN DV TODOs

[GregAC]

OTBN TODOs have been reviewed (for commit https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5). This tracks the remaining TODOs which don't need to be addressed for M2.5.2

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/otbnsim/sim/stats.py#L272

• [x] Small improvement to utility script not directly DV related

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/otbnsim/sim/gpr.py#L119

• [x] We may be missing a check that the secure wipe zeros the call stack. Not a concern for V2.5 as a bad zeroing of the call stack isn't a functional problem and only a minor security concern

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/otbnsim/sim/ext_regs.py#L268

• [x] Suggestion for future design improvement, no concern for V2.5

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/otbn_sim_cfg.hjson#L31

• [x] Minor cleanup, no concern for V2.5

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/otbn_sim_cfg.hjson#L73 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/tb.sv#L276

• [ ] Known issue already triaged out of V2.5 scope

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv#L25

• [x] Minor improvement, no concern for V2.5

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv#L231 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv#L241 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv#L248 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv#L256 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv#L264

• [ ] Potential improvements to how the testbench processes TL register accesses, not a V2.5 concern. Detailed modeling of these is already handled by the python OTBN model

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/model/otbn_model.cc#L712

• [ ] ISS modeling of integrity bits needs improving to handle this case. We skip checks where the ISS thinks a word has valid ECC bits but they're invalid in the RTL. Cases where the RTL has written incorrect ECC bits will be caught by other tests (e.g. spurious alerts from integrity related errors or mismatches aganst the model on instruction execution)

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/model/otbn_core_model.sv#L428

• [ ] Minor cleanup, only an issue if there are mlutiple OTBN instances in a testbench that uses the OTBN model

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/tracer/rtl/otbn_trace_if.sv#L231

• [ ] Minor improvement, no concern for V2.5

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/doc/fcov.md#L181

• [ ] Missing functional coverage point around disabling the promotion of software errors to alerts after running with it enabled. Low concern as the correct functioning of this mechanism is already demonstrated (in particular in the top-level OTBN smoke test).

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/rig/rig/gens/bad_deep_loop.py#L160 https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/rig/rig/gens/bad_deep_loop.py#L187

• [x] Random instruction generator clean-ups

https://github.com/lowRISC/opentitan/blob/be4c7d1a7e72e5d0944d82c250c076d77e4997c5/hw/ip/otbn/dv/rig/rig/gens/loop.py#L405

• [ ] Minor improvements to random instruction generator, no concern for V2.5

[GregAC]

@rswarbrick please review and let me know if you have any concerns.

[rswarbrick]

This is a V3 issue and belongs in M7 instead of M5. Moving accordingly.

[rswarbrick]

Running git grep TODO hw/ip/otbn and dropping checklist points now finds the following 21 items:

• [ ] isa.md:118:TODO:
• [ ] theory_of_operation.md:334:TODO: Specify interactions between idle_o, idle_otp_o and the clock manager fully.
• [ ] theory_of_operation.md:396: (TODO: Not yet implemented.)
• [ ] fcov.md:181:**TODO: This part is not currently tracked.**
• [ ] otbn_core_model.sv:409: // TODO: This bind is by module, rather than by instance, because I couldn't get the by-instance
• [ ] otbn_model.cc:717: // TODO: At the moment, the ISS doesn't track validity bits properly in
• [ ] sim.py:312: # TODO: Passing a nonzero value in ErrBits is incorrect: we don't
• [ ] loop.py:405: # TODO: This means we don't use x1 inside loop bodies; we need to
• [ ] otbn_trace_if.sv:231: // TODO: Tracing for read errors
• [ ] otbn_scoreboard.sv:233: // TODO: Track this more precisely. We know that it should latch !status.busy if intr_enable
• [ ] otbn_scoreboard.sv:243: // TODO: Track states other than IDLE and BUSY_EXECUTE.
• [ ] otbn_scoreboard.sv:250: // TODO: Maybe this could be tracked more precisely. It should only update when an operation
• [ ] otbn_scoreboard.sv:258: // TODO: Maybe this could be tracked more precisely. It should only update when a fatal
• [ ] otbn_scoreboard.sv:266: // TODO: Track this properly. We've got the magic number on the insn_cnt_if interface.
• [ ] otbn_sim_cfg.hjson:72: // TODO(lowrisc/opentitan#16689): Enable cdc instrumentation
• [ ] otbn_idle_checker.sv:131: // TODO(#23903):
• [ ] tb.sv:272: // TODO(#23903): This is probably not the behaviour we want, but would need an RTL fix.
• [ ] tb.sv:293: // TODO (#15710): Fix modelling of URND in the locked state.
• [ ] otbn.core:123: # TODO: set default to DC once
• [ ] otbn_controller.sv:914: // TODO(#18266): Implement GPR to ISPR end to end integrity path (ISPR writes from GPR take data