Re: [PATCH v4 07/11] target/riscv: Support mcycle/minstret write operation
On Wed, Jan 12, 2022 at 3:58 AM Atish Patra wrote:
>
> On Sun, Jan 9, 2022 at 11:51 PM Bin Meng wrote:
> >
> > On Fri, Jan 7, 2022 at 10:14 AM Atish Patra wrote:
> > >
> > > From: Atish Patra
> > >
> > > mcycle/minstret are actually WARL registers and can be written with any
> > > given value. With SBI PMU extension, it will be used to store a initial
> > > value provided from supervisor OS. The Qemu also need prohibit the counter
> > > increment if mcountinhibit is set.
> > >
> > > Support mcycle/minstret through generic counter infrastructure.
> > >
> > > Signed-off-by: Atish Patra
> > > Signed-off-by: Atish Patra
> > > ---
> > > target/riscv/cpu.h | 24 +--
> > > target/riscv/csr.c | 144 ++-
> > > target/riscv/machine.c | 26 ++-
> > > target/riscv/meson.build | 1 +
> > > target/riscv/pmu.c | 32 +
> > > target/riscv/pmu.h | 28
> > > 6 files changed, 200 insertions(+), 55 deletions(-)
> > > create mode 100644 target/riscv/pmu.c
> > > create mode 100644 target/riscv/pmu.h
> > >
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > index 39edc948d703..5fe9c51b38c7 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -101,7 +101,7 @@ typedef struct CPURISCVState CPURISCVState;
> > > #endif
> > >
> > > #define RV_VLEN_MAX 1024
> > > -#define RV_MAX_MHPMEVENTS 29
> > > +#define RV_MAX_MHPMEVENTS 32
> > > #define RV_MAX_MHPMCOUNTERS 32
> > >
> > > FIELD(VTYPE, VLMUL, 0, 3)
> > > @@ -112,6 +112,19 @@ FIELD(VTYPE, VEDIV, 8, 2)
> > > FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
> > > FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
> > >
> > > +typedef struct PMUCTRState PMUCTRState;
> >
> > This 'typedef' can be merged into the definition below
> >
>
> Sure.
>
> >
> > > +struct PMUCTRState {
> > > +/* Current value of a counter */
> > > +target_ulong mhpmcounter_val;
> > > +/* Current value of a counter in RV32*/
> > > +target_ulong mhpmcounterh_val;
> > > +/* Snapshot values of counter */
> > > +target_ulong mhpmcounter_prev;
> > > +/* Snapshort value of a counter in RV32 */
> > > +target_ulong mhpmcounterh_prev;
> > > +bool started;
> > > +};
> > > +
> > > struct CPURISCVState {
> > > target_ulong gpr[32];
> > > uint64_t fpr[32]; /* assume both F and D extensions */
> > > @@ -226,13 +239,10 @@ struct CPURISCVState {
> > >
> > > target_ulong mcountinhibit;
> > >
> > > -/* PMU counter configured values */
> > > -target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
> > > -
> > > -/* for RV32 */
> > > -target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
> > > +/* PMU counter state */
> > > +PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
> > >
> > > -/* PMU event selector configured values */
> > > +/* PMU event selector configured values. First three are unused*/
> > > target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
> > >
> > > target_ulong sscratch;
> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > index 58a9550bd898..d4449ada557c 100644
> > > --- a/target/riscv/csr.c
> > > +++ b/target/riscv/csr.c
> > > @@ -20,6 +20,7 @@
> > > #include "qemu/osdep.h"
> > > #include "qemu/log.h"
> > > #include "cpu.h"
> > > +#include "pmu.h"
> > > #include "qemu/main-loop.h"
> > > #include "exec/exec-all.h"
> > >
> > > @@ -461,41 +462,33 @@ static int write_vcsr(CPURISCVState *env, int
> > > csrno, target_ulong val)
> > > }
> > >
> > > /* User Timers and Counters */
> > > -static RISCVException read_instret(CPURISCVState *env, int csrno,
> > > - target_ulong *val)
> > > +static target_ulong get_icount_ticks(bool brv32)
> >
> > I would use 'rv32' instead of 'brv32'
> >
>
> ok.
>
> > > {
> > > +int64_t val;
> > > +target_ulong result;
> > > +
> > > #if !defined(CONFIG_USER_ONLY)
> > > if (icount_enabled()) {
> > > -*val = icount_get();
> > > +val = icount_get();
> > > } else {
> > > -*val = cpu_get_host_ticks();
> > > +val = cpu_get_host_ticks();
> > > }
> > > #else
> > > -*val = cpu_get_host_ticks();
> > > +val = cpu_get_host_ticks();
> > > #endif
> > >
> > > -return RISCV_EXCP_NONE;
> > > -}
> > > -
> > > -static RISCVException read_instreth(CPURISCVState *env, int csrno,
> > > -target_ulong *val)
> > > -{
> > > -#if !defined(CONFIG_USER_ONLY)
> > > -if (icount_enabled()) {
> > > -*val = icount_get() >> 32;
> > > +if (brv32) {
> > > +result = val >> 32;
> > > } else {
> > > -*val = cpu_get_host_ticks() >> 32;
> > > +result = val;
> > > }
> > > -#else
> > > -*val = cpu_get_host_ticks() >> 32;
> > > -#endif
> > >
> > > -return RISCV_EXCP_NONE;
> > > +return result;
> > > }
> > >
> > > static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong
> > >
Re: [PATCH v4 07/11] target/riscv: Support mcycle/minstret write operation
On Sun, Jan 9, 2022 at 11:51 PM Bin Meng wrote:
>
> On Fri, Jan 7, 2022 at 10:14 AM Atish Patra wrote:
> >
> > From: Atish Patra
> >
> > mcycle/minstret are actually WARL registers and can be written with any
> > given value. With SBI PMU extension, it will be used to store a initial
> > value provided from supervisor OS. The Qemu also need prohibit the counter
> > increment if mcountinhibit is set.
> >
> > Support mcycle/minstret through generic counter infrastructure.
> >
> > Signed-off-by: Atish Patra
> > Signed-off-by: Atish Patra
> > ---
> > target/riscv/cpu.h | 24 +--
> > target/riscv/csr.c | 144 ++-
> > target/riscv/machine.c | 26 ++-
> > target/riscv/meson.build | 1 +
> > target/riscv/pmu.c | 32 +
> > target/riscv/pmu.h | 28
> > 6 files changed, 200 insertions(+), 55 deletions(-)
> > create mode 100644 target/riscv/pmu.c
> > create mode 100644 target/riscv/pmu.h
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 39edc948d703..5fe9c51b38c7 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -101,7 +101,7 @@ typedef struct CPURISCVState CPURISCVState;
> > #endif
> >
> > #define RV_VLEN_MAX 1024
> > -#define RV_MAX_MHPMEVENTS 29
> > +#define RV_MAX_MHPMEVENTS 32
> > #define RV_MAX_MHPMCOUNTERS 32
> >
> > FIELD(VTYPE, VLMUL, 0, 3)
> > @@ -112,6 +112,19 @@ FIELD(VTYPE, VEDIV, 8, 2)
> > FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
> > FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
> >
> > +typedef struct PMUCTRState PMUCTRState;
>
> This 'typedef' can be merged into the definition below
>
Sure.
>
> > +struct PMUCTRState {
> > +/* Current value of a counter */
> > +target_ulong mhpmcounter_val;
> > +/* Current value of a counter in RV32*/
> > +target_ulong mhpmcounterh_val;
> > +/* Snapshot values of counter */
> > +target_ulong mhpmcounter_prev;
> > +/* Snapshort value of a counter in RV32 */
> > +target_ulong mhpmcounterh_prev;
> > +bool started;
> > +};
> > +
> > struct CPURISCVState {
> > target_ulong gpr[32];
> > uint64_t fpr[32]; /* assume both F and D extensions */
> > @@ -226,13 +239,10 @@ struct CPURISCVState {
> >
> > target_ulong mcountinhibit;
> >
> > -/* PMU counter configured values */
> > -target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
> > -
> > -/* for RV32 */
> > -target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
> > +/* PMU counter state */
> > +PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
> >
> > -/* PMU event selector configured values */
> > +/* PMU event selector configured values. First three are unused*/
> > target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
> >
> > target_ulong sscratch;
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 58a9550bd898..d4449ada557c 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -20,6 +20,7 @@
> > #include "qemu/osdep.h"
> > #include "qemu/log.h"
> > #include "cpu.h"
> > +#include "pmu.h"
> > #include "qemu/main-loop.h"
> > #include "exec/exec-all.h"
> >
> > @@ -461,41 +462,33 @@ static int write_vcsr(CPURISCVState *env, int csrno,
> > target_ulong val)
> > }
> >
> > /* User Timers and Counters */
> > -static RISCVException read_instret(CPURISCVState *env, int csrno,
> > - target_ulong *val)
> > +static target_ulong get_icount_ticks(bool brv32)
>
> I would use 'rv32' instead of 'brv32'
>
ok.
> > {
> > +int64_t val;
> > +target_ulong result;
> > +
> > #if !defined(CONFIG_USER_ONLY)
> > if (icount_enabled()) {
> > -*val = icount_get();
> > +val = icount_get();
> > } else {
> > -*val = cpu_get_host_ticks();
> > +val = cpu_get_host_ticks();
> > }
> > #else
> > -*val = cpu_get_host_ticks();
> > +val = cpu_get_host_ticks();
> > #endif
> >
> > -return RISCV_EXCP_NONE;
> > -}
> > -
> > -static RISCVException read_instreth(CPURISCVState *env, int csrno,
> > -target_ulong *val)
> > -{
> > -#if !defined(CONFIG_USER_ONLY)
> > -if (icount_enabled()) {
> > -*val = icount_get() >> 32;
> > +if (brv32) {
> > +result = val >> 32;
> > } else {
> > -*val = cpu_get_host_ticks() >> 32;
> > +result = val;
> > }
> > -#else
> > -*val = cpu_get_host_ticks() >> 32;
> > -#endif
> >
> > -return RISCV_EXCP_NONE;
> > +return result;
> > }
> >
> > static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
> > {
> > -int evt_index = csrno - CSR_MHPMEVENT3;
> > +int evt_index = csrno - CSR_MCOUNTINHIBIT;
> >
> > *val = env->mhpmevent_val[evt_index];
> >
> > @@ -504,7 +497,7 @@ static int read_mhpmevent(CPURISCVState *env, int
> > csrno, target_ulong *val)
> >
> > static int write_mhpmevent(CPURISCVState *env, int csrno, target_ulong
Re: [PATCH v4 07/11] target/riscv: Support mcycle/minstret write operation
On Fri, Jan 7, 2022 at 10:14 AM Atish Patra wrote:
>
> From: Atish Patra
>
> mcycle/minstret are actually WARL registers and can be written with any
> given value. With SBI PMU extension, it will be used to store a initial
> value provided from supervisor OS. The Qemu also need prohibit the counter
> increment if mcountinhibit is set.
>
> Support mcycle/minstret through generic counter infrastructure.
>
> Signed-off-by: Atish Patra
> Signed-off-by: Atish Patra
> ---
> target/riscv/cpu.h | 24 +--
> target/riscv/csr.c | 144 ++-
> target/riscv/machine.c | 26 ++-
> target/riscv/meson.build | 1 +
> target/riscv/pmu.c | 32 +
> target/riscv/pmu.h | 28
> 6 files changed, 200 insertions(+), 55 deletions(-)
> create mode 100644 target/riscv/pmu.c
> create mode 100644 target/riscv/pmu.h
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 39edc948d703..5fe9c51b38c7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -101,7 +101,7 @@ typedef struct CPURISCVState CPURISCVState;
> #endif
>
> #define RV_VLEN_MAX 1024
> -#define RV_MAX_MHPMEVENTS 29
> +#define RV_MAX_MHPMEVENTS 32
> #define RV_MAX_MHPMCOUNTERS 32
>
> FIELD(VTYPE, VLMUL, 0, 3)
> @@ -112,6 +112,19 @@ FIELD(VTYPE, VEDIV, 8, 2)
> FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
> FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
>
> +typedef struct PMUCTRState PMUCTRState;
This 'typedef' can be merged into the definition below
> +struct PMUCTRState {
> +/* Current value of a counter */
> +target_ulong mhpmcounter_val;
> +/* Current value of a counter in RV32*/
> +target_ulong mhpmcounterh_val;
> +/* Snapshot values of counter */
> +target_ulong mhpmcounter_prev;
> +/* Snapshort value of a counter in RV32 */
> +target_ulong mhpmcounterh_prev;
> +bool started;
> +};
> +
> struct CPURISCVState {
> target_ulong gpr[32];
> uint64_t fpr[32]; /* assume both F and D extensions */
> @@ -226,13 +239,10 @@ struct CPURISCVState {
>
> target_ulong mcountinhibit;
>
> -/* PMU counter configured values */
> -target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
> -
> -/* for RV32 */
> -target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
> +/* PMU counter state */
> +PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
>
> -/* PMU event selector configured values */
> +/* PMU event selector configured values. First three are unused*/
> target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
>
> target_ulong sscratch;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 58a9550bd898..d4449ada557c 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -20,6 +20,7 @@
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> #include "cpu.h"
> +#include "pmu.h"
> #include "qemu/main-loop.h"
> #include "exec/exec-all.h"
>
> @@ -461,41 +462,33 @@ static int write_vcsr(CPURISCVState *env, int csrno,
> target_ulong val)
> }
>
> /* User Timers and Counters */
> -static RISCVException read_instret(CPURISCVState *env, int csrno,
> - target_ulong *val)
> +static target_ulong get_icount_ticks(bool brv32)
I would use 'rv32' instead of 'brv32'
> {
> +int64_t val;
> +target_ulong result;
> +
> #if !defined(CONFIG_USER_ONLY)
> if (icount_enabled()) {
> -*val = icount_get();
> +val = icount_get();
> } else {
> -*val = cpu_get_host_ticks();
> +val = cpu_get_host_ticks();
> }
> #else
> -*val = cpu_get_host_ticks();
> +val = cpu_get_host_ticks();
> #endif
>
> -return RISCV_EXCP_NONE;
> -}
> -
> -static RISCVException read_instreth(CPURISCVState *env, int csrno,
> -target_ulong *val)
> -{
> -#if !defined(CONFIG_USER_ONLY)
> -if (icount_enabled()) {
> -*val = icount_get() >> 32;
> +if (brv32) {
> +result = val >> 32;
> } else {
> -*val = cpu_get_host_ticks() >> 32;
> +result = val;
> }
> -#else
> -*val = cpu_get_host_ticks() >> 32;
> -#endif
>
> -return RISCV_EXCP_NONE;
> +return result;
> }
>
> static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
> {
> -int evt_index = csrno - CSR_MHPMEVENT3;
> +int evt_index = csrno - CSR_MCOUNTINHIBIT;
>
> *val = env->mhpmevent_val[evt_index];
>
> @@ -504,7 +497,7 @@ static int read_mhpmevent(CPURISCVState *env, int csrno,
> target_ulong *val)
>
> static int write_mhpmevent(CPURISCVState *env, int csrno, target_ulong val)
> {
> -int evt_index = csrno - CSR_MHPMEVENT3;
> +int evt_index = csrno - CSR_MCOUNTINHIBIT;
>
> env->mhpmevent_val[evt_index] = val;
>
> @@ -513,52 +506,99 @@ static int write_mhpmevent(CPURISCVState *env, int
> csrno, target_ulong val)
>
> static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
> {
> -int ctr_index
[PATCH v4 07/11] target/riscv: Support mcycle/minstret write operation
From: Atish Patra
mcycle/minstret are actually WARL registers and can be written with any
given value. With SBI PMU extension, it will be used to store a initial
value provided from supervisor OS. The Qemu also need prohibit the counter
increment if mcountinhibit is set.
Support mcycle/minstret through generic counter infrastructure.
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.h | 24 +--
target/riscv/csr.c | 144 ++-
target/riscv/machine.c | 26 ++-
target/riscv/meson.build | 1 +
target/riscv/pmu.c | 32 +
target/riscv/pmu.h | 28
6 files changed, 200 insertions(+), 55 deletions(-)
create mode 100644 target/riscv/pmu.c
create mode 100644 target/riscv/pmu.h
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 39edc948d703..5fe9c51b38c7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -101,7 +101,7 @@ typedef struct CPURISCVState CPURISCVState;
#endif
#define RV_VLEN_MAX 1024
-#define RV_MAX_MHPMEVENTS 29
+#define RV_MAX_MHPMEVENTS 32
#define RV_MAX_MHPMCOUNTERS 32
FIELD(VTYPE, VLMUL, 0, 3)
@@ -112,6 +112,19 @@ FIELD(VTYPE, VEDIV, 8, 2)
FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
+typedef struct PMUCTRState PMUCTRState;
+struct PMUCTRState {
+/* Current value of a counter */
+target_ulong mhpmcounter_val;
+/* Current value of a counter in RV32*/
+target_ulong mhpmcounterh_val;
+/* Snapshot values of counter */
+target_ulong mhpmcounter_prev;
+/* Snapshort value of a counter in RV32 */
+target_ulong mhpmcounterh_prev;
+bool started;
+};
+
struct CPURISCVState {
target_ulong gpr[32];
uint64_t fpr[32]; /* assume both F and D extensions */
@@ -226,13 +239,10 @@ struct CPURISCVState {
target_ulong mcountinhibit;
-/* PMU counter configured values */
-target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
-
-/* for RV32 */
-target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
+/* PMU counter state */
+PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
-/* PMU event selector configured values */
+/* PMU event selector configured values. First three are unused*/
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
target_ulong sscratch;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58a9550bd898..d4449ada557c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -20,6 +20,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "cpu.h"
+#include "pmu.h"
#include "qemu/main-loop.h"
#include "exec/exec-all.h"
@@ -461,41 +462,33 @@ static int write_vcsr(CPURISCVState *env, int csrno,
target_ulong val)
}
/* User Timers and Counters */
-static RISCVException read_instret(CPURISCVState *env, int csrno,
- target_ulong *val)
+static target_ulong get_icount_ticks(bool brv32)
{
+int64_t val;
+target_ulong result;
+
#if !defined(CONFIG_USER_ONLY)
if (icount_enabled()) {
-*val = icount_get();
+val = icount_get();
} else {
-*val = cpu_get_host_ticks();
+val = cpu_get_host_ticks();
}
#else
-*val = cpu_get_host_ticks();
+val = cpu_get_host_ticks();
#endif
-return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_instreth(CPURISCVState *env, int csrno,
-target_ulong *val)
-{
-#if !defined(CONFIG_USER_ONLY)
-if (icount_enabled()) {
-*val = icount_get() >> 32;
+if (brv32) {
+result = val >> 32;
} else {
-*val = cpu_get_host_ticks() >> 32;
+result = val;
}
-#else
-*val = cpu_get_host_ticks() >> 32;
-#endif
-return RISCV_EXCP_NONE;
+return result;
}
static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
{
-int evt_index = csrno - CSR_MHPMEVENT3;
+int evt_index = csrno - CSR_MCOUNTINHIBIT;
*val = env->mhpmevent_val[evt_index];
@@ -504,7 +497,7 @@ static int read_mhpmevent(CPURISCVState *env, int csrno,
target_ulong *val)
static int write_mhpmevent(CPURISCVState *env, int csrno, target_ulong val)
{
-int evt_index = csrno - CSR_MHPMEVENT3;
+int evt_index = csrno - CSR_MCOUNTINHIBIT;
env->mhpmevent_val[evt_index] = val;
@@ -513,52 +506,99 @@ static int write_mhpmevent(CPURISCVState *env, int csrno,
target_ulong val)
static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
{
-int ctr_index = csrno - CSR_MHPMCOUNTER3 + 3;
+int ctr_idx = csrno - CSR_MCYCLE;
+PMUCTRState *counter = >pmu_ctrs[ctr_idx];
-env->mhpmcounter_val[ctr_index] = 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_icount_ticks(false);
+ } else {
+/* Other counters
