-
Notifications
You must be signed in to change notification settings - Fork 78
feat: add sie and scountinhibit CSRs #958
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
feat: add sie and scountinhibit CSRs #958
Conversation
spec/std/isa/csr/scountinhibit.yaml
Outdated
| [when="COUNTINHIBIT_EN[3] == true"] | ||
| When bit is set `hpmcounter3.COUNT` does not increment; when 0, it increments normally. | ||
|
|
||
| [when="COUNTINHIBIT_EN[3] == false"] | ||
| Since hpmcounter3 is not implemented, this field is read-only zero. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These [when ..] tags are deprecated. Instead, use:
description:
- id: csr-field-hpmcounter3.COUNT-set-effect
normative: true
when:
param:
name: COUNTINHIBIT_EN
index: 3
value: true
text: |
When bit is set `hpmcounter3.COUNT` does not increment; when 0, `hpmcounter3.COUNT` increments normally.and similar for the false case
(this depends on #891)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, the schema doesn’t support this, but I went ahead and used it anyway. Please let me know if any improvements are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to rebase your branch on #891 for the CI to pass when we get to that point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to define a field(s) for the "other" interrupt enable bits, that are for "platform use"? PR #910 depends on that, since it's defining CSR sieh, the high-order bits of sie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh I'm not sure, but since spec doesn’t define any standard bits in sieh and their semantics are platform-specific, I believe there's no need to define explicit fields. Instead, we could just model it as a single value field, with a note that these bits are reserved for platform use:
fields:
VALUE:
location_rv32: 31-0
description: |
The base spec doesn’t define any standard bits in sieh, those upper interrupt enables are reserved for platform use.
Let me know if I’m overlooking something!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One complication is that sieh needs to serve as an alias for the high-order bits of sie, although it looks like there is a precedent:
spec/std/isa/csr/Zihpm/hpmcounter26h.yaml: alias: mhpmcounter26h.COUNT[63:32]
So, you can move forward with that approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe, putting alias: sie[63:32] in sieh.yaml file will be a right thing to do? Let me know if I'm overlooking or something needs to be added in sie.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see sieh.yaml in the PR. Were you planning on adding that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already in #910. Let me know if I'm supposed to add it in this PR, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfectly fine for me :)
ThinkOpenly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that to realize the layout results (to generate scountinhibit.yaml), you'll need to add a new stanza in /Rakefile. There are examples there already, but ask if you need some help.
Thanks for guidance! I've added that as well, please have a look and let me know if something is wrong, thank! |
ThinkOpenly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're getting close enough to revisit indentation and minor details. :-)
spec/std/isa/csr/scountinhibit.yaml
Outdated
| [when="COUNTINHIBIT_EN[3] == true"] | ||
| When bit is set `hpmcounter3.COUNT` does not increment; when 0, it increments normally. | ||
|
|
||
| [when="COUNTINHIBIT_EN[3] == false"] | ||
| Since hpmcounter3 is not implemented, this field is read-only zero. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to rebase your branch on #891 for the CI to pass when we get to that point.
208fd14 to
2691617
Compare
2691617 to
208fd14
Compare
This is this initial PR; after this merges with main, we should start seeing coverage status checks on subsequent PRs. Initially, this has the following thresholds: - No more than a 1% drop in coverage overall. - At least 90% coverage on any new code in a patch
Should fix the deployment
In (riscv-software-src#686), a new instruction schema was created for types/subtypes. This commit change B/andn instruction to use the new schema. It is also part of effort in (riscv-software-src#655). --------- Signed-off-by: Usman Akinyemi <[email protected]>
…ftware-src#856) PR riscv-software-src#797 refactored a number of things, including encapulating the "Architecture" class in a "Udb" module. `isa_explorer.rb` has a few places where that needs to be accommondated which were missed in riscv-software-src#855.
…src#1004) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.18.8 to 1.18.9. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/releases">nokogiri's releases</a>.</em></p> <blockquote> <h2>v1.18.9 / 2025-07-20</h2> <h3>Security</h3> <ul> <li>[CRuby] Applied upstream libxml2 patches to address CVE-2025-6021, CVE-2025-6170, CVE-2025-49794, CVE-2025-49795, and CVE-2025-49796. See <a href="https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-353f-x4gh-cqq8">GHSA-353f-x4gh-cqq8</a> for more information.</li> </ul> <!-- raw HTML omitted --> <pre><code>5bcfdf7aa8d1056a7ad5e52e1adffc64ef53d12d0724fbc6f458a3af1a4b9e32 nokogiri-1.18.9-aarch64-linux-gnu.gem 55e9e6ca46c4ad1715e313f407d8481d15be1e3b65d9f8e52ba1c124d01676a7 nokogiri-1.18.9-aarch64-linux-musl.gem eea3f1f06463ff6309d3ff5b88033c4948d0da1ab3cc0a3a24f63c4d4a763979 nokogiri-1.18.9-arm64-darwin.gem fe611ae65880e445a9c0f650d52327db239f3488626df4173c05beafd161d46e nokogiri-1.18.9-arm-linux-gnu.gem 935605e14c0ba17da18d203922440bf6c0676c602659278d855d4622d756a324 nokogiri-1.18.9-arm-linux-musl.gem ac5a7d93fd0e3cef388800b037407890882413feccca79eb0272a2715a82fa33 nokogiri-1.18.9.gem 1fe5b7aa4a054eda689a969bb4e03999960a6ea806582d327207d687168bceb5 nokogiri-1.18.9-java.gem 6b4fc1523aa0370c78653e38c94cb50e7f3ab786425de66ba7ad24222c1164a3 nokogiri-1.18.9-x64-mingw-ucrt.gem e0d2deb03d3d7af8016e8c9df5ff4a7d692159cefb135cbb6a4109f265652348 nokogiri-1.18.9-x86_64-darwin.gem b52f5defedc53d14f71eeaaf990da66b077e1918a2e13088b6a96d0230f44360 nokogiri-1.18.9-x86_64-linux-gnu.gem e69359d6240c17e64cc9f43970d54f13bfc7b8cc516b819228f687e953425e69 nokogiri-1.18.9-x86_64-linux-musl.gem </code></pre> <!-- raw HTML omitted --> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md">nokogiri's changelog</a>.</em></p> <blockquote> <h2>v1.18.9 / 2025-07-20</h2> <h3>Security</h3> <ul> <li>[CRuby] Applied upstream libxml2 patches to address CVE-2025-6021, CVE-2025-6170, CVE-2025-49794, CVE-2025-49795, and CVE-2025-49796. See <a href="https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-353f-x4gh-cqq8">GHSA-353f-x4gh-cqq8</a> for more information.</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/sparklemotion/nokogiri/commit/1dcd8ce30365ebd4620a3b823bf806b127eeefc3"><code>1dcd8ce</code></a> version bump to v1.18.9</li> <li><a href="https://github.com/sparklemotion/nokogiri/commit/a05d2b44b930072af70dad12bddbac67f36c6f90"><code>a05d2b4</code></a> Apply upstream patches to address multiple vulnerabilities (<a href="https://redirect.github.com/sparklemotion/nokogiri/issues/3526">#3526</a>)</li> <li><a href="https://github.com/sparklemotion/nokogiri/commit/947a55e87edff3c6d76ffd81f07da728e67c9b82"><code>947a55e</code></a> Apply upstream patches to address multiple vulnerabilities</li> <li>See full diff in <a href="https://github.com/sparklemotion/nokogiri/compare/v1.18.8...v1.18.9">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/riscv-software-src/riscv-unified-db/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Derek Hower <[email protected]>
…e-src#984) Fixes problem where backticks showed up in red in PDFs: <img width="156" height="23" alt="image" src="https://github.com/user-attachments/assets/5c54e238-5357-42dc-aa35-64bb57b9489a" /> now looks like: <img width="163" height="27" alt="image" src="https://github.com/user-attachments/assets/a92626de-6de1-4df1-bca1-e8f0ae3fef2f" /> --------- Signed-off-by: Derek Hower <[email protected]> Co-authored-by: Paul Clarke <[email protected]>
The mstatus.VS field was incorrectly written with the value of the FS field from a CSR write. Update it to use the VS field as expected. Signed-off-by: Jordan Carlin <[email protected]>
…-src#1005) Added YAML specification for Ssdbltrp extension version 1.0.0. Just definition part :) Related to issue riscv-software-src#212 Signed-off-by: Sukuna0007Abhi <[email protected]>
…re-src#1014) Makes it look like this (template stuff will be replaced in deploy flow): <img width="1385" height="805" alt="image" src="https://github.com/user-attachments/assets/4e5d973a-6b20-4c75-a352-63aa3fdd44de" />
…oftware-src#1027) The ADUE field type() logic was backwards in all environment configuration CSRs. Fixed logic: - When Svadu IS implemented ADUE field is Read-Write (software control) - When Svadu is NOT implemented ADUE field is Read-Only-0 (hardwired) Closes riscv-software-src#1026 Signed-off-by: Sukuna0007Abhi <[email protected]>
The ratified version of the Sstc extension is 1.0.0, not 0.9.0. Discovered while creating a UDB configuration for [CVW](github.com/openhwgroup/cvw). --------- Signed-off-by: Jordan Carlin <[email protected]>
…v-software-src#1017) Removes unnecessary "type" from schema definition when a "const" is declared. This prevents problem where code using the jsonschema makes the type mandatory like here: riscv-software-src#636 fixes riscv-software-src#1015 Signed-off-by: Kevin Broch <[email protected]>
) If ruby code uses the "type" to decide the type, then this turns this optional keyword in json schema into a mandatory one. Remove "type" from JSON schema when "enum" present. fixes riscv-software-src#1016 Signed-off-by: Kevin Broch <[email protected]>
…iscv-software-src#1028) - Change default behavior to always recreate .bundle/config - Users will now always get configuration updates - Add --preserve-config option for backward compatibility - Add --help option for usage information - Fix argument processing conflicts when script is sourced by other tools - Prioritizes reliability over performance concerns - Addresses overly cautious guard that prevented config updates - Uses sir @rpsene's argument parsing approach with improved defaults Fixes riscv-software-src#980 Ready for Review sir @ThinkOpenly sir @dhower-qc sir @christian-herber-nxp --------- Signed-off-by: Sukuna0007Abhi <[email protected]> Signed-off-by: GitHub <[email protected]>
208fd14 to
ffc4f01
Compare
Agree it looks messy. It's not quite what I expected. When looking at the commit history, I expected to see the commits from #891 and then your 7 or so commits on top, but they're scattered. It's up to you if you want to wait. I'm not aware of anyone actively waiting for these CSR definitions. #891 may take a while to land, though. |
I'm fine with waiting for #891 to land :) |
ffc4f01 to
d5de362
Compare
|
@ThinkOpenly, I messed up something I believe and going to close this PR, and will open a clean PR again for this shortly. |
Closes #563