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

[edn] D2S Signoff #20982

Closed
msfschaffner opened this issue Jan 25, 2024 · 2 comments
Closed

[edn] D2S Signoff #20982

msfschaffner opened this issue Jan 25, 2024 · 2 comments

Comments

@msfschaffner
Copy link
Contributor

Description

Ensure D2 signoff criteria are fulfilled after focus area changes have landed.

@vogelpi
Copy link
Contributor

vogelpi commented Mar 29, 2024

Commits since Earlgrey-ES tapeout

git rev-parse --short HEAD 9ddf276

git log Earlgrey-M2.5.2-RC0..HEAD --oneline hw/ip/edn
15f5b15 [edn/dv] Force the CSRNG valid signal to high in alert_vseq
e02952c [edn/dv] Add coverage for the new hw_cmd_sts register
5583394 [edn/rtl] Add assertion to guarantee no interactions with CSRNG after err
317a9e7 [edn/dv] Add new alert test to alert vseq
35f2844 [edn/rtl] Add checks to the scoreboard for the new hw_cmd_sts register
30bb26f [edn/dv] Adapt DV to the new changes
747d894 [edn/rtl] Assign signals to the new hw_cmd_sts register
96c75dc [edn/rtl] Enter recoverable alert state when error is received from CSRNG
39af375 [edn/rtl] Add new recoverable alert
7f3b59c [edn/rtl] Add register for displaying HW cmd status
41e4539 [edn/rtl] Signal all recoverable alerts
-> RTL, DV and SW changes to properly handle error responses from CSRNG
eabff1f [edn/dv] Make genbits_vseq wait to enter Idle after boot mode
d3f6260 [edn/dv] Fix newly failing edn_err vseq
7fa4657 [edn/dv] Add checks for sw_cmd_sts to the scoreboard
b495f9d [edn/dv] Fix failing disable_vseq tests
75f6933 [edn/doc] Adapt EDN block diagram to the changes
987d5be [edn/dv] Remove obsolete states from test cases
75df0ae [edn/rtl] Handle backpressure from CSRNG without an output FIFO
57ba114 [edn/rtl] Remove RTL changes done for the output FIFO
-> RTL and DV changes to remove the redudant output FIFO and handle CSRNG back pressure correctly (saves 13*384 FFs) for each instance
5639924 Revert "[edn] Move prim_edn_req out of prim"
c721c51 [rtl, prim] Add 'commit' functionality to prim_count
61a237e [util/reggen] reverse order of substruct generation
6d3bd21 [edn/dv] Add covergroups for the sw_cmd_sts reg and csrng cmd rsp
c6fc953 [edn/doc] Align the documentation with the new sw_cmd_sts register
e76a731 [edn/dv] Add checks to scoreboard for the new cmd_ack and cmd_sts fields
5f16501 [edn/dv] Turn off assertions when disabling and reenabling EDN in err vseq
a974108 [edn/rtl] Change sw_cmd_sts register
-> RTL, DV and SW changes to rework the SW command status register and make it more useful, this is preparatory work for the output FIFO removal but also improves and simplifies the SW API
936fb04 [edn/dv] raise test timeout for edn_disable test
862b811 [edn/dv] Fix failing edn_genbits tests
3b4e36e [edn] Move prim_edn_req out of prim
ad83b93 [edn/doc] Adapt documentation to the new transition to SW mode
da8d6f2 [edn] Adapt coverage to the new transition to SW mode
2f734b3 [edn] Adapt edn_genbits vseq to test the new transition to SW mode
e87d639 [edn] Align the scoreboard with the new transition to SW mode
a47ee96 [edn] Add transition to SW mode after boot sequence is done
deed3d7 [edn/rtl] Set sw_cmd_valid_o to high in sw mode and during auto instantiate
-> RTL and DV changes to transition into SW mode after boot sequence rather than accepting SW commands in boot mode, this improves / simplifies the API but it also beneficial for security
564d414 [edn] Add new states to SM state_e typedef enum
686dc2e [edn/doc] Update documentation to explain when auto cmd FIFOs are cleared
9e97390 [edn] Get the DV environment in line with resetting FIFOs after EDN is disabled
99acbee [edn] Reset auto cmd FIFOs when EDN is disabled
-> Align clearing of command FIFOs between disabling and exiting auto-mode, previously they were not cleared upon disabling
de31bdf [reggen] Remove the devmode input
d3f0ae7 [edn/dv] Fix failing edn_err_vseq tests
e2e668e [edn/dv] Add option to skip wait_cmd_req_done() in wr_cmd()
51b9d9a [edn/dv] Use the new ack_sm enum type for the edn_err_vseq
9682c05 [edn/dv] Add enum type for ack sm states
975a6eb [adc_ctrl,dv] Tidy up access to intr_state in env_cfg files
6124771 [edn/doc] Mention reseed interval reset value in documentation
8daef2c [edn/doc] Update sum_sts strings to main_sm_state
fc1ede3 [edn/test] Add FIFO reset to prevent overflow
2d0778a [edn/cov] Fix small nits
d8c3e83 [edn/dv] Increase the number of reseeds for edn_genbits test
2b010d0 [edn/dv] Add generate cmds with different glen to edn_genbits
daee47b [edn/dv] Add some edge cases to scoreboard for cmds
3ed7b5c [edn/dv] add additional command to edn_genbits_vseq
27bacc6 [edn/dv] add operational mode and cmd src register cps to edn_cs_cmds_cg
b655b0e [edn/dv] add wr_cmd args mode and cmd_src
99ecee7 [edn/dv] Add acmd cover point to cs_cmds cover group
e082349 [edn/dv] move coverage sample call to wr_cmd task
f5e4d57 [edn/dv] Remove todo comment for intr test
e2dc7dd [edn/dv] Add output FIFO errors to edn_err test
4077568 [edn/dv] Add sequence to check issuing cmd during auto_req_mode
-> various DV cleanups and improvements
1a6ca94 [edn/rtl] align reseed cntr resval with MAX_NUM_REQS_BETWEEN_RESEEDS resval
-> simplification of RTL, DV and documentation by aligning reset values
a3b1bde [chip-test] List EDN functional features and add SiVal test plan
b311812 [edn/dv] check commands on EDN-CSRNG interface
-> DV improvement to check commands sent to CSRNG in block-level DV and to check the responses
ce7a92c [edn] Define parameters for register reset values
1b16ca2 [reggen] Add mubi support SWAccess that sets/clears a reg
18426d5 [edn,dv] Fix type of mailbox in edn_disable_auto_req_mode_vseq
b0db0e2 [edn,doc] Fix typo in README.md
59f8142 [doc] Moved badges over to using hosted images
7688e71 [reggen] Add initial support for version and cip_id hjson fields
fbd888e Revert "[reggen] Add CIP_IDs and bump all major versions"
912d2dc [doc] edn registers now using CMDGEN
0ba10b3 [reggen] Add CIP_IDs and bump all major versions

Issues closed since the Earlgrey-ES tapeout

Currently open issues

Summary

For M2, some big and long standing EDN RTL issues have finally been resolved which includes:

These changes mandate a major version increase as the SW API is modified quite heavily (especially around SW commands and error handling). A PR to bump the version is here: #22353

In addition, many little bug fixes and improvements were implemented. For all these changes, documentation, SW and DV were updated inline to keep test pass rates and coverage up and to ensure nothing breaks. No new TODOs have been added to the RTL. All checklist criteria are still met to sign off EDN at D2S and this is what I am proposing.

@andreaskurth , @h-filali , would you mind taking a look at this and let me know what you think?

@vogelpi vogelpi changed the title [edn] D2 Signoff [edn] D2S Signoff Mar 30, 2024
@andreaskurth
Copy link
Contributor

Thanks for this comprehensive signoff analysis, @vogelpi.

I reviewed the following commits, which modify edn's RTL (based on git log --oneline Earlgrey-M2.5.2-RC0..HEAD -- hw/ip/edn/rtl), in detail:

  • 5583394 [edn/rtl] Add assertion to guarantee no interactions with CSRNG after err
    --> RTL change with no functional impact
  • 747d894 [edn/rtl] Assign signals to the new hw_cmd_sts register
    --> RTL change adding bits to the HW_CMD_STS CSR with information on the interaction with CSRNG
  • 96c75dc [edn/rtl] Enter recoverable alert state when error is received from CSRNG
    --> RTL change that makes EDN stop accepting entropy from CSRNG once it has received an error (until EDN is re-enabled)
  • 39af375 [edn/rtl] Add new recoverable alert
    --> RTL change adding a recoverable alert for CSRNG errors
  • 7f3b59c [edn/rtl] Add register for displaying HW cmd status
    --> RTL change provisioning the HW_CMD_STS CSR
  • 41e4539 [edn/rtl] Signal all recoverable alerts
    --> RTL change fixing that all recoverable alerts get sent to alert_handler
  • 75df0ae [edn/rtl] Handle backpressure from CSRNG without an output FIFO
    --> RTL change for getting the interface to CSRNG to work without the output FIFO
  • 57ba114 [edn/rtl] Remove RTL changes done for the output FIFO
    --> RTL change removing the output FIFO
  • 5639924 Revert "[edn] Move prim_edn_req out of prim"
    --> reverts 3b4e36e (below)
  • c721c51 [rtl, prim] Add 'commit' functionality to prim_count
    --> RTL change without functional impact
  • 61a237e [util/reggen] reverse order of substruct generation
    --> RTL change without functional impact
  • a974108 [edn/rtl] Change sw_cmd_sts register
    --> RTL change fixing the SW_CMD_STS CSR
  • 3b4e36e [edn] Move prim_edn_req out of prim
    --> reverted by 5639924 (above)
  • a47ee96 [edn] Add transition to SW mode after boot sequence is done
    --> RTL change fixing SW mode after boot mode
  • deed3d7 [edn/rtl] Set sw_cmd_valid_o to high in sw mode and during auto instantiate
    --> RTL change fixing sw_cmd_valid_o of the main FSM
  • 564d414 [edn] Add new states to SM state_e typedef enum
    --> RTL change with FSM state expansion but no functional change
  • 99acbee [edn] Reset auto cmd FIFOs when EDN is disabled
    --> RTL change fixing the reset of the auto-mode command FIFO when EDN gets disabled
  • de31bdf [reggen] Remove the devmode input
    --> RTL change without functional impact
  • 1a6ca94 [edn/rtl] align reseed cntr resval with MAX_NUM_REQS_BETWEEN_RESEEDS resval
    --> RTL change fixing the reset value of u_prim_count_max_reqs_cntr
  • ce7a92c [edn] Define parameters for register reset values
    --> RTL change without functional impact
  • 1b16ca2 [reggen] Add mubi support SWAccess that sets/clears a reg
    --> RTL change without functional impact
  • fbd888e Revert "[reggen] Add CIP_IDs and bump all major versions"
    --> reverts commit below
  • 0ba10b3 [reggen] Add CIP_IDs and bump all major versions
    --> reverted by commit above

All these changes LGTM, and based on your assessment that they are all covered by DV, I agree with keeping D2S.

There's one open issue, #19789, that tracks a bugfix for EDN. It is part of M4, so no blocker for D2S signoff.

--> Let's keep EDN at D2S.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants