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

Teach Sstc to respect xenvcfg.STCE #1591

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

YenHaoChen
Copy link
Collaborator

@YenHaoChen YenHaoChen commented Feb 2, 2024

When menvcfg.STCE=0, mip.STIP reverts to its defined behavior as if unsupporting Sstc extension. When henvcfg.STCE=0, mip.VSTIP reverts to its defined behavior as if unsupporting Sstc extension.

Reference: riscvarchive/riscv-time-compare#5

Copy link
Contributor

@scottj97 scottj97 left a comment

Choose a reason for hiding this comment

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

Please change the title and commit message to describe what is changing. Explain in the PR what was wrong with the old code, an example of how it could misbehave, and how this fixes it.

I guess the old code is only problematic if --isa=Sstc is enabled? But it's not clear.

@YenHaoChen
Copy link
Collaborator Author

Add additional paragraphs in the commit message to describe an example of misbehaving and the solution's strategy.

The title was a shortened, one-sentence summary from the spec, which the commit aims to conform to.
image
I need help developing a more concise but respectful sentence to the spec.

@scottj97
Copy link
Contributor

scottj97 commented Feb 5, 2024

How about this title:

Teach Sstc to respect xenvcfg.STCE

Followed by the explanation of the problem. The last paragraph in 15cd07d is unnecessary because it's simply describing the implementation which is already easy to understand from the diff.

When menvcfg.STCE=0, mip.STIP reverts to its defined behavior as if
unsupporting Sstc extension. When henvcfg.STCE=0, mip.VSTIP reverts
to its defined behavior as if unsupporting Sstc extension. [riscvarchive/riscv-time-compare#5]

The previous Sstc implementation does not respect the xenvcfg.STCE.
In other words, the Sstc may assert mip.STIP (mip.VSTIP) when
menvcfg.STCE=0 (henvcfg.STCE=0), which is a misbehaving.
@YenHaoChen YenHaoChen changed the title sstc: STIP and VSTIP of mip reverts behavior if xenvcfg.STCE=0 Teach Sstc to respect xenvcfg.STCE Feb 8, 2024
@aswaterman aswaterman merged commit 3a53c80 into riscv-software-src:master Feb 8, 2024
3 checks passed
@YenHaoChen YenHaoChen deleted the pr-sstc-stce branch March 28, 2024 14:04
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