Fix issues related to successive mtspr/mtspr instructions #124
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
Issue openrisc#122 Writing to the tlb rams was dependent on the spr_bus_ack as a means to only write during the first cycle after a spr_bus_stb_i. This works fine for a single write, but if we have a write to itlb_match_spr followed by itlb_trans_spr the spr_bus_ack never goes low so the write to itlb_trans_spr is skipped. Fix this by using the itlb_match_spr_cs_r and itlb_trans_spr_cs_r signals to control the "first cycle after spr_bus_stb_i" condition. This way writes can happen independently. Note, there may still be issues with a write to itlb_match followed by another write to itlb_match. Fixing that requires changing the "first cycle after spr_bus_stb_i" policy, which is similar to dmmu.
This was not connected.
Issue openrisc#122 This fixes case 1 from issue 122 where writes are missed after invalidate state. The change to invalidate_ack to to allow the invalidate_ack to be asserted after the invalidate happens not before. For writes we allow forwarding to WRITE state directly from invalidate at this point pending_write will still be high and we can use that to initialiate the write. When we have delays the cpu_req_i may be low when we get to the write state.
Issue openrisc#122 An additional fix for back to back spr writes. I was seeing that during back to back writes the busy_o signal would go low after the first spr write complete. This caused the fetch to proceed and present bad data to the bus caused by the second in-progress write. This is because ack goes high during the first write and stays high as long as we are writing. Introducing a busy tracker based on the assuming that spr transactions take 2 clocks to complete.
stffrdhrn
changed the title
|
This should fix all of #122 now as the test case passes for me know. This also fixes a trivial issue found related to cache hits not being forwarded to the performance counters. |
|
If anyone has time please help review @olofk @bandvig @wallento @juliusbaxter @skristiansson, it does pass tests. |
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.
Issue #122
Writing to the tlb rams was dependent on the spr_bus_ack as a means
to only write during the first cycle after a spr_bus_stb_i. This works
fine for a single write, but if we have a write to itlb_match_spr
followed by itlb_trans_spr the spr_bus_ack never goes low so the write
to itlb_trans_spr is skipped.
Fix this by using the itlb_match_spr_cs_r and itlb_trans_spr_cs_r
signals to control the "first cycle after spr_bus_stb_i" condition. This
way writes can happen independently.
Note, there may still be issues with a write to itlb_match followed by
another write to itlb_match. Fixing that requires changing the "first
cycle after spr_bus_stb_i" policy, which is similar to dmmu.