fix(csr): csr reserved fields by amkr6207 · Pull Request #1565 · riscv/riscv-unified-db

amkr6207 · 2026-02-07T05:26:21Z

This PR addresses Issue #1186 by explicitly defining all reserved/WPRI fields in mnstatus.yaml as per the RISC-V Smrnmi specification.

Changes:

Explicitly defined Reserved_High (Bits 63-13).
Added MNPELP (Bit 9) from Zicfilp extension.
Added Reserved fields for all gaps (Bits 0-2, 4-6, 8, 10).
Fixed Reserved field locations to use valid rv32/rv64 ranges.

Note: This is a pilot implementation to establish the correct pattern. Once approved, I will apply this pattern to the remaining CSR files in subsequent PRs.
Closes #1186

Explicitly define reserved/WPRI bits in mnstatus.yaml to remove architectural assumptions in the database. Also update .gitignore to exclude docker artifacts. Closes riscv#1186

AFOliveira

Thanks for the contribution! This is new to UDB and may need some discussion. but if you're willing to wait and see, I think this is already along the lines of what to insert, I'm just not sure about the how. @ThinkOpenly and @dhower-qc have worked with CSRs inside UDB way more than me, so let's see what they think.

AFOliveira · 2026-02-08T11:29:29Z

spec/std/isa/csr/Smrnmi/mnstatus.yaml

  extension:
    name: Smrnmi
 fields:
+  Reserved_High:


I'm not super sure about this name for the reserved, locations... @ThinkOpenly @dhower-qc, any thoughts?

Agreed, but I don't have anything that would make it better. Other than maybe having the names with numbers match the order of "location", e.g., Reserved_4_6 --> Reserved_6_4

Okay . I'll rename it to match the location order ( Reserved_4_6 --> Reserved_6_4 )

spec/std/isa/csr/Smrnmi/mnstatus.yaml

AFOliveira · 2026-02-08T11:29:59Z

spec/std/isa/csr/Smrnmi/mnstatus.yaml

      extension:
        name: H
+
+  Reserved_4_6:


AFOliveira · 2026-02-08T11:30:03Z

spec/std/isa/csr/Smrnmi/mnstatus.yaml

    type: RW-H
    reset_value: 0
+
+  Reserved_0_2:


.gitignore

This reverts commit ff1ae5f.

This reverts commit 54514b8.

This reverts commit 97c15cc.

…97c15cc = ignore docker artifacts

dhower-qc

The big-picture question on this is whether or not it's needed. What does explicitly naming the reserved fields add beyond just assuming that any unused bits are unnamed WPRI, as is the case now?

AFOliveira · 2026-02-09T18:32:45Z

The big-picture question on this is whether or not it's needed. What does explicitly naming the reserved fields add beyond just assuming that any unused bits are unnamed WPRI, as is the case now?

If I can recall correctly, the question is that reserved and WPRI mean differnt things.

dhower-qc · 2026-02-10T16:06:42Z

I thought we determined they were the same. Do you have the differing definitions?

AFOliveira · 2026-02-10T16:53:52Z

I thought we determined they were the same. Do you have the differing definitions?

Going down the issue rabbit-hole, I don't think that's true. i.e.

In general the definition of "Reserved" is as specified in the WPRI definition. So yes, in general, Reserved fields MUST be treated as defined.

BUT in specific cases that define a different behavior for a specific field, then that specific field MUST be implemented per that specific definition. In such cases one cannot instead implement the behavior in the WPRI definition since that would be incompatible with what is specified for the field.

Originally posted by @gfavor in #2318

dhower-qc · 2026-02-11T18:48:51Z

Talked about this in the PR meeting -- general direction is that we should treat missing bits as WPRI, and only document the reserved fields that are not WPRI. Much less work!

AFOliveira · 2026-02-11T19:54:06Z

Talked about this in the PR meeting -- general direction is that we should treat missing bits as WPRI, and only document the reserved fields that are not WPRI. Much less work!

That's what this PR does, right?

spec/std/isa/csr/Smrnmi/mnstatus.yaml

ThinkOpenly · 2026-02-12T15:31:50Z

Talked about this in the PR meeting -- general direction is that we should treat missing bits as WPRI, and only document the reserved fields that are not WPRI. Much less work!

That's what this PR does, right?

The guidance in the spec is:

The other bits in mnstatus are reserved; software should write zeros and hardware implementations should return zeros.

I'm not sure I like that guidance, because it breaks forward compatibility. If I always write zeros to those bits, how can they ever be used in the future? Maybe we need to change the spec there. I think it should really be "the other bits in mnstatus are WPRI.`

AFOliveira · 2026-02-12T15:53:01Z

I'm not sure I like that guidance, because it breaks forward compatibility. If I always write zeros to those bits, how can they ever be used in the future? Maybe we need to change the spec there. I think it should really be "the other bits in mnstatus are WPRI.`

Well, I wasn't making judgments on the spec here. I understand your concern, but the spec is ratified as is, so changing that shouldn't be possible without modifying the extension number, right? So, @amkr6207 change should be in UDB as it agrees with current RISC-V specification

ThinkOpenly · 2026-02-12T15:54:50Z

FYI, I reopened riscv/riscv-isa-manual#2318, because the words in the spec seem wrong to me.

AFOliveira · 2026-02-12T15:59:02Z

FYI, I reopened riscv/riscv-isa-manual#2318, because the words in the spec seem wrong to me.

I don't think it's the words. You seem to be disagreeing with the behavior, which is fine, but as of right now, I think the spec mandates what Greg described and I'm afraid that it has been implemented in real HW, so it seems to late to change it?

ThinkOpenly · 2026-02-12T16:07:08Z

FYI, I reopened riscv/riscv-isa-manual#2318, because the words in the spec seem wrong to me.

I don't think it's the words. You seem to be disagreeing with the behavior, which is fine, but as of right now, I think the spec mandates what Greg described and I'm afraid that it has been implemented in real HW, so it seems to late to change it?

I understand your point. I've reopened the issue to get more clarity, for me at least. I will note that "should" != "must". So, that would make actual behavior implementation-dependent, and thus in need of a configuration parameter.

codecov · 2026-02-12T16:36:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.18%. Comparing base (d29399e) to head (7e271ae).
⚠️ Report is 51 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1565      +/-   ##
==========================================
+ Coverage   60.05%   69.18%   +9.13%     
==========================================
  Files          42       43       +1     
  Lines       26120    27059     +939     
  Branches     6246     6475     +229     
==========================================
+ Hits        15686    18721    +3035     
+ Misses      10434     8338    -2096

Flag	Coverage Δ
idlc	`72.92% <ø> (+13.94%)`	⬆️
udb	`62.16% <ø> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

amkr6207 · 2026-02-18T16:00:02Z

Thank you all for the detailed feedback!

Hi @AFOliveira, @ThinkOpenly

Reserved vs WPRI — I see there's an ongoing discussion about whether missing bits should be treated as WPRI by default (riscv/riscv-isa-manual#2318). Should I wait for that to be resolved before extending this to other CSRs, or should I proceed with the current approach?

ThinkOpenly · 2026-02-24T03:59:46Z

Should I wait for that to be resolved before extending this to other CSRs, or should I proceed with the current approach?

I'd wait.

amkr6207 added 2 commits February 7, 2026 10:45

chore: ignore docker and regression artifacts

ff1ae5f

fix(csr): add omitted reserved fields to mnstatus

97c15cc

Explicitly define reserved/WPRI bits in mnstatus.yaml to remove architectural assumptions in the database. Also update .gitignore to exclude docker artifacts. Closes riscv#1186

amkr6207 requested review from ThinkOpenly and dhower-qc as code owners February 7, 2026 05:26

AFOliveira requested changes Feb 8, 2026

View reviewed changes

amkr6207 added 4 commits February 8, 2026 21:59

Revert "chore: ignore docker and regression artifacts"

54514b8

This reverts commit ff1ae5f.

Revert "Revert "chore: ignore docker and regression artifacts""

061b0a0

This reverts commit 54514b8.

Revert "fix(csr): add omitted reserved fields to mnstatus"

a5eb248

This reverts commit 97c15cc.

clarification: swapped commit messages; ff1ae5f = add reserved fields, …

7e271ae

…97c15cc = ignore docker artifacts

amkr6207 changed the title ~~Fix/1186 csr reserved fields~~ fix(csr): CSR reserved fields Feb 9, 2026

amkr6207 changed the title ~~fix(csr): CSR reserved fields~~ fix(csr): csr reserved fields Feb 9, 2026

dhower-qc reviewed Feb 9, 2026

View reviewed changes

ThinkOpenly requested changes Feb 12, 2026

View reviewed changes

spec/std/isa/csr/Smrnmi/mnstatus.yaml Show resolved Hide resolved

amkr6207 mentioned this pull request Feb 25, 2026

Add MNPELP field to mnstatus (Zicfilp extension) #1686

Closed

Conversation

amkr6207 commented Feb 7, 2026

Uh oh!

AFOliveira left a comment

Choose a reason for hiding this comment

Uh oh!

AFOliveira Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

dhower-qc Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

amkr6207 Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AFOliveira Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

AFOliveira Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dhower-qc left a comment

Choose a reason for hiding this comment

Uh oh!

AFOliveira commented Feb 9, 2026

Uh oh!

dhower-qc commented Feb 10, 2026

Uh oh!

AFOliveira commented Feb 10, 2026

Uh oh!

dhower-qc commented Feb 11, 2026

Uh oh!

AFOliveira commented Feb 11, 2026

Uh oh!

Uh oh!

ThinkOpenly commented Feb 12, 2026

Uh oh!

AFOliveira commented Feb 12, 2026

Uh oh!

ThinkOpenly commented Feb 12, 2026

Uh oh!

AFOliveira commented Feb 12, 2026

Uh oh!

ThinkOpenly commented Feb 12, 2026

Uh oh!

codecov bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

amkr6207 commented Feb 18, 2026

Uh oh!

ThinkOpenly commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Feb 12, 2026 •

edited

Loading