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

[TASK] Parametrization #1451

Open
1 task done
JeanRochCoulon opened this issue Sep 19, 2023 · 12 comments
Open
1 task done

[TASK] Parametrization #1451

JeanRochCoulon opened this issue Sep 19, 2023 · 12 comments
Assignees
Labels
Component:RTL For issues in the RTL (e.g. for files in the rtl directory) Component:Verif For issues in the verification environment or test cases (e.g. for testbench, C code, etc.) CV32A60AX Part: Application configuration (former "step1") CV32A65X Part: Embedded configuration Type:Task Project related task

Comments

@JeanRochCoulon
Copy link
Contributor

JeanRochCoulon commented Sep 19, 2023

Is there an existing CVA6 task for this?

  • 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 JeanRochCoulon moved this to In Progress in CVA6 Project Task Board Sep 19, 2023
@JeanRochCoulon JeanRochCoulon added Component:RTL For issues in the RTL (e.g. for files in the rtl directory) CV32A60AX Part: Application configuration (former "step1") CV32A65X Part: Embedded configuration labels Sep 19, 2023
@zarubaf
Copy link
Contributor

zarubaf commented Oct 9, 2023

Ariane_cfg_t should be done.

@JeanRochCoulon
Copy link
Contributor Author

JeanRochCoulon commented Oct 9, 2023

@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
Copy link
Contributor

Jbalkind commented Oct 9, 2023

There was already an existing task for this #1233

@JeanRochCoulon
Copy link
Contributor Author

JeanRochCoulon commented Oct 10, 2023

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
Copy link
Contributor

Hello, I’m working on it

@JeanRochCoulon JeanRochCoulon added the Type:Task Project related task label Oct 18, 2023
@cathales
Copy link
Contributor

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

@JeanRochCoulon JeanRochCoulon added the Component:Verif For issues in the verification environment or test cases (e.g. for testbench, C code, etc.) label Oct 19, 2023
@cathales
Copy link
Contributor

cathales commented Nov 9, 2023

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

@cathales
Copy link
Contributor

cathales commented Nov 15, 2023

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
Copy link
Contributor

wt_cache_pkg is now transferred too.

@cathales
Copy link
Contributor

Here is the current status: #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.

This was referenced Mar 6, 2024
ASintzoff pushed a commit that referenced this issue Mar 15, 2024
This is the third step for #1451. Many values are moved but not all values are moved yet

* move NR_SB_ENTRIES & TRANS_ID_BITS
* remove default rvfi_instr_t from spike.sv
* fifo_v3: ariane_pkg::FPGA_EN becomes a param
* move FPGA_EN
* inline wt_cache_pkg::L15_SET_ASSOC
* move wt_cache_pkg::L15_WAY_WIDTH
* inline wt_cache_pkg::L1I_SET_ASSOC
* inline wt_cache_pkg::L1D_SET_ASSOC
* move wt_cache_pkg::DCACHE_CL_IDX_WIDTH
* move ICACHE_TAG_WIDTH
* move DCACHE_TAG_WIDTH
* move ICACHE_INDEX_WIDTH
* move ICACHE_SET_ASSOC
* use ICACHE_SET_ASSOC_WIDTH instead of $clog2(ICACHE_SET_ASSOC)
* move DCACHE_NUM_WORDS
* move DCACHE_INDEX_WIDTH
* move DCACHE_OFFSET_WIDTH
* move DCACHE_BYTE_OFFSET
* move DCACHE_DIRTY_WIDTH
* move DCACHE_SET_ASSOC_WIDTH
* move DCACHE_SET_ASSOC
* move CONFIG_L1I_SIZE
* move CONFIG_L1D_SIZE
* move DCACHE_LINE_WIDTH
* move ICACHE_LINE_WIDTH
* move ICACHE_USER_LINE_WIDTH
* move DCACHE_USER_LINE_WIDTH
* DATA_USER_WIDTH = DCACHE_USER_WIDTH
* move DCACHE_USER_WIDTH
* move FETCH_USER_WIDTH
* move FETCH_USER_EN
* move LOG2_INSTR_PER_FETCH
* move INSTR_PER_FETCH
* move FETCH_WIDTH
* transform SSTATUS_SD and SMODE_STATUS_READ_MASK into functions
* move [SM]_{SW,TIMER,EXT}_INTERRUPT into a structure
* move SV
* move vm_mode_t to config_pkg
* move MODE_SV
* move VPN2
* move PPNW
* move ASIDW
* move ModeW
* move XLEN_ALIGN_BYTES
* move DATA_USER_EN
* format: apply verible
@cathales
Copy link
Contributor

cathales commented Mar 18, 2024

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 <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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:RTL For issues in the RTL (e.g. for files in the rtl directory) Component:Verif For issues in the verification environment or test cases (e.g. for testbench, C code, etc.) CV32A60AX Part: Application configuration (former "step1") CV32A65X Part: Embedded configuration Type:Task Project related task
Projects
Status: In Progress
Development

No branches or pull requests

4 participants