Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] CVA6Cfg value defined in several files #1836

Closed
1 task done
CoralieAllioux opened this issue Feb 15, 2024 · 2 comments
Closed
1 task done

[BUG] CVA6Cfg value defined in several files #1836

CoralieAllioux opened this issue Feb 15, 2024 · 2 comments
Assignees
Labels
Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system

Comments

@CoralieAllioux
Copy link
Contributor

CoralieAllioux commented Feb 15, 2024

Is there an existing CVA6 bug for this?

  • 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 CoralieAllioux added the Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system label Feb 15, 2024
@CoralieAllioux
Copy link
Contributor Author

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

@cathales
Copy link
Contributor

This issue should be resolved with #1896

If so, can you close it, please @CoralieAllioux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system
Projects
None yet
Development

No branches or pull requests

2 participants