[rtl] Guard against false memory responses for secure configurations #2166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GregAC
force-pushed
the
mem_response_check
branch
from
ba476ed to
2e34f95
Compare
GregAC
force-pushed
the
mem_response_check
branch
from
2e34f95 to
74e2196
Compare
vogelpi
approved these changes
May 17, 2024
rtl/ibex_ex_block.sv
Outdated
| assign sva_multdiv_fsm_idle = 1'b1; | ||
| end | ||
|
|
||
| // Mark the sva_multduv_fsm_idle as unused to avoid lint issues |
There was a problem hiding this comment.
Mintor typo: sva_multduv_fsm_idle -> sva_multdiv_fsm_idle
KatCe
reviewed
May 28, 2024
| @@ -1810,6 +1839,9 @@ module ibex_core import ibex_pkg::*; #( | |||
| // Certain parameter combinations are not supported | |||
| `ASSERT_INIT(IllegalParamSecure, !(SecureIbex && (RV32M == RV32MNone))) | |||
|
|
|||
| // If the ID stage signals its ready the mult/div FSMs must be idle in the following cycle | |||
| `ASSERT(MultDivFSMIdleOnIdReady, id_in_ready |=> ex_block_i.sva_multdiv_fsm_idle) | |||
There was a problem hiding this comment.
ex_block_i.sva_multdiv_fsm_idle is not accessible if INC_ASSERT is not defined
There was a problem hiding this comment.
This is true but ASSERT is just an empty macro when INC_ASSERT isn't defined so this doesn't cause an issue.
GregAC
force-pushed
the
mem_response_check
branch
from
74e2196 to
8fe62ea
Compare
With this change all memory responses are only acted on if Ibex is expecting them for all secure configurations. Previously an error response that was injected onto the bus would trigger an exception that shouldn't occur (in particular breaking the functioning of the multiply state machine). In addition for configurations without the writeback stage an injected load data response could trigger an incorrect write to the register file. This is only applied to the secure configurations, non-secure configurations assume correct adherence to the bus protocol meaning a response will only be seen if a request is outstanding.
GregAC
force-pushed
the
mem_response_check
branch
from
8fe62ea to
2fc670d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this change all memory responses are only acted on if Ibex is expecting them for all secure configurations. Previously an error response that was injected onto the bus would trigger an exception that shouldn't occur (in particular breaking the functioning of the multiply state machine). In addition for configurations without the writeback stage an injected load data response could trigger an incorrect write to the register file.
This is only applied to the secure configurations, non-secure configurations assume correct adherence to the bus protocol meaning a response will only be seen if a request is outstanding.
Fixes #2144