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

Fix delegated interrupt handling #613

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Nov 6, 2024

When the N extension was removed this code was accidentally broken because it doesn't respect mie for delegated interrupts any more.

The internal_errors should be unreachable, at least for the sequential output. I believe formal backends may decide mideleg is going to be non-zero even without S, but I think that was an existing issue.

When the N extension was removed this code was accidentally broken because it doesn't respect mie for delegated interrupts any more.

The `internal_error`s should be unreachable, at least for the sequential output. I believe formal backends may decide `mideleg` is going to be non-zero even without S, but I think that was an existing issue.
Ints_Delegated(d) => {
if not(extensionEnabled(Ext_S)) then {
// Can't delegate to user mode. This code is unreachable because
// `mideleg.bits` is 0 without supervisor mode.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, the CSR doesn't exist so can't be written to by software and thus the Sail register remains its initial 0 value. The comment written here suggests that the CSR exists and is hardwired to 0 (which was the case in early specs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Sail register mideleg always exists. That's what I was referring to. Not sure how to make that clearer...

Copy link

github-actions bot commented Nov 6, 2024

Test Results

396 tests  ±0   396 ✅ ±0   0s ⏱️ ±0s
  4 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit df646de. ± Comparison against base commit b8d1fa5.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Nov 8, 2024

@KotorinMinami does this look right to you?

@KotorinMinami
Copy link
Contributor

@Timmmm
The spec mentions:

In harts with S-mode, the medeleg and mideleg registers must exist, and setting a bit in medeleg or mideleg will delegate the corresponding trap, when occurring in S-mode or U-mode, to the S-mode trap handler. In harts without S-mode, the medeleg and mideleg registers should not exist.

So I think we might need to reflect this in the implementation of the sail's mideleg register, maybe this is the way to solve delegated interrupt handling?

Moreover, based on what is said in spec, we can handle whether medeleg and mideleg exist and the delegated issues by checking misa

In versions 1.9.1 and earlier , these registers existed but were hardwired to zero in M-mode only, or M/U without N harts. There is no reason to require they return zero in those cases, as the misa register indicates whether they exist.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Nov 8, 2024

So I think we might need to reflect this in the implementation of the sail's mideleg register

This is handled by is_CSR_defined() returning false for m*deleg when S mode is not implemented.

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