On Thu, Oct 10, 2024 at 5:44 AM Alexei Filippov
<alexei.filip...@yadro.com> wrote:
>
>
>
> On 10.10.2024 02:09, Atish Patra wrote:
> > Signed-off-by: Atish Patra <ati...@rivosinc.com>
> > ---
> > target/riscv/cpu.h | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 2ac391a7cf74..53426710f73e 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -189,6 +189,28 @@ typedef struct PMUFixedCtrState {
> > uint64_t counter_virt_prev[2];
> > } PMUFixedCtrState;
> >
> > +typedef uint64_t (*PMU_EVENT_CYCLE_FUNC)(RISCVCPU *);
> > +typedef uint64_t (*PMU_EVENT_INSTRET_FUNC)(RISCVCPU *);
> > +typedef uint64_t (*PMU_EVENT_TLB_FUNC)(RISCVCPU *, MMUAccessType
> > access_type);
> > +
> > +typedef struct PMUEventInfo {
> > + /* Event ID (BIT [0:55] valid) */
> > + uint64_t event_id;
> > + /* Supported hpmcounters for this event */
> > + uint32_t counter_mask;
> > + /* Bitmask of valid event bits */
> > + uint64_t event_mask;
> > +} PMUEventInfo;
> > +
> > +typedef struct PMUEventFunc {
> > + /* Get the ID of the event that can monitor cycles */
> > + PMU_EVENT_CYCLE_FUNC get_cycle_id;
> > + /* Get the ID of the event that can monitor cycles */
> > + PMU_EVENT_INSTRET_FUNC get_intstret_id;
> > + /* Get the ID of the event that can monitor TLB events*/
> > + PMU_EVENT_TLB_FUNC get_tlb_access_id;
> Ok, this is kinda interesting decision, but it's not scalable. AFAIU
Yes it is not scalable if there is a need to scale as mentioned earlier.
Do you have any other events that can be emulated in Qemu ?
Having said that, I am okay with single call back though but not too sure
about read/write callback unless there is a use case to support those.
> none spec provide us full enum of existing events. So anytime when
> somebody will try to implement their own pmu events they would have to
> add additional callbacks, and this structure never will be fulled
> properly. And then we ended up with structure 1000+ callback with only 5
> machines wich supports pmu events. I suggest my approach with only
> read/write callbacks, where machine can decide by itself how to handle
> any of machine specific events.
Lot of these events are microarchitectural events which can't be
supported in Qemu.
I don't think it's a good idea at all to add dummy values for all the
events defined in a platform
which is harder to debug for a user.
> > +} PMUEventFunc;
> > +
> > struct CPUArchState {
> > target_ulong gpr[32];
> > target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> > @@ -386,6 +408,9 @@ struct CPUArchState {
> > target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
> >
> > PMUFixedCtrState pmu_fixed_ctrs[2];
> > + PMUEventInfo *pmu_events;
> > + PMUEventFunc pmu_efuncs;
> > + int num_pmu_events;
> >
> > target_ulong sscratch;
> > target_ulong mscratch;
> >
