Skip to content

fix: implement Zawrs in IDL #1131

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: implement Zawrs in IDL #1131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7edcd83
Add IDL for Zawrs
Zain2050 Sep 22, 2025
0bdc53d
Added config params for time limit
Zain2050 Sep 24, 2025
e95c475
make encoding an argument
Zain2050 Sep 24, 2025
07ada2e
Merge branch 'main' into main
Zain2050 Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions spec/std/isa/ext/Zawrs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ description: |
* Code waiting on a flag to be set in memory indicative of an event occurring.
For example, software on a RISC-V hart may wait on a "done" flag to be set in memory by an
accelerator device indicating completion of a job previously submitted to the device.
params:
BOUNDED_TIME_LIMIT:
description: |
Defines the implementation-specific bounded time limit within which a wrs.nto instruction must complete.
schema:
type: integer
STO_DURATION:
description: |
Implementation-defined short timeout for wrs.sto instruction such that the stall is terminated on
the timeout if no other conditions have occurred to terminate the stall.
schema:
type: integer
type: unprivileged
versions:
- version: "1.0.1"
Expand Down
15 changes: 15 additions & 0 deletions spec/std/isa/inst/Zawrs/wrs.nto.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,18 @@ access:
vu: always
data_independent_timing: false
operation(): |
if ((CSR[mstatus].TW == 1'b1) && (mode() != PrivilegeMode::M)) {
if (wait_on_reservation_set_bounded() == false) {
raise (ExceptionCode::IllegalInstruction, mode(), $encoding);
}
}
else if (CSR[misa].H == 1) {
if ((CSR[hstatus].VTW == 1'b1) && ((mode() == PrivilegeMode::VS) || (mode() == PrivilegeMode::VU))) {
if (wait_on_reservation_set_bounded() == false) {
raise (ExceptionCode::VirtualInstruction, mode(), $encoding);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to account for the "implementation-specific bounded time limit" in:

When the TW (Timeout Wait) bit in mstatus is set and WRS.NTO is executed in any privilege mode other than M mode, and it does not complete within an implementation-specific bounded time limit, the WRS.NTO instruction will cause an illegal-instruction exception.

So, I suggest a couple of things:

  1. A new configuration parameter specifying that "bounded time limit" for NTO case.
  2. Some sort of parameterization for the wait_on_reservation_set function to accept that time bound for NTO case and some indicator of the STO case, possibly the same parameter if done carefully.

The wording in the spec seems to imply that the "short" duration for STO is not something formal / predicatable / parameterizable, and probably needs to be left undefined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of adding it but I noticed that the spec also mentions an implementation specific time limit for WFI, but nothing specific was written for it in IDL.

in VS-mode, attempts to execute WFI when hstatus .VTW=1 and mstatus .TW=0, unless the instruction
completes within an implementation-specific, bounded time;

Therefore I didn't include it in Zawrs as well. But, I'll write it as it seems like the right approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change I'll have to represent the permission checks within description of wait_on_reservation_set, right?
Should I write it in code format or just simply describe when it'll fault? What do you suggest?
If we take the code-format approach, I could use the function read_mcycle to keep a count of cycles and compare it with BOUNDED_TIME_LIMIT.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea these situations are tricky to model because (a) IDL has no concept of time and (b) implementations are allowed to do anything, including having "bounded amount time" be variable.

We can (should) try to make this apparent to a reader of the IDL code even if we may never be able to accurately model it. Something like:

if (mode() == PrivilegeMode::M) {
  wait_on_reservation(...);
} else {
  if (try_wait_on_reservation(...) == false) {
    raise(...);
  }
}

Within (try_)wait_on_reservation, it will have to eventually call a builtin to let the "SoC" decide what to do. We can have two variants of that, one for a bounded wait and one for an unbounded wait.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically try_wait_on_reservation for wrs.nto could have a description like

Temporarily stall execution in a low-power state as long as:
    a. The reservation set is valid
    b. Doesn't exceed implementation defined BOUNDED_TIME_LIMIT  
    c. No pending interrupt is observed
    
Returns False if BOUNDED_TIME_LIMIT exceeded; else True

}
}
}
else {
wait_on_reservation_set($encoding);
}
1 change: 1 addition & 0 deletions spec/std/isa/inst/Zawrs/wrs.sto.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ access:
vu: always
data_independent_timing: false
operation(): |
wait_on_reservation_set($encoding);
34 changes: 34 additions & 0 deletions spec/std/isa/isa/builtin_functions.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,40 @@ builtin function wfi {
}
}

builtin function wait_on_reservation_set {
arguments Bits<INSTR_ENC_SIZE> encoding
description {
Hint to temporarily stall execution
in a low-power state as long as:

* The reservation set is valid
* No pending interrupt is observed
* If "wrs.sto", STO_DURATION has not elapsed

A valid implementation is a no-op.
The model will advance the PC; this function does not need to.
}
}

# A special form of "wait_on_reservation_set" when "wrs.nto" must
# complete within an implementation defined bounded time limit
builtin function wait_on_reservation_set_bounded {
returns Boolean
description {
Hint to temporarily stall execution
in a low-power state as long as:

* The reservation set is valid
* No pending interrupt is observed
* BOUNDED_TIME_LIMIT has not elapsed

A valid implementation is a no-op.
The model will advance the PC; this function does not need to.

Returns false if BOUNDED_TIME_LIMIT exceeded, else true
}
}

builtin function pma_applies? {
returns Boolean
arguments
Expand Down
Loading