On 2023/10/11 22:45, Rob Bradford 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>
> ---
> 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 ac2b94b6a6..c9d8fc12fe 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);
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> 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);