search

riscv-non-isa / riscv-sbi-doc

Documentation for the RISC-V Supervisor Binary Interface
https://jira.riscv.org/browse/RVG-49
Creative Commons Attribution 4.0 International
337 stars 88 forks source link

About SBI PMU extension reserved 16 bits in mhpmeventX CSR #157

Open dslin1010 opened 1 month ago

dslin1010 commented 1 month ago

Currently, the SBI PMU spec[1] defines the raw event type as follows:

On RISC-V platform with 64 bits wide mhpmeventX CSRs, the event_data configuration (or parameter) should have the 48-bit value to be programmed in the lower 48-bits of mhpmeventX CSR and the SBI implementation shall determine the value to be programmed in the upper 16 bits of mhpmeventX CSR

However, the current OpenSBI PMU implementation for Sscofpmf extension only reserved 8 bits[2] according to the Sscofpmf extension spec[3].

Should we change the SBI PMU raw event type definition as follows?

On RISC-V platform with 64 bits wide mhpmeventX CSRs, the event_data configuration (or parameter) should have the 56-bit value to be programmed in the lower 56-bits of mhpmeventX CSR and the SBI implementation shall determine the value to be programmed in the upper 8 bits of mhpmeventX CSR

Thanks.

[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-pmu.adoc#event-hardware-raw-events-type-2 [2] https://github.com/riscv-software-src/opensbi/commit/df997c6e55fe5940fd035097e6ecb5612aa95b4b [3] https://github.com/riscvarchive/riscv-count-overflow

avpatel commented 1 month ago

If we restrict SBI PMU raw events to 56-bits then it is not fair to RISC-V platforms without Sscofpmf where entire 64-bits of mhpmeventX CSR is programmable.

dslin1010 commented 1 month ago

Hi @avpatel,

Thanks for the explanation.

But it seems the SBI PMU spec[1] defines more restrictions for raw events that can program only 48-bits of mhpmeventX CSR.

On RISC-V platform with 64 bits wide mhpmeventX CSRs, the event_data configuration (or parameter) should have the 48-bit value to be programmed in the lower 48-bits of mhpmeventX CSR and the SBI implementation shall determine the value to be programmed in the upper 16 bits of mhpmeventX CSR

[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-pmu.adoc#event-hardware-raw-events-type-2

avpatel commented 1 month ago

I think we should not change existing semantics Event: Hardware raw events (Type #2) to maintain backward compatibility.

In addition, to the discrepancy which you pointed, the Priv v1.13 specification makes mhpmeventX CSRs as 64-bits wide for both RV32 and RV64.

I think it is better to define new Event: Hardware raw events v2 (Type #3) which reflects the new reality.

@atishp04 Thoughts ??

atishp04 commented 1 month ago

Priv v1.12 had mhpmevent specified as MXLEN 

The event selector CSRs, mhpmevent3–mhpmevent31, are MXLEN-bit WARL
registers that control which event causes the corresponding counter to increment.

Should we make this change after v1.13 is ratified only ? The SBI implementation can conform to raw events v2 (Type #3) and wider hpmevent for RV32 based on the priv version.

dslin1010 commented 2 weeks ago

Hi @atishp04 ,

Can I ask, will we define the new Event: Hardware raw events v2 (Type #3) in the SBI PMU spec? Since the current Linux riscv pmu driver[1] will limit the raw event only can use lower 48-bits in mhpmeventX CSR

Thanks.

[1] https://github.com/torvalds/linux/blob/master/drivers/perf/riscv_pmu_sbi.c#L531

atishp04 commented 2 weeks ago

@dslin1010 : Yes. But we are waiting for confirmation about how many bits are actually reserved now ?

https://github.com/riscv/riscv-isa-manual/issues/1578