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;
+ 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.1On 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 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 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
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
> > >
> > >
>
© 2016 - 2024 Red Hat, Inc.