Various DV fixes #2183
Merged
Various DV fixes #2183
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
rswarbrick
reviewed
Jul 2, 2024
Where an access is unaligned Ibex splits it into two transactions, each of which undergoes a PMP check. It is possible for the first half to fail a PMP check and the second to succeed and hence produce a request on the memory interface. In Spike it accesses memory byte by byte and if it encounters a PMP error for a particular byte it won't try any further bytes. This results in a mis-match between Ibex and spike when an unaligned transaction is split across two PMP regions, one of which allows the access and the other doesn't. Ibex generates a transaction and spike doesn't producing an error. This adds a fixup into the co-simulation environment. It detects when we have an access that fails PMP that is misaligned. Where this has resulted in Ibex producing a memory request that spike would not we remove it from the list of memory requests to check after checking that the request passes PMP within spike.
The 'pre_val' MIP addresses the scenario where MIP changes as an instruction is excuting, this means a CSR instruction can observe a different MIP from the one that decides whether or not that instruction will be interrupted.
The cosimulation environment does not know if a memory access from spike is due to an instruction fetch or a data memory access. It uses a heuristic to differentiate the two. Any access between the PC and the PC + 8 is considered an instruction fetch. This heuristic did not correctly handle addresses at the top of the range where the PC + 8 calculation overflows. This commit fixes the top of range handling.
When an interrupt is raised the Ibex controller will move from the DECODE state to the IRQ_TAKEN state when it chooses to handle the interrupt. When in IRQ_TAKEN it's possible for the interrupt state to change again which aborts the interrupt entry. This leads to mis-matches against cosim. This change adds a warning to flag up cases where this has occurred to enable quick triage of failures related to this scenario.
GregAC
force-pushed
the
clean_various_fixes
branch
3 times, most recently
from
fa53ff4
to
f94cf96
Compare
GregAC
force-pushed
the
clean_various_fixes
branch
from
f94cf96
to
d5d3f37
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.
As part of my deep dive regression triage I've been fixing up a few things.
With this set of fixes we can hit 95 - 96% pass rate
They require an updated spike, PR is here: lowRISC/riscv-isa-sim#25.
Once that's merged I can kick off a binary build in CI and then update the cosim version we point to which will allow CI to pass on this commit.