Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[keymgr] Don't update and reseed PRNG in Disabled/Invalid state forever #23101

Merged
merged 1 commit into from
May 14, 2024

Conversation

vogelpi
Copy link
Contributor

@vogelpi vogelpi commented May 14, 2024

Previously, keymgr would keep updating and reseeding the PRNG forever once entering the StCtrlDisabled or StCtrlInvalid state. This is not ideal from an entropy and power consumption viewpoint.

This commit changes the design to - once one of the two states is entered - to keep updating the PRNG (which also triggers the reseed operation) until two more PRNG reseed operation have happened.

This also includes the keymgr_DPE specific changes of #23071.
This is related to #22997.

Previously, keymgr would keep updating and reseeding the PRNG forever
once entering the StCtrlDisabled or StCtrlInvalid state. This is not
ideal from an entropy and power consumption viewpoint.

This commit changes the design to - once one of the two states is
entered - to keep updating the PRNG (which also triggers the reseed
operation) until two more PRNG reseed operation have happened.

This also includes the keymgr_DPE specific changes of
lowRISC#23071.
This is related to lowRISC#22997.

Signed-off-by: Pirmin Vogel <[email protected]>
@vogelpi
Copy link
Contributor Author

vogelpi commented May 14, 2024

CHANGE AUTHORIZED: hw/ip/keymgr_dpe/rtl/keymgr_dpe.sv
CHANGE AUTHORIZED: hw/ip/keymgr_dpe/rtl/keymgr_dpe_ctrl.sv

@vogelpi
Copy link
Contributor Author

vogelpi commented May 14, 2024

I ran a full regression and it looks to be okay. I got the following:

util/dvsim/dvsim.py hw/ip/keymgr_dpe/dv/keymgr_dpe_sim_cfg.hjson -t vcs -i all --purge -mp 12
WARNING: [dvsim] resolve_top_chip for hw/ip/keymgr_dpe/dv/keymgr_dpe_sim_cfg.hjson
ERROR: [Modes] Test "keymgr_dpe_stress_all_with_rand_reset" added to regression "V3" not found!
INFO: [FlowCfg] [results]: [keymgr_dpe]:
## KEYMGR_DPE Simulation Results
### Tuesday May 14 2024 10:28:20 UTC
### GitHub Revision: [`86ed6821c5`](https://github.com/lowrisc/opentitan/tree/86ed6821c53a4e368f9bed0ca8f45f585bafc555)
### Branch: integrated_dev
### [Testplan](https://docs.opentitan.org/hw/ip/keymgr_dpe/doc/dv/#testplan)
### Simulator: VCS

### Test Results
|  Stage  |                   Name                    | Tests                                    |  Max Job Runtime  |  Simulated Time  |  Passing  |  Total  |  Pass Rate  |
|:-------:|:-----------------------------------------:|:-----------------------------------------|:-----------------:|:----------------:|:---------:|:-------:|:-----------:|
|   V1    |                   smoke                   | keymgr_dpe_smoke                         |      3.170m       |     39.613ms     |    50     |   50    |  100.00 %   |
|   V1    |               csr_hw_reset                | keymgr_dpe_csr_hw_reset                  |      0.790s       |     38.778us     |     5     |    5    |  100.00 %   |
|   V1    |                  csr_rw                   | keymgr_dpe_csr_rw                        |      3.090s       |     43.764us     |    20     |   20    |  100.00 %   |
|   V1    |               csr_bit_bash                | keymgr_dpe_csr_bit_bash                  |      8.360s       |     4.262ms      |     5     |    5    |  100.00 %   |
|   V1    |               csr_aliasing                | keymgr_dpe_csr_aliasing                  |      8.940s       |     3.672ms      |     5     |    5    |  100.00 %   |
|   V1    |        csr_mem_rw_with_rand_reset         | keymgr_dpe_csr_mem_rw_with_rand_reset    |      3.390s       |    176.778us     |    20     |   20    |  100.00 %   |
|   V1    | regwen_csr_and_corresponding_lockable_csr | keymgr_dpe_csr_rw                        |      3.090s       |     43.764us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_dpe_csr_aliasing                  |      8.940s       |     3.672ms      |     5     |    5    |  100.00 %   |
|   V1    |                                           | **TOTAL**                                |                   |                  |    105    |   105   |  100.00 %   |
|   V2    |                 intr_test                 | keymgr_dpe_intr_test                     |      2.410s       |     39.218us     |    50     |   50    |  100.00 %   |
|   V2    |                alert_test                 | keymgr_dpe_alert_test                    |      2.590s       |     88.915us     |    50     |   50    |  100.00 %   |
|   V2    |           tl_d_oob_addr_access            | keymgr_dpe_tl_errors                     |      17.880s      |    642.248us     |    20     |   20    |  100.00 %   |
|   V2    |            tl_d_illegal_access            | keymgr_dpe_tl_errors                     |      17.880s      |    642.248us     |    20     |   20    |  100.00 %   |
|   V2    |          tl_d_outstanding_access          | keymgr_dpe_csr_hw_reset                  |      0.790s       |     38.778us     |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_dpe_csr_rw                        |      3.090s       |     43.764us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_dpe_csr_aliasing                  |      8.940s       |     3.672ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_dpe_same_csr_outstanding          |      4.390s       |     45.658us     |    20     |   20    |  100.00 %   |
|   V2    |            tl_d_partial_access            | keymgr_dpe_csr_hw_reset                  |      0.790s       |     38.778us     |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_dpe_csr_rw                        |      3.090s       |     43.764us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_dpe_csr_aliasing                  |      8.940s       |     3.672ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_dpe_same_csr_outstanding          |      4.390s       |     45.658us     |    20     |   20    |  100.00 %   |
|   V2    |                                           | **TOTAL**                                |                   |                  |    140    |   140   |  100.00 %   |
|   V2S   |                tl_intg_err                | keymgr_dpe_sec_cm                        |      14.010s      |     5.790ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_dpe_tl_intg_err                   |      13.200s      |     1.685ms      |    20     |   20    |  100.00 %   |
|   V2S   |          shadow_reg_update_error          | keymgr_dpe_shadow_reg_errors             |      12.070s      |    113.356us     |    20     |   20    |  100.00 %   |
|   V2S   |    shadow_reg_read_clear_staged_value     | keymgr_dpe_shadow_reg_errors             |      12.070s      |    113.356us     |    20     |   20    |  100.00 %   |
|   V2S   |         shadow_reg_storage_error          | keymgr_dpe_shadow_reg_errors             |      12.070s      |    113.356us     |    20     |   20    |  100.00 %   |
|   V2S   |           shadowed_reset_glitch           | keymgr_dpe_shadow_reg_errors             |      12.070s      |    113.356us     |    20     |   20    |  100.00 %   |
|   V2S   |    shadow_reg_update_error_with_csr_rw    | keymgr_dpe_shadow_reg_errors_with_csr_rw |      14.100s      |    518.233us     |    20     |   20    |  100.00 %   |
|   V2S   |             prim_count_check              | keymgr_dpe_sec_cm                        |      14.010s      |     5.790ms      |     5     |    5    |  100.00 %   |
|   V2S   |              prim_fsm_check               | keymgr_dpe_sec_cm                        |      14.010s      |     5.790ms      |     5     |    5    |  100.00 %   |
|   V2S   |                                           | **TOTAL**                                |                   |                  |    65     |   65    |  100.00 %   |
|         |                                           | **TOTAL**                                |                   |                  |    310    |   310   |  100.00 %   |


INFO: [FlowCfg] [scratch_path]: [keymgr_dpe] [/home/pirmin/ot/opentitan/scratch/integrated_dev/keymgr_dpe-sim-vcs]

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                          
00:00:24  [    build    ]: [Q: 000, D: 000, P: 001, F: 000, K: 000, T: 001] 100%                                                                                                          
00:06:24  [     run     ]: [Q: 000, D: 000, P: 310, F: 000, K: 000, T: 310] 100%

Copy link
Contributor

@ballifatih ballifatih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Note for the future keymgr_dpe changes: the (old) keymgr runs/seeds LFSR because it wants to keep feeding random data during Disabled/Invalid. In keymgr_dpe RTL we have partially moved away from this "obfuscating" design decision, but since most components are borrowed directly from the (old) keymgr, we see the same pattern for LFSR as well.

Just dropping this comment here to provide a historical context.

@Razer6
Copy link
Member

Razer6 commented May 14, 2024

LGTM. @andreaskurth Can you merge this one?

@andreaskurth
Copy link
Contributor

CHANGE AUTHORIZED: hw/ip/keymgr_dpe/rtl/keymgr_dpe.sv
CHANGE AUTHORIZED: hw/ip/keymgr_dpe/rtl/keymgr_dpe_ctrl.sv

@andreaskurth andreaskurth merged commit 628dfc9 into lowRISC:integrated_dev May 14, 2024
20 checks passed
@vogelpi vogelpi mentioned this pull request Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants