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

hw: Mask TCDM write data stability check on reads #125

Merged
merged 1 commit into from
May 24, 2024

Conversation

paulsc96
Copy link
Member

@paulsc96 paulsc96 commented Apr 2, 2024

pulp-platform/common_cells#200, recently included by a dependency update, may trigger false-positive TCDM stability assertion errors in some cases when using SSRs.

This is because SSR write data is hardwired to the bidirectional data FIFO output. Therefore, on reads, the write data bits may change irrespective of when a read request is pending. This instability is benign, but causes the payload-wide assertion to fail.

This PR, together with pulp-platform/common_cells#219, fixes this issue by masking stability assertion on the write data and mask inside stream_xbar and stream_omega_net and then conditionally asserting write data and mask stability on reads and atomics in snitch_tcdm_interconnect.

A more general discussion is maybe in order on whether SSRs should invest additional logic to instead mask write data on reads, but we should first fix the bug and then ponder design improvements.

TODO:

@paulsc96 paulsc96 force-pushed the paulsc/fix-tcdm-wasrt branch from ebcdcac to f56676e Compare April 2, 2024 20:08
@paulsc96 paulsc96 mentioned this pull request Apr 2, 2024
10 tasks
@paulsc96 paulsc96 force-pushed the paulsc/fix-tcdm-wasrt branch 3 times, most recently from 45fe418 to c4aaaf7 Compare April 2, 2024 20:45
@paulsc96 paulsc96 marked this pull request as ready for review April 2, 2024 20:46
Copy link
Collaborator

@colluca colluca left a comment

Choose a reason for hiding this comment

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

LGTM :)

@paulsc96 paulsc96 force-pushed the paulsc/fix-tcdm-wasrt branch from c4aaaf7 to 31237c5 Compare May 3, 2024 10:50
@colluca colluca force-pushed the paulsc/fix-tcdm-wasrt branch from 31237c5 to 424a346 Compare May 14, 2024 22:31
@colluca colluca force-pushed the paulsc/fix-tcdm-wasrt branch from 424a346 to f9e90e9 Compare May 15, 2024 01:51
@paulsc96 paulsc96 merged commit f80838d into main May 24, 2024
27 checks passed
@paulsc96 paulsc96 deleted the paulsc/fix-tcdm-wasrt branch May 24, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants