[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [v2 4/5] target/riscv: Add cycle & instret privilege mode filtering
|
From: |
Atish Kumar Patra |
|
Subject: |
Re: [v2 4/5] target/riscv: Add cycle & instret privilege mode filtering support |
|
Date: |
Wed, 3 Jan 2024 15:27:24 -0800 |
On Wed, Jan 3, 2024 at 12:18 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 12/28/23 21:49, Atish Patra wrote:
> > From: Kaiwen Xue <kaiwenx@rivosinc.com>
> >
> > QEMU only calculates dummy cycles and instructions, so there is no
> > actual means to stop the icount in QEMU. Hence this patch merely adds
> > the functionality of accessing the cfg registers, and cause no actual
> > effects on the counting of cycle and instret counters.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
> > ---
> > target/riscv/cpu.c | 1 +
> > target/riscv/csr.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 54395f95b299..d24f7ff8b55b 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -1297,6 +1297,7 @@ const char *riscv_get_misa_ext_description(uint32_t
> > bit)
> > const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> > /* Defaults for standard extensions */
> > MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> > + MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
>
>
> As I said in patch 2 this declaration can happen there instead.
>
Yup. Will do it.
>
> > MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
> > MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
> > MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 283468bbc652..618e801a7612 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -233,6 +233,27 @@ static RISCVException sscofpmf_32(CPURISCVState *env,
> > int csrno)
> > return sscofpmf(env, csrno);
> > }
> >
> > +static RISCVException smcntrpmf(CPURISCVState *env, int csrno)
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > +
> > + if (!cpu->cfg.ext_smcntrpmf) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
>
> env_archcpu() should be avoided when possible due to its overhead. You can use
> riscv_cpu_cfg() to retrieve cpu->cfg using 'env':
>
Sure. Will fix it.
> > + if (!riscv_cpu_cfg(env)->ext_smcntrpmf) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
>
>
> Thanks,
>
> Daniel
>
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException smcntrpmf_32(CPURISCVState *env, int csrno)
> > +{
> > + if (riscv_cpu_mxl(env) != MXL_RV32) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + return smcntrpmf(env, csrno);
> > +}
> > +
> > +
> > static RISCVException any(CPURISCVState *env, int csrno)
> > {
> > return RISCV_EXCP_NONE;
> > @@ -818,6 +839,54 @@ static int read_hpmcounterh(CPURISCVState *env, int
> > csrno, target_ulong *val)
> >
> > #else /* CONFIG_USER_ONLY */
> >
> > +static int read_mcyclecfg(CPURISCVState *env, int csrno, target_ulong *val)
> > +{
> > + *val = env->mcyclecfg;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int write_mcyclecfg(CPURISCVState *env, int csrno, target_ulong val)
> > +{
> > + env->mcyclecfg = val;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int read_mcyclecfgh(CPURISCVState *env, int csrno, target_ulong
> > *val)
> > +{
> > + *val = env->mcyclecfgh;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int write_mcyclecfgh(CPURISCVState *env, int csrno, target_ulong
> > val)
> > +{
> > + env->mcyclecfgh = val;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int read_minstretcfg(CPURISCVState *env, int csrno, target_ulong
> > *val)
> > +{
> > + *val = env->minstretcfg;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int write_minstretcfg(CPURISCVState *env, int csrno, target_ulong
> > val)
> > +{
> > + env->minstretcfg = val;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int read_minstretcfgh(CPURISCVState *env, int csrno, target_ulong
> > *val)
> > +{
> > + *val = env->minstretcfgh;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static int write_minstretcfgh(CPURISCVState *env, int csrno, target_ulong
> > val)
> > +{
> > + env->minstretcfgh = val;
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong
> > *val)
> > {
> > int evt_index = csrno - CSR_MCOUNTINHIBIT;
> > @@ -4922,6 +4991,13 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > write_mcountinhibit,
> > .min_priv_ver = PRIV_VERSION_1_11_0 },
> >
> > + [CSR_MCYCLECFG] = { "mcyclecfg", smcntrpmf, read_mcyclecfg,
> > + write_mcyclecfg,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MINSTRETCFG] = { "minstretcfg", smcntrpmf, read_minstretcfg,
> > + write_minstretcfg,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > +
> > [CSR_MHPMEVENT3] = { "mhpmevent3", any, read_mhpmevent,
> > write_mhpmevent },
> > [CSR_MHPMEVENT4] = { "mhpmevent4", any, read_mhpmevent,
> > @@ -4981,6 +5057,13 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_MHPMEVENT31] = { "mhpmevent31", any, read_mhpmevent,
> > write_mhpmevent },
> >
> > + [CSR_MCYCLECFGH] = { "mcyclecfgh", smcntrpmf_32, read_mcyclecfgh,
> > + write_mcyclecfgh,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > + [CSR_MINSTRETCFGH] = { "minstretcfgh", smcntrpmf_32,
> > read_minstretcfgh,
> > + write_minstretcfgh,
> > + .min_priv_ver = PRIV_VERSION_1_12_0 },
> > +
> > [CSR_MHPMEVENT3H] = { "mhpmevent3h", sscofpmf_32,
> > read_mhpmeventh,
> > write_mhpmeventh,
> > .min_priv_ver = PRIV_VERSION_1_12_0 },