[dv] Enable DRAM backdoor for the UVM TB#615
Open
martin-velay wants to merge 5 commits into
Open
Conversation
engdoreis
approved these changes
Jun 18, 2026
engdoreis
left a comment
engdoreis
left a comment
Collaborator
There was a problem hiding this comment.
The SW and build system side LGTM, I also tested the UVM test.
[ legend ]: [S: scheduled, Q: queued, R: running, P: passed, F: failed, K: killed, T: total]
00:03:19 [ build ]: [S: 00, Q: 00, R: 00, P: 01, F: 00, K: 00, T: 01] 100%
00:07:43 [ run ]: [S: 00, Q: 00, R: 00, P: 28, F: 04, K: 00, T: 32] 100%
Contributor
Author
|
@engdoreis, good point, also the Gives: |
marnovandermaas
left a comment
marnovandermaas
left a comment
Collaborator
There was a problem hiding this comment.
Initial review from my end.
| `MEM_BKDR_UTIL_FILE_OP(m_mem_bkdr_util[ChipMemROM], `ROM_MEM_HIER) | ||
|
|
||
| // Zero-init DRAM with a direct SV loop (not clear_mem) before loading the vmem: prim_ram_1p | ||
| // starts as X, and reads outside the binary range propagate into tag-controller FIFOs (DataKnown_A). |
Collaborator
There was a problem hiding this comment.
Hmmm, this sounds like a bug in the tag controller to me.
Contributor
Author
There was a problem hiding this comment.
I am unsure about that one, I don't know the design enough. To whom can I ask to take a look?
Add hw/top_chip/dv/common/ as a shared home for simulation-only modules used by both UVM and Verilator. Move sim_sram_axi/ there, move and rename dram_wrapper_sim -> sim_dram_wrapper for naming consistency, and add a FuseSoC core file for the DRAM wrapper. Update top_chip_sim.core and top_chip_verilator.core to depend on the new cores rather than referencing the files directly. Signed-off-by: martin-velay <mvelay@lowrisc.org>
- Add ChipMemDRAM to chip_mem_e - Instantiate the DRAM mem_bkdr_util in tb.sv - Add zero-init DRAM with a direct SV loop before loading the vmem to prevent X values from uninitialised addresses propagating into the tag-controller FIFO (DataKnown_A). - Extend mem_bkdr_write8/read8 in base_vseq to support ChipMemDRAM. Signed-off-by: martin-velay <mvelay@lowrisc.org>
- Update all 33 tests in top_chip_sim_cfg.hjson to pass ChipMemDRAM_image_file and use DRAM-linked vmem files instead of the SRAM-linked _sram variants. - Remove the cmake if(TRUE) block that built _sram binaries solely for UVM. Remove stale _sram reference from dev_guide. Signed-off-by: martin-velay <mvelay@lowrisc.org>
SW symbols are now in the DRAM-linked binary; pass ChipMemDRAM explicitly instead of relying on the ChipMemSRAM default. Signed-off-by: martin-velay <mvelay@lowrisc.org>
Signed-off-by: martin-velay <mvelay@lowrisc.org>
martin-velay
force-pushed
the
dv_dram_backdoor
branch
from
1f40132 to
2738ba2
Compare
Contributor
Author
|
I have addressed your comments @marnovandermaas except the one related to the assertion. Please let me know if I should remove the initialization |
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
4 participants
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.
This PR addresses issue #514