hpcp: explicit CSR write to minstret does not cancel the in-flight retire increment

[areddyTT]

Summary

A write to minstret (and, by the same logic, mcycle / mhpmcnt*) is followed one
cycle later by the retire increment belonging to the writing instruction, so the counter
ends up one higher than architecturally required.

The RISC-V privileged spec requires the explicit write to take the place of the
side-effect update:

Some CSRs, such as the instructions-retired counter, instret, may be modified as side
effects of instruction execution. In these cases, if a CSR access instruction reads a
CSR, it reads the value prior to the execution of the instruction. If a CSR access
instruction writes such a CSR, the explicit write is done instead of the update from the
side effect. In particular, a value written to instret by one instruction will be the
value read by the following instruction.

Cause

C910_RTL_FACTORY/gen_rtl/pmu/rtl/ct_hpcp_cnt.v

The retire count is registered before use:

else if (cnt_en) begin
    cnt_en_ff         <= cnt_en;
    cnt_adder_ff[3:0] <= cnt_adder[3:0];   // delayed one cycle
end

but the counter update gives priority to cnt_wen only within the same cycle:

if      (cnt_wen)                                        counter <= hpcp_wdata[63:0];
else if (cnt_en_ff && hpcp_cnt_en && (|cnt_adder_ff))    counter <= counter_adder[63:0];
else                                                     counter <= counter[63:0];

The increment produced by the writing instruction's own retirement is still sitting in
cnt_adder_ff when cnt_wen fires, so it is applied on the following cycle, on top of the
value just written.

cycle N     csrrw minstret,0 retires -> cnt_adder=1 latched into cnt_adder_ff
            cnt_wen asserts          -> counter <= 0
cycle N+1   cnt_adder_ff still 1     -> counter <= 0 + 1 = 1     <-- should not happen
later       next instruction retires -> counter <= 2
            csrrs minstret reads 2                                <-- spec requires 1

Reproduction

    csrrw x0, minstret, x0     # write 0
    csrrs x10, mcycle,  x0     # exactly one instruction retires here
    csrrs x10, minstret, x0    # reads 2; per spec should read 1

Observed on commit b91c909 while running the core in instruction-by-instruction lockstep
against a reference ISS. The reference model reads 1, the RTL reads 2. minstret_adder in
ct_hpcp_top.v is itself correct -- it sums each retire packet's instruction count gated by
!split, so this is not micro-op over-counting.

Related

PR #54 ("hpcp: gate counter enables on combinational cnt_mode_dis_pre") fixes a similar
one-cycle-lag problem in the same block, but a different one: it gates the counter enables
on privilege-mode transitions. This report is about the counter update priority on an
explicit write, and is not addressed by that change -- the failure above occurs in M-mode
with no privilege transition anywhere near the affected window.

[abduvTT]

We packaged a reproducer for this in tenstorrent/rv_tester_examples: a directed test (generated by RiescueD) runs on the C910 Verilator model in instruction-by-instruction lockstep against the Whisper ISS.

The test (riescue_test_gen/tests/openc910_issue_69.s) is the sequence from this issue, with a nop as the intervening instruction:

csrw minstret, zero   # explicit write; own retirement must not increment
nop                   # one retirement -> minstret = 1
csrr t0, minstret     # spec: 1.  C910: 2.

To reproduce (needs podman; runs in the public cvm image, first run builds the Verilator model):

git clone https://github.com/tenstorrent/rv_tester_examples && cd rv_tester_examples
./infra/run-bazel.sh test --config=bzlmod \
  //riescue_test_gen:openc910_issue_69_openc910_verilator --test_output=all

Result — the lockstep checker flags the csrr writeback:

Error: Hart 0: Core Arch Checker Mismatch - X5 - csr:minstret DUT: 0x2 ISS: 0x1

The csrw's own retirement increment lands one cycle after the explicit write, matching the cnt_adder_ff analysis above. The same ELF passes on CVA6 and on Whisper standalone.