toggle quoted message
Show quoted text
I still have concerns with "* It is permissible to implement 'time' by always trapping (regardless of U or M mode execution of the access), leaving an M-mode trap handler to take care of things"
What part of the spec allows that? The non-normative comment? But "non-normative text can be skipped if the reader is only interested in the specification itself" which means that the only thing that says that it's OK to trap M mode accesses to time is not part of the spec itself. My understanding is that, if Zicntr is implemented then M mode must be able to access all 3 of the CSRs without trapping. Only if Zicntr is not implemented (if the priv spec even allows it to be unimplemented) can you trap M mode accesses to time.
I'm not opposed to allowing some implementations to unconditionally trap M mode accesses to time (and I know that some existing implementations do this which is fine). I just don't think that this is allowed unless the non-normative text becomes normative.
On Fri, Jan 27, 2023 at 11:46 AM Greg Chadwick <gac@...
Thanks for the discussion everyone, I think I've got the answers I need now, in conclusion:
* mcycle and minstret are mandatory (If implementing Sm).
* Going by latest specification drafts (latest github PDF releases) which include Zicntr: 'instret', 'cycle' and 'time' (and the U-mode view of other HPM counters) are purely part of Zicntr so aren't implemented if Zicntr is supported
* Going by the latest ratified specifications (from here https://riscv.org/technical/specifications/
) that lack Zicntr, 'instret' etc do not have to be implemented for U-mode access but there's an argument they must still be implemented for M-mode access
* It is permissible to implement 'time' by always trapping (regardless of U or M mode execution of the access), leaving an M-mode trap handler to take care of things
I'll put together a PR to attempt to clarify some of this in the specification.
I'll also open a couple of of github issues to discuss the points around whether the 32-bit mode of 64-bit 'instret' and 'cycle' makes any sense given 'mcycle' and 'minstret' are mandatory and whether Zicntr should constrain allowable WARL behaviour for 'mcounteren'
On Thu, Jan 26, 2023 at 1:35 AM Greg Chadwick <gac@...
> I'm not sure I follow your reasoning, would you mind elaborating?
my reading of the spec, I see no ISA mandate that an M-mode execution
environment interface must include the `time` CSR (or Zicntr more
generally). Where are you finding this?
I was basing my reasoning mostly on the privileged spec and looking specifically at the versions given as the latest ratified releases here: https://riscv.org/technical/specifications/
, 'Zicntr' is not part of volume 1 of that specification having been introduced recently in a draft there's a 'Counters' chapter instead. This introduces 'cycle', 'time' and 'instret' CSRs but makes no comment on whether they're optional or not, it does explicitly say 'if implemented' when talking about the rest of the counters which weakly implies 'cycle', 'time' and 'instret' are mandatory with the other counters being the optional extension. Though I agree the latest draft where this has become 'Zicntr' makes it clear these are optional.
My reading of vol II (latest draft) is that the M-mode equivalents are still non-optional (Zicntr and Zihpm being in vol 1 and not relevant to M mode and in particular don't mention the M mode equivalents). As I said in an earlier email unlike other M-mode CSRs there's no clear statement that minstret/mcycle are required but in the absence of a statement saying they're optional I take it they're required. II:3.1.11 mentions 'cycle', 'instret' and 'time' as shadows of the M mode versions, though makes no reference to 'Zicntr'. Pre Zicntr my interpretation was these CSRs must be included though accesses could always trap in U-mode but 'cycle' and 'instret' must be readable in M-mode (with 'time' reads always allowed to trap). Post Zicntr it seems if you don't implement Zicntr 'cycle', 'instret' and 'time' don't need to be implemented for M-mode either (though a note in vol II to explain this would be useful, I'd be happy to draft something and open a PR against the spec). This is the opposite interpretation we'd agreed earlier up the thread ('cycle' and 'instret' must be readable by M-mode regardless of whether they're accessible in U-mode).
I agree that mtime et al. seem to be required by Sm.
I would support a PR to clarify that Sm does not imply Zicntr. It may be as simple as adding the words "if implemented".
> From my reading of II:3.1.11, if an M-mode EEI includes time, then it
must be a read-only shadow of mtime. Perhaps this is cumbersome to
implement, but I don't think that necessarily makes it a bug in the
So are you saying that a 'csrr time' when executed in M-mode cannot trap and instead must do something to read from 'mtime' if you implement Zicntr? What behaviours does 'shadow' actually allow? Must you get the latest value of 'mtime' on any read of 'time' or would be permissible to have some implementation specific backdoor that can update the value 'time' reads receive and then you have some M-mode software triggered on a timer that periodically reads from 'mtime' to update the 'time' value?
I admit I don't know the precise meaning of the technical term "shadow": hopefully the architects can chime in. As a software person, I interpret it as meaning that I should be given a reasonably recent time. If I notice too much lag or jitter in one vendor's implementation I'll take my business elsewhere.
> The current RVA*U* profiles, for example, do mandate Zicntr, so in a
conforming implementation I would expect the U-mode EEI to include `csrr
time`, and this may be handled by taking a trap. However, it sounds
like you are OK with this.
Indeed, but my concern is what an M-mode execution of 'csrr time' is allowed to do.
A couple of other things that concern/puzzle me now I've read the latest spec draft including Zicntr:
1. Mention of software assisted 64-bit counter implementation - 'cycle' and 'instret' must be 64 bit but the specification suggests low-end implementations may use a 32-bit counter with software assistance to provide the upper 32 bits 'We required the counters be 64 bits wide, even when XLEN=32, as otherwise it is very difficult for software to determine if values have overflowed. For a low-end implementation, the upper 32 bits of each counter can be implemented using software counters incremented by a trap handler triggered by overflow of the lower 32 bits.' This seems reasonable but what's the point if you still have to do the full 64-bit implementations of 'mcycle' and 'minstret'? Perhaps the intent of Zicntr is it captures 'mcycle' and 'minstret' too (i.e. no Zicntr no 'mcycle' or 'minstret')?
2. How does 'Zicntr' interact with the following from II: 3.1.11 'In systems with U-mode, the mcounteren must be implemented, but all fields are WARL and may be read-only zero, indicating reads to the corresponding counter will cause an illegal instruction exception when executing in a less-privileged mode'? Is it permissible to implement 'Zicntr' but make 'mcounteren' read-only zero and thus 'cycle', 'instret' etc are present but only accessible in M-mode?
On Wed, Jan 25, 2023 at 5:42 AM Greg Chadwick <gac@...
I can see your point.
Though whilst I've got nothing concrete to point to in the spec I'd say there is a difference between the 'div' and 'csrr time' instructions in that taking a trap from U-mode to implement the CSR read seems reasonable and something the spec suggests as an implementation option whilst trapping on 'div' in U-mode to implement in a M-mode handler given the 'div' instruction must be implemented seems nonsensical.
I'd also say that mandating a 'csrr time' from M-mode must turn into a memory read doesn't feel like sane architecture given it's only for that specific CSR. For one you need some mechanism to tell the hart what address 'mtime' lives at and for another a 'csrr' instruction actually having memory semantics i.e. generates a architecturally visible memory access, which could generate memory related exceptions or interrupts, could cause some surprising action if the 'mtime' address has been mis-configured, are CSR instructions even allowed to produce memory related traps? There's this 'The CSRs defined so far do not have any architectural side effects on reads beyond raising illegal instruction exceptions on disallowed accesses' indicating they're not. Overall it does not seem sensible (were there some general declaration that an implementation may choose to use a memory backed CSR implementation so all csr instructions should be considered potential memory accesses that would be different). Indeed choosing it as an implementation choice regardless of if it's mandated seems to conflict with other architecture, the spec states:
'Standard CSRs do not have side effects on reads but may have side effects on writes'
I'd say a read to an effectively arbitrary address is a side effect (or could cause side effects), the hart would need to somehow guarantee that the address it has for 'mtime' would not generate any architecturally visible side-effects.
There's also this:
'All CSR instructions atomically read-modify-write a single CSR'
So if you have to do an mtime read/write you have to guarantee atomicity of this operation. Implementations are then forced to potentially have a significant/costly addition to their memory system to enable atomics (which it otherwise wouldn't have implemented) purely for the odd side case of an M-mode 'csrr time' execution.
From a micro-architecture point of view having a special case to turn this specific CSR read into a memory access is also awkward.
There's also the point of what the spec is actually saying vs what you believe it should be saying. My core point is if we decide the spec in fact demands a m-mode 'csrr time' turns into a memory access to some platform defined mtime address that should be considered a specification bug and the 'csrr time' always traps should be specifically allowed.
I'm not sure I follow your reasoning, would you mind elaborating?
From my reading of the spec, I see no ISA mandate that an M-mode execution environment interface must include the `time` CSR (or Zicntr more generally). Where are you finding this?
From my reading of II:3.1.11, if an M-mode EEI includes time, then it must be a read-only shadow of mtime. Perhaps this is cumbersome to implement, but I don't think that necessarily makes it a bug in the spec.
The current RVA*U* profiles, for example, do mandate Zicntr, so in a conforming implementation I would expect the U-mode EEI to include `csrr time`, and this may be handled by taking a trap. However, it sounds like you are OK with this.
On Tue, Jan 24, 2023 at 10:45 PM Paul Donahue <pdonahue@...
How is "csrr time" different than "div" in this regard? There was a lot of discussion at some point about whether you could claim M extension compatibility if you trapped and emulated divides in M-mode. My understanding of the outcome is that if an instruction is not supported by the execution environment visible to M-mode then you can't claim to implement that extension. (Of course, the SEE can claim to support emulated instructions but I'm talking about what hardware can claim to support.) The Zmmul extension was created specifically for that case.
Why is there a non-normative comment that says that reads of time can be emulated in M-mode? I agree that the CSR read can be converted into a load of mtime but I don't think that "or emulate this functionality in M-mode software" complies with what I said above. Just like emulation of divides, emulation of the time CSR would be seen by the execution environment visible to M-mode. (Emulating in an implementation-defined mode that's at a higher privilege level than M would be OK.)
From an implementers point of view, that means an CSRR of the time CSR could read the Mtime MMIO location (or a local copy if implemented that way_),
or it could trap and have Mmode firmware do the read and return a value. Any other type of access (CSRRW, CSRS*, CSRC*, etc) will trap
Form the ACT point of view we do not test firmware (e.g. the Mmode handler that would do that),
so we only care whether it traps or returns some value that is monotonically increasing
(at some rate that we need to define in terms of the only deterministically increasing counter we have which is instret),
and whether other variations of the op cause a trap.
So, vendors need to supply a parameter (number of instret counts guaranteed to be longer than a time tick) and the tests will do the rest.
On Mon, Jan 23, 2023 at 1:19 AM Greg Chadwick <gac@...
Thanks Greg and Jeff, useful to have my interpretation confirmed by a couple of people. I might put a PR together for the spec to add some clarification around these points.
On Fri, Jan 20, 2023 at 5:15 PM Greg Favor <gfavor@...
On Fri, Jan 20, 2023 at 1:38 AM Greg Chadwick <gac@...
On a related note I think it's permissible to not implement the 'time' CSR? It's an actual CSR that mirrors the memory mapped 'mtime' and the specification states 'Implementations can convert reads of the time and timeh CSRs into loads to the memory-mapped mtime register, or emulate this functionality in M-mode software' which presumably means M-mode accesses to 'time' can still be emulated by M-mode software (i.e. architecturally it's an illegal instruction exception and you're expecting the software to deal with it which is functionally identical to just not implementing it in hardware).
This is correct. The overarching idea is that the 'time' CSR must appear to be present, i.e. one can execute a CSR instruction accessing the 'time' CRS and the instruction - one way or another - will successfully be completed and will return a correct value. Put differently, from an architectural and software perspective, the 'time' CSR must appear to be implemented - even if it isn't actually implemented in hardware as a physically distinct register.