Skip to content

fix(IDL): implement Smctr/Ssctr in IDL #1164

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 8 commits into
base: main
Choose a base branch
from
Open

fix(IDL): implement Smctr/Ssctr in IDL #1164

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4674bdf
Add Smctr/Ssctr YAML and properly attribute SCTRCLR to them
Mar 27, 2025
dfe4494
Make Smctr/Ssctr require S and Smcsrind/Sscsrind
Mar 27, 2025
8c95734
data(inst): add long_name and description for sctclr
dhower-qc Apr 3, 2025
c133198
refactor(data): convert SCTRCLR description to array form
dhower-qc Apr 17, 2025
74e6237
Update sctrclr.yaml
dhower-qc Apr 29, 2025
f5fad42
fix(idl): add config params to Smctr
Zain2050 Oct 3, 2025
dcc0552
fix(idl): add IDL for Sctrclr
Zain2050 Oct 3, 2025
23e433d
fix: modified params
Zain2050 Oct 4, 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
19 changes: 19 additions & 0 deletions arch/ext/Smctr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@ description: |
which logical entry in the buffer it wishes to read or write.
Logical entry 0 always corresponds to the youngest recorded transfer, followed by entry 1 as the
next youngest, and so on.
params:
CTR_CYCLE_COUNTER:
description: |
The elapsed cycle counter includes a count of CPU cycles elapsed since the prior CTR record.
It is represented by the CC field, which has a 12-bit mantissa component
(Cycle Count Mantissa, or CCM) and a 4-bit exponent component (Cycle Count Exponent, or CCE).
It increments at the same rate as the mcycle counter.
The CtrCycleCounter is reset on writes to xctrctl , and on execution of SCTRCLR, to ensure that any
accumulated cycle counts do not persist across a context switch.
schema:
type: integer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this to represent the optionality of the the CC field in the ctrdata CSR? If so, this seems like this should be a boolean?

Also, it's not clear that this is what the parameter is for, so maybe include a bit more about the purpose of the parameter:

The ctrdata register may optionally include a count of CPU cycles elapsed since the prior CTR record.

Copy link
Author

@Zain2050 Zain2050 Oct 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a bit vague in the spec. From what I've understood. CtrCycleCounter and ctrdata.CC are different. CtrCycleCounter is an independant counter whose value is stored in ctrdata in case of a control transfer. From
section 11.5.3

The CC field is encoded such that CCE holds 0 if the CtrCycleCounter value is less than 4096, otherwise it holds the index of the most significant one bit in the CtrCycleCounter value, minus 11.

ctrdata for each entry is just a copy of CtrCycleCounter at the time of control transfer. Also,

The SCTRCLR instruction performs the following operations:
Zeroes all CTR Entry Registers, for all DEPTH values
Zeroes the CTR cycle counter and CCV

Here, zeroing out entry registers meaning zeroing out ctrdata as well. But it explicitly states to zero out CTR cycle counter and CCV.
As it's a counter therefore, it's type would be integer, right?
I'll improve the description of the param as it seems a bit unclear.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not following, sorry. The purpose of parameters is for configurations to resolve optionality in the spec. (For example, when the spec says "should", "may", or recommends a behavior, but does not mandate it with "must".) What optionality is being resolved with this parameter?

Copy link
Author

Choose a reason for hiding this comment

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

I guess I might be using parameters the wrong way.
Spec states that STRCLR zeros out the CTR cycle counter.

The SCTRCLR instruction performs the following operations:
Zeroes all CTR Entry Registers, for all DEPTH values
Zeroes the CTR cycle counter and CCV

Therefore, I used params to represent the hardware counter and CCV flag. What would be the correct way to represent the counter so that I could zero it out in IDL?

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, so "CCV" is a field in the ctrdata register. This register is not yet defined in UDB, but needs to be for this work to proceed. It would probably be defined much like other CSRs, but it might be a little weird in that I think it can only be accessed indirectly (siselect + sireg3). And there isn't a single set of CTR registers, it's a buffer of up to 256 records that can be accessed by being mapped to indirect register selections 0x200-0x2ff. And, how many records are actually supported is implementation-dependent (needs a configuration parameter) -- see sctrdepth.

So, it's a little complicated. #554 is a start on defining the extension, but isn't merged, and doesn't have the parameters defined.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The response was essentially that CtrCycleCounter is not directly visible machine state, only indirectly through ctrdata register(s). So, I think this needs to be implemented using a callout to a builtin function, akin to the use of read_mcycle() in cycle.yaml.

Copy link
Author

Choose a reason for hiding this comment

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

Okay. What about clearing out the entry registers? We don't have direct access to those registers. Should I use a builtin for that as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

At this point, I don't think it's terribly important whether it is represented as a set of (indirect) actual CSRs, or as a set of indirect CSRs that have sw_write() and sw_read() methods that access some builtin memory buffer. The former is pretty straight-forward. The latter would require defining a builtin accessor function.

Copy link
Author

Choose a reason for hiding this comment

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

So, my current approach makes sense, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you are talking about configuration parameters, then no. :-)

Flipping through the extension documentation, I see at least these as needed parameters:

  • what DEPTH values are supported
  • whether MISP is supported
  • the optionality of each ctrdata field
  • number of supported CCE bits

For registers, you'll need to use indirect access as noted previously.

For CtrCycleCounter, you should implement as a call to a builtin function.

For CCV, each of those bits is contained in each Control Transfer Record, shadowed by the ctrdata register, accessed indirectly through sireg3 (when siselect = buffer offset/register index), so if you're already setting the register to zero, you have also set CCV to zero.

I hope that makes sense. :-)

CCV:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, what is this parameter used for? Is it the "should" in the following description, as to whether that "should" is respected or not?

Copy link
Author

Choose a reason for hiding this comment

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

This represents the value that will be store in the CCV field of ctrdata when the next control transfer occurs. I have updated its description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not following here, either. Are you trying to parameterize the "should" in

This flag should additionally be cleared after any other implementation-specific scenarios where active cycles might not be counted in CTR_CYCLE_COUNTER.

?

And, it's defined as boolean, so what would be the value to be stored? Regardless, I'm not sure this really needs to be parameterized, but I want to understand your thinking first.

description: |
The CC value is valid only when the Cycle Count Valid (CCV) bit is set. The next record will
have CCV=0 after a write to xctrctl , or execution of SCTRCLR, since CtrCycleCounter is reset. CCV
should additionally be cleared after any other implementation-specific scenarios where active cycles
might not be counted in CtrCycleCounter.
schema:
type: boolean
type: privileged
versions:
- version: "1.0.0"
Expand Down