Re: Fast-track "stimecmp / vstimecmp" extension proposal


Greg Favor
 

On Tue, Sep 14, 2021 at 7:13 AM Vedvyas Shanbhogue <ved@...> wrote:
I have a few comments on on the vstimecmp section of the proposal:

1."When this bit is set, access to the vstimecmp register (if implemented) is permitted in VS-mode." contradicts the privileged specification. To support nested virtualization, accesses to the VS CSRs should cause a virtual instruction exception as noted in section 5.2 of the privileged specification. Perhaps this text should be updated to state "When this bit is set, the vstimecmp substitutes for stimecmp so that instructions that would normally read/write stimecmp would instead access vstimecmp."

I think you're looking at an old version of the proposal.  The latest draft, which will be updated after completing Arch Review this week, cleans this up and will be made publicly available very shortly after.

2."when the TM bit in the hcounteren register is clear, attempts to access the vstimecmp register while executing in VS-mode will cause a virtual illegal instruction exception if the same bit in mcounteren is 1" is not clear on the type of exception. Is "virtual illegal instruction" intended to be "illegal instruction" or "virtual instruction" exception?  "illegal instruction exception" looks right.

This has already been cleaned up recently.  And a Virtual Instruction exception is the correct type of exception per the latest H extension draft (which defines when to take a Virtual Instruction versus Illegal Instruction exception).

3."whenever (time + htimedelta) contains a value greater than or equal to vstimecmp" - may be a bit ambiguous as "time" when V=1 is already defined to have the htimedelta offset included. Perhaps this could be restated to replace time with mtime so that there is no ambiguity in the definition - "whenever (mtime + htimedelta) contains a value greater than or equal to vstimecmp"

The architectural definition of htimedelta applies this delta against time, not mtime.

Further, could the proposal be extended to also include the mtimecmp CSR?

Way back this was discussed but was deferred since there is existing mtimecmp arch functionality and not a strong justification to introduce alternative arch functionality that the ecosystem would have to also support.  This doesn't mean that new discussion can't happen to consider this (as another fast track extension if consensus builds towards adding an mtimecmp CSR).

Greg




 

Join tech-privileged@lists.riscv.org to automatically receive all group messages.