add Zicclsm extension #1468

challenger1024 · 2026-01-06T17:39:19Z

Zicclsm Misaligned loads and stores to main memory regions with both the cacheability and coherence PMAs must be supported.
as specified in RVA23.

nadime15 · 2026-01-06T18:09:32Z

model/sys/vmem_utils.sail


 function check_misaligned(vaddr : virtaddr, width : word_width) -> bool = {
-  if plat_enable_misaligned_access then {
+  if plat_enable_misaligned_access | hartSupports(Ext_Zicclsm) then {


I’m afraid it’s not that simple. As you mentioned in your PR message, this applies only to RAM that is coherent and cacheable, so I assume you need to check whether the current RAM meets those criteria (see the config file with memory.region.ram), right?

I think Zicclsm itself should not change any behavior. It should behave more like Zic64b and just enforce additional configuration checks. In this case, any memory region that has both coherent and cacheable set to true must also have misaligned_fault set to NoFault.

Ah yeah even better! So this would mean @challenger1024 you just have to check for coherent and cacheble in check_extension_param_constraints() as part of validate_config.sail.

@jordancarlin Just to clarify we talk only about RAM not other memory regions (ROM, MMIO, etc.)

@nadime15 The PMA config options don't have the concept of "RAM".

The Zicclsm spec specifies "main memory regions with both the cacheability and coherence PMAs" and the PMA section of the priv spec says that main memory regions support read and write of all access widths, are assumed to be idempotent, and have constraints on the memory ordering PMA.

We don't support the memory ordering PMA yet, so I think for Zicclsm we want to ensure that regions with all of the following properties have misaligned_fault set to NoFault:

cacheable

coherent

readable

writable

read_idempotent

write_idempotent

Ahh ok that makes sense now, I was confused by the comments in our config file 😄 So basically a RAM region is implicitly defined by having all those attributes set to true. And the Zicclsm validation just checks, if the extension is enabled, any region matching those attributes must have misaligned_fault == NoFault.

I think it might be simpler for this PR to just do the validation as @jordancarlin suggests. Later, we could repurpose the plat_enable_misaligned_access (and suitably rename it) to configure privileged emulation and relax the validation check. With this PR, plat_enable_misaligned_access will be redundant as it will mean essentially the same thing as Zicclsm. Or, in this PR, we could just remove plat_enable_misaligned_access, and then add the parameter @nadime15 suggests in a later PR.

Interesting development a few days ago related to this: Spike just removed its --misaligned flag and misaligned accesses are now enabled by including Zicclsm in the isa string passed with the --isa flag. See riscv-software-src/riscv-isa-sim#2193.

I agree with @jordancarlin - at least that's what I implemented at Codasip for our check for this requirement.

I always found the description of "main memory" very vague in the ISA manual. Now that I re-read it I think technically our PMAs should have a flag to indicate whether they are "main memory" or "I/O memory", since you can't actually infer it from the other PMAs. You can have an I/O memory region that supports all the same requirements as main memory but you just are declaring that it's actually I/O memory.

However, I think in practice anything that supports read and write and is idempotent (the most important requirements of "main memory") is going to be main memory.

So I would do something like this:

enum MemoryRegionType { MainMemory, IOMemory, } // Technically you can have a region that satisfies these properties // but is still declared as IOMemory but we don't support that yet. function memory_region_type(pma : PMA) -> MemoryRegionType = if pma.readable & pma.writable & pma.read_idempotent & pma.write_idempotent then MainMemory else IOMemory private function check_pma_regions_zicclsm(pmas : list(PMA_Region)) -> bool = match pmas { [||] => true, pma :: rest => { let attr = pma.attributes; if memory_region_type(attr) == MainMemory & attr.cachable & attr.coherent & attr.misaligned_fault != NoFault then { print_endline("nice error message goes here"); return false; }; check_pma_regions_zicclsm(rest) }, } // In validate_config.sail function check_zicclsm() -> bool = { var valid : bool = true; if hartSupports(Ext_Zicclsm) then { if not(plat_enable_misaligned_access) then { valid = false; print_endline("Zicclsm is enabled but misaligned accesses are globally disabled."); } else not(check_pma_regions_zicclsm(pma_regions)) then { valid = false; }; }; valid }

And as @jordancarlin said this shouldn't change any behaviour; it should only be for validation.

Just a random thought but ROM is now classified as IOMemory here but ROM isnt really IOMemory. Maybe the enum should be MainMemory vs NotMainMemory (the debug module adds another region for the program buffer), or we add a third enum entry?

I always found the description of "main memory" very vague in the ISA manual. Now that I re-read it I think technically our PMAs should have a flag to indicate whether they are "main memory" or "I/O memory", since you can't actually infer it from the other PMAs. You can have an I/O memory region that supports all the same requirements as main memory but you just are declaring that it's actually I/O memory.

@Timmmm I think the memory ordering PMA unambiguously determines if a region is main memory or IO:

Regions of the address space are classified as either main memory or I/O for the purposes of ordering by the FENCE instruction and atomic-instruction ordering bits.

@nadime15 I think it is correct to classify ROM as IOMemory according to the spec. It specifies that there are only two types of memory:

Regular main memory is required to have a number of properties, specified below, whereas I/O devices can have a much broader range of attributes. Memory regions that do not fit into regular main memory, for example, device scratchpad RAMs, are categorized as I/O regions.

Main memory is required to be writable, so a ROM must be IO memory.

jordancarlin · 2026-01-06T18:00:57Z

model/core/extensions.sail

+// Main memory supports misaligned loads/stores
+enum clause extension = Ext_Zicclsm
+mapping clause extensionName = Ext_Zicclsm <-> "zicclsm"
+function clause hartSupports(Ext_ZicClsm) = config extensions.Zicclsm.supported


Suggested change

function clause hartSupports(Ext_ZicClsm) = config extensions.Zicclsm.supported

function clause hartSupports(Ext_Zicclsm) = config extensions.Zicclsm.supported

jordancarlin · 2026-01-06T18:02:12Z

model/core/extensions.sail

 mapping clause extensionName = Ext_Smcntrpmf <-> "smcntrpmf"
 function clause hartSupports(Ext_Smcntrpmf) = config extensions.Smcntrpmf.supported
+// Main memory supports misaligned loads/stores
+enum clause extension = Ext_Zicclsm


Move this up to be with the other Zi* extensions.

jordancarlin · 2026-01-06T18:03:51Z

model/core/extensions.sail

    Ext_Zvbb => 2, // > Zve32x
    Ext_Zve32f => 2, // > Zve32x
    Ext_Zve64x => 2, // > Zve32x
+    Ext_Zicclsm => 2,


Unnecessary and not part of any dependency chain. The base case of 2 is fine for Zicclsm.

jordancarlin · 2026-01-06T18:04:49Z

model/core/extensions.sail

+// Main memory supports misaligned loads/stores
+enum clause extension = Ext_Zicclsm
+mapping clause extensionName = Ext_Zicclsm <-> "zicclsm"
+function clause hartSupports(Ext_ZicClsm) = config extensions.Zicclsm.supported


Missing a currentlyEnabled clause. For Zicclsm, I think that can just be

function clause currentlyEnabled(Ext_Zicclsm) = hartSupports(Ext_Zicclsm)

jordancarlin · 2026-01-06T18:13:37Z

model/sys/vmem_utils.sail


 function check_misaligned(vaddr : virtaddr, width : word_width) -> bool = {
-  if plat_enable_misaligned_access then {
+  if plat_enable_misaligned_access | hartSupports(Ext_Zicclsm) then {


I think Zicclsm itself should not change any behavior. It should behave more like Zic64b and just enforce additional configuration checks. In this case, any memory region that has both coherent and cacheable set to true must also have misaligned_fault set to NoFault.

github-actions · 2026-01-06T18:15:44Z

Test Results

2 115 tests ±0 2 115 ✅ ±0 23m 23s ⏱️ -2s
1 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 99c99b7. ± Comparison against base commit 13a6693.

♻️ This comment has been updated with latest results.

allenjbaum · 2026-01-06T18:21:55Z

NOTE: this is a Zi extension, so this is what unprivileged code will see. This means that trapping behavior is permitted (so it may be emulated, and unprivileged code will never know the difference) We've been discussing this with respect to certification behavior, because it's hard to know under what conditions it will trap, and where it won't, Generally (but NOT necessarily always) that will be when some address boundary is crossed, either a fixed boundary (2^n bytes, e.g. 6 for a cacheline) or access width (so an 2^3 byte boundary for an LD) It can also trap if it goes outside/overlaps a main memory region, and may or may not be atomic when it doesn't trap.... There is a misaligned atomicity granule parameter as well which is almost what is needed, but is a stronger guarantee (it requires atomicity, as opposed just being functional, so can't be emulated)

…

On Tue, Jan 6, 2026 at 9:39 AM tongjiacheng ***@***.***> wrote: Zicclsm Misaligned loads and stores to main memory regions with both the cacheability and coherence PMAs must be supported. as specified in RVA23. ------------------------------ You can view, comment on, or merge this pull request online at: #1468 Commit Summary - d8bf75b <d8bf75b> add Zicclsm extension File Changes (3 files <https://github.com/riscv/sail-riscv/pull/1468/files>) - *M* config/config.json.in <https://github.com/riscv/sail-riscv/pull/1468/files#diff-b36f1a8f6855995358dfe9f8843ccb79130c81a8fe726131945d966e8ac3bf50> (3) - *M* model/core/extensions.sail <https://github.com/riscv/sail-riscv/pull/1468/files#diff-1dd6358065cc2bf5bb8b7e300774329202f09a4899787884b24ff675759a60f0> (6) - *M* model/sys/vmem_utils.sail <https://github.com/riscv/sail-riscv/pull/1468/files#diff-a38583222f57f7cc431cd3874c394b6e9e84b466d91898fbe4781790d2d524b5> (2) Patch Links: - https://github.com/riscv/sail-riscv/pull/1468.patch - https://github.com/riscv/sail-riscv/pull/1468.diff — Reply to this email directly, view it on GitHub <#1468>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJXFMRYNXFEHITGOBS34FPXODAVCNFSM6AAAAACQ3NOFM6VHI2DSMVQWIX3LMV43ASLTON2WKOZTG44DKOJVGE2TGNA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

allenjbaum · 2026-01-07T02:25:05Z

I don't quite interpret the spec that way. It supports all address widths from the point of view of the execution environment code; it could invisibly trap and emulate. That means it isn't invisible to Mmode, but.... not part of the profile

…

On Tue, Jan 6, 2026 at 2:22 PM Jordan Carlin ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In model/sys/vmem_utils.sail <#1468 (comment)>: > @@ -109,7 +109,7 @@ function misaligned_order(n) = { // External API function check_misaligned(vaddr : virtaddr, width : word_width) -> bool = { - if plat_enable_misaligned_access then { + if plat_enable_misaligned_access | hartSupports(Ext_Zicclsm) then { I always found the description of "main memory" very vague in the ISA manual. Now that I re-read it I think technically our PMAs should have a flag to indicate whether they are "main memory" or "I/O memory", since you can't actually infer it from the other PMAs. You can have an I/O memory region that supports all the same requirements as main memory but you just are declaring that it's actually I/O memory. @Timmmm <https://github.com/Timmmm> I think the memory ordering PMA unambiguously determines if a region is main memory or IO: Regions of the address space are classified as either main memory or I/O for the purposes of ordering by the FENCE instruction and atomic-instruction ordering bits. @nadime15 <https://github.com/nadime15> I think it is correct to classify ROM as IOMemory according to the spec. It specifies that there are only two types of memory: Regular main memory is required to have a number of properties, specified below, whereas I/O devices can have a much broader range of attributes. Memory regions that do not fit into regular main memory, for example, device scratchpad RAMs, are categorized as I/O regions. Main memory is required to be writable, so a ROM must be IO memory. — Reply to this email directly, view it on GitHub <#1468 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJSPWPMQIB7G6BTUZN34FQYTHAVCNFSM6AAAAACQ3NOFM6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMZSG4ZDGNBXGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Timmmm · 2026-01-07T09:28:34Z

Hmm I think you're right. Now that I double checked the profiles it says

Only user-mode (RVA23U64) and supervisor-mode (RVA23S64) profiles are specified in this family.

So as long as it is possible for machine mode to emulate misaligned access it's fine. So I think the only thing we can check here is that the PMA doesn't cause an access fault on misaligned access (which wouldn't allow emulation).

…ed_access.

challenger1024 · 2026-01-07T17:51:33Z

Recently update:

add currentlyEnabled function for Zicclsm.
add check_pma_regions_zicclsm() function.
remove plat_enable_misaligned_access.

@jordancarlin

nadime15 · 2026-01-07T18:01:20Z

model/postlude/validate_config.sail

  };
+  if hartSupports(Ext_Zicclsm) & not(check_pma_regions_zicclsm(pma_regions)) then {
+    valid = false;
+    print_endline("Zicclsm is enabled but cacheable and coherent memory regions are not set to NoFault.");


Maybe something like?

Suggested change

print_endline("Zicclsm is enabled but cacheable and coherent memory regions are not set to NoFault.");

print_endline("The extension Zicclsm is enabled but at least one main memory region that is cacheable and coherent and has misaligned_fault = NoFault is required.");

challenger1024 · 2026-01-07T18:40:05Z

I think you are right. I have change this message.
@nadime15

jordancarlin · 2026-01-07T18:43:15Z

model/postlude/validate_config.sail

+    [||] => true,
+    pma :: rest => {
+      let attr = pma.attributes;
+      if attr.cacheable & attr.coherent & attr.misaligned_fault != NoFault then {


This isn't right. Like we discussed above, Zicclsm only requires main memory regions that are cacheable and coherent to support misaligned. This is checking all regions that are cacheable and coherent. It needs to only checks regions that have all of the following properties:

cacheable

coherent

readable

writable

read_idempotent

write_idempotent

jordancarlin · 2026-01-07T18:45:22Z

model/postlude/validate_config.sail

  };
+  if hartSupports(Ext_Zicclsm) & not(check_pma_regions_zicclsm(pma_regions)) then {
+    valid = false;
+    print_endline("The extension Zicclsm is enabled but at least one main memory region that is cacheable and coherent and has misaligned_fault = NoFault is required.");


Zicclsm requires all main memory regions that are cacheable and coherent to support misaligned, not just one of them.

Suggested change

print_endline("The extension Zicclsm is enabled but at least one main memory region that is cacheable and coherent and has misaligned_fault = NoFault is required.");

print_endline("The Zicclsm extension is enabled but at least one main memory region that is cacheable and coherent does not support misaligned accesses (misaligned_fault is not NoFault).");

Ups, my bad!

…ion_param_constraints.

challenger1024 · 2026-01-08T03:05:19Z

I have added these conditions and changed that message.

@jordancarlin

jordancarlin · 2026-01-08T03:29:59Z

model/postlude/validate_config.sail

+      if attr.cacheable & attr.coherent
+      & attr.readable & attr.writable
+      & attr.read_idempotent & attr.write_idempotent
+      & attr.misaligned_fault != NoFault then {


Rather than having this all inline, break the read/write/idempotent check out into a helper function that determines whether a region is main memory like @Timmmm suggested. There are several other extensions that impose constraints on main memory, and that would let use reuse the function.

challenger1024 · 2026-01-08T04:18:37Z

I also think Timmmm's suggestion is very good, but I'm not sure where mainMemoryRegion and the memory_region_type function should be placed.

jordancarlin · 2026-01-08T04:37:07Z

I also think Timmmm's suggestion is very good, but I'm not sure where mainMemoryRegion and the memory_region_type function should be placed.

pma.sail seems like a reasonable place. They could also go in validate_config.sail since that’s the only place they’d be used (at least for now).

challenger1024 · 2026-01-09T01:28:08Z

I have added MemoryRegionType enum.
@jordancarlin

pmundkur · 2026-01-13T16:59:41Z

model/core/platform_config.sail

+// whether the platform supports misaligned accesses without trapping to M-mode. if false,
+// misaligned loads/stores are trapped to Machine mode.
+let plat_enable_misaligned_access : bool = config memory.misaligned.supported
+


Perhaps you re-introduced this by mistake?

pmundkur · 2026-01-13T17:00:36Z

config/config.json.in

    // checked before address translation. `misaligned_fault` in
    // `regions` is checked after address translation.
    "misaligned": {
+      "supported": true,


Perhaps you re-introduced this by mistake? I thought the plan was to remove this and use Zicclsm instead? See #1468 (comment)

pmundkur · 2026-01-13T17:01:12Z

model/sys/vmem_utils.sail


 function check_misaligned(vaddr : virtaddr, width : word_width) -> bool = {
-  if currentlyEnabled(Ext_Zicclsm) then {
+  if plat_enable_misaligned_access then {


Same question here.

add Zicclsm extension

d8bf75b

nadime15 reviewed Jan 6, 2026

View reviewed changes

jordancarlin requested changes Jan 6, 2026

View reviewed changes

jordancarlin added the extension Adds support for a RISC-V extension label Jan 6, 2026

challenger1024 added 2 commits January 7, 2026 19:15

Move Zicclsm and add currentlyEnabled function

bb97c57

add check_pma_regions_zicclsm() function, remove plat_enable_misalign…

57d6f92

…ed_access.

nadime15 reviewed Jan 7, 2026

View reviewed changes

challenger1024 added 2 commits January 8, 2026 02:25

revise message in check_extension_param_constraints

e15fe82

Remove extra blank line

66cb81c

jordancarlin requested changes Jan 7, 2026

View reviewed changes

modify logic in check_pma_regions_zicclsm and message in check_extens…

27ea8aa

…ion_param_constraints.

jordancarlin reviewed Jan 8, 2026

View reviewed changes

add MemoryRegionType enum and memory_region_type function.

99c99b7

jordancarlin mentioned this pull request Jan 8, 2026

feat: memory map support from JSON riscv-software-src/riscv-unified-db#1298

Merged

pmundkur reviewed Jan 13, 2026

View reviewed changes

challenger1024 force-pushed the Zicclsm branch from ef3a99d to 99c99b7 Compare January 14, 2026 04:17

	function clause hartSupports(Ext_ZicClsm) = config extensions.Zicclsm.supported
	function clause hartSupports(Ext_Zicclsm) = config extensions.Zicclsm.supported

	print_endline("Zicclsm is enabled but cacheable and coherent memory regions are not set to NoFault.");
	print_endline("The extension Zicclsm is enabled but at least one main memory region that is cacheable and coherent and has misaligned_fault = NoFault is required.");

	print_endline("The extension Zicclsm is enabled but at least one main memory region that is cacheable and coherent and has misaligned_fault = NoFault is required.");
	print_endline("The Zicclsm extension is enabled but at least one main memory region that is cacheable and coherent does not support misaligned accesses (misaligned_fault is not NoFault).");

add Zicclsm extension #1468

Are you sure you want to change the base?

add Zicclsm extension #1468

Conversation

challenger1024 commented Jan 6, 2026

Uh oh!

nadime15 Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

allenjbaum commented Jan 6, 2026 via email

Uh oh!

allenjbaum commented Jan 7, 2026 via email

Uh oh!

Timmmm commented Jan 7, 2026

Uh oh!

challenger1024 commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

challenger1024 commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

challenger1024 commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

challenger1024 commented Jan 8, 2026

Uh oh!

jordancarlin commented Jan 8, 2026

Uh oh!

challenger1024 commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

nadime15 Jan 6, 2026 •

edited

Loading

github-actions bot commented Jan 6, 2026 •

edited

Loading