Merge pull request #65 from riscv-non-isa/dev/beeman/v051-recommendat…

…ions

Recommended text improvements

Makefile

@@ -13,7 +13,7 @@
 # the Doc Template for RISC-V Extensions.
 
 DATE ?= $(shell date +%Y-%m-%d)
-VERSION ?= v0.5.2
+VERSION ?= v0.6.0
 REVMARK ?= Draft
 DOCKER_RUN := docker run --rm -v ${PWD}:/build -w /build \
 riscvintl/riscv-docs-base-container-image:latest

chapter1.adoc

@@ -1,14 +1,9 @@
 [[threatmodel]]
 == External Debug Security Threat model
 
-Modern SoC development consists of several different actors who may not trust each other, resulting in the need to isolate actors’ assets during the development and debugging phases. The current RISC-V Debug specification cite:[dbgspec] grants external debuggers the highest privilege in the system regardless of the privilege level at which the target system is running. This leads to privilege escalation issues when multiple actors are present. 
-
-For example, the owner of a SoC, who needs to debug their M-mode firmware, may be able to use the external debugger to bypass PMP lock (pmpcfg.L=1) and attack Boot ROM (the SoC creator’s asset).
-
-Additionally, RISC-V privilege architecture supports multiple software entities or "supervisor domains" that do not trust each other. The supervisor domains are managed by secure monitor running in M-mode, they are isolated from each other by PMP/IOPMP and they may need different debug policies. The entity that owns secure monitor wants to disable external debug when shipping the secure monitor, however, the entity that owns the supervisor domain needs to enable external debug to develop the supervisor domain. Since the external debugger will be the granted highest privilege in the system, a malicious supervisor domain will be able to compromise M-mode secure monitor with the external debugger.
-
-
-
+Modern SoC development consists of several different actors who may not trust each other, resulting in the need to isolate actors’ assets during the development and debugging phases. The current RISC-V Debug specification cite:[dbgspec] grants external debuggers the highest privilege in the system, regardless of the privilege level at which the target system is running. This leads to privilege escalation issues when multiple actors are present. 
 
+For example, the owner of an SoC, who needs to debug their M-mode firmware, may be able to use the external debugger to bypass PMP lock (pmpcfg.L=1) and attack Boot ROM (the SoC creator’s asset).
 
+Additionally, RISC-V privilege architecture supports multiple software entities, or "supervisor domains," that do not trust each other. The supervisor domains are managed by a secure monitor running in M-mode, are isolated from each other by PMP/IOPMP, and may need different debug policies. The entity that owns the secure monitor wants to disable external debug when shipping the secure monitor; however, the entity that owns the supervisor domain needs to enable external debug to develop the supervisor domain. Since the external debugger will be the granted highest privilege in the system, a malicious supervisor domain will be able to compromise M-mode secure monitor with the external debugger.
 

chapter3.adoc

@@ -1,63 +1,62 @@
 [[dmsext]]
-== Debug Module Security Extension (non-ISA extension)
+== Debug Module Security (non-ISA) Extension
 
-This chapter outlines the security enhancements defined for the Debug Module as non-ISA extension. The debug operations listed below are modified by the non-ISA extension. All features in this chapter must be implemented in Debug Module to achieve external debug security. If any hart in the system implements the Sdsec extension, the Debug Module must also implement the non-ISA extension. The debug operations affected by the non-ISA extension include: 
+This chapter defines the required security enhancements for the Debug Module. The debug operations listed below are modified by the extension. 
 
 * Halt
 * Reset 
 * Keepalive 
 * Abstract commands (Access Register, Quick Access, Access Memory)
 * System bus access
 
+If any hart in the system implements the Sdsec extension, the Debug Module must also implement the Debug Module Security Extension.
+
 === External Debug Security Extensions Discovery 
 
-The ISA and non-ISA external debug security extensions impose security constraints and introduce non-backward-compatible changes. The presence of the extensions can be determined by polling the `allsecured` or/and `anysecured` bits in dmstatus <<regdmstatus>>.  If the field `allsecured` or `anysecured` is set to 1, it represents that all or any selected harts adopt the Sdsec extension. When any hart adopts the Sdsec extension, it indicates the Debug Module implements Debug Module Security Extension as described in this chapter.
+The ISA and non-ISA external debug security extensions impose security constraints and introduce non-backward-compatible changes. The presence of the extensions can be determined by polling the `allsecured` or/and `anysecured` bits in `dmstatus` <<regdmstatus>>.  If the field `allsecured` or `anysecured` is set to 1, it represents that all or any selected harts adopt the Sdsec extension. When any hart adopts the Sdsec extension, it indicates the Debug Module implements Debug Module Security Extension as described in this chapter.
 
 === Halt 
 
-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.
+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 once it has reached a privilege level in which debug is permitted. 
 
 === Reset
 
-The hartreset operation resets selected harts. When M-mode is not allowed to be debugged, the hart will raise a security fault error to Debug Module. The debugger could monitor the error by polling `allsecfault` or/and `anysecfault` in dmstatus. 
+The hartreset operation resets selected harts. When M-mode is not allowed to be debugged, the hart will raise a security fault error to the Debug Module. The debugger could monitor the error by polling `allsecfault` or/and `anysecfault` in `dmstatus`. 
 
-The ndmreset operation is a system-level reset not tied to hart privilege levels and reset the entire system (excluding the Debug Module). It can only be secured by the system. Thus, it must be de-featured. The debugger can determine support for the ndmreset operation by setting the field to 1 and subsequently verifying the returned value upon reading.
+The ndmreset operation is a system-level reset not tied to hart privilege levels and reset the entire system (excluding the Debug Module). Debug Module Security Extension makes ndmreset read-only 0. The debugger can determine support for the ndmreset operation by setting the field to 1 and subsequently verifying the returned value upon reading.
 
 === Keepalive
 
 The keepalive bit serves as an optional request for the hart to remain available for debugging. This bit only takes effect when M-mode is allowed to be debugged; otherwise, the hart behaves as if the bit is not set.
 
 === Abstract Commands 
-The hart's response to abstract commands is detailed in <<sdsecextdbg>>. The following subsection delineates the constraints when the Debug Module issues the abstract commands. 
+The hart's response to abstract commands is detailed in <<sdsecextdbg>>. The following subsection delineates the constraints when the Debug Module issues an abstract command. 
 
 ==== Relaxed Permission Check `relaxedpriv`
 
 The `relaxedpriv` field is hardwired to 0.
 
 ==== Address Translation `aamvirtual`  
 
-The field `aamvirtual` in the command (at 0x17 in the Debug Module) determines whether the Access Memory command uses a physical or virtual address. When an Access Memory command is issued with `aamvirtual`=0, the hart must check whether the physical access is allowed to access memory. The hart responds with an exception to the Debug Module when M-mode is not permitted to debug, `tvm` (in mstatus) is set to 1, and `mode` (in satp) enables any kind of virtual translation. In the event of an exception, the Debug Module set `cmderr` to 3 and clear the data registers to 0.
+The field `aamvirtual` in the command (at 0x17 in the Debug Module) determines whether the Access Memory command uses a physical or virtual address. When an Access Memory command is issued with `aamvirtual`=0, the hart must check whether the physical access is allowed to access memory. The hart responds with an exception to the Debug Module when M-mode is not permitted to debug, `tvm` (in mstatus) is set to 1, and `mode` (in satp) enables any kind of virtual translation. In the event of an exception, the Debug Module set `cmderr` of `abstractcs` (at 0x16 in Debug Module) to 3 and clear the data registers to 0.
 
 ==== Quick Access 
 
-When M-mode debugging is not allowed (`mdbgen`=0) for a hart, any Quick Access operation will be discarded, causing `cmderr` being set to 6.
+When M-mode debugging is not allowed (`mdbgen`=0) for a hart, any Quick Access operation will be discarded, causing `abstractcs.cmderr` to set to 6.
 
 [NOTE] 
 Quick Access abstract commands effect a halt, execution of Program Buffer, and resume of the selected hart. However, it is undesirable for these Quick Access halts to remain pending until debug is allowed, since the debugger blocks while waiting for the Quick Access to complete. Returning an error only for Quick Access commands received when debug is not allowed would require the hart to distinguish between Quick Access halt requests and other halt requests. Because Quick Access is merely an optimized flow and not required for any usage models, it was decided to avoid burdening the hart with extra hardware. Therefore, Quick Access is forbiden when `mdbgen` is 0.
 
 === System Bus Access 
 
-The System Bus Access must be checked by bus initiator protection mechanisms such as IOPMP cite:[iopmp], WorldGuard cite:[worldguard]. The bus protection unit can return error to Debug Module on illegal access, in that case, Debug Module will set `sberror` to 6 (security fault error).
+The System Bus Access must be checked by bus initiator protection mechanisms such as IOPMP cite:[iopmp], WorldGuard cite:[worldguard]. The bus protection unit can return error to Debug Module on illegal access, in that case, Debug Module will set `sberror` of `sbcs` (at 0x38 in Debug Module) to 6 (security fault error).
 
 [NOTE]
 Trusted entities like RoT should configure IOPMP or equivalent protection before granting debug access to M-mode. Similarly, M-mode should apply the protection before enabling supervisor domain debug. 
 
 === Security Fault Error Reporting
 
-A dedicated error code, security fault error (cmderr 6), is included in `cmderr` of `abstractcs` (at 0x16 in Debug Module). Issuance of abstract commands under disallowed circumstance sets `cmderr` to 6. Additionally, the bus security fault error (sberror 6) is introduced in `sberror` of `sbcs` (at 0x38 in Debug Module) to denote errors related to system bus access. 
+A dedicated error code, security fault error (cmderr 6), is included in `cmderr` of `abstractcs`. Issuance of abstract commands under disallowed circumstance sets `cmderr` to 6. Additionally, the bus security fault error (sberror 6) is introduced in `sberror` of `sbcs` to denote errors related to system bus access. 
 
 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.
 

intro.adoc

@@ -1,34 +1,34 @@
 [[intro]]
 == Introduction
-Debugging and tracing are essential for developers to identify and rectify software and hardware issues, optimize performance, and ensure robust system functionality. The debugging and tracing extensions in RISC-V ecosystem play a pivotal role in enabling these capabilities, allowing developers to monitor and control the execution of programs during the development, testing and production phases. However, the current RISC-V Debug and trace specification grants the external debugger highest privilege in the system, regardless of the privilege level at which the target system is running. It leads to privilege escalation issues when multiple actors are present. 
+Debugging and tracing are essential for developers to identify and rectify software and hardware issues, optimize performance, and ensure robust system functionality. The debugging and tracing extensions in the RISC-V ecosystem play a pivotal role in enabling these capabilities, allowing developers to monitor and control the execution of programs during the development, testing and production phases. However, the current RISC-V Debug specification grants the external debugger the highest privilege in the system, regardless of the privilege level at which the target system is running. It leads to privilege escalation issues when multiple actors are present. 
 
 
-This specification defines non-ISA extension <<dmsext, Debug Module Security Extension (non-ISA extension)>> and ISA extension <<Sdsec, Sdsec (ISA extension)>> to address the above security issues in the current _The RISC-V Debug Specification_ cite:[dbgspec] and trace specifications cite:[etrace] cite:[ntrace]. 
+This specification defines <<dmsext, Debug Module Security Extension (non-ISA extension)>> and <<Sdsec, Sdsec (ISA extension)>> to address the above security issues in the current _The RISC-V Debug Specification_ cite:[dbgspec] and trace specifications cite:[etrace] cite:[ntrace]. 
 
-A summary of the changes introduced by _The RISC-V External Debug Security Specification_ follows.:
+A summary of the changes introduced by _The RISC-V External Debug Security Specification_ follows.
 
     - *Per-Hart Debug Control:* Introduce per-hart states to control whether external debug is allowed in M-mode and/or supervisor domains cite:[smmtt].
     - *Per-Hart Trace Control:* Introduce per-hart states to control whether tracing is allowed in M-mode and/or supervisor domains.
     - *Non-secure debug:* Add a non-secure debug state to relax security constraints.
-    - *Debug Mode entry:* External debugger can only halt the hart and enter debug mode when debug is allowed in current privilege mode; all operations are executed with <<dbgaccpriv, debug access privilege>> instead of M-mode privilege.
-    - *Memory Access:* Memory access from a hart’s point of view using the Program Buffer or an Abstract Command must be checked by the hart's memory protection mechanisms as if the hart is running at <<dbgaccpriv, debug access privilege>>; memory access from the Debug Module using System Bus Access must be checked by a system memory protection mechanism, such as IOPMP or WorldGuard.
-    - *Register Access:* Register access using the Program Buffer or an Abstract Command works as if the hart is running in <<dbgaccpriv, debug access privilege>> instead of M-mode privilege. The debug CSRs (`dcsr` and `dpc` ) are shadowed in supervisor domains while Smtdeleg/Sstcfg extensions expose the trigger CSRs to supervisor domains through indirect CSR access. 
-    - *Triggers:* Triggers (with action=1) can only fire or match when external debug is allowed in current privilege.
+    - *Debug Mode entry:* An external debugger can only halt the hart and enter debug mode when debug is allowed in current privilege mode.
+    - *Memory Access:* Memory access from a hart’s point of view, using the Program Buffer or an Abstract Command, must be checked by the hart's memory protection mechanisms as if the hart is running at <<dbgaccpriv, debug access privilege level>>; memory access from the Debug Module using System Bus Access must be checked by a system memory protection mechanism, such as IOPMP or WorldGuard.
+    - *Register Access:* Register access using the Program Buffer or an Abstract Command works as if the hart is running at <<dbgaccpriv, debug access privilege level>> instead of M-mode privilege level. The debug CSRs (`dcsr` and `dpc` ) are shadowed in supervisor domains while Smtdeleg/Sstcfg extensions expose the trigger CSRs to supervisor domains. 
+    - *Triggers:* Triggers (with action=1) can only fire or match when external debug is allowed in the current privilege mode.
 
 === Terminology
 
 [cols="2*"]
 [cols="20%,80%"]
 |=====================================================================================================================================================
-| Abstract command       | A high-level command in Debug Module used to interact with and control harts                                               
+| Abstract command       | A high-level Debug Module operation used to interact with and control harts                                               
 | Debug Access Privilege | The privilege with which an Abstract Command or instructions in the Program Buffer access hardware resources                    
 | Debug Mode             | An additional privilege mode to support off-chip debugging                                                                 
 | Hart                   | A RISC-V hardware thread                                                                                                   
 | IOPMP                  | Input-Output Physical Memory Protection unit                                                                               
 | M-mode                 | The highest privileged mode in the RISC-V privilege model                                                                  
 | PMA                    | Physical Memory Attributes                                                                                                 
 | PMP                    | Physical Memory Protection unit                                                                                            
-| Program buffer         | A buffer in Debug Module to execute arbitrary instructions on a hart                                                       
-| Supervisor domain      | A isolated supervisor execution context defined in RISC-V Supervisor Domains Access Protection cite:[smmtt]                
+| Program buffer         | A mechanism that allows the Debug Module to execute arbitrary instructions on a hart                                                       
+| Supervisor domain      | An isolated supervisor execution context defined in RISC-V Supervisor Domains Access Protection cite:[smmtt]                
 | Trace encoder          | A piece of hardware that takes in instruction execution information from a RISC-V hart and transforms it into trace packets
 |=====================================================================================================================================================