[rom_ctrl, dv] rom_ctrl simulations ported to mocha#435
[rom_ctrl, dv] rom_ctrl simulations ported to mocha#435KolosKoblasz-Semify wants to merge 3 commits intolowRISC:mainfrom
Conversation
KolosKoblasz-Semify
force-pushed
the
kk_rom_ctrl_dv
branch
2 times, most recently
from
bb98462 to
eb4fb89
Compare
KolosKoblasz-Semify
force-pushed
the
kk_rom_ctrl_dv
branch
3 times, most recently
from
846bb35 to
d9d868d
Compare
martin-velay
left a comment
martin-velay
left a comment
There was a problem hiding this comment.
Everything looks great. The commits are well structured IMO and the regression results are looking good. Thanks Kolos!
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
The only concern I have is that this pulls in a lot of IP blocks from OpenTitan. Where are these dependencies coming from. Is there a way to remove those dependencies from ROM control DV?
KolosKoblasz-Semify
force-pushed
the
kk_rom_ctrl_dv
branch
from
abf3232 to
fd2f3b2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Ignore my last review. I need to do a clean build next week and some more investigation. |
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
Ok, it took me a little longer but now I think these are the working changes that remove dependencies for the 5 extra IP blocks:
marnovandermaas@42a3d04
Would you mind encoding this in patch file and then removing all those IP blocks from the vendor script.
Which 5 IPs should be removed? I know about these 4: |
|
Blocks that should not be vendored in this PR:
In my earlier comment where I said 5 I had already removed flash control since you removed that before my latest review. |
KolosKoblasz-Semify
force-pushed
the
kk_rom_ctrl_dv
branch
2 times, most recently
from
ad5dbbb to
804c024
Compare
* The required dependencies added to: hw/vendor/lowrisc_ip.vendor.hjson * Patch file created to apply necessary modifications to rom_ctrl cfg files * rom_ctrl UVM TB simplified and patched to enable fewer modules vendored in as dependencies * rom_ctrl dv needs sram_scrambler_pkg.sv as a patch file to enable reduced number of vendored in modules. Signed-off-by: Kolos Koblasz <kolos.koblasz@semify-eda.com>
* kmac_app_agent modifications added * rom_ctrl modifications added * sram_ctrl modifications added Signed-off-by: Kolos Koblasz <kolos.koblasz@semify-eda.com>
* hw/top_chip/dv/mocha_sim_cfgs.hjson modified to point at the real rom_ctrl configuration files * temporary config files deleted Signed-off-by: Kolos Koblasz <kolos.koblasz@semify-eda.com>
KolosKoblasz-Semify
force-pushed
the
kk_rom_ctrl_dv
branch
from
804c024 to
8b1978c
Compare
martin-velay
left a comment
martin-velay
left a comment
There was a problem hiding this comment.
This PR is looking good, and thanks for having removed the extra-dependencies.
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
Thanks for the progress here. One more dependency to remove: SRAM control.
| - for (genvar k = 1; k < KmacDataIfWidth / 8 - 1; k++) begin : gen_strb_check | ||
| - `ASSERT(StrbAlignLSB_A, kmac_data_req.valid && kmac_data_req.strb[k] === 0 |-> | ||
| + for (genvar k = 1; k < kmac_app_agent_pkg::KmacDataIfWidth / 8 - 1; k++) begin : gen_strb_check | ||
| + `ASSERT(StrbAlignLSB_A, kmac_data_req.valid && kmac_data_req.strb[k] === 0 |-> |
There was a problem hiding this comment.
Nit: extra indent not necessary.
| {from: "hw/ip/rv_timer", to: "ip/rv_timer", patch_dir: "rv_timer"}, // Timer. | ||
| {from: "hw/ip/spi_device", to: "ip/spi_device", patch_dir: "spi_device"}, // SPI device. | ||
| {from: "hw/ip/spi_host", to: "ip/spi_host", patch_dir: "spi_host"}, // SPI host. | ||
| {from: "hw/ip/sram_ctrl", to: "ip/sram_ctrl"}, // SRAM Ctrl. |
There was a problem hiding this comment.
We don't need to pull in SRAM control any more right?
3 participants
rom_ctrl module's simulations relay on OpenTitan project specific paths.
These are modified to fit into Mocha repo.
This PR will close: #177
Command results from:
dvsim hw/vendor/lowrisc_ip/ip/rom_ctrl/dv/rom_ctrl_32kB_sim_cfg.hjson -i nightly`
Failure Buckets
UVM_ERROR (rom_ctrl_corrupt_sig_fatal_chk_vseq.sv:149) [rom_ctrl_corrupt_sig_fatal_chk_vseq] Check failed (cfg.rom_ctrl_vif.pwrmgr_data.done != MuBi4True)has 1 failure:2.rom_ctrl_corrupt_sig_fatal_chk.59291826974623340318801047515561390684876095977744966578288946903992868473900
Line 96, in log /home/kkoblasz/projects/mocha/scratch/kk_rom_ctrl_dv/rom_ctrl_32kB-sim-xcelium/2.rom_ctrl_corrupt_sig_fatal_chk/latest/run.log
if.pwrmgr_data.done != MuBi4True)
UVM_INFO @ 1064671812 ps: (uvm_report_catcher.svh:705) [UVM/REPORT/CATCHER]
--- UVM Report catcher Summary ---
`
Also, integration into the
sim_cfgsbundle (+check if coverage collection works well too):dvsim hw/top_chip/dv/mocha_sim_cfgs.hjson -i smoke --cov