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

Replace WT_DCACHE define by CVA6ConfigCacheType localparam #1127

Merged
merged 8 commits into from
Mar 21, 2023

Conversation

JeanRochCoulon
Copy link
Contributor

@JeanRochCoulon JeanRochCoulon commented Mar 21, 2023

Cache WT or WB was configured by a define.
#603 requests more description of it
#483 tells to not use define
#1122 reports a bug: WB is never used/verified

To solve these issues, WT_DCACHE define has been replaced by CVA6ConfigCacheType localparam

In GitHub Actions CI, cv32a6_imafc_sv32 has been added, this configuration implements WB.

Signed-off-by: Jean-Roch Coulon <[email protected]>
Signed-off-by: Jean-Roch Coulon <[email protected]>
@cfuguet
Copy link
Contributor

cfuguet commented Mar 21, 2023

Hi @JeanRochCoulon, I made an eye-review (no simulation), and it looks good.

However, I have one remark: currently you suppose a binary choice, either the WB or the WT cache. As you know, there is another cache subsystem that is showing the tip of the nose :).

Should not we have some kind of enumeration type that defines legal cache subsystem types:
enum {
WB = 0,
WT = 1,
// HPDcache = 2
} cache_type_t ;

This values can be used in the configuration package to select a given cache subsystem. We can then add new types to it if new cache subsystems are supported.

The question is now where to define this enumeration type. It cannot be in ariane_pkg because we will have a circular dependency between config packages and ariane_pkg. We could create an additional file: ariane_cache_pkg.sv. Constants from it can be imported from the config packages.

Just an idea...

Signed-off-by: Jean-Roch Coulon <[email protected]>
Signed-off-by: Jean-Roch Coulon <[email protected]>
Signed-off-by: Jean-Roch Coulon <[email protected]>
@JeanRochCoulon JeanRochCoulon changed the title Replace WT_DCACHE define by CVA6ConfigWtDcache localparam Replace WT_DCACHE define by CVA6ConfigCacheType localparam Mar 21, 2023
@JeanRochCoulon JeanRochCoulon merged commit 3194885 into openhwgroup:master Mar 21, 2023
niwis added a commit to pulp-platform/cva6 that referenced this pull request Apr 8, 2023
This module was moved to `core/cache_subsystem/axi_adapter.sv`
in openhwgroup#1127.

Signed-off-by: Nils Wistoff <[email protected]>
niwis added a commit to pulp-platform/cva6 that referenced this pull request Apr 8, 2023
This module was moved to `core/cache_subsystem/axi_adapter.sv`
in openhwgroup#1127.

Signed-off-by: Nils Wistoff <[email protected]>
JeanRochCoulon pushed a commit that referenced this pull request Apr 9, 2023
This module was moved to `core/cache_subsystem/axi_adapter.sv`
in #1127.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants