[csrng] List of requested changes and improvements
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
### Description
This issue tracks the open points for the refactor and simplification efforts around the CSRNG.
**Style – All addressed in [28086](https://github.com/lowRISC/opentitan/pull/28086) and [28373](https://github.com/lowRISC/opentitan/pull/28373), still some work left to be done in `csrng_core` and `csrng_cmd_stage`**
- [ ] Consistently put spaces around operators.
- [ ] Drop module-name prefixes for module-local signals and ports
- [ ] Refactor complex nested ternary-if/else `assign` statements into readable `always_comb` processes.
**Architecture**
- [x] Remove most, if not all, FIFOs in the data path, which make up almost 50% of the area of the CSRNG without being required for timing or throughput.
10/13 FIFOs removed in [28428](https://github.com/lowRISC/opentitan/pull/28428), [28633](https://github.com/lowRISC/opentitan/pull/28633), [28611](https://github.com/lowRISC/opentitan/pull/28611), remaining three get removed as part of https://github.com/lowRISC/opentitan/pull/28804
- [x] Simplify the data path. Remove the arbiters between update and cmd/generate stages and block_encrypt and update/generate stages. Fuse the update and generate stages into a single one as the data path of both is almost identical. Give this new unit exclusive access to block_encrypt.
Addressed as https://github.com/lowRISC/opentitan/pull/28804
- [x] Simplify FIFO control signals, make `ready` independent from `valid` wherever possible.
- [x] Remove FIFO error signals, especially the ones that equate to logic zero.
FIFOs got removed; potentially some error signals can still be removed in `csrng_cmd_stage`
- [x] Avoid tracking `inst_id` and `cmd` through the whole data path (instead select one app intf with a pending operation and make the correct state/data belonging to said interface available for all data path units).
Addressed in https://github.com/lowRISC/opentitan/pull/28804
- [ ] Get rid of the main_sm and include its functionality into the cmd stages and the control logic of the unified CTR_DRBG data path block outlined above.
- [ ] Review the (un)packers present in the design. Find a solution to store `adata/pdata` for each application interface with the least amount of flops possible. This is tricky and currently presents the largest flop overhead in the design.
**Features/Improvements**
- [ ] Clarify what the exact condition for skipping the initial update() call on GENerate commands is (pdata/adata being an all-zero bit vector or pdata/adata LENGTH being zero).
- [ ] Add a helper bit for state readout that software can set before starting a readout sequence and that hardware clears upon every write to the state db. This allows software to determine whether the state db has been altered during the multi-register readout process.
- [ ] Only emit requests from command stages to the core data path after required entropy has become available. This is to avoid stalling the core data path for all app interfaces during possibly very long wait times for entropy to become available. This involves moving the interface handling to and from entropy source from the main FSM to the FSMs inside the command stages and adding simple arbitration logic to the top level.
Would allow to reduce the depth of the output FIFO in entropy_src.
First working version on [csrng_archi](https://github.com/glaserf/opentitan/tree/csrng_archi).
- [ ] Add the possibility to abort in-flight generation requests. The EDNs can request very long random bit sequences, during the processing of which it might become necessary to reconfigure an EDN and hence un-instantiate the associated CSRNG instance. Currently, for this to happen, the requested random sequence must be fully consumed first from CSRNG which can take a long time.
Contributor guide
Assessment
This issue has not been assessed yet.