On Fri, Aug 18, 2023 at 1:42 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote:
>
> riscv_trigger_init() had been called on reset events that can happen
> several times for a CPU and it allocated timers for itrigger. If old
> timers were present, they were simply overwritten by the new timers,
> resulting in a memory leak.
>
> Divide riscv_trigger_init() into two functions, namely
> riscv_trigger_realize() and riscv_trigger_reset() and call them in
> appropriate timing. The timer allocation will happen only once for a
> CPU in riscv_trigger_realize().
>
> Fixes: 5a4ae64cac ("target/riscv: Add itrigger support when icount is
> enabled")
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Reviewed-by: LIU Zhiwei <zhiwei_...@linux.alibaba.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/debug.h | 3 ++-
> target/riscv/cpu.c | 8 +++++++-
> target/riscv/debug.c | 15 ++++++++++++---
> 3 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index c471748d5a..5794aa6ee5 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -143,7 +143,8 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
> bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
> bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
>
> -void riscv_trigger_init(CPURISCVState *env);
> +void riscv_trigger_realize(CPURISCVState *env);
> +void riscv_trigger_reset_hold(CPURISCVState *env);
>
> bool riscv_itrigger_enabled(CPURISCVState *env);
> void riscv_itrigger_update_priv(CPURISCVState *env);
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e12b6ef7f6..7e0512dd5f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -904,7 +904,7 @@ static void riscv_cpu_reset_hold(Object *obj)
>
> #ifndef CONFIG_USER_ONLY
> if (cpu->cfg.debug) {
> - riscv_trigger_init(env);
> + riscv_trigger_reset_hold(env);
> }
>
> if (kvm_enabled()) {
> @@ -1475,6 +1475,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>
> riscv_cpu_register_gdb_regs_for_features(cs);
>
> +#ifndef CONFIG_USER_ONLY
> + if (cpu->cfg.debug) {
> + riscv_trigger_realize(&cpu->env);
> + }
> +#endif
> +
> qemu_init_vcpu(cs);
> cpu_reset(cs);
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 75ee1c4971..ddd46b2d3e 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -903,7 +903,17 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs,
> CPUWatchpoint *wp)
> return false;
> }
>
> -void riscv_trigger_init(CPURISCVState *env)
> +void riscv_trigger_realize(CPURISCVState *env)
> +{
> + int i;
> +
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> + env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> + riscv_itrigger_timer_cb, env);
> + }
> +}
> +
> +void riscv_trigger_reset_hold(CPURISCVState *env)
> {
> target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
> int i;
> @@ -928,7 +938,6 @@ void riscv_trigger_init(CPURISCVState *env)
> env->tdata3[i] = 0;
> env->cpu_breakpoint[i] = NULL;
> env->cpu_watchpoint[i] = NULL;
> - env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> - riscv_itrigger_timer_cb, env);
> + timer_del(env->itrigger_timer[i]);
> }
> }
> --
> 2.41.0
>
>
