Re: [PATCH v1 2/4] target/riscv: Add itrigger support when icount is enabled

[Alistair Francis]

On Thu, Oct 13, 2022 at 4:43 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> The max count in itrigger can be 0x3FFF, which will cause a no trivial
> translation and execution overload.
>
> When icount is enabled, QEMU provides API that can fetch guest
> instruction number. Thus, we can set an timer for itrigger with
> the count as deadline.
>
> Only when timer expires or priviledge mode changes, do lazy update
> to count.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.h        |  2 ++
>  target/riscv/cpu_helper.c |  3 ++
>  target/riscv/debug.c      | 59 +++++++++++++++++++++++++++++++++++++++
>  target/riscv/debug.h      |  1 +
>  4 files changed, 65 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 24bafda27d..13ca0f20ae 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -329,6 +329,8 @@ struct CPUArchState {
>      target_ulong tdata3[RV_MAX_TRIGGERS];
>      struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
>      struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
> +    QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
> +    int64_t last_icount;
>
>      /* machine specific rdtime callback */
>      uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 263282f230..7d8089b218 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -676,6 +676,9 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong 
> newpriv)
>      if (newpriv == PRV_H) {
>          newpriv = PRV_U;
>      }
> +    if (icount_enabled() && newpriv != env->priv) {
> +        riscv_itrigger_update_priv(env);
> +    }
>      /* tlb_flush is unnecessary as mode is contained in mmu_idx */
>      env->priv = newpriv;
>      env->xl = cpu_recompute_xl(env);
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 45a3537d5c..5ff70430a1 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -30,6 +30,7 @@
>  #include "trace.h"
>  #include "exec/exec-all.h"
>  #include "exec/helper-proto.h"
> +#include "sysemu/cpu-timers.h"
>
>  /*
>   * The following M-mode trigger CSRs are implemented:
> @@ -569,6 +570,62 @@ void helper_itrigger_match(CPURISCVState *env)
>      }
>  }
>
> +static void riscv_itrigger_update_count(CPURISCVState *env)
> +{
> +    int count, executed;
> +    /*
> +     * Record last icount, so that we can evaluate the executed instructions
> +     * since last priviledge mode change or timer expire.
> +     */
> +    int64_t last_icount = env->last_icount, current_icount;
> +    current_icount = env->last_icount = icount_get_raw();
> +
> +    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> +        if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> +            continue;
> +        }
> +        count = itrigger_get_count(env, i);
> +        if (!count) {
> +            continue;
> +        }
> +        /*
> +         * Only when priviledge is changed or itrigger timer expires,
> +         * the count field in itrigger tdata1 register is updated.
> +         * And the count field in itrigger only contains remaining value.
> +         */
> +        if (check_itrigger_priv(env, i)) {
> +            /*
> +             * If itrigger enabled in this priviledge mode, the number of
> +             * executed instructions since last priviledge change
> +             * should be reduced from current itrigger count.
> +             */
> +            executed = current_icount - last_icount;
> +            itrigger_set_count(env, i, count - executed);
> +            if (count == executed) {
> +                do_trigger_action(env, i);
> +            }
> +        } else {
> +            /*
> +             * If itrigger is not enabled in this priviledge mode,
> +             * the number of executed instructions will be discard and
> +             * the count field in itrigger will not change.
> +             */
> +            timer_mod(env->itrigger_timer[i],
> +                      current_icount + count);
> +        }
> +    }
> +}
> +
> +static void riscv_itrigger_timer_cb(void *opaque)
> +{
> +    riscv_itrigger_update_count((CPURISCVState *)opaque);
> +}
> +
> +void riscv_itrigger_update_priv(CPURISCVState *env)
> +{
> +    riscv_itrigger_update_count(env);
> +}
> +
>  target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
>  {
>      switch (tdata_index) {
> @@ -798,5 +855,7 @@ 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);
>      }
>  }
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index cc3358e69b..c471748d5a 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -146,4 +146,5 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
> CPUWatchpoint *wp);
>  void riscv_trigger_init(CPURISCVState *env);
>
>  bool riscv_itrigger_enabled(CPURISCVState *env);
> +void riscv_itrigger_update_priv(CPURISCVState *env);
>  #endif /* RISCV_DEBUG_H */
> --
> 2.17.1
>
>