riscv-non-isa / tg-nexus-trace Goto Github PK

View Code? Open in Web Editor NEW

33.0 22.0 29.0 13.75 MB

RISC-V Nexus Trace TG documentation and reference code

Home Page: https://jira.riscv.org/browse/RVG-96

License: Creative Commons Attribution 4.0 International

C 94.52% Makefile 1.72% Assembly 3.76%

nexus trace risc-v

tg-nexus-trace's Introduction

TG RISC-V Nexus Trace

Status of each PDF

2024/05/23 status:

PDFs 1.0.0_rc34: Using theme from ISA Manual ADOC and columns adjusted (font is smaller).

Inside of ./pdfs directory:

N-Trace PDF: Version 1.0.0_rc34. Ready for Freeze (accepted by ARC via email).
Controls PDF: Version 1.0.0_rc34. Almost ready for Freeze (see TODO below).
Connectors PDF: Version 1.0.0_rc34. Ready for Freeze (accepted by ARC via email).

TODO (before official freeze)

TODO: Issue of 'minimal reset' (in Control PDF) is discussed with ARC and TG.
TODO: Fix annoying page breaks (manual insert of 'page-break').
TODO: Make release with all PDFs 'Frozen', same date/version and pass back to ARC for official OK-stamp.
TODO: Make Public Review announcement (all PDFs Frozen).

TODO (after ratification approval)

TODO: Change status to Ratified.
TODO: Change date.
TODO: Remove _rc and make version 1.0.0.
TODO: Make official release.
TODO: Place all 3 PDFs into 'public' place (and update/add links).

Repository Overview

Working repository of the RISC-V Nexus Trace TG report. Nexus Trace TG Home page is here.

Main N-Trace specification is defined here: RISC-V-N-Trace.adoc and PDF version is here: PDF.
Trace Control Interface is defined here: RISC-V-Trace-Control-Interface.adoc and PDF version is here: PDF.
Trace connectors are defined here: RISC-V-Trace-Connectors.adoc and PDF version is here: PDF.

Clicking on ADOC file in the github repo viewer will render a usable version as markdown.

For a better rendering to PDF, use Actions in main menu above.

Reference C code for Nexus Trace dumper/encoder/decoder is here - documentation as README.md file is provided.

This work is licensed under a Creative Commons Attribution 4.0 International License. See the LICENSE file for details.

Initial Work (Preserved)

For initial document v0.01 (as PDF from MS Word), click here. Same file in ADOC format is here: TG-RISC-V-Nexus-Trace.adoc.

Additional Resources

The Nexus IEEE-ISTO-5001 Standard (2012-v3.0.1) PDF file.

Documentation generator

PDF version of specification should be generated using Actions menu. See below for more details.

Dependencies

The PDF built in this project uses AsciiDoctor (Ruby). For more information on AsciiDoctor, specification guidelines, or building locally, see the RISC-V Documentation Developer Guide.

Cloning the project

This project uses GitHub Submodules to include the RISC-V docs-resources project to achieve a common look and feel.

When cloning this repository for the first time, you must either use git clone --recurse-submodules or execute git submodule init and git submodule update after the clone to populate the docs-resources directory. Failure to clone the submodule, will result in the PDF build fail with an error message like the following:

$ make
asciidoctor-pdf \
-a toc \
-a compress \
-a pdf-style=docs-resources/themes/riscv-pdf.yml \
-a pdf-fontsdir=docs-resources/fonts \
--failure-level=ERROR \
-o profiles.pdf profiles.adoc
asciidoctor: ERROR: could not locate or load the built-in pdf theme `docs-resources/themes/riscv-pdf.yml'; reverting to default theme
No such file or directory - notoserif-regular-subset.ttf not found in docs-resources/fonts
  Use --trace for backtrace
make: *** [Makefile:7: profiles.pdf] Error 1

tg-nexus-trace's People

Contributors

Stargazers

Watchers

tg-nexus-trace's Issues

Feedback on RISC-V Trace Control Interface Specification - chapter 6

The number of bits requiring reset should be reduced to the minimal needed to ensure that the trace tool can initialize the trace encoder to desired state. For trace encoder, Reset to 0 should be needed only for trTeActive and trTeEnable. When trTeEnable is 0 the trace encoder does not emit any trace and all other register state may then be initialized by the trace tools to the desired state. Read-only fields by definition have a defined read-only value. This feedback applies to all trace control registers.
trTeInstTrigEnable - The description incorrectly states that this signal is set or cleared by the Debug Module instead of the Trigger Module specified by the Sdtrig ISA extension. The description can also be reworded for better readability/clarity.
trTeInstTracing - this signal is volatile and can be set or cleared by triggers. Guideline for a safe sequence to write this bit - for instance as part of writing say 1 to trTeInstStallOrOverflow is missing in the specification. Likewise for trTeDataTracing.
trTeInstExtendAddrMSB - the name of the control bit and its description is misleading as the encoder does not do any extension of address bits. Further it is indicated as being limited to Sv39/48/57 - which contradicts N-trace which allows compression for Sv32 and also physical addresses.
trTeDataDrop - why is this bit not redundant in relation to trTeDataStallOrOverflow
trTeDataStallOrOverflow - why is this bit set when a overflow message is generated and not when overflow is detected? Why is there a subtle difference in behavior compared to trTeInstStallOrOverflow?
trTeDataDropEna - how is this control different than trTeDataStallEna?
Specify behavior if both trTeDataNoValue and trTeDataNoAddr are both set to 1.
trTeDataAddrCompress - For E-trace data trace, the address is always differential and the data can be compressed using XOR or differential encoding. The 4 encodings specified here do not match up with the 4 diff options specified by E-trace data trace protocol.
"In a system with Internal Core Timestamp counter" - the earlier description implied that the core level sharing was only the clock and the timestamp counter is per hart. This text seems to contradict that. Having a shared timestamp counter among multiple harts is not very useful - especially since it may be reset by per-hart controls. This should be changed to a per hart timestamp counter.
trTsCount - this field should be WARL as it is legal to hardwire this unit to 0 when an internal timestamp unit is not implemented.
Timestamp unit - two terms are used "Internal timestamp" and "Internal core timestamp". Are these intended to be different. If not please unify to use a single term.
trTsRunInDebug - this should be renamed "trTsRunWhenHalted" to support additional cases where the timestamp counter may halt - such as low power entry, being held in reset, or due to security reasons as being discussed in context of external debug security.
trTsPrescale - is the prescale to scale up or scale down?
Timestamp units - bits 23:16 - does "System-dependent" mean they are designated for custom extension?
Section 6.2.1 - should be renamed to "Trigger Module"
section 6.2.1 - (breakpoint or watchpoint) - is this an exhaustive list? Why is trace-on/trace-off action prohibited for other forms of triggers?

Feedback on chapter 10 - RISC-V N-Trace (Nexus-based Trace) Specification

"Plain linear instructions" is not a recognized term. RISC-V does not have a
PC relative jump. Suggest using more formal language - e.g., "looking at
binary code"
The two bullet under main rules can be stated:
"Instructions and traps specified in Table 2. result in trace messages."
Detailed rule 1. "If tracing was disabled and restarted" is misleading as
it implies that a state machine is maintained to determine if tracing was
previously disabled and is restarted. The description of ProgTraceSync
accurately states that the message is generated when trace is either started
or restarted. Since the distinction is not important, the first bullet
should be rewritten to be accurate:
"When tracing is started, a ProgTraceSync message is generated"
The sub-bullets do not add any value here as the detailed description of
the message provides the contents of the message.
Detailed rule 2 - is a repeat of main rule.
Detailed rule 3 - "Plain linear" is first used in Chapter 10 and is
not a Priv/Unpriv term.
Detailed rule 3 - does not state a rule for generating messages - it says
how decoder should disassemble the binary using the trace. This should be
removed.
Detailed rule 6 - the rule is not clear about when "stopped or disabled"
occurs.
Detailed rule 7 - does not allow for generating IndirectBranchHistSync?
which is the recommended method for ICNT overflow as specified in section
8.4.4
Section 10.1 - the informative note implies that N-trace existed when
V or Zb or Zcmp were ratfiied. Suggest removing the note.
Section 10.1 - "Custom instruction which may change PC" implies that there
are instructions that can leave PC unchanged. Every instruction by definition
changes the PC.

10.Section 10.1 - the rule can be better stated as simply - map the custom
instruction to a itype category that best matches the custom instruction.
If we want to elaborate more then it could be stated that classify the custom
instruction as:

If it is a jump then classify as the jump as specified in section 4.1.1 of
the ETrace specification. Drive itype corresponding to this classification
to the trace encoder.
With this the example is redundant as decoders just fully the itype
Could move to examples appendix or remove

Further add a statement, if the instruction could not be classified to a
standard itype using rules in section 4 of etrace then use a custom itype

After reading through Chapter 10 - most of the contents here are repeated
from Chapter 7. Avoid repeated and redundant specification.
Section 10.2 - Suggest moving section 10.2 to a non-normative appendix
as it is a simplified version and not normative specification. Also include
a pointer to the github for the reference encoder.

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

Section 6.1 machine readable format specified by NexRvMsg.h:
a) The SRC field is missing the machine readable format.
b) The Error message shows a "PAD" instead of ECODE
c) The ResourceFull does not distinguish between RDATA[0] which
is a Var field and RDATA[1] which is a Cfg field.
Table 10: The SRC field width is controlled by the encoder
configurations. The stronger requirement is for the encoders
to provide identical configuration for trTeSrcBits and trTeInhibitSrc
"Within a given device, the SRC field bit..." ->
"The trTeSrcBits and trTeInhibitSrc control fields must be
implemented identically in all trace encoders that share a
trace stream."
Table 10: I-CNT - "execute/retired" -> "retired". speculative
execution should not be reported.
Table 11: I-CNT
"Most significant bit serves as overflow marker..."
This does not match with the definition I-CNT overflow in section 8.4
where a I-CNT is full when it reaches its maximum value. When a 22
bit wide I-CNT counter is implemented the section 8.4 indicates that
an overflow should occur when I-CNT value reaches 0x3FFFFF whereas
this text implies that an overflow occurs when I-CNT reaches 0x200000.

Editorial:

"Names...used by ..."->
"The terminology 'Indirect Branch' as used by the IEEE-5001
Nexus Standard may lead to confusion, given that the RISC-V
ISA exclusively permits direct conditional branches, which
are always relative. Furthermore, the RISC-V ISA makes a
distinction between 'jumps' (unconditional flow changes)
and 'branches' (conditional flow changes), a differentiation
not observed in Nexus terminology, where any flow change,
including exceptions and interrupts, is uniformly referred
to as a branch. This specification employs the terms 'branch'
and 'jump' as defined within the RISC-V ISA framework."
Section 6.1 - "Table below shows..." ->
"The table presented below enumerates all message types that
can be generated, with each row comprehensively defining the
fields associated with a particular message type. Fields that
are shared across message types are consistently ordered.

Field attributes are described using the following terminology:
- [n]: A fixed-length field that is n bits wide.
- Var: A variable-length field, with a minimum width of 1 bit.
- Cfg: A configurable field, where the existence and size are
  contingent upon the encoder configuration option."
"Vendor defined message (dedicated TCODE range)" ->
"TCODE range designated for use by Vendor Defined messages"
Table 10:
"Message header..." ->
"Specifies the number and/or size of subsequent fields within
the message, as well as their respective interpretations."
Table 10: "this is a number of 16-bit.." -> "this is the number
of 16-bit.."
"Full PC address with least significant bit (which is always 0 for
RISC-V) is skipped." -> "Full PC without the least significant bit.
The least signficant bit is not reported as it is always 0."
Table 10: HIST "Most significant bit = 1..." ->
"The most significant bit serves as the 'stop-bit' and is invariably
set to 1. significant bit represents the most recent branch, and
the bit immediately following the most significant bit reflects the
earliest branch reported."
"Least significant bit is always 0..." ->
"Only 63 bits suffice as the least significant bit of PC is always 0
does not need to be reported."

Feedback on RISC-V Trace Control Interface Specification - Chapter 8

Reword the first paragraph.
trFunnelDisInput - clarify #n corresponds to bit position
n in the field.
trFunnelDisInput - Clarify that when a input is disabled
it is still scheduled (e.g., using round-robin scheme
described before) and the messages are extracted from the
source but discarded.
Funnel timestamp unit is expected to be used as timestamp
source for its sources. The trTsRunInDebug control is
not applicable to a funnel and should be clarified.
Which trTsType are applicable to funnels?

Feedback on chapter 9 - RISC-V N-Trace (Nexus-based Trace) Specification

Section 9.1 - "Both the constant load and the indirect unconditional jump
must be traced as adjacent instructions (in same message/packet) in order
for the decoder to be able to infer the indirect unconditional jump target."

This statement should be reworded to avoid implying that the lui or
auipc are traced in a message/packet. The term packet is also used here
for first time and should be removed. Further, there is an implication that
the decoder should be making this distinction. It should be the encoder that
should use the combination of itype+sijump to make the decision to transform
the itype from 8->9, 10->11, or 14->15. The decoder guidelines should be
written to be clearer i.e. if the itype is a inferrable jump but the
instruction at that pc in the binary is a indirect unconditional jump then
disassemble forward (since disassembling backwards is not deterministic) to
the branch and extract the constant from the instruction preceeding the
branch.
Section 9.1 - why should the section 9.1 not just reference section 3.1.1 of
E-trace specification?
Section 9.2 - The specification should drop the mode 1 and 2. They are not
general and may not work for many programs e.g., C++ programs that do
exception handling, programs tail calls, etc. Is there a reason why these
should be retained as valid options?
Section 9.3 - Reword "Long loops" to "Loops with many iterations such as
those in functions like memcpy/strcpy have identical flow in each iteraction."
Section 9.3 - The second paragraph describes a "simple loop" and third para
a "long loop". To make it clearer the second paragraph should be reworded
as "A typical loop is..." and where it says and "taken once at end" -
prefix "typically" since this is a behavior of a predictable loop.
Section 9.3 - suggest rewording this section to be more clearer. It will
help if there were two subsections - one for BTM and one for HTM. Then
in each subsection the text could be normatively written to explain what
the rules are. Examples, should illustrate the rules and not be a means
to determine the rules. It is also suggested that a formal language be
used in the specification. This section is way too long to describe a very
simple concept. Remove most of the discussion and simply provide the
crisp specification.
There is informative note about HREPEAT field overflow. This is the first
time there is any discussion about HREPEAT field overflowing and it is not
clear what such an overflow mean. Further the guidancec to generate several
messages is not clear as to why on this condition several (how many?)
messages should be generated with a max HREPEAT value.
Is there a limit on HREPEAT value? Is it legal to use HREPEAT to be a 64 bit
counter and so an infinite loop will never generate a resource full?
Chapter 9 - first para - rephrase, not "must be"
Section 9. 1 and 9.2 - These are specified in the ratified e-trace spec.
Consider replacing with a reference to the E-trace spec to avoid repeating.

Feedback on chapter 6 - RISC-V N-Trace (Nexus-based Trace) Specification

Table 9 - Trailing "Table" in TCODE row.
Label the message definition example. Also add some text to state these are
a few samples. Some explanation of the macro names would be useful
Table 9 - SRC - does the "field need not be transmitted" also apply to
when it is enabled by trTeInhibitSrc i.e. it is allowed to ignore that
configuration? The decription for multiple harts states it MUST be
transmitted - this should be qualified by "if enabled".
Using Device or Processor consistently would be better.
Table 9 indicates SYNC is a message by stating "decoding may start from this
message". However SYNC is a field of a message. Perhaps the intent is to state
that "decoding may start from a message containing the SYNC field"
Table 9. I-CNT. The description "16-bit half-instructions" is misleading as
there are full 16-bit instructions. This should be reworded as "I-CNT counter
increments on instruction retirement by a number INST_LEN/2 where
INST_LEN is the length of the retired instruction in bytes."
Section 6.1 -> Rephrase first sentence and the paraphragraph starting with
"Attributes of fields.."
Section 6.1 -> the test starting with "Messages marked as Reserve..." and
rest of this section should be non-normative. A reference to the header file
can be included in the non-normative comment - ideally as a reference in
a bibliography section. The excerpt of the header file content should be
removed.
Table 9 - BTYPE - restate that this field can be 1 or 2 bits.
Table 9 - F-ADDR - redundant "LSB bit". Punctuations are not proper.
Text before Table 10 - Rephrase this paragraph. Original Nexus also
implies there is another version of Nexus being referenced.
Redundant "MSB bit"
Table 9 - TSTAMP - The first sentence of description can be removed.

Feedback on RISC-V Trace Control Interface Specification - Chapter 6

If the WARL is not the norm and there are exceptions then those should be
listed explicitly along with guidance on how to discover such options.
Reset values should be carefully selected. Only fields that absolutely need
a predefined reset should have a reset default. Please scan through the
registers to isolate the subset of state that must be reset. Rest should be
initialized by software.
trTeActive - The description stating this is a enable is misleading and
should be reworded. Since this bit works identically for all components
its description should move to section 4.3 so it does not need to be
repeated.
If trTeActive is written to 1 when it was 0 and some other bits of the
register are also written to 1 in the same write, do the other writes
stick or they are discarded?
The acronymn TE is used for firt time. Expand here or add to glossary.
trTeActive. When trTeActive goes from 1->0, what is the state assumed
by the components registers? Does it retain its state or lose its state?
If trTeActive is written to 0 when trTeEnable is 1 what is the behavior?
General - for single bit bits instead of "1:" one could write as "When set to
1.."
trTeEnable - add that "stops generating trace messages and flushes...".
The bit can be set by writing to it states the obvious. Is it allowed to
write 1 to this along with writing 1 to other settings in this same register
or is the statement only about fields not in this register? Should also
specify the behavior of what happens if other fields are written when this
bit is 1 - it could be unspecified i.e. some implementations may discard the
write, others may act upon some or all of new configuration, etc.
trTeInstTracing - This bit should be read-only and not read-write as it
is a status bit reporting whether tracing is active. The description "
written from a trace tool" seems wrong to have a volatile hardware bit be
also sw writeable. What is the purpose of this status bit to be SW writeable?
"may be subject to additional..in some implementations" - the normative
specification should not make any assumptions about custom extensions.
This sentence should be removed.
trTeEmpty - implies that the TE has internal buffers. Is that the intent?
or is this referring to HIST and CNT fields that may not have been emitted.
What are the conditions in which trTeEmpty will read 0?
Bit field location - please use conventional "msb:lsb" notation.
Fix column width so bit fields do not wrap.
trTeInstMode - the use "Main" implies there are other modes. should be
renamed to "Instruction trace generateion modes"
trTeInstMode encoding 0 - "trace may stil emit some records"
Terminology wise "records" is used for first time. Why does
a disabled encoder still generate messages should be explained.
trTeInstMode - encoding 1-2, 4-5 should just be called Reserved for
future standard use. The parenthetical repetition of for Branch Trace
and Branch History Trace should be avoided.
trTeInstMode - why does this reset to SD value?
trTeControl - bits 8:7 - the description should say Reserved for future
standard extensions. Should not predict what the extensions may be.
trTeContext - the description should exclude scontext/mcontext. The
description should be in form of encoder input which is context and
context may be asid/vmid for example or even hcontext. The description
should be restricted to avoid repeating specification - e.g., when Ownership
messages are generated, etc. The description "Send ownership.." implies
setting this bit will cause the sending. The description should be
"Enable generation of Ownership messages defined by RISC-V N-trace
specification"
trTeInstTrigEnable - The specification is indirect. The text also implies
that only the debug module may be used to do the triggering. Suggest
rewriting this to indicate - "Enable the use of trigger sideband signal
to start and stop instruction trace."
trTeInstStallOrOverflow - "by hardwre when an Overflow message is generated
or when the TE requests a hart stall" - implies that the Overflow message
is generated by some hardware other than the TE. The comment about reset
can be removed as there is a column for that - see also comment about
eliminating resets - this one would qualify as one that should have a reset.
The "Overflow message" is not explained anywhere. The N-trace specification
does not have a definition for such a message - nor does E-trace. This
likely implies a synchronization message generated following a fifo
overflow? It is not clear why this bit should get set once on stall and once
when a synchronization message is generated. For example, if at time T0
the bit was set due to a stall being asserted and SW cleared it at time T1
but the stall conditions persists and then at time T2 the stall condition
goes away leading to a synchronization message will this bit get set again?
Should also clarify that this bit only indicates that a stall has been
asserted since the last time this bit read a value of 0. This bit being 1
does not imply that the hart is currently stalled.
Question: What is behavior for data trace? Is a stall asserted? Can data
trace be active without instruction trace being also active? If so what
is synchronization for data trace?
trTeInstStallEna - do not need to repeat the synchronization message
following unstall that has been specified already.
trTeInhibitSrc - Suggest rewording as "When set to 1, the messages
generated by the trace encoder include a message source field if the
source width held in trTeSrcBits is not 0.
"Disable source.." -> "Disable inclusion of"
What is rationale to make this a inhibit control and not an enable control?
trTeInstSyncMode - "Select periodic.." -> "Select the periodic.."
Since 16-bit is implied by half-word, the parenthesized text can be removed.
Reword "Once the periodic counter is reached" to clarify what.
trTeFormat - "N-trace messages with..." -> Format defined by RISC-V N-trace
specification". The "with 6..." can be removed.
trTeProtocolMajor - "As specified by specification governing trTeFormat"
Table 11 - use period consistently. Follow structure "When set," consistently
trTeInstNoAddrDiff - instead of stating negative and then positive in
parenthesis could reword to state one way "When set, trace messages always
carry a full address"
trTeInstNoTrapAddr - "do not sent" -> "do not include"
Where these options had been discussed in E-trace spec - should copy the text
identically from Table 2.2 for maximal consistency.
trTeInstImplicitReturnMode - the text for encoding 0 is not clear and
should be rewritten. Why is this field R and not WARL.
encoding 1 - why support something that is not recommended to implement?
encoding 2 and 3 - need better descriptions. The logic cost etc. can be left
out of the specification - this has been discussed in the trace specs
themselves extensively and does not need to be repeated in the control spec.
trTeInstEnAllJumps - specify polarity for each action in "... or add..."
The trailing text is covered by the rtace spec and need not be repeated
here.
trTeInstExtendAddrMSB - it would be good if the E-trace and N-trace used
identical name for this optimization. That way the control can refer to the
optimization and not need to say more. The text in parenthesis should be
removed as that is covered by Priv. The last sentence should be removed.
trTeSrcID - reword the description.
trTeSrcBits - unclear why two controls are needed - The effect of setting
trTeSrcBits to 0 is same as setting trTeInhibitSrc. An implementation that
supports only 8-bit src-id may make this field WARL and only support 8 and
0 as the two configurations.
trTeInstFilters - this description should be written to be more clear about
what filters are being discussed and it means for `n' to match. If required
a link back to relevant trace specfication document could be provided.
trTeDataTRacing - this bit should also be a R as it provides status
information. Its unclear why the description notes "from a trace tool"
trTeDataStallOrOverflow - same comment as for corresponding instruction
trace control.
trTeDataStallEna - should clarify that the data trace messages are dropped
when set to 0 and data trace messages cannot be sent.
trTeDataDropEna - how does this control interact with trTeDataStallEna.
Will this bit override that control setting if the trTeDataSTallEna requires
hart stall? Will data trace be dropped if instruction trace is configured
to stall hart instead of dropping instruction trace?
trTeDataNoValue and trTeDataNoAddr - can these be set together. Is there
any value in data trace with trTeDataNoAddr=1 and trTeDataNoValue=0. The
other combination - trTeDataNoAdddr=0 and trTeDataNoValue=1 - seems useful.
Assume (trTeDataNoAddr=1 and trTeDataNoVaLue=1) is equivalent to disabling
data trace.
trTeDataFilters - same comment as trTeInstFilters.
Section 6.1 - "Internal System" - reword the sentence.
Section 6.1 - should put some ground rules on the timestamps. Is the
frequency of the timestamps required to be constant? If so then that should
qualify the clocks used - for example, if a core or a bus implements DVFS -
then is it acceptable to use that clock for timestamping? Is there any
requirement or guidance on the timestamp resolution.
Section 6.1 - its not clear that "internal system" and "internal core" need
be differentiated. The distinction made between accepting a timestamp from
"External" is also not clear. There needs to be some discussion on why these
four need to be exposed to the trace architecture and what logic would be
used to select among them. For example, when should a trace tool decide it
should use the shared vs. core vs. internal - what is the upside and what is
the downside of each choice.
Section 6.1 - "Shared" is this to be selected for the source and the
receivers that share the timestamp should be conifgured as "External"? If so
that should be clarified.
Section 6.1 - "The width of the timestamp..." - the text under parenthesis -
is quite obvious that a 40 bity counter overflows in 4.7 minutes at 1 GHz.
But is not clear what it is trying to imply - is it stating that a overflow
in 4.7 is not acceptable or is sufficient for all implementations?
"An Internal Timestamp Unit may include a prescaler divider" -> clarify this
is a clock divider.
Section 6.1 - "In a system with an Internal core timestamp counter..." The
last sentence is redundant - being optional implies it may or may not be
implemented. The first sentence shoudl be rewritten more formally.
trTsActive - what happens when this bit goes from 0->1 or 1->0. When tracing
is active can this bit be toggled?
Same question as trTsActive for trTsCount and trTsReset. Also what is the
behavior of these bits when trTsType is not 2 or 3.
Since trTsCOunt/trTsReset are only applicable for when trTsType can be
selected as as Internal, should these bits not be WARL?
trTsType - since this is a configuration - should this have been named
trTsMode and not trTsType whihc would imply a reporting facility.
trTsPrescale - its not clarified what n is . Since there are only 4
settings it may simple to just state 0: scale by 1, 1: scale by 4 ...
trTsEnable - Why is it called "Global Enable". Is there another local
enable for the encoder? If so then it should be pointed out.
Table 18 - Action (From debug Spec) -> mcontrol.action
Table 18 - for row 0 and 1 - just reference RISC-V Debug specification.
Table 18 - Action 2/3 - should also be qualified by the trace being enabled
i.e. trace has to be enabled for this action to take effect.
Table 18 - Action 4 - clarify which packet. "If trace is not active...
it should be ignored" - should be rewritten as observable behavior than
spec for hardware.
trTeTrigExtInAction0 - What is motivation to define input triggers as
encoded but output triggers as one-hot.
trTeTrigExtInAction0 - The text can be rewritten for clarifty. a) in
addition to trTeInstTrigEnable, there are other qualifiers such as
trTeEnable which are not included in the description. Is that intentional?
"it will start instruction tracing" - is that a statement about the
TE or the hart? The parenthetical text can be removed as that is a status
indication that tracing is in progress.
trTeTrigExtInAction0 - for trace-notify action should define which
packets are generated for E-trace and which for N-trace. Should also
clarify if this packet can be dropped if at the same instance a
synchronizing packet was also being generated by itself.
Section 6.3 - "And if a match bit in the trTeFilteriControl ...
..the corresponding register ... must have a valid value." - It is not
clear why if a match bit can be set, the corresponding register must
always have a valid value. This sentence may be more appropriate to
say that the corresponding register must exist? But then that statement
is superflous. Alternatively, this sentence could be updated to state
before a match bit in trTeFilteriControl is set, the corresponding register
must be setup with a valid value?
Section 6.3 - "Each of the mentioned ...
so a limited range can be matched with a single comparator unit if needed."
It is not clear what is implied by "so a limited range can be matched."
trTeFilteriMatchChoiceEcause - this is specified as a 32-bit register.
Exceptions cause values in RISC-V can be 64 values - this register should be
updated to support upto 64 exception causes.
Table 24 - trTeFilterMatchValueInterrupt - description is inaccurate
as this is used to match itype and not interrupt level codes.
Table 24 - trTeFilterEnable - what is the use case for an overall filter
enable. It would seem like enabling/disabling would require a single write
to this register and so the motivation to have this bit is not clear.
Suggest removing this as it does not seem to serve a valuable purpose.
trTeFilterMatchValueImpdef - missing parenthesis in description.
trTeFilterMatchMaskImpdef - missing parenthesis in description.
trTeFilterMatch?Impdef - In description indicate "impdef" refers to an
implementation defined set of hart to encoder input signals.
trTeCompPFunction - "less than to" -> "less than". "Greater than to" ->
"Greater than".
trTeCompPFunction - for arithmetic clarify whether the function is a
signed or a unsigned comparison. Also please add clarification that the
number of bits of the trTeCompPMatch used for comparison are the width
of the signals selected by trTeCompPInput.
trTeCompSFunction - instead of repeating the description - point to
trTeCompPFunction
trTeCompMatchMode - encoding 2 indicates - Either primary or secondary
result does not match - but the equation suggested !(P&&S) would match
if neither match - is that expected?
trTeCompPNotify - the parenthetical text should be made expelicit that
this control only applies when iaddr is the input.
trTeCompPNotify - "Final instruction in a block" - every final
instruction of a block generates a packet. Is this an additional packet?
What happens if the final instruction of that block is not reached for
example due to a trap that occurs before that? What happens if a
trigger condition turns off trace before the final instruction of the block
is reached?

Feedback on RISC-V Trace Control Interface Specification - chapter 7

trRamStopOnWrap - Its unclear from the specification what happens to the trace packets when storing is disabled. When storing trace is disabled are trace packets from upstream components dropped or does the trace RAM stop accepting packets and thereby cause a FIFO full leading to a stall/drop at the trace encoder?
trRamAsyncFreq - the name of this field does not match with the description of this field.
The guidance to not use trRamWRpHigh/trRamRPHigh in SMEM mode seems misplaced. While a trace buffer larger than 4 GiB may not be required, the trace buffer need not necessarily be placed in memory below 4 GiB.
Table 47 - explain parameters M and N. For SMEM fixed is the guidance to set limit to trRamStart + 2^N - A correct? The 2^N seems to imply, on reading first column, the alignment such as 64 bytes or 128 bytes. The limit seems impractically small in that case. For SRAM mode, should limit be (max size - 4) instead of (max size); as suggested by the note following the Table 47 that A should be 4.
Not being able to write the trace RAM - in SRAM mode - may lead to leakage of trace data among two traced entities that are mutually distrusting as the trace tool cannot clean up. Has the TG discussed the security model around this aspect?

Discovery related registers

Table 4 lists trTeDiscovery0 - trTeDiscovery7 registers but there is no specification provided for these registers.

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

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."

Feedback on chapter 12 - RISC-V N-Trace (Nexus-based Trace) Specification

There are likely other divergences like the message transmission protocol also seems to diverge.
Is this section supposed to be comprehensive? Why should this be normative specification?
On last sentence - Reference that chapter here

PIB, RAM, and ATB alignment synchronization

PIB, RAM, and ATB have a control field defined to enable generation of alignment synchronization packets. However, these components do not have a trace format (E-trace or N-trace) configuration. As the alignment synchronization sequence is protocol specific, should there should be equivalent of trTeFormat control provided for these components.

Feedback on RISC-V Trace Control Interface Specification - Chapter 4

Section 4
16. Table 1 - the term "trBaseEncoder" etc. used in this table implies it
is a register but it is not. Also the use of that implies there is
one base address in the system.
17. Table 1 - The definition of all the components has been discussed extensively.
Repeating the description is not required - that column should be removed.
18. Section 4.1 - "This also applies to multiple harts..." is redundant wrt to
first normative sentence.
19. Section 4.1.1 - "Optionally (and if accepta..." since this is a
configuration, the text in paraenthesis can be removed. ALso the bullet
before this wants to be rewriiten to state - if the coniguration does not
indicate stall on overflow - otherwise the previous bullet contradicts the
configuration to stall.
20. Section 4.1.1 - last bullet - should be reworded to address a) bandwdith
all along the path, not just at the sink, must be matched to the rate at
which data can be produced. Using trigger windows does not solve the
bandwidth problem for trace generated - it limits the amount of trace
data but not the bandwidth. The FIFO should be clarified as what provide
elasticity for temporary bursts but no amount of buffering can solve a
bandwidth mismatch.
21. Table 2 is quite redundant and not adding much value.
22. Table 2 - why is it disallowed to use ATB unless there are other ARM
components?
23. Section 4.1.1 - informational note - It is unclear why encapsulation in a
nexus message is needed. The ATB has ATID to indetify source of trace and
otherwise is agnostic to the payload. It is for decoder to decide based
on ATID how to interpret the payload. Further for a non RISC-V compliant
implementation there are many other custom aspects to handle like the
trace controls for non-RISC-V compliant components.
24. Section 4.1.2 - the manu diagrams are not adding much value.
25. Section 4.1.2 - information note - since every funnel needs to have a
per input disable, its not clear what the note is trying to say.
26. Section 4.2 - the discussion in this section is quite confusing. It is
well understood that memory mapped registers can be accessed though
SBA or by hart and that such accesses can be controlled by PMP. Since
this specification is to memory map the control registers this whole section
can be removed.
27. Section 4.3 - This table should be updated to only lists 2 registers
Control and implementation. The primary purpose of having the implementation
register at fixed offset is to support discovery of component type to
determine how to interpret the rest of the registers. By keeping just the
two registers and describing the componet type location as being standard
in this section, we can then indicate
If component type is 0x9 - see section 7 register details
The rest of the table is unnecessary as each component has its own table.
and this table starts becoming contradictory using names (e.g. at offset 8
) that dont match the later tables.
The paragraph on version numbering from chapter 5 can be moved here and
chapter 5 removed. The summaries can then be moed into their relevant
chapter.
29. Section 4.3 - note about access to register when tr??Active is 0 should be
normative. Unpredictable is not a good behavior to specify. Is the behavior
UNSPECIFIED? What are the range of behaviors allowed - hang? shutdown?
29. Section 4.3 - "alows debug tools to verify provided base address" - It is
unclear what verification of base address is required. Maybe the guidance
is that given the base address of a component, the tr??Imple register
30. Section 4.3 - "Some WARL fields.." -> this sentence can be removed - this
is already WARL definition.
31. section 4.3.1 - The example imply that those can be implemented as
custom extensions in the register space marked as "Reserved for more
register/sub-components". Is that right? If not then the Reserved should
just be called "Reserved for future standard extensions". It would then
be good to designate a range for custom extensions and mark it as
"Designated for custom use." The trailing normative text can be removed
as its not part of this specification.
32. Table 4 - trTeInstFilters description - "Filters to qualify instruction
trace"
33. Table 5 - trRAMData - since this field is also optional, why separate
the bigger read buffer row?
34. Table 7 - WHy reserve offset 7 for timestamp control? Is there a known
extension planned?

Feedback on chapter 8 - RISC-V N-Trace (Nexus-based Trace) Specification

. Chapter 8 - "Only execution addresses (as seen by the hart)" - reword
Only the pc is reported.

Chapter 8 - "In case MMU..." -> "When virtual memory system is enabled"
"Address fields are being sent..." -> "The address field does not include the
bit 0 as the pc is always aligned to a 2 byte boundary"
The rules are not clearly writen. They would want to be rewriiten for clarity.
- If the refence address is not valid then the current pc is recorded as
  the reference address and a trace message with a F-ADDR is sent.
  - When a message with F-ADDR is received then that address is recorded as the
    reference address.
- The value to transmit in U-ADDR is computed as the value resulting from XOR of
  the pc with the current reference address. This value now is the new reference
  address.
  - The bit 0 of this value is discarded and not transmitted.
  - The high order XLEN-1:Y bits are not transmitted if XLEN-1:Y, where Y
    ranges from XLEN-1 to 1, if they are all 0.
Section 8.2 - the sign extension of the U-ADDR must be done before XOR with
the reference address should be moved into normative text from the informative
text box. The rest of the warning text is redundant and can be removed.
Section 8.2 - the rules for forming U-ADDR or F-ADDR need to be normatively
defined. The section defines ruls for decoder but not for the encoder. There
is hint of it in the information box. That text states "Trace encoder must
implement a MSB extension" - the encoder needs to implement a detetction of a
run of 1 bits and not extension. There are unstated rules about sending at
least 1 bit in the run of 1 bits which are not described normatively.
Section 8.2.1 - there are too many examples for the simple concept. This section could be shortened.
Section 8.2.2 - the use of "MMU enabled" is misleading and should be replaced
with "when virtual memory system is enabled"
Section 8.2.2 - When trace is enabled for VS/VU mode, and Sv57x4 mode is actve
at G-stage and first stage is Bare, the pc is a GPA - to add to the list of 3
addresses - and can be 59 bits wide.
Section 8.2.2 - "Any illegal address.." the text in parenthesis may be removed.
Section 8.3 - reword "instead each taken...is adding a single bit as LSB..."
Section 8.3 - the description should not say HIST field since HIST is a
message field and is not the history accumulator. Further the description is
missing that this accumulator is a shift register where 1 or 0 are shifted
in from the LSB.
"starting from MSB bit (the one before stop-bit = 1)" should be "one after"
A better wording may be to describe it as a shift register for the decoder
as well.
Section 8.3 - the rule for generating the HIST+HREPEAT is not very clearly
defined. It should state when the stop bit is at MSB if the value is
identical to previously transmitted HIST then increment HREPEAT else send a
message with HIST + HREPEAT and initialize HREPEAT to 0.
Last para of section 8.3 seems redundant and not needed for decoders.
Section 8.3 The motivation for N-trace specification to limit HIST to 32-bit is not clear. If there is no functional
reason then it should be for the impleemntation to decide the cost. This limit should be removed.
15.Section 8.3 - "See Repeated..." - last senetnce - should be together with the text before the informational box.
Section 8.4 - the rule for increment of I-CNT can be stated more simply,
accurately, and a forward compatible manner as:
"Every retired instruction MUST increment I-CNT by its ILEN divided by 16.
The first sub-bullet can be removed - its redundant beyond stating "retired
instruction" as all retired instructions change PC.
The next two bullets "An exception or interrupt..." should be replaced with
"Exceptions and interrupts do not update I-CNT" Specifically the third
bullet is wrong as the I-CNT increment is a side effect of the instruction
retiring and not the act there was a interrupt between that instruction and
next. This statement should be a main bullet.
The last bullet can be removed with the wording as above.
Also the statement "are key rules" implies there are more rules?
Section 8.4.1 is a long example about I-CNT. This section should be written
to state the specifications normatively (if any) and the examples (if
required) non-normatively. Specifically this section does not add any new
value to the specification and can just be removed.
Again, maybe these belong in an Examples appendix at the end?
Section 8.4.1 - "Plain linear" is not an Priv. or Unpriv. term.
Section 8.4.2 - is not adding anything new to the I-CNt specification.
The rules for I-CNT increment and reset are agnostic to whether the
trace operates in HTM or BTM mode. This section can also be removed.
Section 8.4.2 - refers to "above code" but there is no code in this section
and lots of code in previous sections.
Too hard to correlate this with the scenarios in the prior section. Would be
better to show the dynamic execution flow, rather than the static code.
Section 8.4.2 - The non-normative note is repeated multiple times and adds
no value.
The entirety of section 8.4 including the 3 subsections can be replaced
with:
"The I-CNT is reset to 0 when trace starts (or restarts). Every retired
instruction increments I-CNT by its ILEN divided by 16. Traps caused by
exceptions and interrupts MUST not affect the I-CNT. The I-CNT is reset
to 0 when its value is transmitted in a trace message."
Section 8.4.4 - The wording is confusing and "(or when HIST buffer is empty)"
as a parenthis after "In BTM mode" implies HIST buffer has meaning in BTM
mode.
"may be reported" wrongly implies that it is optional to report overflow
of I-CNT.

It is not clear why ResourceFull cannot be used when HIST buffer is not
empty. Synchronizing the I-CNT should not require synchronizing the
HIST as well. The HIST can be sent when either it overflows or there is
message that inlcludes HIST that gets generated.

The statement "First choice... - second choice (SYNC=4).."
contradicts the rules of the two bullets. The first bullet states that
when HIST buffer is empty or when in BTM mode, the ResourceFull message
should be generated. But then this sub-bullet states that it is
optional and even in BTM mode a synchronization message can be generated.

The term "Synchronization message" is confusing. It should be specified
which precise *Sync message should be generated.

Suggested rewording:
"
An overflow of I-CNT must be reported to the decoder using one of the
following methods:
1. Using a ResourceFull message with RCODE=0. This method must be used
  when operating in BTM mode. This method may be used when operating in
  HTM mode if the HIST buffer is empty.
2. Using a IndirectBranchHistSync message. This method must be used
  if operating in HTM mode and the HIST buffer is not empty.
  "

The example, by including the c.ebreak muddles the example. That case is
not related to any kind of I-CNT overflow and is just confusing. Should
remove that from the example to keep the example crisp.

Could be Move this to examples in appendix

Section 8.4.4 - Reword "Overflown I-CNT=8 decodes" as decodes is
ambiguous.
Section 8.4.4 - the last paragraph implies periodic sync is mandatory.
The trace controls imply that periodic sync is optional. SYNC is needed for
various other reasons than periodic sync. This paragraph may read that
SYNC itself is not needed if periodic sync is not implemented. This paragraph
is not adding any further value and can be removed since it is misleading.
Section 8.5 - the title should be changed as there is no message called
synchronization. Perhaps "Synchronizing messages" is appropriate.
Section 8.5 - The first sentence should not be bulleted.
Section 8.5 - starting a section with an informational note is okay but
unusual.
Section 8.5 - "Allows the trade" -> "Allows the trace"
Section 8.5 - "Synchronization messages provide SYNC code" -> "Synchronizing
messages are messages with a SYNC field. The SYNC field identifies the reason for
synchronization and such messages include the F-ADDR (full address) field
to synchronize the PC with the PC observed by the encoder"
Table 25 - the header "Encoder Reset" should be first explained. Suggest
adding a few sentences before the table.
Table 25 - Fix the width of the columns
Table 25 - value 0 - "marker of external trigger input" - should this be
reworded as "marker to indicate that trace was enabled due to triggering a
external trigger"? Can external trigger be used to restart a trace that was
disabled? Which input indicates this in trace ingress port? Could we add
the name of the encoder input signal that triggers this? Which message
should be used to carry this SYNC field?
Table 25 - value 1 - Should this always be reset vector? What if trace was
activated only for S-mode. If a reset occured would trace then start tracing
M-mode PCs? Since a trace encoder capable of doing this must be in a
separate reset domain than the hart, should we break this into two - one
where there is a "stop" indicated due to reset assertion. And then if trace
starts there is a "start". So it seems appropriate to use "Entry to Reset"
here and on exit from reset when the trace is qualified to use Trace Enable?
Which message should be used to carry this?
Table 25 - value 2 - which message should be used to carry this sync.
"Clarify what is meant by "wrapped around".
Table 25 - value 3 - add an explicit section reference.
For each SYNC field encoding in Table 25 clarify which message is used to
carry them.
Table 25 - Trace Enable - the text indicates it shold be used only when
trace is re-enabled after being disabled for a gap. This does not match
What should be used when trace is enabled for first time?
Table 25 - Trace Event - should be renamed to more descriptive "
Debug watchpoint triggered". How is this different from external trace
trigger and which input to encoder should be used to determine this?
"with action=4" does this refer to debug spec? If so should provide a
reference. The "encoder state is not reset' is redundant as there is a
explicit column for this.
Table 25 - FIFO overrun - need a definition for FIFO overrun.
Table 25 - Exit from power down - how is it different than exit from reset?
Same questions about privilege mode filtering as for exit from reset. Is
this based on halted input? If not please clarify which input causes this.
Table 25 - code 10-13 - SHould not say Yes for endoder reset - this should
depend on whatever is future definition of these. Should be blank. In
description state "Reserved for future standard use"
Table 25 - code 14-15 - SHould not say Yes for endoder reset - this should
depend on how vendors specify this. In description update to say "Designated
for custom use"
Should resolve the open question about predioci sync usage and debugging.
Clarify what "Decorders should report" means.
Section 8.5 - clarify difference between full and partial TSTAMP. Or was it
meant to be absolute?
Section 8.5 - the informational note about "most synch. fully reset" - this
is not required as the table explicitly states which are and which are not.
Table 26 - please clarify why back to back return/jump/exception or
exception following interrupt are considered as corner cases.
Table 26 - "start of hart" - please clarify this term. If the interrupt is
pending why should it be signaled as a taken interrupt?
Table 26 - Why is excecption at first instruction traced a corner case?
Table 26 - "hart stops" - is that halted or reset? Why should a halt signal
entry to debug mode. Should the I-CNT and HIST not be the value active at
the time the hart stopped?
Section 8.7 - "relative to recently reported" - should it be any recently
reported or should it be fixed as the "last reported"?
Section 8.7 "ALL synchronization messages (which include full address)"
implies that only the synch. messages that include a full address must have
TSTAMP. However all such messages are required to have a full address. So
why is this qualification needed?
Section 8.7 - "Otherwise some sections..." that justification seems
incorrect as all sections are timestamped. The appropriate justification is
that sync. messages may be used to start decoding trace without having
observed any of the previous messages and hence carry an absolute timestamp
Section 8.7 - Is it allowed to omit TSTAMP in a message if the message defines
a TSTAMP field and trTsEnable is 1. The second bullet seems to imply that it
is okay to ignore the configruation. This also conflicts with second
paragraph that states that non sync. messages are required to incldue a
relative timetsamp.
Section 8.7 - it has been established earlier in Chapter 3 that maximum field
size is 64 bits. Why is the bullet 3 specially called out?
Section 8.7 - replace "code correlation" to "trace correlation"
Section 8.7 - reword 5th bullet
Section 8.7 - the first sub-bullet of 5th bullet talks about "distance
between distant events" - this is completely unclear what it means and
what is meant by "reported in a consistent way"
Section 8.7 - The 5th bullet should be reworded to state timestamp is
from the time the itype was presented to the encoder input. The statement
"when exception or interrupt was observed" does not make it clear as to
was observed by who. It should be when it was observed by encoder and not
when it was observed by the hart.
Section 8.7 - Rephrase - last main bullet. Don't need 3 bullets for this.
Section 8.2 - title - "Virtual Addresses Optimization" - consider removing plural
consider a different more descriptive name
"The following additional rules are used (when trTeInstExtendAddrMSB control bit is implemented
and set):" - remove parenthesis

Trace example `t1` makes non-standard use of `IndirectBranchHist` Message

The t1 trace contains multiple messages indicating a indirect branch, but actually being generated for direct, conditional branches.

For example MSG #2:

. 0x70 011100_00: TCODE[6]=28 (MSG #2) - IndirectBranchHist
. 0x10 000100_00: BTYPE[2]=0x0
. 0x55 010101_01: ICNT[10]=0x151 (337)
. 0x45 010001_01: UADDR[6]=0x11 (17)
. 0x00 000000_00:
. 0x00 000000_00:
. 0x00 000000_00:
. 0x00 000000_00:
. 0x00 000000_00:
. 0x0B 000010_11: HIST[36]=0x80000000 (-2147483648)
. next_iaddr=0x200102bc, EmitICNT(n=337,hist=0x80000000)

Before this message, the trace is decoded until PC 0x200102b8.

The disassembly for this address looks as follows:

  200102b2:	fc442703          	lw	a4,-60(s0)
  200102b6:	87c2                	mv	a5,a6
> 200102b8:	00f71363          	bne	a4,a5,200102be <xrle_compress+0x136>

Problem: bne is not an indirect jump.

For me, that seems like a kind of sync message, or somehow replacing the nexus resource-full message type, since always the HIST field is completely used.

Maybe I'm missing something here or have the wrong understanding. Does somebody could provide some clarification, would be awesome.

Best,
Albert

PIB parallel: byte count in header

In "PIB Parallel Protocol" section of the RISC-V-Trace-Control-Interface document, the following sentence is not clear:
"For RISC-V Processor Trace, the header byte includes a byte count."
Where is this byte count specified ? Is it specific to the PIB parallel protocol or should it come from the trace encoder ?

Feedback on RISC-V Trace Control Interface Specification - Chapter 10 through 14

trAtbBridgeAsyncFreq - explain alignment synchronization packets.
Explain "maybe the only choice for ..."
The chapter 11 does not add much to this specification. It
is repeating what are mandatory fields. The fact that a encoder
and sink are needed is obvious. This chapter can be removed.
Chapter 12 is repetition of what has been discussed for the
various field and does not add more value. This chapter can be
removed.
Chapter 13 is similarly not adding much value. The only guideline
that may be useful is to disable trace starting at the source - a
couple of sentences in introduction suffices for this. This chapter
can be removed.
The chapter 11, 12, and 13 may be then replaced with a single
guidelines chapter, if needed, that provides precise guidelines
that are not obvious from previous chapters.
Remove chapter 14. Add a note to trTeVerMajor that encoding of
0 is reserved by this specification. Additional history is not
requireed. Add a note in the the contributors to acknowleged
SiFive contribution - the links to that should be removed.

Feedback on chapter 11 - RISC-V N-Trace (Nexus-based Trace) Specification

"To decode an N-trace encoded stream of messages" - Decoding of the messages
does not require the listed 3 information. This should be reworded to
"To reconstruct the program control flow using the N-trace encoded stream of
messages"
"Size of each instruction (16-bit or 32-bit)." drop text in parenthesis.
Second bullet - the decoder does not have access to itype signal on ingress
port. This bullet should be reworded to be written with respect to decoder.
Third bullet - instead of stating "direct uncondition and direct cond" reword
to state "for inferable obtain the offset from the image" This will then
include the sequential jump optimizaztion - the current text precludes that
by saying "offset encoded in an opcode"
"At the beginning of trace" should be rewritten to "Start decoding from a
synchronizing message. The synch. message provides the complete PC in the
F-ADDR field. Transfers relative to this PC may then be inferred using
subsequent messages till a new PC is transmitted in a subsequent sync.
message"
"full PC is dropped periodically" - why should the decoder drop sync.
messages periodically?
Section 11.1 - "complete PC flow" - complete seem overarching. COuld
reword to "reconstruct control flow of the program". "is very simple"
can be dropped to make the specification formal.
Section 11.1 - Handle HIST ..(and not 0x1) - please expand/reword
"not 0x1"
Section 11.1 - "analyze code from current PC" - "disassemble from the
last PC". "PC will be after last direct conditional" -> "PC will be of
the instruction executed after the last conditional branch"
Section 11.1 - "each encountered instruction should subtract" ->
"For each instruction subract INST_LEN/16" - this will keep it straight
for future and for custom instructions that may be 48 or 64 bit or wider.
Section 11.1 - Handle I-CNT - third sub-bullet - I-CNt being 0 does
not mean an event always. In case of ResourceFull due to I-CNT overflow
it is just the new PC. Suggest rewording as this main bullet as
- First sub-bullet - "Dissaamble instructions starting at last known PC.
  For each disassembled instruction, update PC based on the type of
  instruction and decrementing I-CNT by ILEN/16, till ICNT becomes 0."
Section 11.1 - there seems to be priority between HIST and I-CNT? SO
HIST should be applied first and then I-CNT? This should be stated.
Section 11.1 - missing the handling for HREPEAT and BCNT
Section 11.1 - information note - the "inferable unconditional jump"
is defined before - suggest not repeating
Section 11.1 - information note - Encode under different names is ambigous
and all of this has been covered before. Repetitions should be removed.
Section 11.3 - "(ELF files)" -> "(e.g., ELF files)"
Section 11.3 - Suggest rewording this section to be more precise on
the guidelines on use of Ownership message. What are the expectations
on the operating systems and hypervisors and the decoders.
Section 11.3 - The informative text is not very useful and should be removed.
Section 11.3 - heading - Remove parenthetical, can refer to Linux in the text
Section 11.4 - in informative note please clarify what "compressing
execution flow" means. ALso please clarify how this comment relates
this section. If this is not specific to N-trace is there a reference
that can be provided to other trace systems that have solved thsi problem.
However, The informative text is not very useful and should be removed.
Section 11.1 - Is there a reference decoder we can point to?

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - Chapter 2

Please update the terminology used in Chapter 2 (and the rest of the
specification) as follows:

itype Description
0 No special type
1 Exception
2 Interrupt
3 Trap return
4 Not-taken branch
5 Taken branch
6 Indirect jump (with or without linkage)
7 Reserved
8 Indirect call
9 Direct call
10 Indirect jump (without linkage)
11 Direct jump (without linkage)
12 Co-routine swap
13 Function return
14 Other indirect jump (with linkage)
15 Other direct jump (with linkage)
Both ECALL and EBREAK cause synchronous exceptions, and are not considered
to retire. The epc reported by both is the pc of the instruction itself and
not the address of the following instruction. In table 2 - "ecall is
reported after retirement" is wrong as ecall does not "retire".
For 4-bit itype encodings, The rd != link should be classified further
into two classes - 15 (direct jump with linkage) when rd != x0 and 11
(direct jump without linkage) when rd = x0.
c.j and cm.jt are misclassified. They should be classified as type 11
(direct jump without linkage) and not 15.
"If Pop returns the same address as PC at next valid ingress
port cycle, emit Indirect Branch message with B-TYPE=0.". This needs to be
corrected as the Indirect Branch messages hould be generated if the PC is
not the same as the address Poped from the return address stack.
Table 3 - "Add 0 as least significant bit to HIST field.". Consider updating
to just "Update HIST field" since HIST is not introduced yet.
"Encoder must handle call stack action as described in the Implicit Return
Optimization chapter." Update to clarify that the encoder must handle call
stack actions only if the Implicit return Optimization is implemented.
The last non-normative note - "If optional trTeInstEnAllJumps..." should be
a normative statement.

Editorial and typos:
9. "Table below provides.." -> "The table below provides.."

11."detailed mapping of of causes" -> remove repeated of. This paragraph can
also use rewriting for clarity. Use "operands" instead of "arguements".

All instructions should use uppercase. For instance, "mret" -> "MRET".

13."N-trace is using the..." -> "N-trace uses the same..."

14."detailed mapping of of" -> "detailed mapping of"

15."When ingress port is implemented as 4-bit" -> "When the itype input of
the ingress port is 4-bit wide"

16.Table 4 - Reseved -> Reserved

17."other instruction" -> "Other instructions"

"not listed in this table" - "All other instructions that are not listed in
this table".
"Implicit x1" -> The "Expands to jal x1, offset"
"N-trace encoder does not require cause ..." to
"The N-Trace encoder does not require 'cause' and 'tval' ingress port
signals, which are valid only for exceptions and interrupts, as these
details are not reported in N-Trace messages. Instead, N-Trace solely
provides the address of the exception or interrupt handler."
"As almost every ingress port..." to
"Since almost every ingress port cycle updates I-CNT, there is a possibility
of overflow. For more information, see the I-CNT Details chapter regarding
I-CNT management and overflow handling."
"If the optiona trTeInstEnAllJumps..." to
"If the optional trTeInstEnAllJumps bit is set, the trace ingress port is
required to report itype=5 (Taken branch) for all direct unconditional
jumps, which are normally reported as itype = 0 or itype = 15". See also
comment 8.

Definition of WARL in RISC-V Trace Control Interface 0.9.4

The RISC-V Trace Control Interface 0.9.4 document defines WARL register fields as:

WARL - Write any, read legal. If a non-legal value is written, the written value must be ignored and
register will keep previous, legal value. (...)

This definition requires that when non-legal value is written, the original value must be preserved.
This is a different definition, stronger than WARL from the other specifications: Privileged spec and RISC-V Debug spec. Both of these allow any legal value to be read when illegal value is written. The original value need not be preserved.

Is WARL intentionally defined differently in the Nexus Trace Control document (or is this an oversight)?

If this is intentional, could a different acronym than WARL be introduced to avoid any confusion?

Thank you.

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - Chapter 4

Other trTeData... -> must be 0?
trTeInstMode - HTM should be expanded consistently as
"History Trace Messaging" (instead of History Branch Messaging)
trTeProtocolMajor - "must be rejected by the trace tool" ->
"must be rejected by the trace tool that if it is only compliant
with version 1.0 of the N-trace protocol"
trTeProtocolMinor - "Different values..." ->
"When trTeProtocolMajor is 1, values other than 0 are considered
down-compatible extension and should be accepted by the trace tool"
(Qualification by trTeProtocolMajor is important)
trTeInstMode - the description seems to imply that HTM mode must be
allowed. Is that right? If the "must be allowed" was intended to
qualify "One or more..." then suggest rewriting as:
"One or more of the following values must be supported:
3: BTM (Branch Trace Messaging) mode
6: HTM (History Branch Messaging) mode"

Editorial:

trTeEnable - Remove parenthesis around "potentiall...Unit" ->
"Part of potentially shared Timestamp Unit. Controls generation of
the TSTAMP field. See..."

Ambiguous "may not"

There are several instances of "may not" in https://github.com/riscv/tg-nexus-trace/blob/master/docs/RISC-V-Trace-Control-Interface.adoc

For instance: "This register may not be implemented if the sink type doesn’t require an address." Does that mean that the register must not be implemented or that it might not be implemented?

I think that the document should entirely avoid "may not" to improve clarity. Even in places that are seemingly clear, use "might not" or "must not" as appropriate.

Section 3.4 - PIB Idle cycles

The section 3.4 indicates that idle sequences may be transmitted between packets and the idle sequences may be a sequence of n where n may be 0 or any integer. One key aspect that is not specified is that for the probe to be able to synchronize with the bit stream, the encoder must provide a sequence of 8 idle bits to be able to disambiguate a packet boundary. These 8 idle bits may not occur naturally in the trace stream and may need to be explicitly inserted periodically. Guidance is needed on the need for these synchronizing sequences and the periodicity at which they should be generated if they dont occur naturally.

[discuss] Why are more MDO bits not considered?

An increase of MDO are discouraged in the documentation:
https://github.com/riscv-non-isa/tg-nexus-trace/blob/master/docs/RISC-V-N-Trace.adoc?plain=1#L1858

Currently, you use 2 MSEO & 6 MDO Bits. Which results in a 25% bandwidth overhead.

In my opinion, a 2 MSEO / 30 MDO Bit configuration, would be way more bandwidth optimal. The average message size is definitely above 32 bit, as it contains at least TCode, and often a 32-Bit HIST field or an address.

I would be happy for a discussion and learn more about the motivation against this change.

Feedback on Preamble and Chapter 1 - RISC-V N-Trace (Nexus-based Trace) Specification

Specification state should be Stable till AR completes and specification is Freeze approved.
Copyright and license - please update to follow the template.
"specification for complete end-to-end trace system" may be misleading of the specification as this spec provides the trace contents.
Introduction can be made better and reworded to be a better introduction to the core of the specification.
The statement "will be followed by Nexus compliant data.." - not sure its needed here but it should be non-normative - it "may" be followed by data trace, etc"
The para on trace control layer and connectors should move into the corresponding specifications.
Reword the informative note e.g. "besides PDF files provided below". Usually donations are to a TG and specifications provided by TG are RVI specifications.
Section 1.2 - abruptly starts with a diagram - without enough introduction on what the specification is about.
Fig 1 - It is possible that the Encoder and Control Layer will reside within the hart. It would probably be hard to include both options in a diagram, but a non-normative statement clarifying that the diagram is one possible implementation option would be helpful.
Table 1 - Field - "may span across multiple bytes" -> "may span multiple bytes". Punctuation need to be fixed in various places in the table.
Table 1 - Variable-length Field - "When messages are transferred" -> "When messages are transmitted" to consistently match terminology used in the last sentence of this description.

Feedback on chapter 13 - RISC-V N-Trace (Nexus-based Trace) Specification

None of the sections here seem to qualify as normative specifications. These could be removed or moved to a non-normative appendix. If this is intended to help the TG in future these could be moved into a separate TODO document in the Github.
Section 13.1 is very sparse. Either add more details or remove this. This section is not adding any normative specification value.
Section 13.2 This could be removed, or maybe add as non-normative comment in I-CNT section
Section 13.3 - Table 27 - This table does not have any normative specification. This could be removed.

【Discuss】About NexRv Tool

Hi All，

When I Use the NexRv Tool，I found it expect the first “ProgTraceSync” msg’s “full addr” field match the code’s first PC.
But I think when I enable the trace encoder, the Hart may not start yet.

I wonder if I used the tool wrong ?

Any help would be greatly appreciated!

Best regards,
Micreven

Missing LICENSE file

While the README file references the project is licensed under the CC-BY-4.0 license and provides a link to the full license text. the lack of a LICENSE file prevent GitHub from recognizing the license.

I propose adding a plain text LICENSE file like the one used in the ISA manual.

I'm happy to add this into the project directly and will do so in the coming days. If anyone has concerns, please raise them.

Thanks,
-Jeff

Feedback on chapter 7 - RISC-V N-Trace (Nexus-based Trace) Specification

"messages is provided in Fields in Messages table above" is better to provide
a cross reference to section number of table and avoid "above".
Use same language/format as above in "Fields in Messages" section
Section 7.1 - hypervisors perform context switch for the scontext. Will it be
confusing for decoders if that happens? The encoder only has a context
input. This section discussing hcontext and scontext could be confusing.
This section should be worded with respect to the encoder inputs.
A change in context value just depends on what the hart feeds in the context
input to the TE, so this doesn't have to be scontext/hcontext write. Could also be
a change in ASID/VMID, or even something else. Reword this to follow the language
used for hart->trace-encoder input.
Does Ownership get produced when the instruction executes or when it retires?
If the SRET traps to M-mode due to TSR will that be considered as the SRET
having executed and so produce the ownership message?
"If hcontext is implemented two" -> "if hcontext is implemented then two"
Since there is only a "context" input to the encoder how does it infer whether
to send a first message with hcontext and second message with scontext.
Section 7.2 - The explanation and notes are repeated many times through
the specification. It may be less error prone and easier to comprehend if
this chapter described the message formats and the later sections do the
explanation on how to decode the message.
"Immediately following any trace synchronization message" - this implies that
there cannot be an idle flit between a trace synchronization message and an
ownership message. Is that intentional? Or was the intention "As the next
message following a synchronization message"?
At entry and returns to/from exception and interrupts - this statement is
ambigous. The return from an exception is accomplished by itype=3 classified
instructions. Changing privilege is also only by these instructions and is not
required to change privilege.
Suggest rewording the three bullets as:
1. When the context input to the encoder changes
2. As the next message following a trace synchronization message.
3. Following an interrupt or exception trap
4. When an exception or interrupt return instruction is retired
Table 11 - providing values in decimal and hexadecimal is redundant. Suggest
using hexadecimal (or decimal) consistently.
Table 12 - "V or PRV" -> "V and/or PRV"
Section 7.1 - RTL syntax may not be intuitive to all readers. Consider
a simpler notation that is more universally understood.
Section 7.3 - missing period after first sentence.
The (or interrupt/exception ..) is not in lieu of the the instruction
retiring but seems implied by this text. Suggest breaking it out clearly.
This message is generated when:
a) an instruction causes an indirect unconditional control flow change
b) an interrupt trap is delivered
c) an exception trap is delivered
The explanation that follows table 14 are repeating the text at start of this section.
Section 7.3 - the explanation should remove "described by HIST" as this
message is not applicable in HTM
"Next PC....(either as F-ADDR or as U-ADDR)" is ambigous. This message
only includes U-ADDR and U-ADDR are always in reference to last F-ADDR.
It may also be best practice to avoid duplicating normative text.
Since there is a full section on adddress compression, this text can
simply state:
"Next PC is determine by applying the Address Compression rules using the
U-ADDR in field in this message"
The informative note about not-taken direct conditional branches is also
redundant repetition as that rule has been explained before.
As the leading text in 7.3 also explains when this message is generated
the repeat of that text in the explanation is not required
Since all ETYPE code points are reserved for specific purposes, what the
label Standard implies is unclear. Perhaps only 0-7 are standard and 8-15
are for custom. Suggest
"0 : Queue..."
"1..7: Reserved for future standard extensions."
"8..15: Designated for vendor defined errors"
Reconsider reserving half the encoding space for custom use.
Table 15. Why is it required for TSTAMP to be byte boundar aligned for
Error message but such a constraint is not imposed for any other message?
Is this constraint required? Further since TCODE + SRC + ETYPE can be
variable, how does always producing ECODE enable byte alignment?
Table 15. what do x in the ECODE mean? They can be 0 or 1 or are they
required to be 0? This should be explained.
ECODE field should be Fixed and not Var. Reword "standard:", "Reserved", "Vendor.."
as suggested before for ETYPE
Error message timestamp. Should this be timestamp of the first dropped
message or last dropped message? The text implies "moment when ... dropped"
seems to indicate last dropped message. Is that right?
The statement that "Error message must be produced on a FIFO overflow"
seems like wants to be a normative statement . The informative text box
can then explain why.
Table 16 - since F-ADDR is always provided the utility of I-CNT is not
clear even for the sync caused by external trace trigger.
Table 16 - is the F-ADDR when this sync is caused for extrnal trace trigger
required to be that of the PC that caused the trigger to fire?
Indirect Branch Sync - th eexplanation can be similar to Direct Branch SYnc
and not repeat the reasons for indirect brach message. Same about the
informational note (repeated third time).
Table 19 - same comment about "Standard". Reconsider number of custom encodings.
Table 19 - Is this message generated on the overflow or its generated on the resource
being full. The text implies that it is on overflow and that there is a loss
of recording that occurs. Why not report on full like the message name
implies.
Table 19 - Reserved for vendor... -> Designated for vendor...
Table 19 - Should Opt be labelled Cfg?
Section 7.8 - Explanation
I-CNT - this one can be removed as it is repeated
Not repeated HIST... -> this should be non-normative
RCODE=2 -> integrate this text into the table.
Add an explanation for why it is suggested that I-CNT should be reported first - what bad happens otherwise?
Section 7.10 - to avoid repeatation - the explanation can state - this is
generated when HTM is used and the rules used to generate IndirectBranchSync
apply.
Section 7.11 - the explanation should be updated - its not when an identical
branch message is encountered but when B-CNT number of identical branch
messages are encountered. Are all branch messages allowed to be collapsed or
only those without SYNC?
Table 23 - acronymn CDF used without explanation. Remove "IMPORTANT"
and fix case of "IN"
Table 23 - reword "even if HIST is empty=0x1"

Feedback on RISC-V Trace Control Interface Specification - Chapter 9

trPibDivider - "After PIB reset value of this pin.."
reword to "value of this field". The word "safe" should
be explained.
trPibCalibrate - explain what happens to the trace data
when this bit is 1. Does the Sink backpressure the source
or drops the packets?
Section 9.1 - LSB bytes is redundant. The first bullet
should be reworded to separately state transmission order
for bits and bytes.
Section 9.1 - "In 16-bit mode" - does that refer to
trPibMode=12? "16-bit mode" is not defined before and if so
should replace with the trPibMode value.
Section 9.1 - its more accurate to state "even bytes are
transmitted on TRC_DATA[7:0] and odd bytes on TRC_DATA[15:8]
as many messages have more then 2 bytes.
Section 9.1 - reword first and second sub-bullet of third
bullet.
Section 9.2 - LSB is sometimes used to reference least
significant byte and sometimes least significant bit. Should
use consistent terminology. Explain "next bit period"
Table 53 - Patters -> Patterns

When is Rev 0 the spec going to be published?

When is Rev 0 the spec going to be published, adding a PDF file to the doc directory? @RobertChyla

Feedback on RISC-V Trace Connectors Specification

"MIPI Debug & Trace Connector Recommendations" white paper (dated 7/2/2021) defines two pin options - "Narrow JTAG + Trace" and "JTAG + Trace" - for the 20-pin connector. This pin mapping supports the use of pin 14/16/18/20 as TRC_DATA[0/1/2/3] and pin 12 as TRC_CLK. It is unclear why Chapter 2 indicates that these are new additions to MIPI.
Chapter 2, Table 1 - shows two pins as SerialTrace. What is the function of having two SerialTrace pins and why do they have identical names?
Chapter 2, Table 1 - The layout in Table 1 does not match the mappings in Table 4 of the MIPI white paper. Specifically the JTAG mapping shown as mapping B in Table 1 is deprecated by MIPI. The Table 1 should be reconciled with MIPI and specifically identify why chapter 2 should diverge from MIPI recommendation.
The option of providing power through pin 11/13 is noted by MIPI whitepaper in Table 4 as supported by ARM but not supported by MIPI. The rationale for RISC-V to support this is not clear.
Please provide more rationale for chapter 3 not simply pointing to Table 11 of MIPI white paper i.e. the need for Chapter 3.
Please provide the rationale for chapter 4 not being a reference to chapter 7 of the MIPI white paper.

Please provide further arguements for why this document should not be a white paper (mostly reference ARM and MIPI specifications where appropriate) instead of a normative RISC-V specification.

Adopt the RISC-V documentation format

This is a 3-step update for which I will be submitting a PR:

Import of docs-resources project as a submodule.
Updates to the Makefile to use more options on asciidoctor PDF build, including the RISC-V theme (.yml file) and the fonts directory.
Updates to the main .adoc file to include more variables for the build and the logo for the title page

Once complete, the side-effect for anyone starting to use this repo locally will be that they'll need to learn to use Git Submodules. Plenty of useful information is here. But the key step is to issue a git submodule update --init after a new clone or the first pull after this PR.

At key points in the future, we may want to use the git submodule update to update to a new level because Submodules always "link" at a given commit level. (More discussion here as we work with this.)

If anyone has questions about the process, please free to let me know.

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

Section 8.1: "following Sv39/Sv48/Sv57 address generation rules"
needs to be updated. The paged virtual-memory system does not provide rules
for address generation but for address validity checks. Suggest update as follows:
"Address transmission in RISC-V N-trace is in compliance with the IEEE-5001
Nexus Standard. Per the IEEE-5001 Nexus Standard, the most significant
address bits that are zero are not transmitted. Additionally, for RV64,
RISC-V N-trace introduces an extension that allows for the omission of
identical most significant bits during address transmission. This extension
exploits the rules set forth by the RISC-V paged virtual-memory system for
valid virtual addresses when SXLEN=64. For further details on this
optimization, please refer to the 'Virtual Addresses Optimization' section.
section 8.1 :
"Rules when generating addresses" -> "Rules for generating addresses"
The bullet 4 is not rules for generating addresses. The bullet 4 can be made
a sub-bullet of bullet 3.
Section 8.2 - "Trace encoder must..." the rules in this paragraph is not
complete. Applying this rule at encoder as stated will not lead to the
desired result or match the examples. For example, applying this rule will
lead to add address=0xFFFF_FFFE_3FFF_FFFE being encoded as 0x1FFF_FFFF
which when decoded will lead to a wrong address.

A more detailed and correct algorithm should be included. For example
something like follows.

The process for detecting and skipping most significant identical bits in an
address A is as follows.
1. Let A be the XLEN (64 or 32) bit wide address.
2. Let i be XLEN-1
3. if bits i:i-5 are not identical to bit i-6 then goto step "vi"
4. Let i = i - 6
5. if i > 6 go to step "ii"
6. Let B[i-1:0] = A[i:1]
7. Let p = (i-1) % 6
8. if i < XLEN-1, then set bits B[i+p-1:i] to the value of bit B[i-1]
9. transmit B
Section 8.4 - the statement "Field I-CNT..." seems to conflate the field in the
message with the counter in the encoder.
"The I-CNT field, present in most messages, transmits the value of the
I-CNT counter, which counts the number of halfwords used to encode
retired instructions."

"The I-CNT counter in the trace encoder is reset to 0, in accordance with
the IEEE-5001 Nexus Standard, under one of the following two conditions:
- When tracing starts or is restarted for any reason.
- After the I-CNT counter value has been transmitted in a message."
section 8.4.1 -> remove reference to "prefetch fault" as there is no
such exception.
Table 26 should indicate which messages are used for each of the cases in
BTM and in HTM. The examples in section 8.8 do not provide all details.
Specifically, the details around treatment of HIST/HREPEAT/B-COUNT are not
provided in this section or in the examples. For BTM, if the choice between
use of a ProgramTraceSync with code=4 or ResourceFull message is a
implementation choice then some guidance for that choice should be provided;
and if it is not then under which circumstances the choice is made should be
specified.
Section 8.6: Timestamp recording -> Timestamp reporting

Editorial:

Section 8.1 - "Normally (with above bit enabled..." ->
"Normally (without the above bit enabled or implemented), addresses with many
most significant bits set to 1 will be sent as long packets (as variable size
fields skip only the most significant bit set to 0). The following address,
0xFFFF_FFFF_8000_31F4 (a real address from the Linux kernel), will be encoded
as F-ADDR=0x7FFF_FFFF_C000_18FA (with the least significant 0-bit skipped).
Such a 63-bit variable field value will require 11 bytes to be sent (as we
have 6 MDO bits in each byte)."
Section 8.3 and 8.3.1

"When operating in HTM mode, the encoder does not generate messages for
conditional branches. Instead, it maintains a HIST register or accumulator to
record the outcomes of these branches, whether taken or not. Each conditional
branch contributes a single bit to the HIST register, as follows:
- A bit with a value of 1 is appended at the least significant position for a
  taken conditional branch.
- A bit with a value of 0 is appended at the least significant position for a
  not-taken conditional branch.
The HIST register may be implemented as a left-shift register. Initially, when
the HIST register is empty, bit 0 of the register is set to 1, with all other
bits set to 0. Subsequent conditional branches cause the register to shift left,
recording each taken or not-taken outcome in bit 0. The transition of the most
significant bit from 0 to 1 indicates the register is full. At this point, the
entire register, including the most significant bit — which serves as the
stop-bit — is transmitted. After transmission, the register is reset to its
initial, empty state. The HIST register is transmitted using a ResourceFull
message with the RCODE field set to either 1 or 2.

Decoders must initiate the interpretation of the HIST field starting from the
second most significant bit. The most significant bit, designated as the
stop-bit, is invariably set to 1. This second most significant bit—immediately
following the stop-bit—encodes the outcome of the first conditional branch
captured in the HIST register. Conversely, the least significant bit represents
the outcome of the last conditional branch prior to the transmission of the HIST
register.

When a HIST register is full, if its value is the same as that of the HIST
field transmitted in the last ResourceFull message, then the encoder may
increment an internal HREPEAT counter (history repeat counter) instead of
generating a ResourceFull message if the Repeated History Optimization is
enabled. See Repeated History Optimization section for further details."
Section 8.4 "Every retired instruction..." and its sub bullets->
"Every retired instruction increments the I-CNT counter by 1 for 16-bit
instructions and by 2 for 32-bit instructions. This increment applies to
all forms of branch instructions as well. However, instructions that either
raise exceptions or are interrupted prior to retirement do not increment
the I-CNT counter.
Section 8.4 "When I-CNT counter is full..." ->
"When the I-CNT counter reaches its maximum value, becoming full, it can be
reported in one of two ways:
- By using a ResourceFull message with RCODE=0. This method is applicable
  to both BTM and HTM.
- By using a Synchronizing message with SYNC=4 (Sequential Instruction
  Counter), which is exclusive to BTM. For HTM, since no SYNC code exists
  to report HIST register overflow, employing a ResourceFull message is
  necessary. Thus, using a ResourceFull message in HTM for I-CNT overflow
  reporting ensures consistency across reporting mechanisms."
section 8.4.1 "(... does not matter)" -> "(Specific operations are abstracted
as "..." as they do not matter for this example)"
Table 26 "but HIST and I-CNT should provide the PC " ->
"The I-CNT and HIST fields may be used determine the PC of the last
instruction retired before reset".
"If timestamp is enabled all" -> "If the timestamp is enabled, all"
"The TSTAMP field is a variable-length field and most ..." ->
"The TSTAMP field is a variable-length field, and most significant bits set
to 0 will not be transmitted. This approach provides good compression for
both relative and absolute timestamps."
Section 8.6
"The following rules must be observed..."->
"The following rules must be observed:
- If timestamps are enabled, ALL Synchronizing Messages must include
  an absolute TSTAMP value.
- It is not required for all non-synchronizing messages to always
  report a timestamp. Doing so may be opted for saving trace bandwidth or
  in the case of sending back-to-back messages.
- The absolute timestamp cannot exceed 64 bits (even with 1ps resolution,
  64-bit counters will overflow in about 584 years).
  - Implementations may choose a smaller counter. Trace tools may assume
    the timestamp will not overflow in a single session, although adding
    support for overflow is not significantly challenging.
- It is suggested that in multi-hart systems, all Trace Encoders use a
  shared timestamp (for better trace correlation), but it is not mandatory.
- In all cases, when an address is provided, the timestamp should reflect
  the time when an event leading to that particular address being sent
  occurred."
"If the above is not possible..."->
"If the above is not feasible, timestamps should at least be reported
consistently, ensuring that the time distance between distant events (for
example, a periodic timer interrupt) can be reliably calculated. It is necessary
to ensure that the time reported at exceptions/interrupt handlers reflects the
moment when the exception or interrupt was observed."
Make section 8.8 a sub-section of 8.6. In section 8.8 correct spelling of
"ProgramTraceSyn". Either explain the red color coding or remove it if it is
not significant. The examples seem to refer to foot notes 1, 2, and 3 but
the foot notes are missing. For case 6, what does "After P" mean?
The case 6 seems to imply that "trTeInstTracing" has pulses periodically
causing it to transition from 1->0 and 0->1 at P intervals. Is that
intended? Same question about trTeInstTracing dropping to 0 when SYNC TRIGGER
occurs.

Feedback on RISC-V Trace Control Interface Specification - Chapter 5

Section 5

See note about moving the description of this register to the section 4.3
"The major version...should change" -> "The major version is incremented
when the modification breaks backward compatibility". " The minor version
is incremented when the modification maintains backward compatibility"
"Versions must be always.." -> is that a guideline for software? If so
it should not be normative specfication.
The long list of examples should be specified as a rule instead of specifying
through examples.
"Software should use the major version number to determine its compatibility
with the component's implementation. Software that was implemented to be
compatible with a major version may continue to operate even if the minor
version of the component is higher than the version it was designed for.
This is because minor versions, including experimental minor versions, are
backward-compatible."
The rest of the specification about causing warnings and aborts should not
be normative.
The note about what software should do is not useful as it does not add any
information about this specification and is generally best practice that
people versed in the art would know.

SBA acronym is not introduced in docs/RISC-V-Trace-Control-Interface.adoc

SBA is never introduced and is not contained in the Glossary, but used quite a bit throughout the document

Feedback on chapter 2 - RISC-V N-Trace (Nexus-based Trace) Specification

This issue summarizes the feedback and comments from AR on Chapter 2.

There may be many purposes for a specification. The second part of the
below sentence should be removed:
"Table below provides a detailed mapping of encodings of instructions
into itype signal - it should be used during development of ingress
port logic inside of a hart."
Further, it is not right to say that the itype is an encoding of the
instructions. While some instructions may lead to a control tranfer
out of a block of instructions, other causes of transfer out of a
block of instructions exists.
Suggest to use "Table below provides a mapping of causes for
terminating an instruction block to the corresponding itype encoding"
The title of the first column of table 2 is misleading by stating it
relates to the instruction that retired. As exceptions and traps are
included, the more appropriate heading would be "Cause for instruction
block termination".
The term "Interrupted instruction" may be misleading. While
instructions may be interrupted in some cases such as with vector
instructions, interrupts usually interrupt the instruction stream and
occur between instructions. The distinction is not material for N-trace
and so stating "Interrupt" as the what caused transfer out of the block
suffices. Reword "Interrupted instruction" to "An interrupt trap occurred
following the final retired instruction in the block."
The term "Exception in instruction" is misleading. While exceptions can
occur due to instruction execution, they may also occur during the act of
fetching an instruction - e.g., an instruction page-fault, an instruction
access fault, etc. Reword to "An exception trap that occurred following the
final retired instruction in the block".
uret is not a ratified instruction and should be removed.
cm.jt is a indirect table jump. It should be classified as uninferrable jump
Since non-control-transfer instructions do not lead to a termination of the
block, why do we need to add a row for "non-jump". Is the expectation that
every instruction when N-trace is active produces a trace packet?
cm.jalt is classified as a inferable call. However, it is unclear if the JVT
is always part of the instruction image. While cm.jalt treats accesses to
the JVT as implicit access the Zc allows for this to be dynamically located.
Should this be classiffied as a uninferrable call instead?
Table 2-2 - first instance of cm.popret* - capitalizes Z in Zcmp
Readability of Chapter 2 would be greatly enhanced if the terms I-CNT
BTM, and HTM and their concepts were introduced early.
Table 3 - I-CNT should not be updated by exception.
Table 3 - I-CNt should not be updated by interrupt. When the interrupt is in
boundary of two instructions, the previous instruction has updated the I-CNT
on its retirement. When an instruction itself is interruptible, then the I-CNT
should not be updated.
Table 4 - The header row is missing?
Table 4 - what does Push and Pop signify?
Table 4 - What is meant by "pop returns the same address as PC"
Table 4 - statements like "at next ingress port cycle" imply assumption
about a certain implementation. The architecture should be describe based
on the ISA property and not an implementation. For example, would the
implementation be non-compliant if it could do many taken branches in a cycle?
Or if it sends the address after 2 cycles?
The note about trTeInstEnaAllJumps implies that direct unconditional jumps
are reported as itype=0. Per Table 2 inferrable jumps use itype=15/14. Only
when implicit return optimization is not available they use itype=0 - should
this text be qualified by itype_width_p
The specification suggests using a reserved encoding. Specifications should
not encourage use of reserved encodings to preserve them for future standard
use. A better recommendation might have been to use encoding of 5 instead of
0. That could avoid the - encode as 7 and interpret as 0 or 5 guideline.
The cause and tval inputs being disabled should be stated normatively and
not in a informative note as it is an important aspect of N-trace. tval is
spelt incorrectly as tvar.
un-inferable -> uninferable
Table 2 - This table duplicates the information in the E-trace spec (table 4.4 and
section 4.1.1). There may be improvements needed in that table (e.g., updates for
Zc), but those should be made in the form of PRs to the E-trace spec.
Page 9 - informative note - Only the comment about 4 bit itypes and implicit
return optimization is needed. Rest of this comment can be removed.
Table 3 - Exception - Directive to emit 2 or 1 for BTYPE should be
qualified by what guides that decision.
Table 3 - Exception - Clarify what "address emitted is known at next ingress
port cycle"
Table 4 - Uninferable tail call - This is just an indirect jump (JALR) that
is not a call or return. Why is this not possible? This should match
behavior for itype=14. Same comment for itype=11/15

Feedback on RISC-V Trace Control Interface Specification - Chapter 1, 2, and 3

Document state should be Stable. The copyright should match the RVI template.
Chapter 1 - "Both Trace Working Groups..." this sentence be removed.

Chapter 2:

Reword first and second paragraph.
First bullet - "It describes RISC-V Trace Ingress..." can be removed
Page 6 - replace "master" with a better and inclusive term.

Chapter 3:

It is customary to have the glossary in the Introduction chapter.
Section 3.1 - For terms defined in E-trace glossary would want to
use identically to avoid subtly different terminology.
- Trace Encoder
- Trace Message/Packet
- Trace Decoder
Section 3.1 - Trace funnel - the definition should be expanded to
include inputs from other trace funnels - as allowed by section 3.8
Section 3.1 - R - usually R denotes readable and not read-only. The
conventional notation for read-only is RO.
Bit/field can be replaced with field everywhere as fields can be
single bit wide.
Section 3.1 - RW - we should avoid bits that have side effects.
If there really are no bits that are volative and RW then please
update the description. If there are please justify the definition.
Section 3.1 - WARL - please adopt the Priv spec definition for WARL
Section 3.1 - ATB - parenthesis can be removed. Please add a bibliography
and full reference to the ARM specification.
Section 3.2 - the heading of the section does not match the contents.
Section 3.6 - informational note - The note about Nexus Format
should be stated as an example.
Section 3.6.2 - Any reason for omitting (usually bytes) for System
Memory Sink as compared to SRAM sink? "DMA-type bus master" -> "DMA
controller".
Section 3.7 - "ATB bus master" -> "ATB initiator"
Section 3.7 - Reword second sentence.
Section 3.7 - ATB Bridge - Could the trace funnel or trace encoder
not directly emit on a ATB. Please explain why a bridge is required
and what is bridge to ATB.
Section 3.8 - is it assumed or it is required to have unique source ID?
Should the note should be rewritten to state that trace messages that are
input to the funnel are required to include a unique message source ID?
The teSrcID is not a funnel control register so the parenthetical use
somehow implies these signals are input to the funnel.

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

Section 11.1
The decoding algorithm does not consideration whether BTM or HTM is
being used. Without that differentiation the algorithm looks incomplete
and in some cases wrong. For instance, the handling of I-CNT in the first
bullet indicates that conditional branches should be treated as not-taken,
which is correct for BTM, but for HTM the outcome of the conditional
branch should be determined using HIST field.

Suggest writing chapter 11 with two algorithms - one for BTM and one
for HTM. For HTM, the I-CNT and HIST have be processed together
and not sequentially as currently suggested.

Editorial:

First note in chapter 11
"messages with F-ADDR is" -> "messages with F-ADDR are"
"wrapped around buffers" -> "circular buffers"
Section 11.2
"Decoder assigned to a specific hart should process only those messages
tagged with a SRC value corresponding to that hart. To facilitate this,
all encoders operating within the same trace stream must configure the
'trTeSrcBits' field identically to ensure a consistent source identifier bit width,
and must each be assigned a unique 'trTeSrcID' field value. This arrangement
ensures that messages can be accurately attributed to their originating hart,
allowing for precise and isolated trace analysis per hart."

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - chapter 9 and 10.

Chapter 9:
Editorial suggestions:

Section 9.1 "Jump targets that supplied via" -> "Some jump targets may be supplied via:"
Section 9.2 - "and is reset to 0 whenever a synchronization packet" was that meant to say Synchronizing ?
Suggest using messages or packets consistently. Message seems to be used more often than packet in this specification.
Section 9.2 "Such deep calls will be most likely 'broken' by other events/messages" - replace broken by interrupted.

Chapter 10:

Calling interrupts and exceptions as non-inferable instructions is not right. Restate main rules as
"Main Rules
- Inferable Instructions: This category includes instructions that neither perform control transfers nor
  are direct jumps. The subsequent program counter (PC) for these instructions can be accurately
  predicted through static analysis of the binary code. Because these instructions exhibit a predictable
  execution flow, they are termed "inferable," and no trace is generated for them. In BTM, a not-taken
  conditional branch is also classified as inferable.

Non-Inferable Instructions: This category comprises conditional and indirect branches, distinguished
by their ability to generate a trace. Due to the unpredictability of the next PC as determined through
static analysis alone, non-inferable instructions require trace generation for precise control flow tracking.
Interrupts and Exceptions: Control flow changes caused by interrupts and exceptions necessitate
trace generation. These events alter the flow in an unpredictable manner, similar to non-inferable
instructions, thereby requiring their occurrences to be traced for accurate analysis."

First detailed rule implies that ProgTraceSync is generated only when tracing is started after it was
disabled. Is that intended that the very first enabling of trace does not generate ProgTraceSync. If
not then suggest updating as:
"If tracing is started (or restarted after being disabled), a ProgTraceSync message is generated. This
message specifies the reason for the start in the SYNC field and includes the full address in the F-ADDR field."
"Any retired instruction increments I-CNT field (+1 or +2)." -> the increment should be on the counter and not
a message field.
"A retired 16-bit instruction increments the I-CNT counter by 1, while a retired 32-bit instruction increments it by 2."
The third bullet of the detailed rules repeats the main rule for inferable instructions. Can replace the
third point and its sub-bullets with a single statement "Inferable instructions do not generate any trace"
First extended rule states that call and return maintain call stack. However, implicit return optimization does
not require maintaining a call stack as there is an option to implement a counter.

Editorial:

"This chapter..."->
"This chapter explicitly addresses 16-bit and 32-bit instructions as defined in the currently
ratified instruction set. Nonetheless, the guidelines provided herein are applicable to any
instruction size that is a multiple of 16-bit, should such instructions be defined in the future. "
Detailed rules 4th point
"Indirect branch instructions are handled as follows:
- In BTM mode, an IndirectBranch message is generated.
- In HTM mode, an IndirectBranchHist message is produced. Should the HIST field be
  empty, an IndirectBranch message may optionally be generated instead."
Detailed rules 5th point
"Conditional Branch Instructions are handled as follows:
- In BTM mode, a DirectBranch message is generated, but only if the branch is taken.
- In HTM mode, the outcome of the branch (1 for taken, 0 for not taken) is appended
  as a single bit to the branch history buffer (HIST register).
Detailed rules 6th point
"When tracing is stopped or disabled, a ProgTraceCorrelation message is generated.
This message includes the reason for stopping or disabling (specified in the EVCODE
field), the I-CNT, and an optional HIST field. These details allow for the calculation of the last PC."
Detailed rules 7th point
"When a generated message includes I-CNT counter value or HIST register value, the
corresponding counter and/or register are reset.
- If the I-CNT counter reaches its capacity, a ResourceFull message, indicating that the
  I-CNT counter is full, is generated. Subsequently, the I-CNT counter is reset.
- Similarly, if the HIST register reaches its capacity, a ResourceFull message, specifying
  that the HIST register is full, is generated. The HIST register is then reset."

Feedback on chapter 5 - RISC-V N-Trace (Nexus-based Trace) Specification

chapter 5 - Nexus defines a single Branch Trace Messaging with two types -
BTM using traditional messages and BTM using Branch History Messages.
This section should be fixed to use the right terminology.
Nexus standard defined -> Nexus standard defines
Rephrase "Encoder must implement at least one ..."

Feedback on RISC-V Trace Control Interface Specification - chapter 10 and 11

Chapter 10 - introductory text discusses that there is a field to indicate "Trace Source ID". This however seems to actually imply the ATID - trAtbBridgeID. This text should be reworded to avoid confusion that the trAtbBridgeID is used to replaced source ID in trace packets by replacing "Trace Source ID" by "ATID". Further description of trAtbBridgeID should use "ATID" instead of "ID" to make this clear. The trAtbBridgeID should be WARL instead of RW to support static/fixed ID implemntation. The reset value for trAtbBridgeID is stated as 0 - which is prohibited ATID per the description.
Chapter 10 - trAtbBridgeAsyncFreq - Is this configuration optional if SYNCREQ is implemented?
Section 11.2 requires saving the tr??Control registers after placing the component in reset. Was this intended to be after the reset is de-asserted? The rest of the text "it may prevent a trace tool to do read-modify operation later" needs more explanation.
Section 11.4 claims that version 0 implies an original version of this specification. The text in this section is misleading as there is no RISC-V ratified version of trace control specification. While there may be many proprietary/custom implementations claiming version 0 being a legacy version of this specification is misleading.

Feedback on RISC-V N-Trace Specification 1.0.0 rc2.0 - Chapter 5

IEEE-5001 defines the two modes but it is not mentioned
that RISC-V N-trace also defines these two modes. Suggest
"RISC-V N-trace, like IEEE-5001 Next Standard, defines..."
Traditional messages is used only here. What is a traditional
message?

Editorial:

"every taken .." ->
"Each taken direct conditional branch generates a minimum
two-byte message. However, repeated branches can be
aggregated and reported as a single message with a count,
rather than numerous identical messages. This document
refers to this mode as BTM (Branch Trace Messaging)."
"every direct.."->
"Every direct conditional branch, whether taken or not-taken,
contributes a single bit to the history buffer, significantly
enhancing efficiency. This document designates this mode as
HTM (History Trace Messaging)."

Feedback on chapter 3 - RISC-V N-Trace (Nexus-based Trace) Specification

This issue summarizes the feedback and comments from AR on Chapter 3.

The specification states that 38 bytes is the maximum message size. It was
not apparent that the protocol itself has such a limit on the maximum message
size. The last row of the comparison table should be updated to state
"In this version of the specification is 38 bytes (sum of all field lengths)"
instead of "38 bytes (worst sum of all fields" - which would imply a
inflexible upper bound fixed at 38 bytes. Same comment about the following
sentence - "Max message size (38 bytes) " should be reworded as

"The maximum message size of 38 bytes in this version of the specification is
to transmit the IndirectBranchHistSync message which includes TCODE/ SRC/
SYNC/ B-TYPE (5 bytes total), I-CNT(30 bits, 5 bytes), F-ADDR(63 bits, 11 bytes),
HIST(32 bits, 6 bytes), and TSTAMP(64 bits, 11 bytes).

Did not include "worst" as assumption was that it just means maximum size. If
there is a different meaning please clarify.
Reword "Particular hardware ...."
"While implementations may have a shorter maximum message size (e.g, due to I-CNT
being smaller), they must assure that the internal FIFOs are designed to hold at
least two maximum sized messages that the implementation can produce."
This does not make assumptions about the implementation being hardware or
firmware or a combination of both.
Reword "Decoding software..." for better clarity
"While decoding software may be designed to avoid dynamic memory allocation, it
must nonetheless be robust enough to handle messages of any size. This is to
account for scenarios where trace memory could be corrupted, such as a trace
consisting entirely of zeros, which could be interpreted as an unusually long
variable-length field."
Or alternatively error out.
Section 3.1, "on LSB part of each byte" -> "located in the least signficant
bits of each byte"
Section 3.1 Second sub-bullet of second bullet should be split into two. The
sub-bullet should only describe the non-last bytes of a variable length field.
A separate main bullet created for fixed-length fields.

The statement "initial parts of longer variable" may be misread to imply that
the rule applies to only some initial bytes. Its better to write it as "The
non-last bytes of a variable-length field are indicated by MSEO[1:0]=00"
Section 3.1 "The last byte of a variable-length.." - should be written to
"The last byte of a variable-length field, that is not the last byte of a
message, is indicated by MSEO[1:0]=01". Else it contradicts a bit with the
later bullets. Similar wording can be used for the fixed-length fields rule.
Section 3.1 - "The last byte of a message..." - there is a stray ** in the
text.
Table 5 - these transitions are not factoring in the Last byte of variable-length
field also being last byte of message i.e. 00-11 (or 01) should be a end of
variable length message.
Table 5 - Since the last byte of variable length message must be 11 if it is
also last byte of message, the 01-11-(more 11s) transition is not allowed for
End of message. Should it not just say "00-11-(more 11s)"
Table 5 - Idle is stated as 11s. It should be 11-11s?
Tabel 5 - message transmission can also see 00-01-00 and not just 00s
Section 3.1 - is there a unstated rule that first field of message cannot be a
6-bit variable-length field? If that is allowed then Start of message could
also be 11s->00(or 01)? If it is disallowed then should state that rule.
Since the N-trace reproduces/copies parts of the IEEE-ISTO 5001-2012 - like
section 3.1 - the N-trace specifcation should include copyright notice and
the rules in the copyright are followed. Either please remove the
reproductions or ensure that the document is compliant with the IEEE
copyright rules.
Section 3.3 - "Idle state must be...all MSEO..." -> "...all 1s MSEO..."
Section 3.3 -> do not split "The following 4 example sequences:" and "are
all valid"
The message transmission protocol of N-trace is different than that of Nexus
Adding a diagram along the lines of Figure 5-2 of Nexust specification is
good to highlight the divergence.
"LSB bits" is redundant. Better to just say "least significant bits"
"N-trace messages transmission protocol..." -> "N-trace message transmission
protocol..." ->
What does "each LSB first" mean?
Table 5 - This table is confusing. I guess it is only listing the MSEO bits
of each byte. Term "Message transmission" should be clarified.
Section 3.2 : ALWAYS -> always
Section 3.3 - All of this seems implicit from prior rules. Maybe just state
the first bullet as a sentence in an earlier section, and remove this section?
Section 3.4 - Should this entire section be non-normative?
Table 6 - ICNT[10]=0x7D - Re-phrase the description.
Table 6 - Rephrase "Here we could have TSTAMP..."

Feedback on chapter 4 - RISC-V N-Trace (Nexus-based Trace) Specification

Since the primary purpose of the table is to show which controls are required and which are optional, the first paragraph should be reworded - specifically "describes fields and bits" as this table is not intended to describe the fields and bits and how they influence the encoder.
"May be hard-coded" - dont need to state this as these are WARL fields.
trTeInstSyncMax - "Controls generation of Sychronization Messages (with SYNC field=2)". The parenthesis is not needed as this only affects periodic synchronization?
trTeProtocolMajor - The second sentence should be removed.
trTeProtocolMinor - The sentence "Different values are..." should be removed. The last sentence should be reworded as the protocol major/minor are not part of messages or the wire protocol. These are trace encoder inputs. The verbiage implies that tools will have these trace controls as input
rows marked as "Future" should be marked as Not applicable and set to 0 since this is a specification for this version of the specification.
Why is trTsEnable in a informative note and not in the table? What is treatment for other trTs* fields?
Instead of repeating "See RISC-V Trace Control Interface Specification" - this can be in the leading paragraph. The cell can be left empty if there is no specific comment to be made. Alternatively only the rows that have N-trace specific clarification can be listed and rest referred to the control interface specification without repeating them here. This would also avoid ambiguity about fields left out of this table.
A one bit field is also a field. Does the bit/field need to be differentiated?

riscv-non-isa / tg-nexus-trace Goto Github PK

tg-nexus-trace's Introduction

TG RISC-V Nexus Trace

Status of each PDF

TODO (before official freeze)

TODO (after ratification approval)

Repository Overview

Initial Work (Preserved)

Additional Resources

Documentation generator

Dependencies

Cloning the project

tg-nexus-trace's People

Contributors

Stargazers

Watchers

Forkers

tg-nexus-trace's Issues

Recommend Projects

Recommend Topics

Recommend Org