Re: [PATCH] target/riscv: Allocate itrigger timers only once

[Philippe Mathieu-Daudé]

On 16/8/23 18:27, Akihiko Odaki 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>
---
  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..7edc31e7cc 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(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..3bc3f96a58 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(env);

Maybe name _reset_hold()? Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>

      }