[BUG] `minstret` and `mcycle` do not increment in debug mode, while `dcsr.stopcount` is set to 0 (normal mode)

[xiaoweish]

Is there an existing CVA6 bug for this?

• [X] I have searched the existing bug issues

Bug Description

When debug_req_i is asserted and DUT enters into debug mode, minstret and mcycle do not increment in debug mode (while dcsr.stopcount==0), as below figure shows,

riscv-debug-spec says,

    <field name="stopcount" bits="10" access="WARL" reset="Preset">
        <value v="0" name="normal">
        Increment counters as usual.
        </value>

        <value v="1" name="freeze">
        Don't increment any hart-local counters while in Debug Mode or
        on `ebreak` instructions that cause entry into Debug Mode.
        These counters include the `instret` CSR. On single-hart cores
        `cycle` should be stopped, but on multi-hart cores it must keep
        incrementing.
        </value>

        An implementation may hardwire this bit to 0 or 1.
    </field>

also, here

. If {dcsr-stopcount} is 0 then counters continue. If it is 1 then counters are stopped.

So, we can see that current cva6 behavior: minstret and mcycle stopped is conflict with its dcsr.stopcount==0 setting.

[JeanRochCoulon]

Good catch !

[xiaoweish]

Here is my proposed fix:

https://github.com/openhwgroup/cva6/blob/master/core/csr_regfile.sv#L827

    // --------------------
    // Counters
    // --------------------
    cycle_d         = cycle_q;
    instret_d       = instret_q;
-    if (!debug_mode_q) begin
+    if ( !(debug_mode_q==1'b1 && dcsr_d.stopcount==1'b1) ) begin
      // increase instruction retired counter
      for (int i = 0; i < CVA6Cfg.NrCommitPorts; i++) begin
        if (commit_ack_i[i] && !ex_i.valid && (!CVA6Cfg.PerfCounterEn || (CVA6Cfg.PerfCounterEn && !mcountinhibit_q[2])))
          instret++;
      end
      instret_d = instret;
      // increment the cycle count
      if (!CVA6Cfg.PerfCounterEn || (CVA6Cfg.PerfCounterEn && !mcountinhibit_q[0]))
        cycle_d = cycle_q + 1'b1;
      else cycle_d = cycle_q;
    end

[JeanRochCoulon]

Hello @xiaoweish It is difficult to me saying whether the modification is right. What could be great is to add a debug job to the CI to monitor the regression of debug mode. Do you think this would be possible ?

[xiaoweish]

Hi @JeanRochCoulon , Yes, it would be beneficial to incorporate the debug job into CI once debug_test is implemented. This issue was indeed discovered during the debug_test. Once PR https://github.com/openhwgroup/cva6/pull/2293 is approved, I'll proceed with additional PR(s) for this task.

[JeanRochCoulon]

Let's do as you said :-) The PR will be reviewed begin of next week