| Message ID | 20231003125107.34859-2-rbradford@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support discontinuous PMU counters | expand |
On Tue, Oct 3, 2023 at 10:53 PM Rob Bradford <rbradford@rivosinc.com> wrote: > > More closely follow the QEMU style by returning an Error and propagating > it there is an error relating to the PMU setup. > > Further simplify the function by removing the num_counters parameter as > this is available from the passed in cpu pointer. > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.c | 8 +++++++- > target/riscv/pmu.c | 19 +++++++++---------- > target/riscv/pmu.h | 3 ++- > 3 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 4140899c52..9d79c20c1a 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1488,7 +1488,13 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp) > } > > if (cpu->cfg.pmu_num) { > - if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) { > + riscv_pmu_init(cpu, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + > + if (cpu->cfg.ext_sscofpmf) { > cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > riscv_pmu_timer_cb, cpu); > } > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c > index 36f6307d28..13801ccb78 100644 > --- a/target/riscv/pmu.c > +++ b/target/riscv/pmu.c > @@ -434,22 +434,21 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx) > } > > > -int riscv_pmu_init(RISCVCPU *cpu, int num_counters) > +void riscv_pmu_init(RISCVCPU *cpu, Error **errp) > { > - if (num_counters > (RV_MAX_MHPMCOUNTERS - 3)) { > - return -1; > + uint8_t pmu_num = cpu->cfg.pmu_num; > + > + if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) { > + error_setg(errp, "Number of counters exceeds maximum available"); > + return; > } > > cpu->pmu_event_ctr_map = g_hash_table_new(g_direct_hash, g_direct_equal); > if (!cpu->pmu_event_ctr_map) { > - /* PMU support can not be enabled */ > - qemu_log_mask(LOG_UNIMP, "PMU events can't be supported\n"); > - cpu->cfg.pmu_num = 0; > - return -1; > + error_setg(errp, "Unable to allocate PMU event hash table"); > + return; > } > > /* Create a bitmask of available programmable counters */ > - cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, num_counters); > - > - return 0; > + cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num); > } > diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h > index 2bfb71ba87..88e0713296 100644 > --- a/target/riscv/pmu.h > +++ b/target/riscv/pmu.h > @@ -17,13 +17,14 @@ > */ > > #include "cpu.h" > +#include "qapi/error.h" > > bool riscv_pmu_ctr_monitor_instructions(CPURISCVState *env, > uint32_t target_ctr); > bool riscv_pmu_ctr_monitor_cycles(CPURISCVState *env, > uint32_t target_ctr); > void riscv_pmu_timer_cb(void *priv); > -int riscv_pmu_init(RISCVCPU *cpu, int num_counters); > +void riscv_pmu_init(RISCVCPU *cpu, Error **errp); > int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value, > uint32_t ctr_idx); > int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx); > -- > 2.41.0 > >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 4140899c52..9d79c20c1a 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1488,7 +1488,13 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp) } if (cpu->cfg.pmu_num) { - if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) { + riscv_pmu_init(cpu, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + + if (cpu->cfg.ext_sscofpmf) { cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, riscv_pmu_timer_cb, cpu); } diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c index 36f6307d28..13801ccb78 100644 --- a/target/riscv/pmu.c +++ b/target/riscv/pmu.c @@ -434,22 +434,21 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx) } -int riscv_pmu_init(RISCVCPU *cpu, int num_counters) +void riscv_pmu_init(RISCVCPU *cpu, Error **errp) { - if (num_counters > (RV_MAX_MHPMCOUNTERS - 3)) { - return -1; + uint8_t pmu_num = cpu->cfg.pmu_num; + + if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) { + error_setg(errp, "Number of counters exceeds maximum available"); + return; } cpu->pmu_event_ctr_map = g_hash_table_new(g_direct_hash, g_direct_equal); if (!cpu->pmu_event_ctr_map) { - /* PMU support can not be enabled */ - qemu_log_mask(LOG_UNIMP, "PMU events can't be supported\n"); - cpu->cfg.pmu_num = 0; - return -1; + error_setg(errp, "Unable to allocate PMU event hash table"); + return; } /* Create a bitmask of available programmable counters */ - cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, num_counters); - - return 0; + cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num); } diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h index 2bfb71ba87..88e0713296 100644 --- a/target/riscv/pmu.h +++ b/target/riscv/pmu.h @@ -17,13 +17,14 @@ */ #include "cpu.h" +#include "qapi/error.h" bool riscv_pmu_ctr_monitor_instructions(CPURISCVState *env, uint32_t target_ctr); bool riscv_pmu_ctr_monitor_cycles(CPURISCVState *env, uint32_t target_ctr); void riscv_pmu_timer_cb(void *priv); -int riscv_pmu_init(RISCVCPU *cpu, int num_counters); +void riscv_pmu_init(RISCVCPU *cpu, Error **errp); int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value, uint32_t ctr_idx); int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx);
More closely follow the QEMU style by returning an Error and propagating it there is an error relating to the PMU setup. Further simplify the function by removing the num_counters parameter as this is available from the passed in cpu pointer. Signed-off-by: Rob Bradford <rbradford@rivosinc.com> --- target/riscv/cpu.c | 8 +++++++- target/riscv/pmu.c | 19 +++++++++---------- target/riscv/pmu.h | 3 ++- 3 files changed, 18 insertions(+), 12 deletions(-)