Allow debug module to be at a base address other than 0 #2649
Conversation
|
CSR changes look good. I assume the same debug ROM code copes with either case? |
|
Nevermind, I see, it's different code for the two cases. Makes sense. |
| @@ -9,7 +9,7 @@ import chisel3.util._ | |||
| import freechips.rocketchip.config._ | |||
| import freechips.rocketchip.diplomacy._ | |||
| import freechips.rocketchip.regmapper._ | |||
| import freechips.rocketchip.rocket.Instructions | |||
| import freechips.rocketchip.rocket.{CSRs, Instructions} | |||
There was a problem hiding this comment.
I wonder how much work it would be to move these two helpers to util so that Rocket and Debug can both depend on them, rather than having a circular dependency between these packages?
There was a problem hiding this comment.
The Instructions.scala file that has these objects looks like it used to be generated from the riscv/riscv-opcodes repository. I ran the chisel generator there, but the result was much different from Instructions.scala, so it's clear the two have diverged at some time in the past. The Spike CSR list used by the Debug ROM code is yet another divergent copy of this info. It makes sense to me to have this info in a foundation like riscv-opcodes and generate all uses from there.
| @@ -1579,13 +1643,14 @@ class TLDebugModuleInner(device: Device, getNComponents: () => Int, beatBytes: I | |||
| val commandWrIsAccessRegister = (COMMANDWrData.cmdtype === DebugAbstractCommandType.AccessRegister.id.U) | |||
| val commandRegIsAccessRegister = (COMMANDReg.cmdtype === DebugAbstractCommandType.AccessRegister.id.U) | |||
|
|
|||
| val commandWrIsUnsupported = COMMANDWrEn && !commandWrIsAccessRegister; | |||
| val commandWrIsUnsupported = COMMANDWrEn && !commandWrIsAccessRegister | |||
There was a problem hiding this comment.
a semi-colon?! that must be a @mwachs5 2017 original 😂
| @@ -7,6 +7,7 @@ import Chisel._ | |||
| import Chisel.ImplicitConversions._ | |||
| import chisel3.withClock | |||
| import freechips.rocketchip.config.Parameters | |||
| import freechips.rocketchip.devices.debug.DebugModuleKey | |||
There was a problem hiding this comment.
This is definitely the dark side of the implicit p pattern; we're going to feel the pain if we ever have more than one debug unit and want them to differ according to any of the parameters used here. I am not really sure how to make it diplomatic though, is the only extant direct wire between the core and debug unit that one interrupt line?
There was a problem hiding this comment.
Yes -- a debug interrupt, a TL slave port from cbus, and a TL master port to fbus. I guess if there is more than one DM in the address space, then the one whose interrupt is attached is the one this Tile will use and the debug exception address should be customized to that one. This PR doesn't add to the problem -- before, it was a hardcoded 0x800 which didn't allow multiple DMs either.
Related issue:
Type of change: feature request
Impact: API addition (no impact on existing code)
Development Phase: implementation
Release Notes
A new baseAddress parameter is added to
DebugModuleParams. This defaults to 0 but can be changed to another 4K-aligned address in the config.The
dataaddrfield of DMI registerhartinfoindicates the address of the DATA registers in memory and the debug spec states that these may be accessed usingdataaddroffset fromx0. When the DM is not at zero, this is no longer true. Therefore,hartinfois forced to 0, which the debug spec says means it is nonexistent. This will prevent debuggers from attempting to use the program buffer to directly access DATA.The number of Debug Scratch registers is increased from 1 to 2 when the base address is non-zero. Besides saving
s0to use as a temporary for moving data values, the debug ROM must now save another register which serves as a base pointer for data accesses within the debug module (e.g. FLAGS register).The debug ROM code is changed to accommodate a non-zero base address. There are now two different debug ROMs since the non-zero version assumes
dscratch1exists and has about 20% more code.The constructed opcodes in ABSTRACT in
Debug.scalaare changed to accommodate a non-zero base address when necessary. Basically, DATA(s1) or DATA(s0) is used to load/store registers, depending on which register the abstract command is accessing. s0/s1 is restored to the customer value before ABSTRACT falls through to PROGBUF.Hardcoded debug vector addresses in
CSR.scalanow come fromDebugModuleKeyand reflectbaseAddress. The debug vector address computation is done at elaboration.