bcstrongx
commented
5 months ago 7. Some of the inhibit bits in *ctrctl are documented thus:
INDJUMPINH Inhibit recording of indirect jumps.
DIRJUMPINH Inhibit recording of direct jumps.
INDOJMPINH Inhibit recording of other indirect jumps.
DIROJMPINH Inhibit recording of other direct jumps.To most readers, it's going to be inscrutable how, next to the category of "indirect jumps", there could be any "other indirect jumps" not covered by the first category. The same for "direct jumps" and "other direct jumps". Furthermore, the catch-all categories of "indirect jumps" and "direct jumps" would presumably subsume many of the other categories: "indirect calls", "direct calls", "co-routine swaps", etc.
So by plain reading of the document, it would seem that setting both bits INDJUMPINH and DIRJUMPINH should prevent all JAL and JALR instructions (including their compressed forms, C.J etc.) from being recorded. In this case, we can infer that the other inhibit bits for jumps (e.g. for subroutine calls and returns) become irrelevant and ignored.
The same issue affects the description of the TYPE subfield of ctrdata. Some of the documented values are:
1010 - Indirect jump
1011 - Direct jump
1110 - Other indirect jump
1111 - Other direct jumpIt's impossible to divine what exactly distinguishes binary code 1010 from 1110, and 1011 from 1111.
After referring to the E-trace document, I believe what Smctr/Ssctr says is not what is actually intended. If my understanding is correct (and please correct me if I'm wrong), the labels for the jump categories for ctrdata.TYPE would more accurately be:
1010 - Other indirect jump without linkage
1011 - Direct jump without linkage
1110 - Other indirect jump with linkage
1111 - Other direct jump with linkageBy linkage here I mean that the jump instruction has a destination register that is not x0. (So a jump "without linkage" has no destination register other than the zero-only register x0.)
Assuming that the inhibit bits in *ctrctl are supposed to match the types recorded in ctrdata.TYPE (and I believe they should), then they would be:
bit 42 Inhibit recording of other indirect jumps without linkage.
bit 43 Inhibit recording of direct jumps without linkage.
bit 46 Inhibit recording of other indirect jumps with linkage.
bit 47 Inhibit recording of other direct jumps with linkage.One way or another, the labelling for these categories needs to be fixed in the document. This implies also that some of the official subfield names (INDJUMPINH etc.) ought to change, since they currently mimic the misleading labels.
The Architecture Review Committee agreed it would be better for the Smctr/Ssctr document to provide its own sufficiently detailed descriptions of each control-transfer category and not just defer to the E-trace document for this information. This may entail some cut- and-paste from the E-trace document. Some statement that the two are intended to be aligned would be appropriate.
However, to the extent that the E-trace document is the original source for some of the mislabelling I identified, it still needs to get corrected in the Smctr/Ssctr document in the process.
1. As the RISC-V ISA and all ratified extensions are currently defined, if S-mode isn't implemented, no S-level CSRs exist.
This draft extension requires an S-level CSR, sctrstatus, yet claims not to require that S-mode be implemented. The Architecture Review Committee discussed the matter and concluded it leans against having sctrstatus be the first (and only?) S-level CSR that may exist without S-mode.
The ARC asks the TG to consider whether Smctr/Ssctr should have a dependence on S-mode, much as it already requires XLEN >= 64 in order for the extension to be fully controllable through the 64-bit *ctrctl registers.
If the TG is sure Smctr/Ssctr is needed for 64-bit RISC-V processors without S-mode (though not 32-bit ones, evidently), then it appears it will be necessary to add an mctrstatus CSR that aliases sctrstatus.
2. The width of CSR sctrstatus isn't stated.
To avoid unnecessary complexity for a possible future RV32 version, I recommend 32 bits, unless the TG has reason to believe it needs to be more.
3. In the *ctrctl registers, the order of bits U, S, and M, and the order of STE and MTE, is backwards from other RV CSRs.
Assuming there is a desire to keep MTE and STE lined up with M and S, then for better RV consistency, presumably bits STE and S should remain where they are, bits U and M swapped, and MTE moved to bit 10.
4. Within CTRs, for both ctrsource and ctrtarget, we have:
The RISC-V *tval registers serve a similar purpose on memory access traps, and the Privileged ISA says of mtval:
Is the TG absolutely sure that ctrsource and ctrtarget don't need to support at least one invalid address?
5. Section 6.1.2, "External Traps", says:
This seems wrong to me. Hence, I believe the document deserves an explanation for why it's actually desirable.
6. According to the chapter "Custom Extensions", the value of the Custom field in each *ctrctl register is reset to an implementation-specific "default value", which configures the extension for its standard- defined behavior. Values of the Custom fields other than this default value select custom behavior.
The ARC strongly believes that a value of zero in a Custom field should select standard-defined behavior. Software can then know that a value of zero gives standard behavior, rather than some implementation- specific value (the "default") that may or may not be known to programmers. If it is even necessary to initialize these fields at reset (existing RISC-V convention usually leaves initialization to initial boot code whenever possible), they will of course reset to zero.