Anup,

Thanks.  Comments below.

Greg

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR

This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module (e.g. to start counting when a certain instruction executed and to stop counting when another address is executed).  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  Currently cross-trigger capabilities like this aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires. 

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 30 July 2020 12:57
To: alankao <alankao@...>
Cc: tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Alan,

 

I'm fine with taking the lead on this architecture extension.  But it should follow a proper process as directed by the TSC.  Thus far this would mean getting a new TG created or doing something less formally under an existing TG.  But for smaller extension proposals like this there is need for a proper lighter weight and faster process.  Need for this is recognized and I suspect will probably be promulgated by the TSC some time soon.

 

So I suggest we pause for a short bit, and then see if we can follow that expedited process once it is available.  In the meantime I/we can prepare what we can in advance.  (I don't think this will represent a material slow down to getting to a frozen spec and then to ratification.)

 

Greg

 

On Wed, Jul 29, 2020 at 5:24 PM alankao <alankao@...> wrote:

Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan