Re: [PATCH v1 4/4] target/riscv: Add itrigger_enabled field to CPURISCVState

[Alistair Francis]

On Thu, Oct 13, 2022 at 4:51 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> Avoid calling riscv_itrigger_enabled() when calculate the tbflags.
> As the itrigger enable status can only be changed when write
> tdata1, migration load or itrigger fire, update env->itrigger_enabled
> at these places.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>  target/riscv/cpu.h        |  1 +
>  target/riscv/cpu_helper.c |  3 +--
>  target/riscv/debug.c      |  3 +++
>  target/riscv/machine.c    | 15 +++++++++++++++
>  4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 13ca0f20ae..44499df9da 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -331,6 +331,7 @@ struct CPUArchState {
>      struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
>      QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
>      int64_t last_icount;
> +    bool itrigger_enabled;
>
>      /* machine specific rdtime callback */
>      uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 7d8089b218..95c766aec0 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -106,8 +106,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, 
> target_ulong *pc,
>                             get_field(env->mstatus_hs, MSTATUS_VS));
>      }
>      if (riscv_feature(env, RISCV_FEATURE_DEBUG) && !icount_enabled()) {
> -        flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER,
> -                           riscv_itrigger_enabled(env));
> +        flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
>      }
>  #endif
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index db7745d4a3..2c0c8b18db 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -565,6 +565,7 @@ void helper_itrigger_match(CPURISCVState *env)
>          }
>          itrigger_set_count(env, i, count--);
>          if (!count) {
> +            env->itrigger_enabled = riscv_itrigger_enabled(env);
>              do_trigger_action(env, i);
>          }
>      }
> @@ -662,6 +663,8 @@ static void itrigger_reg_write(CPURISCVState *env, 
> target_ulong index,
>                  /* set the count to timer */
>                  timer_mod(env->itrigger_timer[index],
>                            env->last_icount + itrigger_get_count(env, index));
> +            } else {
> +                env->itrigger_enabled = riscv_itrigger_enabled(env);
>              }
>          }
>          break;
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index c2a94a82b3..cd32a52e19 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -21,6 +21,8 @@
>  #include "qemu/error-report.h"
>  #include "sysemu/kvm.h"
>  #include "migration/cpu.h"
> +#include "sysemu/cpu-timers.h"
> +#include "debug.h"
>
>  static bool pmp_needed(void *opaque)
>  {
> @@ -229,11 +231,24 @@ static bool debug_needed(void *opaque)
>      return riscv_feature(env, RISCV_FEATURE_DEBUG);
>  }
>
> +static int debug_post_load(void *opaque, int version_id)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    if (icount_enabled()) {
> +        env->itrigger_enabled = riscv_itrigger_enabled(env);
> +    }
> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_debug = {
>      .name = "cpu/debug",
>      .version_id = 2,
>      .minimum_version_id = 2,

The versions here should be bumped

Alistair

>      .needed = debug_needed,
> +    .post_load = debug_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
>          VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
> --
> 2.17.1
>
>