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

config_pkg: Update configurations with new flush parameter #2704

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

niwis
Copy link
Contributor

@niwis niwis commented Jan 14, 2025

#2691 extended the cva6_user_cfg_t struct by two new parameters to control the cache's flush behaviour. Add these new parameters to all configs to fix compilation errors due to incomplete struct literals.

PR openhwgroup#2691 extended the `cva6_user_cfg_t` struct by two new parameters
to control the cache's flush behaviour. Add these new parameters to all
configs to fix compilation errors due to incomplete struct literals.

Signed-off-by: Nils Wistoff <[email protected]>
Copy link
Contributor

✔️ successful run, report available here.

@JeanRochCoulon
Copy link
Contributor

@niwis The two parameters are not defined for all configurations (60x and 65x for instance). I think it would be better to define it for all.

@niwis
Copy link
Contributor Author

niwis commented Jan 15, 2025

@JeanRochCoulon sure, done

@@ -88,8 +91,8 @@ package cva6_config_pkg;
DcacheByteSize: unsigned'(2028),
DcacheSetAssoc: unsigned'(2),
DcacheLineWidth: unsigned'(128),
DcacheFlushOnFence: bit'(0),
DcacheInvalidateOnFlush: bit'(0),
DcacheFlushOnFence: bit'(CVA6ConfigDcacheFlushOnFence),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DcacheFlushOnFence: bit'(CVA6ConfigDcacheFlushOnFence),
DcacheFlushOnFence: bit'(1'b0),

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to remove the localparam from config_pkg to avoid bugs, because a discrepancy can occur between localparam and cva6config parameter (if driven from outside).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry @JeanRochCoulon, I guess I misunderstood your previous request. 60x and 65x already define this parameter in their cva6_cfg struct on master, what else should I do then?

DcacheFlushOnFence: bit'(0),
DcacheInvalidateOnFlush: bit'(0),

I'll revert the previous commit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I was not aware that the two parameters were already present in 60x and 65x. I will merge

core/include/cv32a60x_config_pkg.sv Outdated Show resolved Hide resolved
core/include/cv32a65x_config_pkg.sv Outdated Show resolved Hide resolved
core/include/cv32a65x_config_pkg.sv Outdated Show resolved Hide resolved
Copy link
Contributor

❌ failed run, report available here.

Copy link
Contributor

✔️ successful run, report available here.

Copy link
Contributor

❌ failed run, report available here.

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