[dv][kmac] Integrate KMAC Block-level DV from OpenTitan#650
[dv][kmac] Integrate KMAC Block-level DV from OpenTitan#650tchilikov-semify wants to merge 5 commits into
Conversation
|
Implements #430 |
martin-velay
left a comment
martin-velay
left a comment
There was a problem hiding this comment.
Thanks for this PR Christian. I have some comments, but you've done a good job!
| function new(string name = "app_cg"); | ||
| app_cg = new(name); | ||
| app_cfg_reg_cg = new(name); | ||
| app_cfg_reg_cg = new({name, "_cfg_reg"}); |
There was a problem hiding this comment.
Why this change? Is it to avoid having 2 times the same name? If yes, that's probably something we should fix upstream too?
There was a problem hiding this comment.
That's right - the issue here is that theres duplicate naming for different coverage properties. In the upstream where vcs was used this wasn't an issue it seems, but xcelium throws an error. this change makes it so the different properties have unique names. Youre right that this should probably be changes upstream for compatibility across simulators.
| if (common_seq_type inside {"shadow_reg_errors", "shadow_reg_errors_with_csr_rw"}) begin | ||
| csr_excl_item csr_excl = ral.get_excl_item(); | ||
| // Shadow storage fatal error might cause req to drop without ack. | ||
| `ifdef KMAC_MASKING |
There was a problem hiding this comment.
We should avoid the ifdef as much as possible (at least on the DV side). And I think we already have the material to do so in the cfg class. Could you try this instead and remove the +define+KMAC_MASKING?
if (cfg.enable_masking) begin
$assertoff(0, "tb.dut.gen_entropy...SyncReqAckHoldReq");
$assertoff(0, "tb.dut.gen_entropy...SyncReqAckAckNeedsReq");
endThere was a problem hiding this comment.
Good catch, I didn't notice that this already exists. Will update it to use the cfg class instead of a macro
| `uvm_object_new | ||
|
|
||
| virtual function void disable_asserts(); | ||
| `ifdef KMAC_MASKING |
2 participants
KMAC sanity regression can be ran with: