pulp-platform / riscv-dbg

RISC-V Debug Support for our PULP RISC-V Cores
Other
198 stars 70 forks source link

Make DM support CPU with bus width different from core data width #83

Open JeanRochCoulon opened 3 years ago

JeanRochCoulon commented 3 years ago

Hi @zarubaf

This PR is relative to the issue #541 of openHWgroup/CVA6

Cheers Jean-Roch

JeanRochCoulon commented 3 years ago

Sorry it is a PR relative to Tag v0.1 and not master !

zarubaf commented 3 years ago

Unfortunately, I am still skeptical about the solution here. The debug module should, ideally, support all variations of XLEN and BusWidth. It is unclear to me what happens in your system if there is a 32 and 64-bit core and the bus architecture is 32 bit.

JeanRochCoulon commented 3 years ago

Thank you to challenge the PR, it is good for code quality !!

To my mind, we are facing to 3 different variables: CORE DATA width, CORE BUS width and DTM width So ideally DTM need to handle 8 combinations. We can assume that cases when CORE BUS width is lesser than CORE DATA width are Non Applicable

CORE DATA width | CORE BUS width| DTM width 1) 64 | 64 | 64 => CV64A6, trivial case 2) 32 | 32 | 32 => maybe LowRiscv and Ibex, trivial cases 3) 64 | 32 | 64 => Non Applicable 4) 64 | 32 | 32 => Non Applicable 5) 32 | 64 | 64 => CV32A6 6) 64 | 64 | 32
7) 32 | 32 | 64
8) 32 | 64 | 32

If I remove the trivial and non-applicable cases, it remains 4 cases del cazzo (ask Davide for the italian translation!)

In the current DTM github version, DTM looks at DTM width (buswidth variable) to determine the ISA variant (32 or 64 bits). In that case, 5) 6) and 7) are FAILED. This solution is not acceptable for CV32A6.

In the PR, DTM looks at CORE DATA width, only 6) is FAILED. I think that 6) corresponds to the case you spoke about. On my side, I have never seen such a case. Would it be acceptable according to you to follow the constraint: COREBUS width always equal to DTM width ? If yes, 5), 7) and 8) would be non-applicable.

Kind Regards