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

Update mstatus bits guarded by privilege modes #750

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

Conversation

jordancarlin
Copy link
Collaborator

A recent PR against the isa manual (riscv/riscv-isa-manual#1864) clarified that mstatus SIE and SPIE should be read-only zero when S mode is not supported. While updating, I noticed several other bits that should also be guarded by this constraint according to the spec.

Copy link

Test Results

392 tests  ±0   392 ✅ ±0   1m 20s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 8de2591. ± Comparison against base commit 80c4928.

@trdthg
Copy link
Contributor

trdthg commented Feb 19, 2025

I have always had a small question, what version of spec should we develope with, the release version or the in develop version?
Will sail have a config to change different spec versions in the future?
If yes, then both behaviors will be supported when we have that config?

@jordancarlin
Copy link
Collaborator Author

I have always had a small question, what version of spec should we develope with, the release version or the in develop version? Will sail have a config to change different spec versions in the future? If yes, then both behaviors will be supported when we have that config?

The plan is definitely to support multiple versions of the spec (primarily the privileged specs) once config is up and running.

All of the mstatus bits other than SIE and SPIE that I changed have had the constraint that they were read-only zero if supervisor mode was not implemented going back to at least priv 1.10, so versions shouldn't affect those parts.

SIE and SPIE are a trickier case because it is being presented as just a clarification, not an actual change. If there were implementations that didn't make these bits read only zero though, then it is most definitely a breaking change. Maybe for now we update it accordingly and then add some configuration option later if needed.

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