openhwgroup / cva6

The CORE-V CVA6 is an Application class 6-stage RISC-V CPU capable of booting Linux
https://docs.openhwgroup.org/projects/cva6-user-manual/
Other
2.27k stars 688 forks source link

[TASK] Parametrization #1451

Open JeanRochCoulon opened 1 year ago

JeanRochCoulon commented 1 year ago

Is there an existing CVA6 task for this?

Task Description

To be able to instantiate several CVA6 with different configurations, parameters must be defined as cva6 input parameters.

Required Changes

Move parameters from packages to cva6 input parameters. But as not all the parameters can be configured, identify:

Current Status

NOC done RVFI done Ariane_cfg_t ongoing All others to be done

Risks

To not finish the modification, to be in the middle of the bridge.

Prerequisites

system verilog limitations

KPI (KEY Performance Indicators)

remaining number of parameters to be moved from package to input parameters

Description of Done

All parameters are moved from package to input parameters

Associated PRs

No response

zarubaf commented 1 year ago

Ariane_cfg_t should be done.

JeanRochCoulon commented 1 year ago

@zarubaf I listed the parameters belonging to the config package to identify which are already in CVACfg ("done") and the others, some could be easily transformed as parameter ("easy"), some less ("hard"). image

Jbalkind commented 1 year ago

There was already an existing task for this https://github.com/openhwgroup/cva6/issues/1233

JeanRochCoulon commented 1 year ago

Yes, #1233 was not defined as Kanban board task, but normal github issue. Thank for having added the link. @Jbalkind Any news about parametrization ?

cathales commented 1 year ago

Hello, I’m working on it

cathales commented 1 year ago

I have started working on it last week but this week I did not have time to work on this topic.

cathales commented 12 months ago

Back from holiday, I am rebasing my previous work and will continue working on it.

cathales commented 11 months ago

I have finished transferring ariane_pkg and riscv values which depend on configuration to the new parametrization system. However there are values in other packages that depend on these ariane_pkg "variable" values so I have to transfer them too. It is not possible to elaborate until everything is transferred so I have to continue to completion before submitting changes.

It will require me a few more weeks before I can complete this task.

cathales commented 11 months ago

wt_cache_pkg is now transferred too.

cathales commented 11 months ago

Here is the current status: https://github.com/openhwgroup/cva6/pull/1704

Once this PR is completed and merged, the planned work will be to:

  1. Remove all localparams from config files so that only the cva6_cfg definition remains a) requires moving all remaining user-config-dependent values from all packages b) requires HPDCache update c) requires CoreV Verif update about CV-X-IF
  2. Update the config generator accordingly.

It would be nice if someone else can help me, especially for points b) and c) which I cannot handle myself. These two points can be started in parallel of my current work.

cathales commented 7 months ago

All the three steps announced on Mattermost are now merged. Many values have been moved to the new structure but the work is not over yet! Below are points that still need to be addressed.

Update submodules

Submodules currently depend on configuration values from packages. It prevents form moving these values and their dependencies into the structure.

RVFI (done)

This task has been done by @yanicasa `core/cva6_rvfi.sv` is re-building values which are now into `CVA6Cfg`. This should not be needed anymore. A cleanup would help making the code more *DRY*. @yanicasa

Update user configs and config generator

core/include/cv[36][24]a6*_config_pkg.sv define localparam values which are used to build the structure. We should remove localparam definitions except the one of the structure. It would help us making sure that the old values are not used anymore.

However, util/config_pkg_generator.py first needs to be updated to handle both localparam field = value; and field: cast'(value); for the transition.

Move more values to the configuration structure

Not all values are parametrized yet, and updating user configurations would help finding them.

Rules of thumb to help moving values into the configuration structure. ### Identify items to move Find potential dependents of `` to change. ```bash grep -nrw core/include ``` Start with the leaves of the dependency tree. ### Add the item to the configuration - Remove it from its package. - `core/include/config_pkg.sv`: add the fields to appropriate structures. - `core/include/build_config_pkg.sv`: add assign the field with the value. `core/include/build_config_pkg.sv` is built last so you can temporarily have dependencies from other packages for the transition. Commit your changes before massive edits. You will be able to amend your commit after them. ### Massive edits All commands below can have false positive. Make sure you understand the command before running it (mind the placeholders). **Make sure your have nothing to commit**; it will make it easy to cancel your command with `git checkout .` Feel free to modify the commands to better suit your needs. #### Add `` in all configurations Before ``: ```bash sed -i "/:/i\ : unsigned'(CVA6Config)," core/include/cv[36][24]a6*_config_pkg.sv ``` As the last field (known to have a false positive in comments from one configuration): ```bash sed -i "s/)$/),/" core/include/cv[36][24]a6*_config_pkg.sv && sed -i "/};/i\ : unsigned'(CVA6Config)" core/include/cv[36][24]a6*_config_pkg.sv ``` #### Use `` ```bash sed 's/\b\(std_cache_pkg::\|wt_cache_pkg::\|ariane_pkg::\|riscv::\|CVA6Cfg.\)\?\(\)\b/CVA6Cfg.\2/g' -i **/*.{v,sv,svh} ``` Remove unwanted modifications (often in `core/include`) while still reviewing them: ```bash git checkout -p core/include ```

Update pmp

The pmp takes CVA6Cfg but does not use it yet. A choice should be made:

cathales commented 7 months ago

Another input from @zarubaf

The types could be at the beginning of the module instead of inside the parameters.

 module the_module #(
-  localparam type the_type = ...
 ) ( ... );

+  localparam type the_type = ...;

 ...

 endmodule