Add Smstateen/Ssstateen Extension and CSRs #592

neverlandiz · 2025-04-04T02:15:02Z

This PR addresses Issue #547 and adds the Smstateen extension, along with the following Smstateen/Ssstateen CSRs

mstateen0, mstateen1, mstateen2, mstateen3
mstateen0h, mstateen1h, mstateen2h, mstateen3h
hstateen0, hstateen1, hstateen2, hstateen3
hstateen0h, hstateen1h, hstateen2h, hstateen3h
sstateen0, sstateen1, sstateen2, sstateen3

arch/csr/hstateen0.yaml

ThinkOpenly

My comments in hstateen0/hstateen0h generally apply to the others as well.

arch/csr/hstateen0.yaml

arch/csr/hstateen0h.yaml

arch/csr/sstateen0.yaml

arch/ext/Smstateen.yaml

Signed-off-by: Katherine Hsu <[email protected]>

dhower-qc

The comments apply to all the CSRs.

.pre-commit-config.yaml

arch/csr/hstateen0.yaml

.pre-commit-config.yaml

arch/csr/hstateen0.yaml

arch/csr/hstateen0h.yaml

dhower-qc

We also need an arch/ext/Ssstateen.yaml file.

neverlandiz · 2025-04-30T23:05:14Z

I think there is already a Ssstateen.yaml file?

dhower-qc · 2025-04-30T23:37:53Z

I think there is already a Ssstateen.yaml file?

You're right. Ignore my ignorance please ;)

arch/csr/mstateen0.yaml

jordancarlin · 2025-05-01T22:32:43Z

arch/csr/hstateen0.yaml

+      The SE0 bit in `hstateen0` controls access to the `sstateen0` CSR.
+    type: RW
+    reset_value: UNDEFINED_LEGAL
+  ENVCFG:


The {h/s}stateen bits are 0 if the corresponding mstateen bit is 0. It doesn't look like that is captured here right now. I assume this needs a sw_read() call to represent this?

Yep, that's right.

@jordancarlin Thanks for pointing that out! Could you check if this implementation is correct (for hstateen0)?

sw_read(): | # for every bit in an mstateen CSR that is zero, the same bit # appears as read-only zero in the matching hstateen CSR Bit<64> mstateen0_mask = CSR[mstateen0].csr_value; Bit<64> hstateen0_value = csr_value & mstateen0_mask; return hstateen0_value;

I think that seems reasonable. For sstateen it will probably be a little more complicated since it changes depending on if the H extension is enabled.

Do we need a sw_write() here, too, that enforces read-only for corresponding bits that are zero in mstateen0?

Would it make sense to use type() instead of sw_write() to set the specific field to RO whenever the corresponding field is zero? For example, when CSR[mstateen0].JVT is 0, then CSR[sstateen0].JVT would have type RO.

I understand type() to be an expression that must be knowable at compile time. Since JVT can vary during runtime, I believe it needs to be a dynamic check with each write operation.

I thought to look at the definition of that attribute in the schema to see if what I was saying was (1) accurate, and (2) documented, and nicely it is:

"type()": { "type": "string", "description": "Function that returns a configuration-dependent type. The return value should be a CsrFieldType enum, and must be compile-time-known." },

I see, thanks for the clarification!

neverlandiz · 2025-05-05T17:46:18Z

Regression tests are failing due to missing Sscsrind (depends on Issue #557) and Zfinx extensions.

ThinkOpenly · 2025-05-05T20:47:15Z

Regression tests are failing due to missing Sscsrind

Sscsrind is going in as we speak.

and Zfinx extensions.

I don't even see an issue open for that. :-/

ThinkOpenly · 2025-05-05T21:06:58Z

There are still a couple of unresolved comments about changing a reset_value and adding a sw_read().

As for Zfinx, I'd be tempted to comment it out in the PR, since Zfinx isn't even started, then open an issue for Zfinx to be added, including a note to uncomment the content here.

neverlandiz · 2025-05-05T21:22:17Z

Ok, sounds good. Should I open an issue for Zfinx?

I'm still working on resolving the remaining comments, but I'll probably finish them by the end of today.

jordancarlin · 2025-05-05T21:30:21Z

#711 adds the Zfinx (and other Z*inx) extensions.

ThinkOpenly · 2025-05-05T21:35:05Z

#711 adds the Zfinx (and other Z*inx) extensions.

Ah, so it does. I searched issues and PRs, but unsurprisingly didn't search for Z.*f.*inx.

Still, that PR and this one depend on each other. I suggest proceeding as planned... comment out Zfinx here, and move this one along.

@neverlandiz , if you would, please add a comment there requesting that your commented out content be uncommented via that PR.

neverlandiz · 2025-05-05T21:51:39Z

Sounds good, thanks!

ThinkOpenly · 2025-05-08T18:13:34Z

arch/csr/hstateen0.yaml

+  # for every bit in an mstateen CSR that is zero, the same bit
+  # appears as read-only zero in the matching hstateen CSR
+
+  Bits<64> mstateen0_mask = CSR[mstateen0].csr_value;


The CI failures are complaining about this. Looking at other files, when the entire CSR value is accessed, it seems you should use:

$bits(CSR[mstateen0])

here and elsewhere

dhower-qc · 2025-05-08T17:36:25Z

arch/csr/hstateen0.yaml

@@ -160,6 +160,6 @@ sw_read(): |
  # for every bit in an mstateen CSR that is zero, the same bit
  # appears as read-only zero in the matching hstateen CSR

-  Bits<64> mstateen0_mask = CSR[mstateen0];
+  Bits<64> mstateen0_mask = CSR[mstateen0].csr_value;


To turn an entire CSR into a Bits type, you want to cast:

Bits<64> mstateen0_mask = $bits(CSR[mstateen0]);

dhower-qc · 2025-05-08T19:16:20Z

arch/csr/sstateen0.yaml

+    sw_write(csr_value): |
+      if (CSR[mstateen0].JVT == 1'b0){
+        return 0;
+      }
+      else if (mode() == PrivilegeMode::VS && CSR[hstateen0].JVT == 1'b0) {
+        return 0;
+      }


This brings up an interesting question. What happens if you write '1' to sstateen0.JVT while mstateen0.JVT is 0, and then change mstateen0.JVT to 1? Should sstateen0.JVT read as 0, 1, or undefined? I don't see any clarity on this in the spec. The answer will dictate whether this gets handled via sw_write() or sw_read().

While we figure that out...

All the stateen bits are WARL, and can thus be configured to be RW, read-only-0, or read-only-1. This will require a parameter for every bit in every mode. For example, for JVT:

arch/ext/Zcmt.yaml

parameters: MSTATEEN_JVT_TYPE: when: Smstateen schema: enum: [rw, read-only-0, read-only-1] description: | Behavior of the mstateen0.JVT bit: * 'rw': read-write * 'read-only-0': read-only, fixed to 0 * 'read-only-1': read-only, fixed to 1 HSTATEEN_JVT_TYPE: when: allOf: - H - Ssstateen schema: enum: [rw, read-only-0, read-only-1] description: | Behavior of the hstateen0.JVT bit: * 'rw': read-write * 'read-only-0': read-only, fixed to 0 * 'read-only-1': read-only, fixed to 1 extra_validation: | assert HSTATEEN_JVT_TYPE == 'read-only-0' if MSTATEEN_JVT_TYPE == 'read-only-0' assert HSTATEEN_JVT_TYPE == 'read-only-1' if MSTATEEN_JVT_TYPE == 'read-only-1' SSTATEEN_JVT_TYPE: when: allOf: - S - Sstateen schema: enum: [rw, read-only-0, read-only-1] description: | Behavior of the sstateen0.JVT bit: * 'rw': read-write * 'read-only-0': read-only, fixed to 0 * 'read-only-1': read-only, fixed to 1 extra_validation: | assert SSTATEEN_JVT_TYPE == 'read-only-0' if MSTATEEN_JVT_TYPE == 'read-only-0' assert SSTATEEN_JVT_TYPE == 'read-only-0' if HSTATEEN_JVT_TYPE == 'read-only-0' assert SSTATEEN_JVT_TYPE == 'read-only-1' if MSTATEEN_JVT_TYPE == 'read-only-1' assert SSTATEEN_JVT_TYPE == 'read-only-1' if HSTATEEN_JVT_TYPE == 'read-only-1'

Thanks for the feedback! Could you explain the purpose of this line when: Smstateen? When I tried adding it, it resulted in a schema error because Smstateen is not a when_condition. I looked at other extension yaml files and noticed that some use also_defined_in, but I'm not sure if it applies in this case.

My bad. After checking the schema, it's looking for an extension requirement (e.g., "~> 1.0"), which is assumed to be a requirement of the extension being defined (Zcmt here). That's not descriptive enough for us in this case, so we'll need a schema change. I will get that in ASAP and point you to it.

#751 adds the support. You can say:

when: name: Smstateen version: ~> 1.0

Ok, thanks!

Will this schema also be added?

when: allOf: - S - Sstateen

ThinkOpenly · 2025-05-13T15:35:31Z

There was a newer version of The RISC-V Instruction Set Manual: Volume II: Privileged Archtecture last week (20250508). Sorry to do this to you, but could you verify the current content in this PR is up-to-date? @syedowaisalishah mentioned in #653 that, for example, mstateen0 now has more defined bits (SRMCFG and CTR).

neverlandiz · 2025-05-13T20:07:14Z

Ok, sounds good!

add smstateen/ssstateen CSRs

6a3c81c

neverlandiz requested a review from dhower-qc as a code owner April 4, 2025 02:15

AFOliveira requested changes Apr 16, 2025

View reviewed changes

arch/csr/hstateen0.yaml Show resolved Hide resolved

Merge branch 'main' into add-smstateen-sstateen

be264c6

ThinkOpenly requested changes Apr 23, 2025

View reviewed changes

neverlandiz added 4 commits April 24, 2025 01:23

lots of minor fixes

f981030

merging

4348f66

merge

f147046

lots of backticks

601efbe

ThinkOpenly reviewed Apr 26, 2025

View reviewed changes

arch/csr/sstateen0.yaml Outdated Show resolved Hide resolved

ThinkOpenly reviewed Apr 26, 2025

View reviewed changes

arch/ext/Smstateen.yaml Outdated Show resolved Hide resolved

neverlandiz and others added 3 commits April 28, 2025 20:27

minor fixes

a5108d2

modified descriptions

8bb0d30

Merge branch 'main' into add-smstateen-sstateen

0807a91

Signed-off-by: Katherine Hsu <[email protected]>

dhower-qc requested changes Apr 29, 2025

View reviewed changes

dhower-qc mentioned this pull request Apr 30, 2025

feat: add Zfinx, Zdinx, Zhinxmin extensions, modify check_f_ok for them #711

Draft

dhower-qc reviewed Apr 30, 2025

View reviewed changes

ThinkOpenly mentioned this pull request May 1, 2025

Add CSR Definitions for Trigger Module Registers #524

Open

neverlandiz and others added 3 commits May 1, 2025 22:15

remove pre-commit-config.yaml from pr

e5fa958

remove pre-commit file changes

fffabda

Merge branch 'main' into add-smstateen-sstateen

daa12b4

jordancarlin reviewed May 1, 2025

View reviewed changes

neverlandiz added 2 commits May 1, 2025 22:52

add definedBy for fields

354ed45

add sw_write and sw_read for alias

9484789

syedowaisalishah mentioned this pull request May 5, 2025

Add ssqosid extension and csr yaml #653

Open

neverlandiz added 3 commits May 5, 2025 16:39

add long names

493e97c

structured format descriptions

843c1c6

finish adding structured format

a3f75bb

change mstateen* reset values to 0

f4c73f3

neverlandiz and others added 5 commits May 6, 2025 15:48

docs(stateen): comment out Zfinx & Zdinx code + add sw_read

de434ae

docs(stateen): change C field description

d074a3e

Merge branch 'main' into add-smstateen-sstateen

4321a85

docs(stateen): fix syntax errors

efc3bff

docs(stateen): syntax

659aa3a

dhower-qc mentioned this pull request May 6, 2025

Additional fixes for Xqci v0.11.0 #729

Merged

neverlandiz and others added 3 commits May 8, 2025 17:22

docs(stateen): add sw_write for sstateen CSRs

405103b

docs(stateen): add sw_write for hstateen* CSRs

240b895

Merge branch 'main' into add-smstateen-sstateen

59a027a

ThinkOpenly reviewed May 8, 2025

View reviewed changes

neverlandiz added 5 commits May 8, 2025 18:19

docs(stateen): syntax fixes

838355d

docs(stateen): syntax fixes

eb9b003

docs(stateen): more fixes

5d4e1be

docs(stateen): syntax fix

3bc9359

fix

a439395

dhower-qc requested changes May 8, 2025

View reviewed changes

docs(stateen): add params to zcmt

7dae65a

docs(stateen): update fields based on newest specs

a0d910c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Smstateen/Ssstateen Extension and CSRs #592

Add Smstateen/Ssstateen Extension and CSRs #592

neverlandiz commented Apr 4, 2025

ThinkOpenly left a comment

dhower-qc left a comment

dhower-qc left a comment

neverlandiz commented Apr 30, 2025

dhower-qc commented Apr 30, 2025

jordancarlin May 1, 2025

dhower-qc May 5, 2025

neverlandiz May 5, 2025 •

edited

Loading

jordancarlin May 6, 2025

neverlandiz May 6, 2025

ThinkOpenly May 6, 2025

neverlandiz May 8, 2025

ThinkOpenly May 8, 2025

ThinkOpenly May 8, 2025

neverlandiz May 8, 2025

neverlandiz commented May 5, 2025 •

edited

Loading

ThinkOpenly commented May 5, 2025

ThinkOpenly commented May 5, 2025

neverlandiz commented May 5, 2025

jordancarlin commented May 5, 2025

ThinkOpenly commented May 5, 2025

neverlandiz commented May 5, 2025

ThinkOpenly May 8, 2025

dhower-qc May 8, 2025

dhower-qc May 8, 2025

neverlandiz May 9, 2025

dhower-qc May 9, 2025

dhower-qc May 9, 2025

neverlandiz May 9, 2025

neverlandiz May 9, 2025 •

edited

Loading

ThinkOpenly commented May 13, 2025

neverlandiz commented May 13, 2025

Add Smstateen/Ssstateen Extension and CSRs #592

Are you sure you want to change the base?

Add Smstateen/Ssstateen Extension and CSRs #592

Conversation

neverlandiz commented Apr 4, 2025

ThinkOpenly left a comment

Choose a reason for hiding this comment

dhower-qc left a comment

Choose a reason for hiding this comment

dhower-qc left a comment

Choose a reason for hiding this comment

neverlandiz commented Apr 30, 2025

dhower-qc commented Apr 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverlandiz May 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverlandiz commented May 5, 2025 • edited Loading

ThinkOpenly commented May 5, 2025

ThinkOpenly commented May 5, 2025

neverlandiz commented May 5, 2025

jordancarlin commented May 5, 2025

ThinkOpenly commented May 5, 2025

neverlandiz commented May 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neverlandiz May 9, 2025 • edited Loading

Choose a reason for hiding this comment

ThinkOpenly commented May 13, 2025

neverlandiz commented May 13, 2025

neverlandiz May 5, 2025 •

edited

Loading

neverlandiz commented May 5, 2025 •

edited

Loading

neverlandiz May 9, 2025 •

edited

Loading