Skip to content

Don't access time/timeh in ZicntrU test if UDB_TIME_CSR_IMPLEMENTED is not defined - #2155

Open
Luke Wren (Wren6991) wants to merge 1 commit into
riscv:act4from
Wren6991:fix-time-timeh-unimplemented
Open

Don't access time/timeh in ZicntrU test if UDB_TIME_CSR_IMPLEMENTED is not defined#2155
Luke Wren (Wren6991) wants to merge 1 commit into
riscv:act4from
Wren6991:fix-time-timeh-unimplemented

Conversation

@Wren6991

Copy link
Copy Markdown

These CSRs are optional even when Zicntr is implemented, and even when the address-mapped time/timeh registers (e.g. in CLINT) are implemented.

UDB configuration can express this using the TIME_CSR_IMPLEMENTED boolean, but the SAIL model just provides Zicntr.supported and clint.supported, so it's not sufficiently granular to cover this (common) configuration.

As a compromise, skip the time/timeh tests when the UDB says the CSR is not implemented.

…s not defined

These CSRs are optional even when Zicntr is implemented, and even when
the address-mapped time/timeh registers (e.g. in CLINT) are implemented.

UDB configuration can express this using the TIME_CSR_IMPLEMENTED
boolean, but the SAIL model just provides Zicntr.supported and
clint.supported, so it's not sufficiently granular to cover this
(common) configuration.

As a compromise, skip the time/timeh tests when the UDB says the CSR is
not implemented.
@github-actions github-actions Bot added status: needs-review PR is ready for a review Tests Issues or pull requests related to the test generator or handwritten tests labels Aug 22, 2026
@jordancarlin

Copy link
Copy Markdown
Collaborator

See related discussion in #1924.

The Zicntr extension defines (and as far as I can tell, requires) the time CSR. If we don't test the time CSR, then we are not testing the full Zicntr extension. From an unpriv perspective, there is no optionality in the spec.

The priv spec then has the following non-normative note:

Implementations can convert reads of the time and timeh CSRs into loads to the memory-mapped mtime register, or emulate this functionality on behalf of less-privileged modes in M-mode software.

The UDB parameter seems to come from that non-normative note. That note does not make the time CSR optional, it just suggests one implementation of it is to trap and emulate. RISC-V technically allows trapping and emulating on any instruction when in lower priv modes. I don't think this case is any different than testing something like fdiv on a core that traps and emulates that instruction due to a bug in the hardware divider. In this case it is a design choice instead of a bug, but it is still a required CSR that should be accessible from a User/Supervisor perspective.

The current proposed solution is to catch this CSR access in the trap handler when TIME_CSR_IMPLEMENTED is false and invisibly emulate it.

@jordancarlin Jordan Carlin (jordancarlin) added status: needs-author The issue reporter or PR author must reply or update the item and removed status: needs-review PR is ready for a review labels Aug 22, 2026
@Wren6991

Copy link
Copy Markdown
Author

I don't think trap-and-emulate is applicable here because the test is checking whether the access traps. These are both valid answers to that question, per the spec:

  • Trap always (to permit M-mode emulation)

  • Trap when mcounteren.tm is false.

Unfortunately this type of optionality is difficult to support when the tests are checked with signature diffing instead of in-test logic.

So there is an actual structural issue with the tests that is not solved by installing a trap handler, IMO. Maybe the trap-and-emulate-style behaviour just needs to be supported in the SAIL model?

@Wren6991

Copy link
Copy Markdown
Author

The priv spec then has the following non-normative note:

This is non-normative because it gives no additional permission: an implementation can always provide S- or U-mode functionality through M-mode emulation.

Looking at this pragmatically, mtime/mtimeh and time/timeh are in a bit of an odd position because mtime was retconned from a normal CSR to a weird memory-mapped hybrid fairly late in the ratification process, and I think it's worth handling this one as a special case. A commenter on the other issue already mentioned SiFive U74 as an example of an implementation that doesn't implement time. Hazard3 is another example.

@jordancarlin Jordan Carlin (jordancarlin) added status: needs-decision A maintainer design, policy, or scope decision is required and removed status: needs-author The issue reporter or PR author must reply or update the item labels Aug 23, 2026
@allenjbaum

Copy link
Copy Markdown
Collaborator

@jordancarlin

Copy link
Copy Markdown
Collaborator

I'm working on an example implementation of this and will hopefully get it pushed in the next few days. The standard trap handler that we provide will automatically emulate accesses to time if TIME_CSR_IMPLEMENTED is false by reading from mtime. There will also be an RVMODEL hook for additional trap-and-emulate instructions so users don't need to modify the trap handler if they need to catch other instructions. These trap-and-emulate routines will happen at the beginning of the trap handler before any of the signature logic, so the test results will not change when an invisible trap is taken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs-decision A maintainer design, policy, or scope decision is required Tests Issues or pull requests related to the test generator or handwritten tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants