qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ RFC v2 3/9] target/riscv: pmu: Make number of counters configurabl


From: Atish Patra
Subject: Re: [ RFC v2 3/9] target/riscv: pmu: Make number of counters configurable
Date: Thu, 16 Sep 2021 11:52:23 -0700

On Wed, Sep 15, 2021 at 7:51 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Sep 10, 2021 at 4:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > The RISC-V privilege specification provides flexibility to implement
> > any number of counters from 29 programmable counters. However, the Qemu
>
> nits: %s/Qemu/QEMU
>
> > implements all the counters.
> >
> > Make it configurable through pmu config parameter which now will indicate
> > how many programmable counters should be implemented by the cpu.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  target/riscv/cpu.c |  2 +-
> >  target/riscv/cpu.h |  2 +-
> >  target/riscv/csr.c | 96 ++++++++++++++++++++++++++++++----------------
> >  3 files changed, 65 insertions(+), 35 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 7a486450ebc6..eba6050324a0 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -587,7 +587,7 @@ static Property riscv_cpu_properties[] = {
> >      DEFINE_PROP_BOOL("x-b", RISCVCPU, cfg.ext_b, false),
> >      DEFINE_PROP_BOOL("x-h", RISCVCPU, cfg.ext_h, false),
> >      DEFINE_PROP_BOOL("x-v", RISCVCPU, cfg.ext_v, false),
> > -    DEFINE_PROP_BOOL("pmu", RISCVCPU, cfg.ext_pmu, true),
> > +    DEFINE_PROP_UINT16("pmu", RISCVCPU, cfg.ext_pmu, 16),
>
> UINT8 should be enough. The name should better be changed to pmu-num
> (cfg.pmu_num).

ok. will change it.

>
> >      DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> >      DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> >      DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 5e67003e58a3..0e2e88f3bbea 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -293,9 +293,9 @@ struct RISCVCPU {
> >          bool ext_u;
> >          bool ext_h;
> >          bool ext_v;
> > -        bool ext_pmu;
> >          bool ext_ifencei;
> >          bool ext_icsr;
> > +        uint16_t ext_pmu;
> >
> >          char *priv_spec;
> >          char *user_spec;
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index c3ce7d83a6b2..fa014bac72ab 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -57,15 +57,45 @@ static RISCVException vs(CPURISCVState *env, int csrno)
> >      return RISCV_EXCP_ILLEGAL_INST;
> >  }
> >
> > +static RISCVException mctr(CPURISCVState *env, int csrno)
> > +{
> > +#if !defined(CONFIG_USER_ONLY)
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +    int ctr_index;
> > +    int base_csrno = CSR_MHPMCOUNTER3;
> > +
> > +    if (riscv_cpu_is_32bit(env) && csrno >= CSR_MCYCLEH) {
> > +        /* Offset for RV32 hpmcounternh counters */
>
> should be mhpmcounternh

yeah.

>
> > +        base_csrno += 0x80;
> > +    }
> > +    ctr_index = csrno - base_csrno;
> > +    if (!cpu->cfg.ext_pmu || ctr_index > cpu->cfg.ext_pmu) {
>
> ctr_index >= cpu->cfg.ext_pmu

Yup.

>
> > +        /* The Counters extensions is not enabled or out of range*/
>
> PMU extension
>
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    }
> > +
> > +    return RISCV_EXCP_NONE;
> > +#endif
> > +}
> > +
> >  static RISCVException ctr(CPURISCVState *env, int csrno)
> >  {
> >  #if !defined(CONFIG_USER_ONLY)
> >      CPUState *cs = env_cpu(env);
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> >      int ctr_index;
> > +    int base_csrno = CSR_CYCLE;
> > +    bool brv32 = riscv_cpu_is_32bit(env);
> > +
> > +    if (brv32 && csrno >= CSR_CYCLEH) {
> > +        /* Offset for RV32 hpmcounternh counters */
> > +        base_csrno += 0x80;
> > +    }
> > +    ctr_index = csrno - base_csrno;
> >
> > -    if (!cpu->cfg.ext_pmu) {
> > -        /* The Counters extensions is not enabled */
> > +    if (!cpu->cfg.ext_pmu || ctr_index > (cpu->cfg.ext_pmu + 3)) {
>
> ctr_index >=
>
> > +        /* The Counters extensions is not enabled or out of range */
>
> PMU extension

Thanks. I will address all these in v3.

>
> >          return RISCV_EXCP_ILLEGAL_INST;
> >      }
> >
> > @@ -93,7 +123,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> >              }
> >              break;
> >          }
> > -        if (riscv_cpu_is_32bit(env)) {
> > +        if (brv32) {
> >              switch (csrno) {
> >              case CSR_CYCLEH:
> >                  if (!get_field(env->mcounteren, HCOUNTEREN_CY)) {
> > @@ -148,7 +178,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> >              }
> >              break;
> >          }
> > -        if (riscv_cpu_is_32bit(env)) {
> > +        if (brv32) {
> >              switch (csrno) {
> >              case CSR_CYCLEH:
> >                  if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
> > @@ -1721,35 +1751,35 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >      [CSR_HPMCOUNTER30]   = { "hpmcounter30",   ctr,    read_zero },
> >      [CSR_HPMCOUNTER31]   = { "hpmcounter31",   ctr,    read_zero },
> >
> > -    [CSR_MHPMCOUNTER3]   = { "mhpmcounter3",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER4]   = { "mhpmcounter4",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER5]   = { "mhpmcounter5",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER6]   = { "mhpmcounter6",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER7]   = { "mhpmcounter7",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER8]   = { "mhpmcounter8",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER9]   = { "mhpmcounter9",   any,    read_zero },
> > -    [CSR_MHPMCOUNTER10]  = { "mhpmcounter10",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER11]  = { "mhpmcounter11",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER12]  = { "mhpmcounter12",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER13]  = { "mhpmcounter13",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER14]  = { "mhpmcounter14",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER15]  = { "mhpmcounter15",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER16]  = { "mhpmcounter16",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER17]  = { "mhpmcounter17",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER18]  = { "mhpmcounter18",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER19]  = { "mhpmcounter19",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER20]  = { "mhpmcounter20",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER21]  = { "mhpmcounter21",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER22]  = { "mhpmcounter22",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER23]  = { "mhpmcounter23",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER24]  = { "mhpmcounter24",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER25]  = { "mhpmcounter25",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER26]  = { "mhpmcounter26",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER27]  = { "mhpmcounter27",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER28]  = { "mhpmcounter28",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER29]  = { "mhpmcounter29",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER30]  = { "mhpmcounter30",  any,    read_zero },
> > -    [CSR_MHPMCOUNTER31]  = { "mhpmcounter31",  any,    read_zero },
> > +    [CSR_MHPMCOUNTER3]   = { "mhpmcounter3",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER4]   = { "mhpmcounter4",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER5]   = { "mhpmcounter5",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER6]   = { "mhpmcounter6",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER7]   = { "mhpmcounter7",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER8]   = { "mhpmcounter8",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER9]   = { "mhpmcounter9",   mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER10]  = { "mhpmcounter10",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER11]  = { "mhpmcounter11",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER12]  = { "mhpmcounter12",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER13]  = { "mhpmcounter13",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER14]  = { "mhpmcounter14",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER15]  = { "mhpmcounter15",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER16]  = { "mhpmcounter16",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER17]  = { "mhpmcounter17",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER18]  = { "mhpmcounter18",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER19]  = { "mhpmcounter19",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER20]  = { "mhpmcounter20",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER21]  = { "mhpmcounter21",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER22]  = { "mhpmcounter22",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER23]  = { "mhpmcounter23",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER24]  = { "mhpmcounter24",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER25]  = { "mhpmcounter25",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER26]  = { "mhpmcounter26",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER27]  = { "mhpmcounter27",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER28]  = { "mhpmcounter28",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER29]  = { "mhpmcounter29",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER30]  = { "mhpmcounter30",  mctr,   read_zero },
> > +    [CSR_MHPMCOUNTER31]  = { "mhpmcounter31",  mctr,   read_zero },
> >
> >      [CSR_MHPMEVENT3]     = { "mhpmevent3",     any,    read_zero },
> >      [CSR_MHPMEVENT4]     = { "mhpmevent4",     any,    read_zero },
> > --
>
> Regards,
> Bin
>


-- 
Regards,
Atish



reply via email to

[Prev in Thread] Current Thread [Next in Thread]