Suggested improvements in text of spec

[rsnikhil]

Here are some suggestions to improve the text in:

Version 1.0.0-rc3, Revised 2024-05-15: Frozen
https://github.com/riscv/riscv-debug-spec/releases/download/1.0.0-rc3/riscv-debug-specification.pdf

• Sec 1.2 Context: Should 'Zicsr' be included in this list?

• Sec 3.7.1.3 Access Memory:
  "... with the exact same memory view and permissions as the selected hart has."

  But a hart often has two views into memory: Instruction view and
  Load/Store/AMO view, which may not be exactly in sync (needing
  FENCE.I).  Suggest that the text clarifies which view it refers to.
  

• Sec 4.7 Halt:
  "2. prv and v are set to reflect the current privilege mode"

  Suggest change to:
  "2. prv and v are set to reflect the current privilege and virtualization mode"
  (This makes the language symmetric with 4.8,
  "2. The current privilege mode and virtualization mode are
  changed to that specified by prv and v.")

• Apx B.2.6: The examples use the Program Buffer to read MSTATUS/F1
  into s0, then finally use an Abstract command to read s0.

  Why would one do this 2-step process, since abstract commands are
  cabable of reading MSTATUS/F1 directly?

  Also, these examples overwrite s0. Would it make more sense to use
  dscratch0? Would be good to clarify.

• Apx B.2.8:
  "(depending on and other system configuration")
  Dodgy English?

[rsnikhil]

Question re. Sec 3.14.2 Debug Module Control (dmcontrol, at 0x10)

On any given write, a debugger may only write 1 to at most one of
the following bits: resumereq, hartreset, ackhavereset,
setresethaltreq, and clrresethaltreq. The others must be written 0."

What happens if the write violates this? For abstract commands there
is an abstractcs[cmderr] for reporting errors, but what happens for
this dmcontrol error?

[rtwfroody]

• Sec 1.2 Context: Should 'Zicsr' be included in this list?

Yes. The spec includes features specific to accessing CSRs, which means it's addressing Zicsr. #1064

• Sec 3.7.1.3 Access Memory:
  "... with the exact same memory view and permissions as the selected hart has."

  But a hart often has two views into memory: Instruction view and
  Load/Store/AMO view, which may not be exactly in sync (needing
  FENCE.I).  Suggest that the text clarifies which view it refers to.
  

I'm not sure if the RISC-V architecture agrees with you. Filed #1065 as a starting point for further discussion.

• Sec 4.7 Halt:
  "2. prv and v are set to reflect the current privilege mode"
  Suggest change to:
  "2. prv and v are set to reflect the current privilege and virtualization mode"

Sounds good. #1066.

To be continued...

[rtwfroody]

• Apx B.2.6: The examples use the Program Buffer to read MSTATUS/F1
  into s0, then finally use an Abstract command to read s0.
  Why would one do this 2-step process, since abstract commands are
  cabable of reading MSTATUS/F1 directly?

Because abstract commands are only required to access GPRs. Accessing CSR/FPRs is optional. #1067

Also, these examples overwrite s0. Would it make more sense to use
dscratch0? Would be good to clarify.

dscratch0 is optional so it might not exist, and abstract command access to CSRs is also optional so even if it does exist it might not be accessible that way.

• Apx B.2.8:
  "(depending on and other system configuration")
  Dodgy English?

Yes. #1068

What happens if the write violates this? For abstract commands there
is an abstractcs[cmderr] for reporting errors, but what happens for
this dmcontrol error?

The behavior is unspecified. The whole document is full of what the debugger must and mustn't do. I don't think it's necessary to state for each one that if the debugger violates that then the behavior is unspecified. (It's literally not in the spec what happens in that case.)

[rsnikhil]

On HALTREQ, the halt actions update DPC, DCSR.CAUSE, DCSR.PRV, DCSR.V. What if current instruction (at which we halt) happens to be a CSRRxx instruction that writes DPC or DCSR? What should be the resulting value of the CSR?

Should the spec clarify this situation?

[pdonahue-ventana]

Speaking about writes to CSRs other than these two: dpc is defined to be "the virtual address of the next instruction to be executed." If the CSR write instruction has already executed then dpc will point to the following instruction (which is the next instruction to be executed). If the CSR write instruction has not already executed then dpc will point to the CSR write instruction (which is the next instruction to be executed). This is exactly like the mepc/sepc/vsepc behavior on an interrupt.

But more specifically about writes to dpc and dcsr, this question is about a situation that cannot happen. If you are not in Debug mode and you try to write dcsr or dpc then it will raise an illegal instruction exception and the value of dpc/dcsr is clear (i.e. they will not be modified by such an attempted write). If you are already in Debug mode then haltreq has no effect because you're already halted.