Fix for bus_imem_fault issue in #44 #47
Open
+13
−5
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.
When a 16-bit instruction occurs immediately before a faulting instruction fetch, the
bus_imem_fault
check erroneously asserts that it must fault, as though the instruction were 32-bit and the fetch had occurred in its second half.Avoid this by introducing a new free variable
imem_prev_32bit
which conditionally enables the problematic check onpc + 2
, and also constrains fetch from the halfword beforeimem_addr
to start with2'b11
, i.e. to be a 32-bit instruction.This might seem to weaken the check because if the data at
imem_addr - 2
is actually the second half of a previous 32-bit instruction, this would force two bits in the middle of that instruction to be2'b11
, limiting the possible instructions. However in this caseimem_addr
will take exactly the value ofpc
so, assuming fetches are at least 32 bits in size, thepc+2
assertions are unnecessary. If thepc+2
assertions aren't needed then theimem_prev_32bit
variable can be clear, meaning the2'b11
constraint doesn't apply. Therefore I don't think this restricts the instruction sequences that can be checked, at least for a processor with at least 32-bit-wide fetch.See discussion in #44
The
bus_imem_fault
check fails on Hazard3 without this patch, and passes with it. Repro:(note this is a different git hash than the one I gave in the issue thread because I had to make a couple of changes to my RVFI monitor)