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

csr_regfile: Fix S-mode traps when H extension is enabled #2587

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

niwis
Copy link
Contributor

@niwis niwis commented Nov 8, 2024

Since #2346, if Hypervisor extension is enabled, the logic required to properly trap to S mode is excluded. Fix this by adjusting the if condition.

If Hypervisor extension is enabled, the logic required to properly trap
to S mode is currently excluded. Fix this by adjusting the if block.

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

github-actions bot commented Nov 8, 2024

❌ failed run, report available here.

@AyoubJalali
Copy link
Contributor

Hello @niwis the logic doesn't change here ? that's right ?
Because of the reasons of the PR #2346 is increase code coverage, the simulator doesn't recognize the condition in if (CVA6Cfg.RVH && trap_to_v) begin as a constant to exclude the code related in code coverage, that's why we dispatch the condition

@niwis
Copy link
Contributor Author

niwis commented Nov 11, 2024

@AyoubJalali #2346 did change the logic in a breaking way since the else block below the diff is no longer taken if (RVH && !trap_to_v). This is a critical bug since S-mode traps no longer update sstatus, scause, sepc, and stval if CVA6Cfg.RVH == 1. This PR fixes this by reverting the breaking part of #2346.

@JeanRochCoulon JeanRochCoulon merged commit 485c382 into openhwgroup:master Nov 12, 2024
4 checks passed
JeanRochCoulon added a commit that referenced this pull request Nov 12, 2024
JeanRochCoulon added a commit that referenced this pull request Nov 12, 2024
@JeanRochCoulon
Copy link
Contributor

@niwis I merged, then reverted this PR because the CI was failed (my bad). Feel free to submit again.

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.

3 participants