[prim_count] Register err_o to avoid potential CDC issues downstream #24125

vogelpi · 2024-07-25T12:55:28Z

This is related to #24119.

This is related to lowRISC#24119. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

rswarbrick

I guess this will slightly change which cycle the signal appears at, but maybe we only expect this to happen when someone injects an error (so we don't really care about the timings). Is that correct?

rswarbrick · 2024-07-25T13:17:49Z

CHANGE AUTHORIZED: hw/ip/prim/rtl/prim_count.sv

msfschaffner

Yes, I think this will mostly break DV or FPV if anything.
The error condition itself is treated as asynchronous in general.

msfschaffner · 2024-07-25T13:58:34Z

CHANGE AUTHORIZED: hw/ip/prim/rtl/prim_count.sv

msfschaffner · 2024-07-25T14:04:33Z

You may want to double check this with the corresponding FPV TB:
https://github.com/lowRISC/opentitan/blob/master/hw/ip/prim/fpv/tb/prim_count_tb.sv

vogelpi · 2024-07-25T14:19:30Z

Thanks for the pointers @msfschaffner and @rswarbrick , FPV looks fine:

dev@2f8cf6446c25:~/src$ util/dvsim/dvsim.py hw/top_earlgrey/formal/top_earlgrey_fpv_prim_cfgs.hjson --select-cfg=prim_count_max_reset_fpv                                                 
INFO: [dvsim] [proj_root]: /home/dev/src                                                                                                                                                  
INFO: [OneShotCfg] [scratch_path]: [prim_count_max_reset_fpv] [/home/dev/src/scratch/prim-count-fix/prim_count_max_reset_fpv-formal-fpv-jaspergold]                                       
WARNING: [Deploy] prim_count_max_reset_fpv:default: jaspergold is unsupported for job runtime extraction. Using dvsim-maintained job_runtime instead.                                     
INFO: [FlowCfg] [results]: [prim_count_max_reset_fpv]:                                                                                                                                    
## PRIM_COUNT_MAX_RESET_FPV Formal FPV Results                                                                                                                                            
                                                                                                                                                                                          
### Thursday July 25 2024 14:16:12 UTC
### GitHub Revision: [`3b00ed85f6`](https://github.com/lowrisc/opentitan/tree/3b00ed85f668db9c8cf6f8e62a2ed638cd87cea2)
### Branch: prim-count-fix
### Tool: JASPERGOLD


## Formal FPV Results
|           name           |  errors  |  warnings  |  proven  |  cex  |  undetermined  |  covered  |  unreachable  |  pass_rate  |  cov_rate  |
|:------------------------:|:--------:|:----------:|:--------:|:-----:|:--------------:|:---------:|:-------------:|:-----------:|:----------:|
| prim_count_max_reset_fpv |   0 E    |    0 W     |   14 G   |  0 E  |      0 W       |   13 G    |      0 E      |  100.00 %   |  100.00 %  |

## Coverage Results
### Coverage html file dir: /home/dev/src/scratch/prim-count-fix/prim_count_max_reset_fpv-formal-fpv-jaspergold/default/formal-icarus

|  formal  |  stimuli  |  checker  |
|:--------:|:---------:|:---------:|
| 96.08 %  | 100.00 %  |  94.74 %  |

INFO: [FlowCfg] [scratch_path]: [prim_count_max_reset_fpv] [/home/dev/src/scratch/prim-count-fix/prim_count_max_reset_fpv-formal-fpv-jaspergold]
INFO: [FormalCfg] [result summary]: ## TOP_EARLGREY_PRIM_FPV Formal FPV Results (Summary)

### Thursday July 25 2024 14:16:12 UTC
### GitHub Revision: [`3b00ed85f6`](https://github.com/lowrisc/opentitan/tree/3b00ed85f668db9c8cf6f8e62a2ed638cd87cea2)
### Branch: prim-count-fix

|           name           |  pass_rate  |  formal_cov  |  stimuli_cov  |  checker_cov  |
|:------------------------:|:-----------:|:------------:|:-------------:|:-------------:|
| prim_count_max_reset_fpv |  100.00 %   |   96.08 %    |   100.00 %    |    94.74 %    |

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                          
00:00:18  [    build    ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%

and

dev@2f8cf6446c25:~/src$ util/dvsim/dvsim.py hw/top_earlgrey/formal/top_earlgrey_fpv_prim_cfgs.hjson --select-cfg=prim_count_zero_reset_fpv
INFO: [dvsim] [proj_root]: /home/dev/src
INFO: [OneShotCfg] [scratch_path]: [prim_count_zero_reset_fpv] [/home/dev/src/scratch/prim-count-fix/prim_count_zero_reset_fpv-formal-fpv-jaspergold]
WARNING: [Deploy] prim_count_zero_reset_fpv:default: jaspergold is unsupported for job runtime extraction. Using dvsim-maintained job_runtime instead.
INFO: [FlowCfg] [results]: [prim_count_zero_reset_fpv]:
## PRIM_COUNT_ZERO_RESET_FPV Formal FPV Results

### Thursday July 25 2024 14:17:38 UTC
### GitHub Revision: [`3b00ed85f6`](https://github.com/lowrisc/opentitan/tree/3b00ed85f668db9c8cf6f8e62a2ed638cd87cea2)
### Branch: prim-count-fix
### Tool: JASPERGOLD


## Formal FPV Results
|           name            |  errors  |  warnings  |  proven  |  cex  |  undetermined  |  covered  |  unreachable  |  pass_rate  |  cov_rate  |
|:-------------------------:|:--------:|:----------:|:--------:|:-----:|:--------------:|:---------:|:-------------:|:-----------:|:----------:|
| prim_count_zero_reset_fpv |   0 E    |    0 W     |   14 G   |  0 E  |      0 W       |   13 G    |      0 E      |  100.00 %   |  100.00 %  |

## Coverage Results
### Coverage html file dir: /home/dev/src/scratch/prim-count-fix/prim_count_zero_reset_fpv-formal-fpv-jaspergold/default/formal-icarus

|  formal  |  stimuli  |  checker  |
|:--------:|:---------:|:---------:|
| 96.08 %  | 100.00 %  |  94.74 %  |

INFO: [FlowCfg] [scratch_path]: [prim_count_zero_reset_fpv] [/home/dev/src/scratch/prim-count-fix/prim_count_zero_reset_fpv-formal-fpv-jaspergold]
INFO: [FormalCfg] [result summary]: ## TOP_EARLGREY_PRIM_FPV Formal FPV Results (Summary)

### Thursday July 25 2024 14:17:38 UTC
### GitHub Revision: [`3b00ed85f6`](https://github.com/lowrisc/opentitan/tree/3b00ed85f668db9c8cf6f8e62a2ed638cd87cea2)
### Branch: prim-count-fix

|           name            |  pass_rate  |  formal_cov  |  stimuli_cov  |  checker_cov  |
|:-------------------------:|:-----------:|:------------:|:-------------:|:-------------:|
| prim_count_zero_reset_fpv |  100.00 %   |   96.08 %    |   100.00 %    |    94.74 %    |

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                          
00:00:16  [    build    ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%

vogelpi · 2024-07-25T15:05:31Z

This looks good from a CI perspective, too. There are some FPGA test failures but the failing tests are known to be flaky or currently broken USB tests:

//sw/device/silicon_creator/rom/e2e/reset_reason:reset_reason_check_disabled_no_fault_fpga_cw310_rom_with_fake_keys FLAKY, failed in 1 out of 2 in 12.8s
  Stats over 2 runs: max = 12.8s, min = 1.6s, avg = 7.2s, dev = 5.6s
//sw/device/silicon_creator/rom/e2e/reset_reason:reset_reason_check_disabled_with_fault_fpga_cw310_rom_with_fake_keys FLAKY, failed in 1 out of 2 in 12.5s
  Stats over 2 runs: max = 12.5s, min = 1.6s, avg = 7.1s, dev = 5.4s
//sw/device/silicon_creator/rom/e2e/rom_ext_upgrade_interrupt:rom_ext_upgrade_interrupt_rma_fpga_cw310_rom_with_fake_keys FLAKY, failed in 1 out of 2 in 13.4s
  Stats over 2 runs: max = 13.4s, min = 0.8s, avg = 7.1s, dev = 6.3s
//sw/device/tests:usbdev_mixed_test_fpga_cw310_rom_with_fake_keys       TIMEOUT in 2 out of 2 in 60.0s
  Stats over 2 runs: max = 60.0s, min = 60.0s, avg = 60.0s, dev = 0.0s
//sw/device/tests:usbdev_stream_test_fpga_cw310_rom_with_fake_keys      TIMEOUT in 2 out of 2 in 60.0s
  Stats over 2 runs: max = 60.0s, min = 60.0s, avg = 60.0s, dev = 0.0s

With lowRISC#24125, we added a register to the error output of all prim_count instances. This adds an additional delay until errors become actually visible via CSRs (e.g. main_sm_state, err_code). This commit adds a corresponding wait statement to take this into account. This resolves lowRISC#24226. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

With #24125, we added a register to the error output of all prim_count instances. This adds an additional delay until errors become actually visible via CSRs (e.g. main_sm_state, err_code). This commit adds a corresponding wait statement to take this into account. This resolves #24226. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

With lowRISC#24125, we added a register to the error output of all prim_count instances. This adds an additional delay until errors become actually visible via CSRs (e.g. main_sm_state, err_code). This commit adds a corresponding wait statement to take this into account. This resolves lowRISC#24226. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

With lowRISC#24125, we added a register to the error output of all prim_count instances. This adds an additional delay until errors become actually visible via CSRs (e.g. main_sm_state, err_code). This commit adds a corresponding wait statement to take this into account. This resolves lowRISC#24226. This is a cherry pick of commit a43f5a3 to branch earlgrey_1.0.0. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

With #24125, we added a register to the error output of all prim_count instances. This adds an additional delay until errors become actually visible via CSRs (e.g. main_sm_state, err_code). This commit adds a corresponding wait statement to take this into account. This resolves #24226. This is a cherry pick of commit a43f5a3 to branch earlgrey_1.0.0. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

[prim_count] Register err_o to avoid potential CDC issues downstream

3b00ed8

This is related to lowRISC#24119. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

vogelpi requested review from msfschaffner, GregAC, moidx, OTshimeon, meisnere and andreaskurth July 25, 2024 12:55

rswarbrick reviewed Jul 25, 2024

View reviewed changes

rswarbrick approved these changes Jul 25, 2024

View reviewed changes

msfschaffner approved these changes Jul 25, 2024

View reviewed changes

vogelpi added the ECO_C A potential candidate for an ECO label Jul 25, 2024

This was referenced Jul 25, 2024

[ECO] prim_count.sv CDC #24119

Closed

[rv_core_ibex] Flop combinatorial escalation IRQ before 2-flop sync #24121

Closed

[prim_esc_receiver] Register esc_req_o to avoid potential CDC issues #24127

Merged

moidx approved these changes Jul 25, 2024

View reviewed changes

vogelpi merged commit a39b4c7 into lowRISC:master Jul 25, 2024
31 of 33 checks passed

vogelpi added ECO Accepted as an ECO and removed ECO_C A potential candidate for an ECO labels Jul 25, 2024

vogelpi mentioned this pull request Aug 6, 2024

[edn/dv] Allow flopped error output of prim_count to propagate to CSRs #24237

Merged

vogelpi deleted the prim-count-fix branch September 17, 2024 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[prim_count] Register err_o to avoid potential CDC issues downstream #24125

[prim_count] Register err_o to avoid potential CDC issues downstream #24125

vogelpi commented Jul 25, 2024

rswarbrick left a comment

rswarbrick commented Jul 25, 2024

msfschaffner left a comment

msfschaffner commented Jul 25, 2024

msfschaffner commented Jul 25, 2024

vogelpi commented Jul 25, 2024

vogelpi commented Jul 25, 2024

[prim_count] Register err_o to avoid potential CDC issues downstream #24125

[prim_count] Register err_o to avoid potential CDC issues downstream #24125

Conversation

vogelpi commented Jul 25, 2024

rswarbrick left a comment

Choose a reason for hiding this comment

rswarbrick commented Jul 25, 2024

msfschaffner left a comment

Choose a reason for hiding this comment

msfschaffner commented Jul 25, 2024

msfschaffner commented Jul 25, 2024

vogelpi commented Jul 25, 2024

vogelpi commented Jul 25, 2024