Skip to content

Conversation

@cnokes14
Copy link

Fixes this issue: #530
Arrays/buffers in cv32e40s_obi_integrity_fifo.sv and cv32e40s_lsu_response_filter.sv did not have proper bound checking (two bit index with a default buffer size of three, meaning that in cases of 2'b11 an undefined signal would occur).
Specifically, the LSU response filter only checked increments, not decrements, and the integrity FIFO did not ever check bounds.
As discussed, this resulted in X's appearing on the instr_req_o and instr_reqpar_o lines, among others. This occurred frequently during fuzz testing.
My fix uses logic similar to the increment bound checking in the LSU response filter on the other three cases (LSU response filter decrement, integrity FIFO increment, and integrity FIFO decrement).

@Silabs-ArjanB
Copy link
Contributor

Hi @cnokes14 Thank you very much for your further analysis and proposed fix. After my initial pos, based on running your testbench ourselves we had found the same out-of-bounds issue that you highlighted. I agree this needs to be fixed. If you would contrain the environment to follow the OBI protocol, then the out-of-bound issue cannot occur, but it should be fixed nonetheless. I was thinking that maybe making the indexed arrays a bit wider would maybe be a good fix.

@cnokes14
Copy link
Author

cnokes14 commented Feb 1, 2025

Sorry I took a while to get back, it's been a long work week.
I've pushed a commit with the widened arrays. A long fuzz test on it shows no undefined signals--looks like this is fixed.
Let me know if there are any more changes you'd like me to add to the fix.

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.

2 participants