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] EDN doesn't support backpressure from CSRNG on RES/GEN commands #15469

Closed
vogelpi opened this issue Oct 13, 2022 · 5 comments
Closed

[edn] EDN doesn't support backpressure from CSRNG on RES/GEN commands #15469

vogelpi opened this issue Oct 13, 2022 · 5 comments
Assignees
Labels
Component:DV DV issue: testbench, test case, etc. Component:RTL IP:edn Priority:P1 Priority: high Type:Bug Bugs
Milestone

Comments

@vogelpi
Copy link
Contributor

vogelpi commented Oct 13, 2022

Factored out from #15402.

While working on #14240 it has been noticed that EDN currently doesn't support backpressure from CSRNG when transmitting contents of the command FIFOs. As a result, whenever a RES/GEN command contains more than 1 word of additional data, word 2 and 3 are dropped by CSRNG which then keeps waiting for the last two words forever. Similarly, EDN keeps waiting for the final ACK of CSRNG. Both IPs lock up.

The waveform below shows this. csrng_req_ready goes low at the curser but EDN keeps updating the request bus csrng_req_bus. Words 1 and 2 of the seed (0x21CA8E3F and 0x5BA3DBA1) are not being picked up by CSRNG - they are missing in the rdata_o sequence at the bottom. cmd_len_q stays at 2 and CSRNG remains in the SendMOP state forever.

Screenshot from 2022-10-11 17-35-52

I've worked out a first hot fix here #15402 but something more robust is needed. In addition also verification is needed for this feature.

@vogelpi vogelpi added Component:DV DV issue: testbench, test case, etc. Priority:P1 Priority: high Type:Bug Bugs Component:RTL IP:edn labels Oct 13, 2022
@vogelpi vogelpi added this to the Project: M2 milestone Oct 13, 2022
@vogelpi
Copy link
Contributor Author

vogelpi commented Oct 13, 2022

Hey @mwbranstad & @senelson7,
knowing that you're already working on this I thought it's maybe easier to factor out the actual issue out of my hot fix PR. You might even want to create a separate issue for the DV part.

@vogelpi
Copy link
Contributor Author

vogelpi commented Oct 13, 2022

FYI @martin-lueker , I've now assigned a default P1.

@vogelpi
Copy link
Contributor Author

vogelpi commented Oct 13, 2022

FYI @tjaychen @moidx

@tjaychen tjaychen mentioned this issue Oct 13, 2022
19 tasks
@mwbranstad
Copy link
Contributor

PR #15478 will functionally handle this issue. However, this is a brute force fix by adding an output FIFO on the CSRNG command interface.
For a cost reduction fix, the FIFO should be removed. All of the command logic must then respect the ready signal when de-asserted and hold state until ready again.

@vogelpi
Copy link
Contributor Author

vogelpi commented Oct 24, 2022

A first fix for this issue based on adding a FIFO has been merged with #15478, the DV part has been added with #15570. Issue #15561 is tracking the effort for reworking the handshakes to again remove the additional FIFO. I am thus closing this issue.

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. Component:RTL IP:edn Priority:P1 Priority: high Type:Bug Bugs
Projects
None yet
Development

No branches or pull requests

3 participants