Add Smstateen/Ssstateen extension regs #694

trdthg · 2025-01-22T07:55:01Z

Add some basic things, including

cli param
regs
- bitfield
- legalize_xxx
- csr_name_map
- is_CSR_defined
- read_CSR
- write_CSR

This PR does not include any actual functionality

I plan to split other parts like FCSR into other PRs

For convenience, when the stateen CSRs are implemented and misa.F = 0, then if the FCSR bit of a controlling stateen0 CSR is zero, all floating-point instructions cause an illegal instruction trap (or virtual instruction trap, if relevant),

Currently although many registers in this extension has no fields defined, they already have their own name, encoding, ... so I think we can put them here.

Another small thing is，sail doesn't support empty bitfield now，so we can't write bitfield Mstateen1 : xlenbits = {} and I replace it with type Mstateen2 = xlenbits. I've opened an issue about it: rems-project/sail#899

github-actions · 2025-01-22T08:21:02Z

Test Results

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

Results for commit e6ccff4. ± Comparison against base commit 2dfc4ff.

♻️ This comment has been updated with latest results.

jordancarlin

Haven't looked at the functionality of its interaction with the rest of the system yet, but here are a few initial comments.

model/riscv_extensions.sail

jordancarlin · 2025-01-23T02:24:44Z

model/riscv_stateen_regs.sail

+
+register sstateen1 : Sstateen1
+mapping clause csr_name_map = 0x10D  <-> "sstateen1"
+function clause is_CSR_defined(0x10D) = extensionEnabled(Ext_Ssstateen)


From the spec:

The Smstateen extension specification comprises the mstateen*, sstateen*, and hstateen* CSRs and their functionality. The Ssstateen extension specification comprises only the sstateen* and hstateen* CSRs and their functionality.

This seems to mean that Sstateen1 (and all of the other S and H CSRs defined here) should accessible if extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_ Ssstateen).

Alternatively, we could potentially define extensionEnabled(Ext_Ssstateen) = sys_enable_ssstateen() | extensionEnabled(Ext_Smstateen).

I didn't find the dependency between Sm and Ss in the spec, so I replaced them with

function enable_mstatenn() -> bool = extensionEnabled(Ext_Smstateen) function enable_sstatenn() -> bool = extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_Ssstateen) function enable_hstatenn() -> bool = extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_Ssstateen)

What do you think?

I don’t love introducing yet another level of indirection to enabling/disabling extensions, but it seems like an acceptable solution for this situation.

Add two additional constraints

// 'If supervisor mode is implemented' function enable_sstatenn() -> bool = extensionEnabled(Ext_S) & (extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_Ssstateen)) // 'And if the hypervisor extension is implemented' function enable_hstatenn() -> bool = false & (extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_Ssstateen))

jordancarlin · 2025-01-23T02:30:03Z

model/riscv_stateen_regs.sail

+function clause read_CSR(0x30F) = zero_extend(mstateen3)
+function clause write_CSR(0x30F, value) = { mstateen3 = legalize_mstateen3(mstateen3, value); zero_extend(mstateen3) }
+
+type Mstateen0h = xlenbits


This doesn't seem true? Mstateen0h should be the top 32 bits of the Mstateen register, just like all of the other h CSRs.

Mstateen should be defined as 64 bits and the write/read should always access that register, either the entire register (for rv64) or the appropriate half of the register (for rv32). This issue is present for all of the mstateen and hstateen registers. See how menvcfg and menvcfgh are implemented.

nadime15 · 2025-02-28T21:47:48Z

model/riscv_stateen_regs.sail

+function enable_hstatenn() -> bool = extensionEnabled(Ext_Smstateen) | extensionEnabled(Ext_Ssstateen)
+
+bitfield Mstateen0 : bits(64) = {
+    SEO     : 63,


Small detail but its SE0 (zero) and not SEO (letter).

thanks! fixed

nadime15

Additionally, on chip reset mstateen* CSR's are set to 0.

nadime15 · 2025-02-28T22:31:04Z

model/riscv_stateen_regs.sail

+bitfield Mstateen0 : bits(64) = {
+    SEO     : 63,
+    ENVCFG  : 62,
+    WRPI    : 61,


Seems like there is no reason to define this bit at all. We don't define any of the other WPRI bits since they should all be read-only zero.

nadime15 · 2025-03-03T04:50:37Z

model/riscv_stateen_regs.sail

+
+function legalize_sstateen0(s : Sstateen0, v : bits(32)) -> Sstateen0 = {
+  let v = Mk_Sstateen0(v);
+  if mstateen0[SE0] == 0b0 then {


What happens if mstateen is not defined because Smstateen is not enabled?

nadime15 · 2025-03-03T05:15:17Z

model/riscv_stateen_regs.sail

+      JVT  = if mstateen0[JVT]  == 0b0 then v[JVT] else 0b0,
+      FCSR = if mstateen0[FCSR] == 0b0 & misa[F] == 0b1 then v[FCSR] else 0b0,
+      C    = if mstateen0[C]    == 0b0 then v[C]   else 0b0,


I am not sure if the following is correct, so please double check what I say, but the specification says:

For every bit in an mstateen CSR that is zero (whether read-only zero or set to zero), the same bit
appears as read-only zero in the matching hstateen and sstateen CSRs.

So in your case, even if mstateen0[JVT] == 0b0 you still update s[JVT] with the new value from v.

You rather want:

JVT = if mstateen0[JVT] == 0b1 then v[JVT] else 0b0,

Same for reading, this must be checked beforehand as well, so something like lower_mstateen() (?) would probably make sense similar to how sstatus is handled for reads.

nadime15 · 2025-03-03T06:09:21Z

model/riscv_stateen_regs.sail

+function legalize_sstateen0(s : Sstateen0, v : bits(32)) -> Sstateen0 = {
+  let v = Mk_Sstateen0(v);
+  if mstateen0[SE0] == 0b0 then {
+    s


Do you really return s in this case? It seems to me that the spec isn't very clear here, but:

The SE0 bit in mstateen0 controls access to the hstateen0, hstateen0h, and the sstateen0 CSRs. The SE0
bit in hstateen0 controls access to the sstateen0 CSR.

I think you would throw some kind of exception (illegal instruction exception?)

trdthg · 2025-03-05T03:24:33Z

You are right, I will check them again with spike and qemu.

trdthg force-pushed the smstateen branch from 404e24d to b7f213f Compare January 22, 2025 08:07

trdthg force-pushed the smstateen branch 3 times, most recently from 7fa372b to fe215cc Compare January 23, 2025 01:39

jordancarlin added the extension Adds support for a RISC-V extension label Jan 23, 2025

trdthg force-pushed the smstateen branch from fe215cc to 6a3bd5c Compare January 23, 2025 02:18

jordancarlin requested changes Jan 23, 2025

View reviewed changes

trdthg force-pushed the smstateen branch 4 times, most recently from 0b935cb to dc437ee Compare January 23, 2025 06:07

nadime15 mentioned this pull request Feb 28, 2025

Add support for Zcmt extension #757

Open

2 tasks

nadime15 reviewed Feb 28, 2025

View reviewed changes

trdthg force-pushed the smstateen branch 3 times, most recently from 5debc31 to b294115 Compare March 3, 2025 02:31

Add stateen support

e6ccff4

trdthg force-pushed the smstateen branch from b294115 to e6ccff4 Compare March 3, 2025 02:31

nadime15 reviewed Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Smstateen/Ssstateen extension regs #694

Add Smstateen/Ssstateen extension regs #694

trdthg commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

jordancarlin left a comment

jordancarlin Jan 23, 2025 •

edited

Loading

trdthg Jan 23, 2025 •

edited

Loading

jordancarlin Jan 23, 2025

trdthg Mar 3, 2025

jordancarlin Jan 23, 2025

nadime15 Feb 28, 2025 •

edited

Loading

trdthg Mar 3, 2025

nadime15 left a comment

nadime15 Feb 28, 2025

jordancarlin Mar 1, 2025

trdthg Mar 3, 2025

nadime15 Mar 3, 2025

nadime15 Mar 3, 2025

nadime15 Mar 3, 2025

trdthg commented Mar 5, 2025

Add Smstateen/Ssstateen extension regs #694

Are you sure you want to change the base?

Add Smstateen/Ssstateen extension regs #694

Conversation

trdthg commented Jan 22, 2025 • edited Loading

github-actions bot commented Jan 22, 2025 • edited Loading

Test Results

jordancarlin left a comment

Choose a reason for hiding this comment

jordancarlin Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

trdthg Jan 23, 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

nadime15 Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadime15 left a 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

trdthg commented Mar 5, 2025

trdthg commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

jordancarlin Jan 23, 2025 •

edited

Loading

trdthg Jan 23, 2025 •

edited

Loading

nadime15 Feb 28, 2025 •

edited

Loading