Re: [PATCH v4 5/5] target/riscv: Implement privilege mode filtering for cycle/instret

[Alistair Francis]

On Wed, Jan 24, 2024 at 10:15 AM Atish Kumar Patra <atishp@rivosinc.com> wrote:
>
> On Sun, Jan 21, 2024 at 9:04 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Jan 9, 2024 at 10:29 AM Atish Patra <atishp@rivosinc.com> wrote:
> > >
> > > Privilege mode filtering can also be emulated for cycle/instret by
> > > tracking host_ticks/icount during each privilege mode switch. This
> > > patch implements that for both cycle/instret and mhpmcounters. The
> > > first one requires Smcntrpmf while the other one requires Sscofpmf
> > > to be enabled.
> > >
> > > The cycle/instret are still computed using host ticks when icount
> > > is not enabled. Otherwise, they are computed using raw icount which
> > > is more accurate in icount mode.
> > >
> > > Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > ---
> > >  target/riscv/cpu.h        | 11 +++++
> > >  target/riscv/cpu_helper.c |  9 +++-
> > >  target/riscv/csr.c        | 95 ++++++++++++++++++++++++++++++---------
> > >  target/riscv/pmu.c        | 43 ++++++++++++++++++
> > >  target/riscv/pmu.h        |  2 +
> > >  5 files changed, 136 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > index 34617c4c4bab..40d10726155b 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -136,6 +136,15 @@ typedef struct PMUCTRState {
> > >      target_ulong irq_overflow_left;
> > >  } PMUCTRState;
> > >
> > > +typedef struct PMUFixedCtrState {
> > > +        /* Track cycle and icount for each privilege mode */
> > > +        uint64_t counter[4];
> > > +        uint64_t counter_prev[4];
> >
> > Are these two used?
> >
>
> Yes. That's where it tracks the current/previous value cycle/instret.
> riscv_pmu_icount_update_priv/riscv_pmu_cycle_update_priv
>
> The priv mode based filtering is enabled in 
> riscv_pmu_ctr_get_fixed_counters_val
> using "counter" afterwards.

Ah! Yeah sorry was not reading this correctly

Alistair