openhwgroup / cva6

The CORE-V CVA6 is an Application class 6-stage RISC-V CPU capable of booting Linux
https://docs.openhwgroup.org/projects/cva6-user-manual/
Other
2.14k stars 651 forks source link

Bug : Spike raise an exception on reading some legal CSRs #2198

Closed AyoubJalali closed 1 month ago

AyoubJalali commented 1 month ago

Is there an existing CVA6 bug for this?

Bug Description

Hello, Spike raise exception on reading from hpmcounter3-31h , these CSRs are implemented in CV32A65X and it's RO-ZERO CSRs.

image

MarioOpenHWGroup commented 1 month ago

The zihpm is not enabled as far as I know, we should enable it in spike if we want it

JeanRochCoulon commented 1 month ago

The Zicnt and Zihpm are not supported by 65x, but the CSRs referenced by @AyoubJalali are MHPMEVENT and MPPMCOUNTERS/H at 0xC** addresses (there are not part of the previous listed extensions).

image

zchamski commented 1 month ago

As far as I can tell from reading the RTL, the Zihpm extension is implemented: I see both MHPMCOUNTERn CSRs (range 0xB03..0xB9F) and those without the leading M (range 0xC03..0xC9F), cf. https://github.com/openhwgroup/cva6/blob/master/core/csr_regfile.sv#L669 and ff.

The privilege check on the 0xC.. CSRs says 'no violation' if the privilege mode is M (https://github.com/openhwgroup/cva6/blob/master/core/csr_regfile.sv#L2125).

JeanRochCoulon commented 1 month ago

Zihpm does not correpond to 0xB03..0xB9F and range 0x323..0x33f !! But to 0xC.. addresses. So Zihpm is not supported by 65x. Please @zchamski confirm it.

AyoubJalali commented 1 month ago

The Zicnt and Zihpm are not supported by 65x, but the CSRs referenced by @AyoubJalali are MHPMEVENT and MPPMCOUNTERS/H at 0xC** addresses (there are not part of the previous listed extensions).

image

Just to confirm spike support MHPMEVENT and MPPMCOUNTERS/H, i'm issue talk about hpmcounter[n]3-31h form 0xC03 to 0xC1F and from 0xC83 to 0xC9F

JeanRochCoulon commented 1 month ago

Spike seems aligned with 65x specification, but RTL is not aligned. RTL implements Zihpm, but i should not. RTL need to be fixed.

AyoubJalali commented 1 month ago

so this is an RTL issue @JeanRochCoulon you can confirm this to create an issue in the CVA6 and close this

AyoubJalali commented 1 month ago

Spike seems aligned with 65x specification, but RTL is not aligned. RTL implements Zihpm, but i should not. RTL need to be fixed.

need to add a new extension to condition the RTL Zihpm

JeanRochCoulon commented 1 month ago

This gihub issue is superseeded by #2201