Improve RISC-V Debug support

[PATCH 6/9] target/riscv: debug: Create common trigger actions function

Posted by frank.chang@sifive.com 3 years, 8 months ago

From: Frank Chang <frank.chang@sifive.com>

Trigger actions are shared among all triggers. Extract to a common
function.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 target/riscv/debug.c | 55 ++++++++++++++++++++++++++++++++++++++++++--
 target/riscv/debug.h | 13 +++++++++++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 1668b8abda..ab23566113 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -91,6 +91,35 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
     return extract_trigger_type(env, env->tdata1[trigger_index]);
 }
 
+static trigger_action_t get_trigger_action(CPURISCVState *env,
+                                           target_ulong trigger_index)
+{
+    target_ulong tdata1 = env->tdata1[trigger_index];
+    int trigger_type = get_trigger_type(env, trigger_index);
+    trigger_action_t action = DBG_ACTION_NONE;
+
+    switch (trigger_type) {
+    case TRIGGER_TYPE_AD_MATCH:
+        action = (tdata1 & TYPE2_ACTION) >> 12;
+        break;
+    case TRIGGER_TYPE_INST_CNT:
+    case TRIGGER_TYPE_INT:
+    case TRIGGER_TYPE_EXCP:
+    case TRIGGER_TYPE_AD_MATCH6:
+    case TRIGGER_TYPE_EXT_SRC:
+        qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+                      trigger_type);
+        break;
+    case TRIGGER_TYPE_NO_EXIST:
+    case TRIGGER_TYPE_UNAVAIL:
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return action;
+}
+
 static inline target_ulong build_tdata1(CPURISCVState *env,
                                         trigger_type_t type,
                                         bool dmode, target_ulong data)
@@ -181,6 +210,28 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
     }
 }
 
+static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
+{
+    trigger_action_t action = get_trigger_action(env, trigger_index);
+
+    switch (action) {
+    case DBG_ACTION_BP:
+        riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+        break;
+    case DBG_ACTION_DBG_MODE:
+    case DBG_ACTION_TRACE0:
+    case DBG_ACTION_TRACE1:
+    case DBG_ACTION_TRACE2:
+    case DBG_ACTION_TRACE3:
+    case DBG_ACTION_EXT_DBG0:
+    case DBG_ACTION_EXT_DBG1:
+        qemu_log_mask(LOG_UNIMP, "action: %d is not supported\n", action);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
@@ -381,11 +432,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
     if (cs->watchpoint_hit) {
         if (cs->watchpoint_hit->flags & BP_CPU) {
             cs->watchpoint_hit = NULL;
-            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+            do_trigger_action(env, DBG_ACTION_BP);
         }
     } else {
         if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
-            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+            do_trigger_action(env, DBG_ACTION_BP);
         }
     }
 }
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 9f69c64591..0e4859cf74 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -44,6 +44,19 @@ typedef enum {
     TRIGGER_TYPE_NUM
 } trigger_type_t;
 
+/* actions */
+typedef enum {
+    DBG_ACTION_NONE = -1,           /* sentinel value */
+    DBG_ACTION_BP = 0,
+    DBG_ACTION_DBG_MODE,
+    DBG_ACTION_TRACE0,
+    DBG_ACTION_TRACE1,
+    DBG_ACTION_TRACE2,
+    DBG_ACTION_TRACE3,
+    DBG_ACTION_EXT_DBG0 = 8,
+    DBG_ACTION_EXT_DBG1
+} trigger_action_t;
+
 /* tdata1 field masks */
 
 #define RV32_TYPE(t)    ((uint32_t)(t) << 28)
-- 
2.36.1

Re: [PATCH 6/9] target/riscv: debug: Create common trigger actions function

Posted by Bin Meng 3 years, 7 months ago

On Fri, Jun 10, 2022 at 1:21 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Trigger actions are shared among all triggers. Extract to a common
> function.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  target/riscv/debug.c | 55 ++++++++++++++++++++++++++++++++++++++++++--
>  target/riscv/debug.h | 13 +++++++++++
>  2 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 1668b8abda..ab23566113 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -91,6 +91,35 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
>      return extract_trigger_type(env, env->tdata1[trigger_index]);
>  }
>
> +static trigger_action_t get_trigger_action(CPURISCVState *env,
> +                                           target_ulong trigger_index)
> +{
> +    target_ulong tdata1 = env->tdata1[trigger_index];
> +    int trigger_type = get_trigger_type(env, trigger_index);
> +    trigger_action_t action = DBG_ACTION_NONE;
> +
> +    switch (trigger_type) {
> +    case TRIGGER_TYPE_AD_MATCH:
> +        action = (tdata1 & TYPE2_ACTION) >> 12;
> +        break;
> +    case TRIGGER_TYPE_INST_CNT:
> +    case TRIGGER_TYPE_INT:
> +    case TRIGGER_TYPE_EXCP:
> +    case TRIGGER_TYPE_AD_MATCH6:
> +    case TRIGGER_TYPE_EXT_SRC:
> +        qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> +                      trigger_type);
> +        break;
> +    case TRIGGER_TYPE_NO_EXIST:
> +    case TRIGGER_TYPE_UNAVAIL:
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return action;
> +}
> +
>  static inline target_ulong build_tdata1(CPURISCVState *env,
>                                          trigger_type_t type,
>                                          bool dmode, target_ulong data)
> @@ -181,6 +210,28 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
>      }
>  }
>
> +static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
> +{
> +    trigger_action_t action = get_trigger_action(env, trigger_index);
> +
> +    switch (action) {
> +    case DBG_ACTION_BP:
> +        riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +        break;
> +    case DBG_ACTION_DBG_MODE:
> +    case DBG_ACTION_TRACE0:
> +    case DBG_ACTION_TRACE1:
> +    case DBG_ACTION_TRACE2:
> +    case DBG_ACTION_TRACE3:
> +    case DBG_ACTION_EXT_DBG0:
> +    case DBG_ACTION_EXT_DBG1:
> +        qemu_log_mask(LOG_UNIMP, "action: %d is not supported\n", action);
> +        break;

case DBG_ACTION_NONE should be added here as get_trigger_action() may
return such value

> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  /* type 2 trigger */
>
>  static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
> @@ -381,11 +432,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
>      if (cs->watchpoint_hit) {
>          if (cs->watchpoint_hit->flags & BP_CPU) {
>              cs->watchpoint_hit = NULL;
> -            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +            do_trigger_action(env, DBG_ACTION_BP);
>          }
>      } else {
>          if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
> -            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +            do_trigger_action(env, DBG_ACTION_BP);
>          }
>      }
>  }
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 9f69c64591..0e4859cf74 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -44,6 +44,19 @@ typedef enum {
>      TRIGGER_TYPE_NUM
>  } trigger_type_t;
>
> +/* actions */
> +typedef enum {
> +    DBG_ACTION_NONE = -1,           /* sentinel value */
> +    DBG_ACTION_BP = 0,
> +    DBG_ACTION_DBG_MODE,
> +    DBG_ACTION_TRACE0,
> +    DBG_ACTION_TRACE1,
> +    DBG_ACTION_TRACE2,
> +    DBG_ACTION_TRACE3,
> +    DBG_ACTION_EXT_DBG0 = 8,
> +    DBG_ACTION_EXT_DBG1
> +} trigger_action_t;
> +
>  /* tdata1 field masks */
>
>  #define RV32_TYPE(t)    ((uint32_t)(t) << 28)

Regards,
Bin