Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - chapter 7

Question

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - chapter 7

ved-rivos opened this issue 7 months ago · 1 comments

ved-rivos commented 7 months ago

Ownership:
The third bullet covers the case of instructions which may lead to
privilege mode change. Further instructions - ecall, ebreak - that
cause privilege mode change do not retire - they only raise an
exception - so the first bullet "an instruction which is changing
privilege mode retired" is not correct. Suggest fixing as follows:
1. Upon the retirement of an instruction that writes to the
  scontext/hcontext CSR.
2. Following any trace synchronizing message, specifically any
  message that includes the SYNC field. Importantly:
  - Should hcontext be implemented, the protocol requires two
    consecutive messages: the first presenting hcontext information
    and the second scontext information. This sequence is critical
    for enabling the decoder to precisely identify the code
    associated with a specific process.
3. In the event of a trap or trap return that results in a change in
  privilege mode.
Ownership: Is the requirement to send the hcontext first and scontext second a
hard requirement? A decoder has to wait for both since with either
order a decode cannot proceed before both have been received. Since
the order is not material the second bullet could be rephrased to
"requires two consecutive messages: one to present the hcontext
information and a second to present the scontext information. The
order of the two messages is not material but they must be
generated consecutively."
DirectBranch
Section 7.2 - "This message is..." ->
Qualifying with "direct" is not required and may be misleading as there
are no indirect conditional branches.
Suggest:
"This message is generated exclusively in BTM mode, upon the retirement
of a taken conditional branch."
In rest of section "direct conditional branch" -> "conditional branch"
Error:
"It is suggested to have a timestamp at the moment when the first trace
messages got dropped, but it is not required." - "It is recommended that
the timestamp reported in the message corresponds to the moment when the
first trace message was dropped; however, this is not a requirement." to
make it clear that the timestamp referenced is the timestamp carried in
the message.
The note for Error Message states that the message must be produced even
if trace is stopped. Is this problematic? If trTeEnable is cleared the
encoder must not generate any messages. Generating messages when disabled
may lead to unexpected behavior such as corrupting trace RAM.
RepeatBranch: A statement that RepeatBranch must only follow one of
the DirectBranch* or IndirectBranch* messages should be included.
Is there a limit on B-CNT?
What happens on a Debug Mode entry if HREPEAT counter was not 0 or B-CNT
counter was not zero is not specified? Should encoder generate a RepeatBranch or ResourceFull
message before sending the ProgTraceCorrelationMessage?
Section 7.8 : "when the HIST mask or I-CNT counter has reached.." ->
"when the HIST register is full or the I-CNT counter has reached maximum"
Should section 7.8 title be "ResourceFull" instead of "Resource Full" to
match format of name used for all other messages.

Editorial:

"Original IEE..." ->
"The IEEE-5001 Nexus Standard presents tables with TCODE (which
is sent first) in the last row. In contrast, this specification
describes Fields in Messages in the order they are sent (the
first field sent is described first), aligning with the order
of storage, processing, and text dumps."
Ownership: "This message..." ->
"This message furnishes the requisite context (privileged mode and
Context ID, as assigned by the operating system or hypervisor),
enabling the decoder to correlate program flow with distinct code
segments associated with various programs. Activation of this
feature is contingent upon the explicit enabling of the trTeContext
control bit. Reporting of this information occurs under one of the
following three conditions:"
Ownership: "Bit layout can be defined.." -> "Bit layout is defined.."
DirectBranch:
"Next PC is determined.."->
"Next PC is determined by decoding the conditional branch to determine
the encoded signed offset and adding it to the address of the conditional
branch instruction."
IndirectBranch:
"This message is generated..." ->
"This message is generated under two conditions: (1) when an instruction
that causes an indirect unconditional control flow change has retired,
and (2) when an interrupt or exception is delivered. This specification
is applicable exclusively to BTM mode."

"Last instruction..." ->
"The last instruction within the code block(s), as specified by the
I-CNT field, either represents an indirect unconditional control flow
change (i.e., jump, call, or return) or this packet is generated in
response to an exception or interrupt reported on the ingress port.
The next PC is determined by applying the Address Compression rules
to the U-ADDR field present in this message."

"Not-taken direct..." ->
"Not-taken conditional branches and direct unconditional jumps do not
generate any trace; however, they do increase the I-CNT. Additionally,
direct unconditional jumps modify the PC to the destination address
specified in the instruction. Consequently, the PC of the last
instruction in a code block(s) can be determined."
Error:
"Error message..."->
"An error message must be generated in the event of an internal messages
FIFO overflow, resulting in the loss of a trace message."

"FIFO overrun caused messages (one or..." ->
"A FIFO overrun has resulted in the loss of one or more messages."

"Implementation may always ..."->
"The field must be generated even if the reported value is 0, to guarantee
that the TSTAMP field aligns at the byte boundary."
ProgTraceSync:
"This message is generated.."
"This message is produced at the start or restart of trace. In such instances,
the I-CNT field is required to be set to 0. However, under certain conditions
associated with the SYNC parameter (e.g., External Trace Trigger), the I-CNT
field may not be zero. Instead, it serves to pinpoint the precise Program
Counter (PC) location at which the specified trigger or event occurred.
Additionally, the F-ADDR field provides the complete PC address at the moment
the trigger was activated."
DirectBranchSync:
"This message.."->
"This message is produced under the same conditions as the DirectBranch message.
However, it further includes details on the reason for synchronization via the
SYNC field, as well as the full Program Counter (PC) address through the
F-ADDR field."
Section 7.10 -> IndirectBranchHistHist -> IndirectBranchHist
Resource Full
"This message is ..."->
"This message is emitted when either the HIST register is full or the I-CNT counter
reaches its maximum value for a given encoder implementation. This mechanism
ensures that no information is lost, as it enables the decoder to reconstruct
larger I-CNT and HIST fields by adding or concatenating the emitted values."

"Not repeated HIST..."->
"When RCODE is set to 1, this signifies that the HIST register is full and will
not be repeated. Under these circumstances, the HIST field generally encapsulates
the maximum number of history bits implemented within the HIST register.
Nonetheless, implementations may opt to include any quantity of history bits in
this field, with the range extending from a minimum of 2 bits up to the maximum
defined by NTRACE_MAX_HIST bits.

Should the I-CNT counter and the HIST register simultaneously reach their
respective capacity limits, it is mandatory to emit two successive ResourceFull
messages."
ProgTraceCorrelation
"It provides..."->
"This message includes the EVCODE field, which specifies the reason for
generating this message, alongside the I-CNT and HIST fields. These fields
collectively enable the decoder to accurately identify the PC location where
execution or tracing was halted."

Answer 1 · 2024-04-01T11:00:36.000Z

Addressed in 1.0.0_rc25 (or even earlier ...) version.