Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

- Update the trace description by removing the interface signal details #53

Merged
merged 4 commits into from
Aug 13, 2024
Merged

- Update the trace description by removing the interface signal details #53

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6e9b4b
- Update the trace description by removing the interface signal details
AoteJin Aug 5, 2024
8ce33df
- Update the trace and debug control by removing interface description
AoteJin Aug 12, 2024
6882057
- Remove unnecessary denotation i for hart
AoteJin Aug 13, 2024
f713b3c
- update wording
AoteJin Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 5 additions & 18 deletions chapter2.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,33 +92,20 @@ The interrupt can be disabled by `stepie` in the dcsr during single stepping. Wh
When debugging is only allowed for the supervisor domain, M-mode interrupts must not be disabled. Otherwise, debugging might impact the behavior of other parts of the system. For example, if a context switch for the supervisor domain triggered by a timer interrupt is suppressed, some real-time workloads might not be completed on time, resulting in unexpected errors.

=== Trace
When Sdsec is supported, the optional sideband signal to trace encoder, sec_check[i] cite:[etrace], must be implemented for each hart i. The sec_check[i] signal is only cleared when trace is allowed by <<mtrcctl, M-mode trace control>> and/or <<sdtrcctl, supervisor domain trace control>>.
When Sdsec is supported, trace, as a non-intrusive debug method, will be constrained based on RISC-V privilege level. The availability of trace output is indicated through the interface defined in <<<_reference to the trace interface doc_>>> to trace module.

[mtrcctl]
==== M-Mode Trace Control
For each hart i, an input port, mtrcen[i], controls M-mode trace availability. Setting mtrcen[i] to 1 enables M-mode and supervisor domain trace by clearing the sec_check[i] signal to 0 across all privilege levels. Conversely, if mtrcen[i] is set to 0, the sec_check[i] signal cannot be cleared when the hart i runs in M-mode.
For each hart i, the input port mtrcen[i] controls the availability M-mode tracing. Setting mtrcen[i] to 1 enables trace for both M-mode and the supervisor domain. Conversely, setting mtrcen[i] to 0 disables trace output when hart i is running in M-mode.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mtrcen is another signal. I thought the intent was to remove references to signals and focus on the behavioural requirements (hence removing references to sec_check). How is mtrcen supposed to be controlled? This is what should be described in my view.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec defines the mtrcen to indicate that the M-mode trace capability is granted by higher-privileged entity than M-mode SW (e.g. a fuse or BROM to toggle the signal and lock it). To avoid ambiguity, we explicitly name the signal. I will add a note to describe how it is controlled, as you suggested.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point by Iain. Is there a requirement that the hart implements this mtrcen signal interface, or does there just need to be a way to disable tracing in M-mode? When I look at the RV debug spec, it never specifies a haltreq signal from the DM to the hart, it could instead be transported by a bus request, or something else. Those are implementation details. In this case, mtrcen could be established via a handshake between the hart and the RoT, rather than via a wire routed to each hart, right? Obviously all of this would apply to mdbgen too.

Also, there's a typo:

controls the availability of M-mode tracing

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the intention here is not to define an interface, but rather to explicitly highlight that the debug/trace capability of M-mode is provided by the RoT. I agree that this could potentially be broadcasted via a protocol, depending on the implementation. The key point is to convey this idea in a straightforward manner, similar to how MEIP/SEIP interrupts are described. By using terms like "signal" or "port," the reader can easily visualize the concept.
What do you think about adding a non-normative description to explain how this is controlled and how it might be implemented differently up to the HW choice?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest replacing line 99 with something like:

Each hart will add a new state element, mtrcen, which controls the availability M-mode tracing. mtrcen may be controlled via a new input port to the hart, or by handshake with the system Root of Trust, or by other methods. Setting mtrcen to 1 enables trace for both M-mode and the supervisor domain. Conversely, setting mtrcen to 0 disables trace output when hart i is running in M-mode.

The same could be done for mdbgen. This allows using these m*en names to simplify references to the enables, without appearing to dictate the implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I updated it accordingly in latest commit below.


[NOTE]
For a homogeneous computing system, similar to M-mode debug control, the implementation can consolidate all mtrcen[i] into a single port to constrain trace capability across all harts.
Similar to M-mode debug control, the implementation can choose to group several harts together and use one signal to drive their mtrcen ports or assign each hart a dedicated signal.

[sdtrcctl]
==== Supervisor Domain Trace Control
The Smsdetrc extension introduces `sdetrcalw` field (bit 8) in CSR <<Sdseccsr,msdcfg>> within hart i. The sec_check[i] signal for hart i in supervisor domain is determined by the `sdetrcalw` field and mtrcen[i]. When the logical-OR of `sdetrcalw` and mtrcen[i] is 1, the sec_check[i] signal is cleared while the hart i runs in supervisor domain.
The Smsdetrc extension introduces `sdetrcalw` field (bit 8) in CSR <<Sdseccsr,msdcfg>> within hart i. The trace availability for hart i in supervisor domain is determined by the `sdetrcalw` field and mtrcen[i]. If either `sdetrcalw` or mtrcen[i] is set to 1, the trace output is allowed when hart i runs in the supervisor domain.

When both `sdetrcalw` and mtrcen[i] are set to 0, the sec_check[i] signal cannot be cleared at all.

[[trcctl]]
[options="header"]
.Status of the sec_check[i] sideband signal across privilege levels
|===========================================================
| mtrcen| sdetrcalw| M-mode | Supervisor domain
| 1 | x | sec_check[i] = 0 | sec_check[i] = 0
| 0 | 1 | sec_check[i] = 1 | sec_check[i] = 0
| 0 | 0 | sec_check[i] = 1 | sec_check[i] = 1
|===========================================================

[NOTE]
The sec_check signal serves as an additional signal for the trace module, indicating that trace output is prohibited due to security controls. Functionally, sec_check behaves identically to the halted signal. Both sec_check and halted signals cannot be active simultaneously. Reserved for future applications, the combined state of [sec_check, halted] as 0b11 remains unutilized. In cases where a trace module lacks support for the sec_check signal, the hart may alternatively toggle the halted signal to restrict trace output.
When both `sdetrcalw` and mtrcen[i] are set to 0, trace output is inhibited at all privilege levels.

=== Trigger (Sdtrig)

Expand Down
Binary file modified external-debug-security.pdf
View file
Open in desktop
Binary file not shown.
Loading