On Thu, Aug 21, 2025 at 7:25 PM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> Hi,
>
> (old patch merged as b2d7a7c7e4e30fb5341d38deac968de675f9419c)
>
> On 18/7/24 04:10, Alistair Francis wrote:
> > From: Atish Patra <[email protected]>
> >
> > 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.
> >
> > Co-Developed-by: Rajnesh Kanwal <[email protected]>
> > Signed-off-by: Rajnesh Kanwal <[email protected]>
> > Reviewed-by: Daniel Henrique Barboza <[email protected]>
> > Acked-by: Alistair Francis <[email protected]>
> > Signed-off-by: Atish Patra <[email protected]>
> > Message-ID: <[email protected]>
> > Signed-off-by: Alistair Francis <[email protected]>
> > ---
> >   target/riscv/cpu.h        |  11 ++++
> >   target/riscv/pmu.h        |   2 +
> >   target/riscv/cpu_helper.c |   9 ++-
> >   target/riscv/csr.c        | 117 ++++++++++++++++++++++++++------------
> >   target/riscv/pmu.c        |  92 ++++++++++++++++++++++++++++++
> >   5 files changed, 194 insertions(+), 37 deletions(-)
>
>
> > +#if defined(CONFIG_USER_ONLY)
> >   /* User Timers and Counters */
> > -static target_ulong get_ticks(bool shift, bool instructions)
> > +static target_ulong get_ticks(bool shift)
> >   {
> > -    int64_t val;
> > -    target_ulong result;
> > -
> > -#if !defined(CONFIG_USER_ONLY)
> > -    if (icount_enabled()) {
> > -        if (instructions) {
> > -            val = icount_get_raw();
> > -        } else {
> > -            val = icount_get();
> > -        }
> > -    } else {
> > -        val = cpu_get_host_ticks();
> > -    }
>
> I see this comes from old commit c7b95171881 ("RISC-V: Implement
> modular CSR helper interface"), ...
>
> > -#else
> > -    val = cpu_get_host_ticks();
> > -#endif
> > -
> > -    if (shift) {
> > -        result = val >> 32;
> > -    } else {
> > -        result = val;
> > -    }
> > +    int64_t val = cpu_get_host_ticks();
> > +    target_ulong result = shift ? val >> 32 : val;
> >
> >       return result;
> >   }
>
>
> > +{
> > +    uint64_t *snapshot_prev, *snapshot_new;
> > +    uint64_t current_ticks;
> > +    uint64_t *counter_arr;
> > +    uint64_t delta;
> > +
> > +    if (icount_enabled()) {
> > +        current_ticks = icount_get();
> > +    } else {
> > +        current_ticks = cpu_get_host_ticks();
> > +    }
>
> ... but still I wonder, why not use cpus_get_elapsed_ticks()?

I don't think there is a specific reason, I didn't realised
cpus_get_elapsed_ticks() is the way to go. Should this be changed to
use cpus_get_elapsed_ticks()?

Alistair

>
> Regards,
>
> Phil.
>

Reply via email to