Re: [PATCH v1 3/4] target/riscv: Enable native debug itrigger

[Alistair Francis <alistair23@gmail.com>]

On Thu, Oct 13, 2022 at 4:38 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> When QEMU is not in icount mode, execute instruction one by one. The
> tdata1 can be read directly.
>
> When QEMU is in icount mode, use a timer to simulate the itrigger. The
> tdata1 may be not right because of lazy update of count in tdata1. Thus,
> We should pack the adjusted count into tdata1 before read it back.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

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

Alistair

> ---
>  target/riscv/debug.c | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 5ff70430a1..db7745d4a3 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -626,10 +626,80 @@ void riscv_itrigger_update_priv(CPURISCVState *env)
>      riscv_itrigger_update_count(env);
>  }
>
> +static target_ulong itrigger_validate(CPURISCVState *env,
> +                                      target_ulong ctrl)
> +{
> +    target_ulong val;
> +
> +    /* validate the generic part first */
> +    val = tdata1_validate(env, ctrl, TRIGGER_TYPE_INST_CNT);
> +
> +    /* validate unimplemented (always zero) bits */
> +    warn_always_zero_bit(ctrl, ITRIGGER_ACTION, "action");
> +    warn_always_zero_bit(ctrl, ITRIGGER_HIT, "hit");
> +    warn_always_zero_bit(ctrl, ITRIGGER_PENDING, "pending");
> +
> +    /* keep the mode and attribute bits */
> +    val |= ctrl & (ITRIGGER_VU | ITRIGGER_VS | ITRIGGER_U | ITRIGGER_S |
> +                   ITRIGGER_M | ITRIGGER_COUNT);
> +
> +    return val;
> +}
> +
> +static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
> +                               int tdata_index, target_ulong val)
> +{
> +    target_ulong new_val;
> +
> +    switch (tdata_index) {
> +    case TDATA1:
> +        /* set timer for icount */
> +        new_val = itrigger_validate(env, val);
> +        if (new_val != env->tdata1[index]) {
> +            env->tdata1[index] = new_val;
> +            if (icount_enabled()) {
> +                env->last_icount = icount_get_raw();
> +                /* set the count to timer */
> +                timer_mod(env->itrigger_timer[index],
> +                          env->last_icount + itrigger_get_count(env, index));
> +            }
> +        }
> +        break;
> +    case TDATA2:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "tdata2 is not supported for icount trigger\n");
> +        break;
> +    case TDATA3:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "tdata3 is not supported for icount trigger\n");
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return;
> +}
> +
> +static int itrigger_get_adjust_count(CPURISCVState *env)
> +{
> +    int count = itrigger_get_count(env, env->trigger_cur), executed;
> +    if ((count != 0) && check_itrigger_priv(env, env->trigger_cur)) {
> +        executed = icount_get_raw() - env->last_icount;
> +        count += executed;
> +    }
> +    return count;
> +}
> +
>  target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
>  {
> +    int trigger_type;
>      switch (tdata_index) {
>      case TDATA1:
> +        trigger_type = extract_trigger_type(env, env->tdata1[env->trigger_cur]);
> +        if ((trigger_type == TRIGGER_TYPE_INST_CNT) && icount_enabled()) {
> +            return deposit64(env->tdata1[env->trigger_cur], 10, 14,
> +                             itrigger_get_adjust_count(env));
> +        }
>          return env->tdata1[env->trigger_cur];
>      case TDATA2:
>          return env->tdata2[env->trigger_cur];
> @@ -658,6 +728,8 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>          type6_reg_write(env, env->trigger_cur, tdata_index, val);
>          break;
>      case TRIGGER_TYPE_INST_CNT:
> +        itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
> +        break;
>      case TRIGGER_TYPE_INT:
>      case TRIGGER_TYPE_EXCP:
>      case TRIGGER_TYPE_EXT_SRC:
> --
> 2.17.1
>
>