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
+62 −0
Open

fix: implement Zawrs in IDL #1131

Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 11 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,14 @@ access:
vu: always
data_independent_timing: false
operation(): |
if ((CSR[mstatus].TW == 1'b1) && (mode() != PrivilegeMode::M)) {
raise (ExceptionCode::IllegalInstruction, mode(), $encoding);
}
if (CSR[misa].H == 1) {
if ((CSR[hstatus].VTW == 1'b1)) {
if ((mode() == PrivilegeMode::VS) || (mode() == PrivilegeMode::VU)) {
raise (ExceptionCode::VirtualInstruction, mode(), $encoding);
Copy link
Collaborator

@ThinkOpenly ThinkOpenly Sep 22, 2025

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

@Zain2050 Zain2050 Sep 23, 2025

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

@Zain2050 Zain2050 Sep 23, 2025

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

@dhower-qc dhower-qc Sep 23, 2025

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

@Zain2050 Zain2050 Sep 23, 2025

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

Copy link
Collaborator

@dhower-qc dhower-qc Oct 8, 2025

Choose a reason for hiding this comment

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

ping, @Zain2050, are you able to make this change?

Copy link
Author

@Zain2050 Zain2050 Oct 9, 2025

Choose a reason for hiding this comment

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

The ISS implementation? Sure, I'll give it a try. Can you give me some pointers here as I have no experience with ISS.

}
}
}
wait_on_reservation_set();
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();
15 changes: 15 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,21 @@ builtin function wfi {
}
}

builtin function wait_on_reservation_set {
description {
Wait-on-reservation-set: hint to temporarily
stall execution in a low-power state as long as:

a. The reservation set is valid
b. If "wrs.sto" , a short duration since start of stall has not elapsed
c. No pending interrupt is observed

A valid implementation is a no-op.

The model will advance the PC; this function does not need to.
}
}

builtin function pma_applies? {
returns Boolean
arguments
Expand Down
Loading