sail-riscv
sail-riscv copied to clipboard
Fix handling of mstatus.MIE in M-mode only case.
Previously if neither user or supervisor mode are present dispatchInterrupt ignored mstatus.MIE leading to an infinite interrupt loop.
Unit Test Results
712 tests ±0 712 :heavy_check_mark: ±0 0s :stopwatch: ±0s 6 suites ±0 0 :zzz: ±0 1 files ±0 0 :x: ±0
Results for commit 1e983ea8. ± Comparison against base commit 9547a30b.
:recycle: This comment has been updated with latest results.
Any objections to this?
Yes, that is more elegant. I will push an alternative version.
That does sound like the correct version to determine that an interrupt cannot be delegated, irrespective of delegation bits and enabling
On Wed, Jan 25, 2023 at 2:32 AM Robert Norton @.***> wrote:
@.**** commented on this pull request.
In model/riscv_sys_control.sail https://github.com/riscv/sail-riscv/pull/190#discussion_r1086463844:
@@ -342,9 +342,9 @@ function dispatchInterrupt(priv : Privilege) -> option((InterruptType, Privilege if (~ (haveUsrMode())) | ((~ (haveSupMode())) & (~ (haveNExt()))) then { assert(priv == Machine, "invalid current privilege");
On the subject of that condition I think it could also be expressed as ~(haveSupMode()) | (haveUsrMode() & ~haveNExt()) which might more clearly express the intent?
The above is incorrect but a more concise version might be ~(haveSupMode() | haveNExt())?
— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/pull/190#discussion_r1086463844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJUAG6EYCA2UQXFCOSTWUD6KXANCNFSM6AAAAAARW772AA . You are receiving this because you are subscribed to this thread.Message ID: @.***>
OK, I'll revise this again to remove the assertion. I'll also flip the if so we don't need to invert the condition.
Please correct the typo (msatus) in the commit message and PR title.
Also this whole check is only a performance optimisation. Maybe it's better just to remove it (it probably doesn't make too much difference because it's all xlenbits so GMP won't get involved).
We ended up doing this so the behaviour should be correct now. Please open an issue if it isn't!