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

[entropy_src] Functional coverage metrics dropped #21888

Closed
vogelpi opened this issue Mar 7, 2024 · 6 comments
Closed

[entropy_src] Functional coverage metrics dropped #21888

vogelpi opened this issue Mar 7, 2024 · 6 comments
Assignees
Labels
Component:DV DV issue: testbench, test case, etc. Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:entropy_src Subsystem:Entropy entropy_src, csrng, or edn related issues

Comments

@vogelpi
Copy link
Contributor

vogelpi commented Mar 7, 2024

Description

After merging #21369 which made the rng_fips_o request bit to the RNG/noise source as well as the fips_compliance flag signaled to CSRNG software controllable, the functional coverage for ENTROPY_SRC dropped massively from above 96% to around 46%. The main reason for this massive drop is the coverage cross for the different fields of the CONF register. The PR added two new fields to the CONF register, meaning unless we substantially increase the number of seeds run in simulation, we can cover only about a 4th of the possible configuration settings.

Screenshot from 2024-03-07 15-28-10

Screenshot from 2024-03-07 15-28-48

Since the two new configuration fields directly control a single output bit each and not more, I think we shouldn't spend effort/so much simulation time into getting coverage again to an acceptable level for V2/V2S (90%) and instead not consider these two settings into the crosses.

What is your view @h-filali, @andreaskurth ?

Update:
In addition, also toggle coverage dropped below 90% (currently at 88%). After all these RTL changes, we should probably regenerate the coverage exclusion files.

@vogelpi vogelpi added Component:DV DV issue: testbench, test case, etc. Subsystem:Entropy entropy_src, csrng, or edn related issues IP:entropy_src labels Mar 7, 2024
@vogelpi vogelpi added this to the Earlgrey-PROD.M4 milestone Mar 7, 2024
@vogelpi
Copy link
Contributor Author

vogelpi commented Mar 7, 2024

@vogelpi vogelpi added the Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones label Mar 7, 2024
@andreaskurth
Copy link
Contributor

I agree that the having the two new control fields in the cross doesn't add much value. Do we already measure non-crossed coverage and check their correct implementation in the scoreboard, though?

vogelpi added a commit to vogelpi/opentitan that referenced this issue Mar 29, 2024
Since ES tapeout, several API breaking changes had to be implemented.
The DV side was mostly updated in sync but there are some reasons why
we cannot meet V2S currently:
- Functional and code coverage metrics are currently not met (V2 & V2S).
  See lowRISC#21888.
- The entropy_src_err_vseq sequence should be extended to test the
  hardened prim_fifo_sync primitives (V2S).
  See lowRISC#22326.

Signed-off-by: Pirmin Vogel <[email protected]>
This was referenced Mar 29, 2024
vogelpi added a commit that referenced this issue Mar 29, 2024
Since ES tapeout, several API breaking changes had to be implemented.
The DV side was mostly updated in sync but there are some reasons why
we cannot meet V2S currently:
- Functional and code coverage metrics are currently not met (V2 & V2S).
  See #21888.
- The entropy_src_err_vseq sequence should be extended to test the
  hardened prim_fifo_sync primitives (V2S).
  See #22326.

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

moidx commented Jun 11, 2024

Discussed moving as P1 to M5 due to RTL risk analysis performed during last issue triage.

@h-filali
Copy link

It might be more interesting to just have a separate cover group which is only for the different fips configurations and remove the fips settings from the other corver groups. Since the fips bit is now software controllable, it should not be related to the other settings.

@vogelpi
Copy link
Contributor Author

vogelpi commented Jun 18, 2024

Update: @h-filali has been investigating this and it is not super straight forward. Even after removing some coverpoints from the crosses as mentioned previously, the cross coverage is lower than expected. The investigation continues as we really need this for V2S.

@vogelpi
Copy link
Contributor Author

vogelpi commented Jun 28, 2024

This has been taken care of by @h-filali . All coverage metrics should now be above the V2/V2S threshold again. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. Earlgrey-PROD Candidate Temporary label to triage issues into Earlgrey-PROD Milestones IP:entropy_src Subsystem:Entropy entropy_src, csrng, or edn related issues
Projects
None yet
Development

No branches or pull requests

4 participants