cva6 icon indicating copy to clipboard operation
cva6 copied to clipboard

[BUG] CVA6Cfg value defined in several files

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 CVA6Cfg (config_pkg::cva6_cfg_t type) localparam, which propagates the CVA6 configuration parameters to testbench and RTL.

Currently, CVA6Cfg is defined in cva6/verif/tb/uvmt/uvmt_cva6_tb.sv (set to cva6_config_pkg::cva6_cfg), which propagates it until cva6/core/cva6.sv and cva6/verif/tb/uvmt/uvmt_cva6_tb_ifs.sv. Moreover, there are two files that modify CVA6Cfg in the same way to then propagate it as CVA6ExtendCfg, which is the real issue for us:

  • cva6/core/cva6.sv
  • cva6/core/cva6_rvfi.sv

It would be cleaner to have only one definition of CVA6ExtendCfg. The best solution from our point of view would be to have two different types for the CVA6 configuration:

  • one for all the parameter that are chosen by the user: new type config_pkg::cva6_param_t (type name proposal), set in files cva6/core/include/cvXXa6_XX_pkg.sv
  • one for computed attributes that depends on parameters previously set by the user: config_pkg::cva6_cfg_t (to avoid modifying type everywhere in RTL and UVM) that could be set in a new file in cva6/core/include/ We are ready to submit a PR for this issue, after aligning with you on this topic.

Do you have any feedback? Are you already thinking about evolution of CVA6Cfg?

CoralieAllioux avatar Feb 15 '24 09:02 CoralieAllioux

This Issue should be resolved with PR #1704 , which splits the user parameters and the extended ones.

CoralieAllioux avatar Feb 23 '24 11:02 CoralieAllioux

This issue should be resolved with #1896

If so, can you close it, please @CoralieAllioux

cathales avatar Mar 18 '24 15:03 cathales