Make test benches work for DPLLC branch (Gwaihir) - #45
Open
midnightveil wants to merge 2 commits into
Open
Conversation
…flush_partition This would read 'X memory, and would likely misbehave on synthesised implementation. There's also undocumented behaviour where writing MaxPart+1 will isolate the LLC (via ar/aw_bypass variables) and so we maintain this.
…-ish-tests I have no idea what these tests are supposed to be doing. The previous code configured the cache to exist with 159 lines of 256 in partition 7. It then tested flushing only partition 7 - which failed to compare memory equal as there would be writes that end up in the shared partition (why is this a thing that even happens???) and so memory would be different between cpu & mem as the cache wasn't flushed. We can't configure partition 7 as all 256 lines, and configuring two partitions to fill up the shared partition also doesn't work, as it generates many warnings. (Which should be fixed). The flushing of partition 11 and 13 instead was just flushing the 'Shared Partition' (MaxPartition); how this worked I have no idea, as partition 7 should still be containing data. With my changes the tests pass now, but they don't really test much of anything at all, aside from that data doesn't get corrupted.
Contributor
Author
|
Note; Also this probably doesn't synthesise because of I'm willing to put in work to improve the tests here (I'd actually be willing to mostly redo how the user-visible parts work, it's rather confusing to deal with and has some strange semantics), so if people have ideas for how to test things... (I'm not familiar with how to write good testbenches for hardware; most things I want to test are somewhat internal details...) |
Contributor
Author
|
(Pinging @ricted98 here too) |
midnightveil
commented
Sep 9, 2026
| to_flush_set_d = conf_regs_i_cfg_flush_set & ~conf_regs_i_flushed_set; | ||
| index_based_flush_d = 1'b1; // meaning that the current flush operation is index-based | ||
| if (conf_regs_i.cfg_flush_partition > (MaxPartition + 1)) begin | ||
| $fatal("flushing partition outside of range [MaxPartition:0] & MaxPartition+1(all): %d", conf_regs_i.cfg_flush_partition); |
Contributor
Author
There was a problem hiding this comment.
This probably shouldn't stay in the code, but needs a discussion of what should happen here.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
See individual commits for details.
Basically, this fixes one bug where out-of-bound array access ocurred, and makes the
tb_axi_llc_partitiontests pass.I don't know how the tests were supposed to work before, however; please see the code.