Added cv64a65x_config_pkg.sv for CVA6 64bits config. To be reviewed! #2655
Conversation
|
✔️ successful run, report available here. |
|
"65" means superscalar is enabled, which is not the case in this new configuration. |
|
Hi, Any other feedback before making the change? Thanks |
|
Yes. We highly suggest to remove all the localparams from this file. Because this produces RTL issues: CVA6config can be override from outside (when instantiating the CVA6) while localparam cannot be redefined. |
|
Ok, no problem to remove localparam. Current localparam were just based on the format of the cv32a65x config pkg file. |
Configuration names are vetted by the Cores TG, either in monthly meetings or on the Mattermost channel. With what I read, the likely name would be CV64A60AX (the second A for Application = MMU, X for CV-X-IF). I think it collides with what @zarubaf has in mind for his company's configuration (but it's not vetted). The first action would for @ludovicpion and @zarubaf to get in touch together and identify the differences between their configurations. |
| localparam config_pkg::cache_type_t CVA6ConfigDcacheType = config_pkg::HPDCACHE; | ||
|
|
||
| localparam config_pkg::cva6_user_cfg_t cva6_cfg = '{ | ||
| XLEN: unsigned'(CVA6ConfigXlen), |
There was a problem hiding this comment.
In cv32a65x_config_pkg.sv there is a parameter called VLEN (Virtual address length) defined as well. It is a bit of a surprise that the CV32A65X should define anything about virtual addressing, but I would expect to see it defined for an Application-class core such as the CV64A65X.
There was a problem hiding this comment.
Maybe it is a parameter that we have missed. I will check.
Thanks for the feedback, it is useful.
There was a problem hiding this comment.
In CPU core, addresses are virtual.
if no MMU, PLEN address = VLEN
if MMU, PLEN address = translation(VLEN)
There was a problem hiding this comment.
This is great @ludovicpion, thanks for this. I am happy with it, but will leave it up to either @JeanRochCoulon or @jquevremont to approve. I've added comments directly to specific lines in the pkg file and will also make the following comments and queries:
- I see that you have not set
SuperscalarEN. If we follow our current naming strategy, that implies that this core should be the CV64A60X. - Are you interested in defining a core that aligns with one of the RVI Profiles? At this point, this config is close to the
RVB23U64. To get there we'll need to add Zicsr, Zicntr, Zihpm, Ziccif, Ziccrse, Ziccamoa, Za64rs and Zihintpause (whew!). As the CVA6 predates the definition of RVI Profiles, the config_pkg.sv does not have variables defined for many of these Zi* features. Please give it some thought - at a minimum it would be useful to add these variables to the config package and set them to zero.
| AxiUserWidth: unsigned'(CVA6ConfigDataUserWidth), | ||
| MemTidWidth: unsigned'(CVA6ConfigAxiIdWidth), | ||
| NrLoadBufEntries: unsigned'(8), | ||
| RVF: bit'(1), |
There was a problem hiding this comment.
Happy to see the F and D ISAs included in this core! I would strongly encourage you to update the floating-point unit the in CVA6 to the latest version of cvfpu as the floating-point unit currently in CVA6 is very out of date and can be expected to have many known and unknown bugs. A couple of additional things to note:
- The F ISA of cvfpu has been extensively verified by v1.83. of CV32E40P. Please check out the CV32E40P user Manual to see which parameters of the cvfpu have been verified.
- Support for the D ISA has never been verified in an OpenHW core. I do not know the status of this feature in cvfpu, but it is safe to assume that the CV64A65X will have a lot of work to do for double-precision floating point verification.
| XF16: bit'(0), | ||
| XF16ALT: bit'(0), | ||
| XF8: bit'(0), | ||
| RVA: bit'(1), |
| RVZiCond: bit'(1), | ||
| NrScoreboardEntries: unsigned'(CVA6ConfigNrScoreboardEntries), | ||
| PerfCounterEn: bit'(1), | ||
| MmuPresent: bit'(1), |
There was a problem hiding this comment.
As I am sure you now, the MMU, RVS and RVU features are not well verified. We will need a lot of work for these.
| CachedRegionAddrBase: 1024'({64'h8000_0000}), | ||
| CachedRegionLength: 1024'({64'h40000000}), | ||
| MaxOutstandingStores: unsigned'(7), | ||
| DebugEn: bit'(1), |
There was a problem hiding this comment.
Again, the Debug feature is not well verified.
| MaxOutstandingStores: unsigned'(7), | ||
| DebugEn: bit'(1), | ||
| AxiBurstWriteEn: bit'(0), | ||
| IcacheByteSize: unsigned'(32768), |
There was a problem hiding this comment.
These updates to the I and D caches will be "all new verification territory".
As discussed during CVA6 Project Meeting on 2024-12-03, I added the cv64a65x_config_pkg.sv for CVA6 64bits config.
To be reviewed