On Tue, Oct 3, 2023 at 5:51 AM Rob Bradford <rbradf...@rivosinc.com> wrote: > > There is no requirement that the enabled counters in the platform are > continuously numbered. Add a "pmu-mask" property that, if specified, can > be used to specify the enabled PMUs. In order to avoid ambiguity if > "pmu-mask" is specified then "pmu-num" must also match the number of > bits set in the mask. > > Signed-off-by: Rob Bradford <rbradf...@rivosinc.com> > --- > target/riscv/cpu.c | 1 + > target/riscv/cpu_cfg.h | 1 + > target/riscv/pmu.c | 15 +++++++++++++-- > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 9d79c20c1a..b89b006a76 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1817,6 +1817,7 @@ static void riscv_cpu_add_misa_properties(Object > *cpu_obj) > static Property riscv_cpu_extensions[] = { > /* Defaults for standard extensions */ > DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), > + DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, 0), > DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false), > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index 0e6a0f245c..40f7d970bc 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -124,6 +124,7 @@ struct RISCVCPUConfig { > bool ext_XVentanaCondOps; > > uint8_t pmu_num; > + uint32_t pmu_mask; > char *priv_spec; > char *user_spec; > char *bext_spec; > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > index 13801ccb78..f97e25a1f6 100644 > --- a/target/riscv/pmu.c > +++ b/target/riscv/pmu.c > @@ -437,6 +437,13 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t > value, uint32_t ctr_idx) > void riscv_pmu_init(RISCVCPU *cpu, Error **errp) > { > uint8_t pmu_num = cpu->cfg.pmu_num; > + uint32_t pmu_mask = cpu->cfg.pmu_mask; > + > + if (pmu_mask && ctpop32(pmu_mask) != pmu_num) { > + error_setg(errp, "Mismatch between number of enabled counters in " > + "\"pmu-mask\" and \"pmu-num\""); > + return; > + } >
Is that necessary for the default case? I am thinking of marking pmu-num as deprecated and pmu-mask as the preferred way of doing things as it is more flexible. There is no real benefit carrying both. The default pmu-mask value will change in that case. We can just overwrite pmu-num with ctpop32(pmu_mask) if pmu-mask is available. Thoughts ? > if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) { > error_setg(errp, "Number of counters exceeds maximum available"); > @@ -449,6 +456,10 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp) > return; > } > > - /* Create a bitmask of available programmable counters */ > - cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num); > + /* Create a bitmask of available programmable counters if none supplied > */ > + if (pmu_mask) { > + cpu->pmu_avail_ctrs = pmu_mask; > + } else { > + cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num); > + } > } > -- > 2.41.0 >