cva6 icon indicating copy to clipboard operation
cva6 copied to clipboard

[BUG] Missing explicit import in riscv-dv gen tb

Open CoralieAllioux opened this issue 1 year ago • 1 comments

Is there an existing CVA6 bug for this?

  • [X] I have searched the existing bug issues

Bug Description

We are currently porting the verification to be compliant with Cadence Xcelium simulator. The objective is to use the current verification flow and adapt it, to add the Cadence simulator. This issue concerns the test cva6/verif/regress/smoke_gen-test.sh.

The file which generate the instructions, called cva6/verif/sim/dv/test/riscv_instr_gen_top.sv, has an implicit dependency with cva6/verif/env/corev-dv/cva6_instr_test_pkg.sv. Xcelium must have this import explicit to create that link. Therefore, the explicit import should be added, as following:

module riscv_instr_gen_tb_top;

  import uvm_pkg::*;
  import riscv_instr_test_pkg::*;
  import cva6_instr_test_pkg::*;


  initial begin
    run_test();
  end

endmodule

Issue: This solution cannot be resolved by a fix on riscv-dv submodule, since this import is cva6-dependent.

Suggested solutions:

  • since riscv_instr_gen_tb_top becomes cva6-dependent, copy it and adapt it in cva6/verif/env/corev-dv
  • vendorization of riscv-dv
  • patch to be applied locally ==> then could be hard to maintain and not a clean solution (from my point of view)

Please @ASintzoff and @MikeOpenHWGroup, what is your opinion on how to resolve this issue? We are open to discussion to find the best file organization.

CoralieAllioux avatar Feb 08 '24 08:02 CoralieAllioux

Hello @mike, this issue is related to the organization setup to use and modify easily riscv-dv. @CoralieAllioux raises a challenging question, maybe you will have an opinion on it?

JeanRochCoulon avatar Feb 08 '24 10:02 JeanRochCoulon

My apologies @CoralieAllioux, I have not checked my Issues in a couple of weeks and I have missed this one. About your "suggested solutions":

since riscv_instr_gen_tb_top becomes cva6-dependent, copy it and adapt it in cva6/verif/env/corev-dv

Yes, this is the preferred approach, and I think #1862 implements this.

vendorization of riscv-dv

This can work, but we have not done this up to now. My preferred approach is to have the scriptware (e.g. cva6.py) clone a specific hash of riscv-dv and to implement corev-dv extensions.

patch to be applied locally ==> then could be hard to maintain and not a clean solution (from my point of view)

Yes, you are right. Locally applied patches are hard to maintain. Vendorization is better, but I still prefer class extensions of a specific hash riscv-dv.

MikeOpenHWGroup avatar Mar 04 '24 15:03 MikeOpenHWGroup