Address mismatch when reading the performance counters

[ying27]

The issue comes from the csr_regfile module. This module addresses the CSR registers using the enums riscv::CSR_XXXXX, but in the perf_counters module, the performance counters are addressed using the enums riscv::PERF_XXXXX.

The csr_regfile passes the address to the perf_counters as is, without modifying the actual value (output port called perf_addr_o):

/* csr_regfile.sv */

    // ----------------
    // CSR Read logic
    // ----------------
    always_comb begin : csr_read_process
        ...
        // feed through address of performance counter
        perf_addr_o = csr_addr.address;
    ...

So for example, if we want to read the L1_DCM, the riscv::CSR_L1_DCACHE_MISS value is 12'hC04, while the riscv::PERF_L1_DCACHE_MISS is 1. The way how Ariane handles the performance counters is the following:

/* perf_counters.sv */
   ...
    always_comb begin : perf_counters
        perf_counter_d = perf_counter_q;
        ...
        if (l1_dcache_miss_i)
            perf_counter_d[riscv::PERF_L1_DCACHE_MISS] = perf_counter_q[riscv::PERF_L1_DCACHE_MISS] + 1'b1;
        ...
        // Read Port
        if (!we_i) begin
            data_o = perf_counter_q[addr_i[2:0]];
    ...

When we wanna read a counter, as we are chosing only the 3 last bits from addr_i, its value, which is 12'hC04 is going to be 3'h4, and instead of reading the L1_DCM which are stored in the position 1, we will be reading the counter in the position number 4, which is riscv::PERF_LOAD. Note that the addr_i value comes from the perf_addr_o output port of the csr_regfile module.

An easy solution is to decrease the address in the perf_counters module:

/* perf_counters.sv */
   ...
    always_comb begin : perf_counters
        ...
        // Read Port
        if (!we_i) begin
            data_o = perf_counter_q[addr_i[2:0]-3];
   ...

Or set the correct address value in the csr_regfile module:

/* csr_regfile.sv */

    // ----------------
    // CSR Read logic
    // ----------------
    always_comb begin : csr_read_process
        ...
        // feed through address of performance counter
        perf_addr_o = csr_addr.address - 3;
    ...

[zarubaf]

Mhm, I see - definitely problematic. I think your proposed solution, to adjust the address offset by 3, works well for the moment. I would welcome a merge request ;-)

[msfschaffner]

Thanks for pointing this one out. I pushed a slightly more generic fix that removes some redundant defines.