Merge pull request #62 from riscv-non-isa/rev0.5_feedback

Update the intro and appendix

appendix.adoc

@@ -1,26 +1,32 @@
 [appendix]
 == Theory of Operation 
 
-This chapter explains the theory of operation for the External Debug Security Extension. The subsequent diagram illustrates the reference implementation of security control for the Debug Module and trace encoder, respectively.
+This chapter explains the theory of operation for the External Debug Security Extension. The subsequent diagram illustrates the reference implementation of security control for the debug and trace, respectively.
 
-=== Debug Module security control
+=== Debug Security Control
 
-As outlined in the specification, the security control on the Debug Module can vary for each hart. The dedicated security policy for hart i is enforced by the input port mdbgen[i] and the `sdedbgalw` field inside CSR msdcfg. The security control logic examines all debug operations and triggers (with action=1) firing/matching based on mdbgen[i], `sdedbgalw`, and the privilege level of the hart. The failed action will either be dropped or pending. Additionally, the platform-specific external trigger inputs must obey to platform constraints, which must be carefully handled by platform owner. The mdbgen[i] can be bundled in an MMIO (Memory-Mapped I/O) outside the hart, such as in the Debug Module, or implemented as fuses. 
+As outlined in the specification, the dedicated debug security policy for a hart is enforced by platform state `nsecdbg`, hart state `mdbgen`, and the `sdedbgalw` field inside the `msdcfg` CSR. Both the `nsecdbg` and `mdbgen` states can be accommodated in MMIO outside the harts, such as in the Debug Module registers, or implemented as fuses.  
 
-The privilege level of the hart is determined by code execution, while the debug requests are validated against the privilege level generated by the hart. This process involves two actors, which may lead to a potential Time-of-Check Time-of-Use (TOCTOU) issue. To mitigate this, the implementation must ensure that the inspection and execution of debug requests occur within the same privilege level of the hart. Failure to do so could result in debug requests bypassing access controls intended for higher privilege levels. If the accesses fail the security check, it must prompt an immediate termination of access to prevent any information leakage. 
+The security control logic validates all debug requests and triggers (with action=1) firing/matching based on `nsecdbg`, `mdbgen`, and `sdedbgalw` against the privilege level of the hart. Debug requests that fail validation will either be dropped or kept pending. Additionally, the platform-specific external trigger inputs must obey platform constraints, which must be carefully handled by the platform implementation. 
+
+When `nsecdbg` is set to 0, the validation process involves two actors, which may lead to a potential Time-of-Check Time-of-Use (TOCTOU) issue. To mitigate this, the implementation must ensure that both the validation and execution of debug requests occur under the same privilege level and the same debug security policy. Failing to do so may allow debug requests to bypass security controls. 
+
+[[extdbg]]
+image::external_debug_dm.png[title="The debug security control",align="center"]
 
 When the external debugger is stepping through an instruction that triggers a transition to a higher privilege level, the security control logic must verify against debug capability according to <<dbgpriv>> before entering Debug Mode. If debugging is permitted, the hart re-enters Debug Mode after executing the instruction. Otherwise, the hart continues executing with the pending single step request until it becomes debuggable and can re-enter Debug Mode. In scenarios where multiple supervisor domains are debuggable, the secure monitor in M-mode may switch the context during single stepping. In such cases, the debugger might stop in a different application than the original one. Users of the debugger should be mindful of this possibility.
 
-Application-level debugging is primarily accomplished through self-hosted debugging, allowing the management of debug policies at the supervisor/hypervisor level. As a result, user-level debugging management is not addressed within this extension.
+Application-level debugging is primarily accomplished through self-hosted debugging, allowing the management of debug policies by supervisor domains. As a result, user-level debugging management is not addressed within this extension.
 
-[[extdbg]]
-image::external_debug_dm.png[title="The security control on Debug Module",align="center"]
+=== Trace security control 
+
+Similar to debug security, trace is controlled by platform state `nsecdbg`, hart state `mtrcen`, and `sdetrcalw` in CSR `msdcfg` for each hart. The sec_inhibit sideband signal indicates the availability of trace to the trace encoder.
 
-=== Trace Encoder security control 
+image::external_debug_trace.png[title="The trace security control",align="center"]
 
-Similar to the Debug Module, the trace encoder is controlled by the mtrcen[i] and `sdetrcalw` in CSR msdcfg for each hart i. The halted sideband signal to the trace encoder is determined by <<trcctl>>.
+[appendix]
+== Execution Based Implementation with Sdsec
 
-image::external_debug_trace.png[title="The security control on trace module",align="center"]
+In an execution-based implementation, the code executing the "park loop" can always run with M-mode privilege to access the memory and CSR. However, once execution is dispatched to an Abstract Command or the program buffer, the privilege level for accessing memory and CSR should be restricted to <<dbgaccpriv, debug access privilege>>. 
 
-=== Execution Based Implementation with Sdsec
-<TBD>
+To achieve this, a Debug Mode only state element (e.g., a field in a custom CSR) may be introduced to control the privilege level in Debug Mode. When the state is set to 1, Debug Mode allows M-mode privilege; when cleared to 0, it enforces the <<dbgaccpriv, debug access privilege>>. The hardware sets this state to 1 upon entering the park loop and clears it to 0 by the final instruction of the park loop, right before execution is transferred to an Abstract Command or the program buffer.

chapter2.adoc

@@ -154,7 +154,7 @@ The `sdcsr` register exposes a subset of `dcsr`, formatted as shown in <<sdcsr32
 Unlike `dcsr` and `dpc`, the scratch registers do not have supervisor access, and external debuggers with S-mode privilege cannot not use them as scratch memory.
 
 [caption="Register {counter:rimage}: ", reftext="Register {rimage}"]
-[title="Supervisor debug control and status register (sdcsr) for RV32"]
+[title="Supervisor debug control and status register (sdcsr)"]
 [id=sdcsr32]
 [wavedrom, ,svg]
 ....

chapter3.adoc

@@ -18,7 +18,7 @@ The ISA and non-ISA external debug security extensions impose security constrain
 The halt behavior for a hart is detailed in <<sdsecextdbg>>. According to _The RISC-V Debug Specification_ cite:[dbgspec],  a halt request must be responded within one second. However, this constraint must be removed as the request might be pending due to the situations where debugging is disallowed. In the case of halt-on-reset request, the request is only acknowledged by the hart when it is permitted to debug after the deassertion of reset. Besides, when a Quick Access abstract command is issued to a hart while the hart is not yet allowed to debug, the Quick Access cannot halt the hart, and the Debug Module will receive a security error fault (`cmderr`=6).
 
 [NOTE]
-The halt action in Quick Access is handled differently because other types of halts can be canceled by external debugger when debugging is disallowed, while the Quick Access command can only complete successfully or respond with an error. Otherwise, the debug interface to the selected hart will appear to be hung. Additionally, reset is not always applicable to the hart to recover from a hang situation. To avoid such situations, the halt action in Quick Access must be handled separately by the hart.
+The halt action in Quick Access is handled differently because other types of halts can be canceled by external debugger when debugging is disallowed, while the Quick Access command can only complete successfully or respond with an error. Otherwise, the debug interface to the selected hart will appear to be hung. Additionally, reset is not always applicable to the hart to recover from a hang situation. To avoid such situations, the halt action in Quick Access must be handled separately.
 
 === Reset
 
@@ -61,16 +61,17 @@ A dedicated error code, security fault error (cmderr 6), is included in `cmderr`
 
 The error raised by resethaltreq, reset can be identified through the fields `allsecfault` and `anysecfault` in dmstatus. Error status bits are internally maintained for each hart, with the `allsecfault` and `anysecfault` fields indicating the error status of the currently selected harts. These error statuses are sticky and can only be cleared by writing 1 to `acksecfault` in dmcs2.
 
-=== Non-secure Debug control
+=== Non-secure Debug 
 
 The state element `nsecdbg` is introduced to retain full debugging capabilities, as if the extensions in this specification were not implemented. When `nsecdbg` is set to 1:
 
-* All debug operations are executed with M-mode privilege (equivalent to having `mdbgen` set to 1) for all harts in the system.
+* All <<dbops, debug operations>> are executed with M-mode privilege (equivalent to having `mdbgen` set to 1) for all harts in the system.
 * The ndmreset operation is allowed.
 * The `relaxedpriv` field may be configurable.
 * System Bus Access may bypass the bus initiator protections.
+* Trace output is enabled in all privilege modes.  
 
-[NOTE] During the early stages of a chip's lifecycle, such as when developing the boot process, it is desirable to debug from the initial system state. The `nsecdbg` should only be set to 1 when the entire system is authorized for unrestricted debugging.
+[NOTE] During the early stages of a chip's lifecycle, such as when developing the boot process, it is desirable to debug from the initial system state. The `nsecdbg` should only be set to 1 when the entire system is authorized for unrestricted debugging and tracing.
 
 === Update of Debug Module Registers