cva6 icon indicating copy to clipboard operation
cva6 copied to clipboard

[TASK] Parametrization

Open JeanRochCoulon opened this issue 1 year ago • 12 comments

Is there an existing CVA6 task for this?

  • [x] I have searched the existing task issues

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:

  • The paramaters that are actually used to configure the core.
  • The parameters that are not used now, e.g. the WB cache.
  • The parameters that are not used anymore, e.g. the renaming is not needed from GCC 13. Remove ifdef from RTL, for instance openpiton ifdef

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

JeanRochCoulon avatar Sep 19 '23 15:09 JeanRochCoulon

Ariane_cfg_t should be done.

zarubaf avatar Oct 09 '23 12:10 zarubaf

@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

JeanRochCoulon avatar Oct 09 '23 12:10 JeanRochCoulon

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

Jbalkind avatar Oct 09 '23 15:10 Jbalkind

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

JeanRochCoulon avatar Oct 10 '23 02:10 JeanRochCoulon

Hello, I’m working on it

cathales avatar Oct 11 '23 12:10 cathales

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

cathales avatar Oct 19 '23 08:10 cathales

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

cathales avatar Nov 09 '23 10:11 cathales

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 avatar Nov 15 '23 08:11 cathales

wt_cache_pkg is now transferred too.

cathales avatar Dec 12 '23 13:12 cathales

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 avatar Dec 13 '23 10:12 cathales

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.

  • HPDCache (see core/include/cva6_hpdcache_default_config_pkg.sv @cfuguet)
  • CoreV Verif (about CV-X-IF @MarioOpenHWGroup)

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 <the item> to change.

grep -nrw <the item> 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 <an item> in all configurations

Before <another item>:

sed -i "/<another item>:/i\      <an item>: unsigned'(CVA6Config<an item>)," core/include/cv[36][24]a6*_config_pkg.sv

As the last field (known to have a false positive in comments from one configuration):

sed -i "s/)$/),/" core/include/cv[36][24]a6*_config_pkg.sv && sed -i "/};/i\      <an item>: unsigned'(CVA6Config<an item>)" core/include/cv[36][24]a6*_config_pkg.sv

Use <the moved item>

sed 's/\b\(std_cache_pkg::\|wt_cache_pkg::\|ariane_pkg::\|riscv::\|CVA6Cfg.\)\?\(<the moved item>\)\b/CVA6Cfg.\2/g' -i **/*.{v,sv,svh}

Remove unwanted modifications (often in core/include) while still reviewing them:

git checkout -p core/include

Update pmp

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

  • Either use the values from CVA6Cfg
    • It might reduce the number of parameters.
    • The testbench of pmp might not work anymore.
  • or remove the CVA6Cfg parameter from pmp.

cathales avatar Mar 18 '24 15:03 cathales

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

cathales avatar Mar 21 '24 14:03 cathales