| Message ID | 20240109002554.646572-6-atishp@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add ISA extension smcntrpmf support | expand |
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? Alistair > + /* Track cycle and icount for each privilege mode when V = 1*/ > + uint64_t counter_virt[2]; > + uint64_t counter_virt_prev[2]; > +} PMUFixedCtrState; > + > struct CPUArchState { > target_ulong gpr[32]; > target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */ > @@ -334,6 +343,8 @@ struct CPUArchState { > /* PMU event selector configured values for RV32 */ > target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS]; > > + PMUFixedCtrState pmu_fixed_ctrs[2]; > + > target_ulong sscratch; > target_ulong mscratch; > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index e7e23b34f455..3dddb1b433e8 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) > { > g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED); > > - if (icount_enabled() && newpriv != env->priv) { > - riscv_itrigger_update_priv(env); > + if (newpriv != env->priv) { > + if (icount_enabled()) { > + riscv_itrigger_update_priv(env); > + riscv_pmu_icount_update_priv(env, newpriv); > + } else { > + riscv_pmu_cycle_update_priv(env, newpriv); > + } > } > /* tlb_flush is unnecessary as mode is contained in mmu_idx */ > env->priv = newpriv; > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 3bd4aa22374f..307d052021c5 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int csrno, target_ulong val) > return RISCV_EXCP_NONE; > } > > +#if defined(CONFIG_USER_ONLY) > /* User Timers and Counters */ > static target_ulong get_ticks(bool shift) > { > - int64_t val; > - target_ulong result; > - > -#if !defined(CONFIG_USER_ONLY) > - if (icount_enabled()) { > - val = icount_get(); > - } else { > - val = cpu_get_host_ticks(); > - } > -#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; > } > > -#if defined(CONFIG_USER_ONLY) > static RISCVException read_time(CPURISCVState *env, int csrno, > target_ulong *val) > { > @@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, int csrno, target_ulong val) > return RISCV_EXCP_NONE; > } > > +static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env, > + int counter_idx, > + bool upper_half) > +{ > + uint64_t curr_val = 0; > + target_ulong result = 0; > + uint64_t *counter_arr = icount_enabled() ? env->pmu_fixed_ctrs[1].counter : > + env->pmu_fixed_ctrs[0].counter; > + uint64_t *counter_arr_virt = icount_enabled() ? > + env->pmu_fixed_ctrs[1].counter_virt : > + env->pmu_fixed_ctrs[0].counter_virt; > + uint64_t cfg_val = 0; > + > + if (counter_idx == 0) { > + cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) : > + env->mcyclecfg; > + } else if (counter_idx == 2) { > + cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) : > + env->minstretcfg; > + } else { > + cfg_val = upper_half ? > + ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) : > + env->mhpmevent_val[counter_idx]; > + } > + > + if (!cfg_val) { > + if (icount_enabled()) { > + curr_val = icount_get_raw(); > + } else { > + curr_val = cpu_get_host_ticks(); > + } > + goto done; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_MINH)) { > + curr_val += counter_arr[PRV_M]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_SINH)) { > + curr_val += counter_arr[PRV_S]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_UINH)) { > + curr_val += counter_arr[PRV_U]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_VSINH)) { > + curr_val += counter_arr_virt[PRV_S]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_VUINH)) { > + curr_val += counter_arr_virt[PRV_U]; > + } > + > +done: > + if (riscv_cpu_mxl(env) == MXL_RV32) { > + result = upper_half ? curr_val >> 32 : curr_val; > + } else { > + result = curr_val; > + } > + > + return result; > +} > + > static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > { > int ctr_idx = csrno - CSR_MCYCLE; > @@ -941,7 +989,8 @@ static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > counter->mhpmcounter_val = val; > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - counter->mhpmcounter_prev = get_ticks(false); > + counter->mhpmcounter_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > + ctr_idx, false); > if (ctr_idx > 2) { > if (riscv_cpu_mxl(env) == MXL_RV32) { > mhpmctr_val = mhpmctr_val | > @@ -968,7 +1017,8 @@ static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val) > mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32); > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - counter->mhpmcounterh_prev = get_ticks(true); > + counter->mhpmcounterh_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > + ctr_idx, true); > if (ctr_idx > 2) { > riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx); > } > @@ -1009,7 +1059,8 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val, > */ > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - *val = get_ticks(upper_half) - ctr_prev + ctr_val; > + *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx, upper_half) - > + ctr_prev + ctr_val; > } else { > *val = ctr_val; > } > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > index 0e7d58b8a5c2..8b6cc4c6bb4d 100644 > --- a/target/riscv/pmu.c > +++ b/target/riscv/pmu.c > @@ -19,6 +19,7 @@ > #include "qemu/osdep.h" > #include "qemu/log.h" > #include "qemu/error-report.h" > +#include "qemu/timer.h" > #include "cpu.h" > #include "pmu.h" > #include "sysemu/cpu-timers.h" > @@ -176,6 +177,48 @@ static int riscv_pmu_incr_ctr_rv64(RISCVCPU *cpu, uint32_t ctr_idx) > return 0; > } > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv) > +{ > + uint64_t delta; > + uint64_t *counter_arr; > + uint64_t *counter_arr_prev; > + uint64_t current_icount = icount_get_raw(); > + > + if (env->virt_enabled) { > + counter_arr = env->pmu_fixed_ctrs[1].counter_virt; > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_virt_prev; > + } else { > + counter_arr = env->pmu_fixed_ctrs[1].counter; > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_prev; > + } > + > + counter_arr_prev[newpriv] = current_icount; > + delta = current_icount - counter_arr_prev[env->priv]; > + > + counter_arr[env->priv] += delta; > +} > + > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv) > +{ > + uint64_t delta; > + uint64_t *counter_arr; > + uint64_t *counter_arr_prev; > + uint64_t current_host_ticks = cpu_get_host_ticks(); > + > + if (env->virt_enabled) { > + counter_arr = env->pmu_fixed_ctrs[0].counter_virt; > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_virt_prev; > + } else { > + counter_arr = env->pmu_fixed_ctrs[0].counter; > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_prev; > + } > + > + counter_arr_prev[newpriv] = current_host_ticks; > + delta = current_host_ticks - counter_arr_prev[env->priv]; > + > + counter_arr[env->priv] += delta; > +} > + > int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx) > { > uint32_t ctr_idx; > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > index 505fc850d38e..50de6031a730 100644 > --- a/target/riscv/pmu.h > +++ b/target/riscv/pmu.h > @@ -31,3 +31,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx); > void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name); > int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, > uint32_t ctr_idx); > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv); > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv); > -- > 2.34.1 > >
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. Did I misunderstand your question ? > Alistair > > > + /* Track cycle and icount for each privilege mode when V = 1*/ > > + uint64_t counter_virt[2]; > > + uint64_t counter_virt_prev[2]; > > +} PMUFixedCtrState; > > + > > struct CPUArchState { > > target_ulong gpr[32]; > > target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */ > > @@ -334,6 +343,8 @@ struct CPUArchState { > > /* PMU event selector configured values for RV32 */ > > target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS]; > > > > + PMUFixedCtrState pmu_fixed_ctrs[2]; > > + > > target_ulong sscratch; > > target_ulong mscratch; > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > index e7e23b34f455..3dddb1b433e8 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) > > { > > g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED); > > > > - if (icount_enabled() && newpriv != env->priv) { > > - riscv_itrigger_update_priv(env); > > + if (newpriv != env->priv) { > > + if (icount_enabled()) { > > + riscv_itrigger_update_priv(env); > > + riscv_pmu_icount_update_priv(env, newpriv); > > + } else { > > + riscv_pmu_cycle_update_priv(env, newpriv); > > + } > > } > > /* tlb_flush is unnecessary as mode is contained in mmu_idx */ > > env->priv = newpriv; > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 3bd4aa22374f..307d052021c5 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int csrno, target_ulong val) > > return RISCV_EXCP_NONE; > > } > > > > +#if defined(CONFIG_USER_ONLY) > > /* User Timers and Counters */ > > static target_ulong get_ticks(bool shift) > > { > > - int64_t val; > > - target_ulong result; > > - > > -#if !defined(CONFIG_USER_ONLY) > > - if (icount_enabled()) { > > - val = icount_get(); > > - } else { > > - val = cpu_get_host_ticks(); > > - } > > -#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; > > } > > > > -#if defined(CONFIG_USER_ONLY) > > static RISCVException read_time(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > > @@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, int csrno, target_ulong val) > > return RISCV_EXCP_NONE; > > } > > > > +static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env, > > + int counter_idx, > > + bool upper_half) > > +{ > > + uint64_t curr_val = 0; > > + target_ulong result = 0; > > + uint64_t *counter_arr = icount_enabled() ? env->pmu_fixed_ctrs[1].counter : > > + env->pmu_fixed_ctrs[0].counter; > > + uint64_t *counter_arr_virt = icount_enabled() ? > > + env->pmu_fixed_ctrs[1].counter_virt : > > + env->pmu_fixed_ctrs[0].counter_virt; > > + uint64_t cfg_val = 0; > > + > > + if (counter_idx == 0) { > > + cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) : > > + env->mcyclecfg; > > + } else if (counter_idx == 2) { > > + cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) : > > + env->minstretcfg; > > + } else { > > + cfg_val = upper_half ? > > + ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) : > > + env->mhpmevent_val[counter_idx]; > > + } > > + > > + if (!cfg_val) { > > + if (icount_enabled()) { > > + curr_val = icount_get_raw(); > > + } else { > > + curr_val = cpu_get_host_ticks(); > > + } > > + goto done; > > + } > > + > > + if (!(cfg_val & MCYCLECFG_BIT_MINH)) { > > + curr_val += counter_arr[PRV_M]; > > + } > > + > > + if (!(cfg_val & MCYCLECFG_BIT_SINH)) { > > + curr_val += counter_arr[PRV_S]; > > + } > > + > > + if (!(cfg_val & MCYCLECFG_BIT_UINH)) { > > + curr_val += counter_arr[PRV_U]; > > + } > > + > > + if (!(cfg_val & MCYCLECFG_BIT_VSINH)) { > > + curr_val += counter_arr_virt[PRV_S]; > > + } > > + > > + if (!(cfg_val & MCYCLECFG_BIT_VUINH)) { > > + curr_val += counter_arr_virt[PRV_U]; > > + } > > + > > +done: > > + if (riscv_cpu_mxl(env) == MXL_RV32) { > > + result = upper_half ? curr_val >> 32 : curr_val; > > + } else { > > + result = curr_val; > > + } > > + > > + return result; > > +} > > + > > static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > > { > > int ctr_idx = csrno - CSR_MCYCLE; > > @@ -941,7 +989,8 @@ static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > > counter->mhpmcounter_val = val; > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > - counter->mhpmcounter_prev = get_ticks(false); > > + counter->mhpmcounter_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > > + ctr_idx, false); > > if (ctr_idx > 2) { > > if (riscv_cpu_mxl(env) == MXL_RV32) { > > mhpmctr_val = mhpmctr_val | > > @@ -968,7 +1017,8 @@ static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val) > > mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32); > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > - counter->mhpmcounterh_prev = get_ticks(true); > > + counter->mhpmcounterh_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > > + ctr_idx, true); > > if (ctr_idx > 2) { > > riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx); > > } > > @@ -1009,7 +1059,8 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val, > > */ > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > - *val = get_ticks(upper_half) - ctr_prev + ctr_val; > > + *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx, upper_half) - > > + ctr_prev + ctr_val; > > } else { > > *val = ctr_val; > > } > > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > > index 0e7d58b8a5c2..8b6cc4c6bb4d 100644 > > --- a/target/riscv/pmu.c > > +++ b/target/riscv/pmu.c > > @@ -19,6 +19,7 @@ > > #include "qemu/osdep.h" > > #include "qemu/log.h" > > #include "qemu/error-report.h" > > +#include "qemu/timer.h" > > #include "cpu.h" > > #include "pmu.h" > > #include "sysemu/cpu-timers.h" > > @@ -176,6 +177,48 @@ static int riscv_pmu_incr_ctr_rv64(RISCVCPU *cpu, uint32_t ctr_idx) > > return 0; > > } > > > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv) > > +{ > > + uint64_t delta; > > + uint64_t *counter_arr; > > + uint64_t *counter_arr_prev; > > + uint64_t current_icount = icount_get_raw(); > > + > > + if (env->virt_enabled) { > > + counter_arr = env->pmu_fixed_ctrs[1].counter_virt; > > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_virt_prev; > > + } else { > > + counter_arr = env->pmu_fixed_ctrs[1].counter; > > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_prev; > > + } > > + > > + counter_arr_prev[newpriv] = current_icount; > > + delta = current_icount - counter_arr_prev[env->priv]; > > + > > + counter_arr[env->priv] += delta; > > +} > > + > > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv) > > +{ > > + uint64_t delta; > > + uint64_t *counter_arr; > > + uint64_t *counter_arr_prev; > > + uint64_t current_host_ticks = cpu_get_host_ticks(); > > + > > + if (env->virt_enabled) { > > + counter_arr = env->pmu_fixed_ctrs[0].counter_virt; > > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_virt_prev; > > + } else { > > + counter_arr = env->pmu_fixed_ctrs[0].counter; > > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_prev; > > + } > > + > > + counter_arr_prev[newpriv] = current_host_ticks; > > + delta = current_host_ticks - counter_arr_prev[env->priv]; > > + > > + counter_arr[env->priv] += delta; > > +} > > + > > int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx) > > { > > uint32_t ctr_idx; > > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > > index 505fc850d38e..50de6031a730 100644 > > --- a/target/riscv/pmu.h > > +++ b/target/riscv/pmu.h > > @@ -31,3 +31,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx); > > void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name); > > int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, > > uint32_t ctr_idx); > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv); > > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv); > > -- > > 2.34.1 > > > >
On Tue, Jan 23, 2024 at 4:15 PM 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. > > Did I misunderstand your question ? > > ping ? > > Alistair > > > > > + /* Track cycle and icount for each privilege mode when V = 1*/ > > > + uint64_t counter_virt[2]; > > > + uint64_t counter_virt_prev[2]; > > > +} PMUFixedCtrState; > > > + > > > struct CPUArchState { > > > target_ulong gpr[32]; > > > target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */ > > > @@ -334,6 +343,8 @@ struct CPUArchState { > > > /* PMU event selector configured values for RV32 */ > > > target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS]; > > > > > > + PMUFixedCtrState pmu_fixed_ctrs[2]; > > > + > > > target_ulong sscratch; > > > target_ulong mscratch; > > > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > > index e7e23b34f455..3dddb1b433e8 100644 > > > --- a/target/riscv/cpu_helper.c > > > +++ b/target/riscv/cpu_helper.c > > > @@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, > target_ulong newpriv) > > > { > > > g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED); > > > > > > - if (icount_enabled() && newpriv != env->priv) { > > > - riscv_itrigger_update_priv(env); > > > + if (newpriv != env->priv) { > > > + if (icount_enabled()) { > > > + riscv_itrigger_update_priv(env); > > > + riscv_pmu_icount_update_priv(env, newpriv); > > > + } else { > > > + riscv_pmu_cycle_update_priv(env, newpriv); > > > + } > > > } > > > /* tlb_flush is unnecessary as mode is contained in mmu_idx */ > > > env->priv = newpriv; > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > > index 3bd4aa22374f..307d052021c5 100644 > > > --- a/target/riscv/csr.c > > > +++ b/target/riscv/csr.c > > > @@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int > csrno, target_ulong val) > > > return RISCV_EXCP_NONE; > > > } > > > > > > +#if defined(CONFIG_USER_ONLY) > > > /* User Timers and Counters */ > > > static target_ulong get_ticks(bool shift) > > > { > > > - int64_t val; > > > - target_ulong result; > > > - > > > -#if !defined(CONFIG_USER_ONLY) > > > - if (icount_enabled()) { > > > - val = icount_get(); > > > - } else { > > > - val = cpu_get_host_ticks(); > > > - } > > > -#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; > > > } > > > > > > -#if defined(CONFIG_USER_ONLY) > > > static RISCVException read_time(CPURISCVState *env, int csrno, > > > target_ulong *val) > > > { > > > @@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, > int csrno, target_ulong val) > > > return RISCV_EXCP_NONE; > > > } > > > > > > +static target_ulong > riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env, > > > + int > counter_idx, > > > + bool > upper_half) > > > +{ > > > + uint64_t curr_val = 0; > > > + target_ulong result = 0; > > > + uint64_t *counter_arr = icount_enabled() ? > env->pmu_fixed_ctrs[1].counter : > > > + env->pmu_fixed_ctrs[0].counter; > > > + uint64_t *counter_arr_virt = icount_enabled() ? > > > + env->pmu_fixed_ctrs[1].counter_virt : > > > + env->pmu_fixed_ctrs[0].counter_virt; > > > + uint64_t cfg_val = 0; > > > + > > > + if (counter_idx == 0) { > > > + cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) : > > > + env->mcyclecfg; > > > + } else if (counter_idx == 2) { > > > + cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) : > > > + env->minstretcfg; > > > + } else { > > > + cfg_val = upper_half ? > > > + ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) : > > > + env->mhpmevent_val[counter_idx]; > > > + } > > > + > > > + if (!cfg_val) { > > > + if (icount_enabled()) { > > > + curr_val = icount_get_raw(); > > > + } else { > > > + curr_val = cpu_get_host_ticks(); > > > + } > > > + goto done; > > > + } > > > + > > > + if (!(cfg_val & MCYCLECFG_BIT_MINH)) { > > > + curr_val += counter_arr[PRV_M]; > > > + } > > > + > > > + if (!(cfg_val & MCYCLECFG_BIT_SINH)) { > > > + curr_val += counter_arr[PRV_S]; > > > + } > > > + > > > + if (!(cfg_val & MCYCLECFG_BIT_UINH)) { > > > + curr_val += counter_arr[PRV_U]; > > > + } > > > + > > > + if (!(cfg_val & MCYCLECFG_BIT_VSINH)) { > > > + curr_val += counter_arr_virt[PRV_S]; > > > + } > > > + > > > + if (!(cfg_val & MCYCLECFG_BIT_VUINH)) { > > > + curr_val += counter_arr_virt[PRV_U]; > > > + } > > > + > > > +done: > > > + if (riscv_cpu_mxl(env) == MXL_RV32) { > > > + result = upper_half ? curr_val >> 32 : curr_val; > > > + } else { > > > + result = curr_val; > > > + } > > > + > > > + return result; > > > +} > > > + > > > static int write_mhpmcounter(CPURISCVState *env, int csrno, > target_ulong val) > > > { > > > int ctr_idx = csrno - CSR_MCYCLE; > > > @@ -941,7 +989,8 @@ static int write_mhpmcounter(CPURISCVState *env, > int csrno, target_ulong val) > > > counter->mhpmcounter_val = val; > > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > > - counter->mhpmcounter_prev = get_ticks(false); > > > + counter->mhpmcounter_prev = > riscv_pmu_ctr_get_fixed_counters_val(env, > > > + > ctr_idx, false); > > > if (ctr_idx > 2) { > > > if (riscv_cpu_mxl(env) == MXL_RV32) { > > > mhpmctr_val = mhpmctr_val | > > > @@ -968,7 +1017,8 @@ static int write_mhpmcounterh(CPURISCVState *env, > int csrno, target_ulong val) > > > mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32); > > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > > - counter->mhpmcounterh_prev = get_ticks(true); > > > + counter->mhpmcounterh_prev = > riscv_pmu_ctr_get_fixed_counters_val(env, > > > + > ctr_idx, true); > > > if (ctr_idx > 2) { > > > riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx); > > > } > > > @@ -1009,7 +1059,8 @@ static RISCVException > riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val, > > > */ > > > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > > > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > > > - *val = get_ticks(upper_half) - ctr_prev + ctr_val; > > > + *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx, > upper_half) - > > > + ctr_prev + > ctr_val; > > > } else { > > > *val = ctr_val; > > > } > > > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > > > index 0e7d58b8a5c2..8b6cc4c6bb4d 100644 > > > --- a/target/riscv/pmu.c > > > +++ b/target/riscv/pmu.c > > > @@ -19,6 +19,7 @@ > > > #include "qemu/osdep.h" > > > #include "qemu/log.h" > > > #include "qemu/error-report.h" > > > +#include "qemu/timer.h" > > > #include "cpu.h" > > > #include "pmu.h" > > > #include "sysemu/cpu-timers.h" > > > @@ -176,6 +177,48 @@ static int riscv_pmu_incr_ctr_rv64(RISCVCPU *cpu, > uint32_t ctr_idx) > > > return 0; > > > } > > > > > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong > newpriv) > > > +{ > > > + uint64_t delta; > > > + uint64_t *counter_arr; > > > + uint64_t *counter_arr_prev; > > > + uint64_t current_icount = icount_get_raw(); > > > + > > > + if (env->virt_enabled) { > > > + counter_arr = env->pmu_fixed_ctrs[1].counter_virt; > > > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_virt_prev; > > > + } else { > > > + counter_arr = env->pmu_fixed_ctrs[1].counter; > > > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_prev; > > > + } > > > + > > > + counter_arr_prev[newpriv] = current_icount; > > > + delta = current_icount - counter_arr_prev[env->priv]; > > > + > > > + counter_arr[env->priv] += delta; > > > +} > > > + > > > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong > newpriv) > > > +{ > > > + uint64_t delta; > > > + uint64_t *counter_arr; > > > + uint64_t *counter_arr_prev; > > > + uint64_t current_host_ticks = cpu_get_host_ticks(); > > > + > > > + if (env->virt_enabled) { > > > + counter_arr = env->pmu_fixed_ctrs[0].counter_virt; > > > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_virt_prev; > > > + } else { > > > + counter_arr = env->pmu_fixed_ctrs[0].counter; > > > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_prev; > > > + } > > > + > > > + counter_arr_prev[newpriv] = current_host_ticks; > > > + delta = current_host_ticks - counter_arr_prev[env->priv]; > > > + > > > + counter_arr[env->priv] += delta; > > > +} > > > + > > > int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx > event_idx) > > > { > > > uint32_t ctr_idx; > > > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > > > index 505fc850d38e..50de6031a730 100644 > > > --- a/target/riscv/pmu.h > > > +++ b/target/riscv/pmu.h > > > @@ -31,3 +31,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum > riscv_pmu_event_idx event_idx); > > > void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char > *pmu_name); > > > int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, > > > uint32_t ctr_idx); > > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong > newpriv); > > > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong > newpriv); > > > -- > > > 2.34.1 > > > > > > >
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]; > + /* Track cycle and icount for each privilege mode when V = 1*/ > + uint64_t counter_virt[2]; > + uint64_t counter_virt_prev[2]; > +} PMUFixedCtrState; > + > struct CPUArchState { > target_ulong gpr[32]; > target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */ > @@ -334,6 +343,8 @@ struct CPUArchState { > /* PMU event selector configured values for RV32 */ > target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS]; > > + PMUFixedCtrState pmu_fixed_ctrs[2]; > + > target_ulong sscratch; > target_ulong mscratch; > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index e7e23b34f455..3dddb1b433e8 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) > { > g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED); > > - if (icount_enabled() && newpriv != env->priv) { > - riscv_itrigger_update_priv(env); > + if (newpriv != env->priv) { > + if (icount_enabled()) { > + riscv_itrigger_update_priv(env); > + riscv_pmu_icount_update_priv(env, newpriv); > + } else { > + riscv_pmu_cycle_update_priv(env, newpriv); > + } > } > /* tlb_flush is unnecessary as mode is contained in mmu_idx */ > env->priv = newpriv; > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 3bd4aa22374f..307d052021c5 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int csrno, target_ulong val) > return RISCV_EXCP_NONE; > } > > +#if defined(CONFIG_USER_ONLY) > /* User Timers and Counters */ > static target_ulong get_ticks(bool shift) > { > - int64_t val; > - target_ulong result; > - > -#if !defined(CONFIG_USER_ONLY) > - if (icount_enabled()) { > - val = icount_get(); > - } else { > - val = cpu_get_host_ticks(); > - } > -#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; > } > > -#if defined(CONFIG_USER_ONLY) > static RISCVException read_time(CPURISCVState *env, int csrno, > target_ulong *val) > { > @@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, int csrno, target_ulong val) > return RISCV_EXCP_NONE; > } > > +static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env, > + int counter_idx, > + bool upper_half) > +{ > + uint64_t curr_val = 0; > + target_ulong result = 0; > + uint64_t *counter_arr = icount_enabled() ? env->pmu_fixed_ctrs[1].counter : > + env->pmu_fixed_ctrs[0].counter; I don't follow why we access different arrays depending if icount_enabled(). Can we at least comment this? Alistair > + uint64_t *counter_arr_virt = icount_enabled() ? > + env->pmu_fixed_ctrs[1].counter_virt : > + env->pmu_fixed_ctrs[0].counter_virt; > + uint64_t cfg_val = 0; > + > + if (counter_idx == 0) { > + cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) : > + env->mcyclecfg; > + } else if (counter_idx == 2) { > + cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) : > + env->minstretcfg; > + } else { > + cfg_val = upper_half ? > + ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) : > + env->mhpmevent_val[counter_idx]; > + } > + > + if (!cfg_val) { > + if (icount_enabled()) { > + curr_val = icount_get_raw(); > + } else { > + curr_val = cpu_get_host_ticks(); > + } > + goto done; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_MINH)) { > + curr_val += counter_arr[PRV_M]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_SINH)) { > + curr_val += counter_arr[PRV_S]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_UINH)) { > + curr_val += counter_arr[PRV_U]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_VSINH)) { > + curr_val += counter_arr_virt[PRV_S]; > + } > + > + if (!(cfg_val & MCYCLECFG_BIT_VUINH)) { > + curr_val += counter_arr_virt[PRV_U]; > + } > + > +done: > + if (riscv_cpu_mxl(env) == MXL_RV32) { > + result = upper_half ? curr_val >> 32 : curr_val; > + } else { > + result = curr_val; > + } > + > + return result; > +} > + > static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > { > int ctr_idx = csrno - CSR_MCYCLE; > @@ -941,7 +989,8 @@ static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) > counter->mhpmcounter_val = val; > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - counter->mhpmcounter_prev = get_ticks(false); > + counter->mhpmcounter_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > + ctr_idx, false); > if (ctr_idx > 2) { > if (riscv_cpu_mxl(env) == MXL_RV32) { > mhpmctr_val = mhpmctr_val | > @@ -968,7 +1017,8 @@ static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val) > mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32); > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - counter->mhpmcounterh_prev = get_ticks(true); > + counter->mhpmcounterh_prev = riscv_pmu_ctr_get_fixed_counters_val(env, > + ctr_idx, true); > if (ctr_idx > 2) { > riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx); > } > @@ -1009,7 +1059,8 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val, > */ > if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || > riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { > - *val = get_ticks(upper_half) - ctr_prev + ctr_val; > + *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx, upper_half) - > + ctr_prev + ctr_val; > } else { > *val = ctr_val; > } > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > index 0e7d58b8a5c2..8b6cc4c6bb4d 100644 > --- a/target/riscv/pmu.c > +++ b/target/riscv/pmu.c > @@ -19,6 +19,7 @@ > #include "qemu/osdep.h" > #include "qemu/log.h" > #include "qemu/error-report.h" > +#include "qemu/timer.h" > #include "cpu.h" > #include "pmu.h" > #include "sysemu/cpu-timers.h" > @@ -176,6 +177,48 @@ static int riscv_pmu_incr_ctr_rv64(RISCVCPU *cpu, uint32_t ctr_idx) > return 0; > } > > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv) > +{ > + uint64_t delta; > + uint64_t *counter_arr; > + uint64_t *counter_arr_prev; > + uint64_t current_icount = icount_get_raw(); > + > + if (env->virt_enabled) { > + counter_arr = env->pmu_fixed_ctrs[1].counter_virt; > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_virt_prev; > + } else { > + counter_arr = env->pmu_fixed_ctrs[1].counter; > + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_prev; > + } > + > + counter_arr_prev[newpriv] = current_icount; > + delta = current_icount - counter_arr_prev[env->priv]; > + > + counter_arr[env->priv] += delta; > +} > + > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv) > +{ > + uint64_t delta; > + uint64_t *counter_arr; > + uint64_t *counter_arr_prev; > + uint64_t current_host_ticks = cpu_get_host_ticks(); > + > + if (env->virt_enabled) { > + counter_arr = env->pmu_fixed_ctrs[0].counter_virt; > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_virt_prev; > + } else { > + counter_arr = env->pmu_fixed_ctrs[0].counter; > + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_prev; > + } > + > + counter_arr_prev[newpriv] = current_host_ticks; > + delta = current_host_ticks - counter_arr_prev[env->priv]; > + > + counter_arr[env->priv] += delta; > +} > + > int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx) > { > uint32_t ctr_idx; > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > index 505fc850d38e..50de6031a730 100644 > --- a/target/riscv/pmu.h > +++ b/target/riscv/pmu.h > @@ -31,3 +31,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx); > void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name); > int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, > uint32_t ctr_idx); > +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv); > +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv); > -- > 2.34.1 > >
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
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]; + /* Track cycle and icount for each privilege mode when V = 1*/ + uint64_t counter_virt[2]; + uint64_t counter_virt_prev[2]; +} PMUFixedCtrState; + struct CPUArchState { target_ulong gpr[32]; target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */ @@ -334,6 +343,8 @@ struct CPUArchState { /* PMU event selector configured values for RV32 */ target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS]; + PMUFixedCtrState pmu_fixed_ctrs[2]; + target_ulong sscratch; target_ulong mscratch; diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index e7e23b34f455..3dddb1b433e8 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv) { g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED); - if (icount_enabled() && newpriv != env->priv) { - riscv_itrigger_update_priv(env); + if (newpriv != env->priv) { + if (icount_enabled()) { + riscv_itrigger_update_priv(env); + riscv_pmu_icount_update_priv(env, newpriv); + } else { + riscv_pmu_cycle_update_priv(env, newpriv); + } } /* tlb_flush is unnecessary as mode is contained in mmu_idx */ env->priv = newpriv; diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 3bd4aa22374f..307d052021c5 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int csrno, target_ulong val) return RISCV_EXCP_NONE; } +#if defined(CONFIG_USER_ONLY) /* User Timers and Counters */ static target_ulong get_ticks(bool shift) { - int64_t val; - target_ulong result; - -#if !defined(CONFIG_USER_ONLY) - if (icount_enabled()) { - val = icount_get(); - } else { - val = cpu_get_host_ticks(); - } -#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; } -#if defined(CONFIG_USER_ONLY) static RISCVException read_time(CPURISCVState *env, int csrno, target_ulong *val) { @@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, int csrno, target_ulong val) return RISCV_EXCP_NONE; } +static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env, + int counter_idx, + bool upper_half) +{ + uint64_t curr_val = 0; + target_ulong result = 0; + uint64_t *counter_arr = icount_enabled() ? env->pmu_fixed_ctrs[1].counter : + env->pmu_fixed_ctrs[0].counter; + uint64_t *counter_arr_virt = icount_enabled() ? + env->pmu_fixed_ctrs[1].counter_virt : + env->pmu_fixed_ctrs[0].counter_virt; + uint64_t cfg_val = 0; + + if (counter_idx == 0) { + cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) : + env->mcyclecfg; + } else if (counter_idx == 2) { + cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) : + env->minstretcfg; + } else { + cfg_val = upper_half ? + ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) : + env->mhpmevent_val[counter_idx]; + } + + if (!cfg_val) { + if (icount_enabled()) { + curr_val = icount_get_raw(); + } else { + curr_val = cpu_get_host_ticks(); + } + goto done; + } + + if (!(cfg_val & MCYCLECFG_BIT_MINH)) { + curr_val += counter_arr[PRV_M]; + } + + if (!(cfg_val & MCYCLECFG_BIT_SINH)) { + curr_val += counter_arr[PRV_S]; + } + + if (!(cfg_val & MCYCLECFG_BIT_UINH)) { + curr_val += counter_arr[PRV_U]; + } + + if (!(cfg_val & MCYCLECFG_BIT_VSINH)) { + curr_val += counter_arr_virt[PRV_S]; + } + + if (!(cfg_val & MCYCLECFG_BIT_VUINH)) { + curr_val += counter_arr_virt[PRV_U]; + } + +done: + if (riscv_cpu_mxl(env) == MXL_RV32) { + result = upper_half ? curr_val >> 32 : curr_val; + } else { + result = curr_val; + } + + return result; +} + static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) { int ctr_idx = csrno - CSR_MCYCLE; @@ -941,7 +989,8 @@ static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val) counter->mhpmcounter_val = val; if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { - counter->mhpmcounter_prev = get_ticks(false); + counter->mhpmcounter_prev = riscv_pmu_ctr_get_fixed_counters_val(env, + ctr_idx, false); if (ctr_idx > 2) { if (riscv_cpu_mxl(env) == MXL_RV32) { mhpmctr_val = mhpmctr_val | @@ -968,7 +1017,8 @@ static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val) mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32); if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { - counter->mhpmcounterh_prev = get_ticks(true); + counter->mhpmcounterh_prev = riscv_pmu_ctr_get_fixed_counters_val(env, + ctr_idx, true); if (ctr_idx > 2) { riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx); } @@ -1009,7 +1059,8 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val, */ if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) || riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) { - *val = get_ticks(upper_half) - ctr_prev + ctr_val; + *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx, upper_half) - + ctr_prev + ctr_val; } else { *val = ctr_val; } diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c index 0e7d58b8a5c2..8b6cc4c6bb4d 100644 --- a/target/riscv/pmu.c +++ b/target/riscv/pmu.c @@ -19,6 +19,7 @@ #include "qemu/osdep.h" #include "qemu/log.h" #include "qemu/error-report.h" +#include "qemu/timer.h" #include "cpu.h" #include "pmu.h" #include "sysemu/cpu-timers.h" @@ -176,6 +177,48 @@ static int riscv_pmu_incr_ctr_rv64(RISCVCPU *cpu, uint32_t ctr_idx) return 0; } +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv) +{ + uint64_t delta; + uint64_t *counter_arr; + uint64_t *counter_arr_prev; + uint64_t current_icount = icount_get_raw(); + + if (env->virt_enabled) { + counter_arr = env->pmu_fixed_ctrs[1].counter_virt; + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_virt_prev; + } else { + counter_arr = env->pmu_fixed_ctrs[1].counter; + counter_arr_prev = env->pmu_fixed_ctrs[1].counter_prev; + } + + counter_arr_prev[newpriv] = current_icount; + delta = current_icount - counter_arr_prev[env->priv]; + + counter_arr[env->priv] += delta; +} + +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv) +{ + uint64_t delta; + uint64_t *counter_arr; + uint64_t *counter_arr_prev; + uint64_t current_host_ticks = cpu_get_host_ticks(); + + if (env->virt_enabled) { + counter_arr = env->pmu_fixed_ctrs[0].counter_virt; + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_virt_prev; + } else { + counter_arr = env->pmu_fixed_ctrs[0].counter; + counter_arr_prev = env->pmu_fixed_ctrs[0].counter_prev; + } + + counter_arr_prev[newpriv] = current_host_ticks; + delta = current_host_ticks - counter_arr_prev[env->priv]; + + counter_arr[env->priv] += delta; +} + int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx) { uint32_t ctr_idx; diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h index 505fc850d38e..50de6031a730 100644 --- a/target/riscv/pmu.h +++ b/target/riscv/pmu.h @@ -31,3 +31,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx); void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name); int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx); +void riscv_pmu_icount_update_priv(CPURISCVState *env, target_ulong newpriv); +void riscv_pmu_cycle_update_priv(CPURISCVState *env, target_ulong newpriv);