Re: [PATCH] Add performance monitoring unit extension
I think there is some confusion here.
The “counter_info” does not related to the mhpmevent CSR value. Rather, the “counter_info” only provides more details about a counter (hardware/software).
Only “event_idx” and “event_info” decide the value to be programmed in the mhpmevent CSR.
From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Greg Favor
Sent: 30 October 2020 05:58
To: Sean Anderson <seanga2@...>; Anup Patel <Anup.Patel@...>; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>; Al Stone <ahs3@...>; Tommy Thorn <tommy.thorn@...>
Cc: Greg Favor <gfavor@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add performance monitoring unit extension
I want to ask or encourage that this not be done. Here's why:
It will be desirable for new CPU implementations to adopt a mhpmevent CSR format that matches what is naturally coming through OpenSBI - instead of each having their own special formats. This way there can be a default code path in OpenSBI that simply writes this 20 bits of event_idx.type/code straight into the low 20 bits of an mhpmevent CSR, and all implementations that adopt this format will not need to provide implementation-specific code to support recoding and reformatting this event_idx information. OpenSBI won't require implementations to "conform" in this way, but ones that do can benefit from adopting this format.
At the same time, other fields of the mhpmevent CSRs will gradually be architecturally standardized (some in the near future). And in the meantime new implementations will have non-standard fields in these CSRs (which will be usable through OpenSBI as part of RAW events).
My point being that it would be cleanest for these 20 bits of event_idx.type/code to stay packed together and in the low 20 bits - which by default would translate into being written into the low 20 bits of an mhpmevent CSR. For RAW events OpenSBI supports the upper XLEN-20 also being specifiable and these would be passed through and, by default, written into the upper XLEN-20 bits of mhpmevent. All making for a clean simple picture for hardware and software where implementation-specific OpenSBI PMU code can be avoided for implementations that choose to "conform".
P.S. As a CPU implementer, this is exactly what we want to be doing. In lieu of an existing official architecture standard for this part of the information that wants to go into an mhpmevent CSR, OpenSBI conveniently provides the basis for what can become a de facto (or eventual official) standard. And as other fields of mhpmevent become architecturally standardized, then OpenSBI can explicitly support those for non-RAW events as well as implicitly for RAW events. (Further, new architecturally standardized fields will mindfully avoid being positioned in the low 20 bits of mhpmevent. They will probably roughly start from the "top" of the CSR and work they way down towards the low 20 bits over time.)