[PATCH v2] target/riscv: set tval for triggered watchpoints

Sergey Matyukevich posted 1 patch 1 year, 2 months ago
target/riscv/cpu_helper.c | 6 ++++++
target/riscv/debug.c      | 1 -
2 files changed, 6 insertions(+), 1 deletion(-)
[PATCH v2] target/riscv: set tval for triggered watchpoints
Posted by Sergey Matyukevich 1 year, 2 months ago
From: Sergey Matyukevich <sergey.matyukevich@syntacore.com>

According to priviledged spec, if [sm]tval is written with a nonzero
value when a breakpoint exception occurs, then [sm]tval will contain
the faulting virtual address. Set tval to hit address when breakpoint
exception is triggered by hardware watchpoint.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>

---

v1 -> v2
- do not set tval blindly for every breakpoint exception,
  handle current specific case under consideration

 target/riscv/cpu_helper.c | 6 ++++++
 target/riscv/debug.c      | 1 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9a28816521..f1a0c65ad3 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1641,6 +1641,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
             tval = env->bins;
             break;
+        case RISCV_EXCP_BREAKPOINT:
+            if (cs->watchpoint_hit) {
+                tval = cs->watchpoint_hit->hitaddr;
+                cs->watchpoint_hit = NULL;
+            }
+            break;
         default:
             break;
         }
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index bf4840a6a3..b091293069 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -761,7 +761,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
 
     if (cs->watchpoint_hit) {
         if (cs->watchpoint_hit->flags & BP_CPU) {
-            cs->watchpoint_hit = NULL;
             do_trigger_action(env, DBG_ACTION_BP);
         }
     } else {
-- 
2.39.0
Re: [PATCH v2] target/riscv: set tval for triggered watchpoints
Posted by Alistair Francis 1 year, 2 months ago
On Wed, Feb 1, 2023 at 3:35 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> From: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
>
> According to priviledged spec, if [sm]tval is written with a nonzero
> value when a breakpoint exception occurs, then [sm]tval will contain
> the faulting virtual address. Set tval to hit address when breakpoint
> exception is triggered by hardware watchpoint.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>

Thanks!

Applied to riscv-to-apply.next with the typo fixed

Alistair

>
> ---
>
> v1 -> v2
> - do not set tval blindly for every breakpoint exception,
>   handle current specific case under consideration
>
>  target/riscv/cpu_helper.c | 6 ++++++
>  target/riscv/debug.c      | 1 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9a28816521..f1a0c65ad3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1641,6 +1641,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
>              tval = env->bins;
>              break;
> +        case RISCV_EXCP_BREAKPOINT:
> +            if (cs->watchpoint_hit) {
> +                tval = cs->watchpoint_hit->hitaddr;
> +                cs->watchpoint_hit = NULL;
> +            }
> +            break;
>          default:
>              break;
>          }
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index bf4840a6a3..b091293069 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -761,7 +761,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
>
>      if (cs->watchpoint_hit) {
>          if (cs->watchpoint_hit->flags & BP_CPU) {
> -            cs->watchpoint_hit = NULL;
>              do_trigger_action(env, DBG_ACTION_BP);
>          }
>      } else {
> --
> 2.39.0
>
>
Re: [PATCH v2] target/riscv: set tval for triggered watchpoints
Posted by Alistair Francis 1 year, 2 months ago
On Wed, Feb 1, 2023 at 3:35 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> From: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
>
> According to priviledged spec, if [sm]tval is written with a nonzero
> value when a breakpoint exception occurs, then [sm]tval will contain
> the faulting virtual address. Set tval to hit address when breakpoint
> exception is triggered by hardware watchpoint.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>

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

Alistair

>
> ---
>
> v1 -> v2
> - do not set tval blindly for every breakpoint exception,
>   handle current specific case under consideration
>
>  target/riscv/cpu_helper.c | 6 ++++++
>  target/riscv/debug.c      | 1 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9a28816521..f1a0c65ad3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1641,6 +1641,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          case RISCV_EXCP_VIRT_INSTRUCTION_FAULT:
>              tval = env->bins;
>              break;
> +        case RISCV_EXCP_BREAKPOINT:
> +            if (cs->watchpoint_hit) {
> +                tval = cs->watchpoint_hit->hitaddr;
> +                cs->watchpoint_hit = NULL;
> +            }
> +            break;
>          default:
>              break;
>          }
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index bf4840a6a3..b091293069 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -761,7 +761,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
>
>      if (cs->watchpoint_hit) {
>          if (cs->watchpoint_hit->flags & BP_CPU) {
> -            cs->watchpoint_hit = NULL;
>              do_trigger_action(env, DBG_ACTION_BP);
>          }
>      } else {
> --
> 2.39.0
>
>
Re: [PATCH v2] target/riscv: set tval for triggered watchpoints
Posted by Bin Meng 1 year, 2 months ago
On Wed, Feb 1, 2023 at 1:35 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> From: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
>
> According to priviledged spec, if [sm]tval is written with a nonzero

typo: privileged

> value when a breakpoint exception occurs, then [sm]tval will contain
> the faulting virtual address. Set tval to hit address when breakpoint
> exception is triggered by hardware watchpoint.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
>
> ---
>
> v1 -> v2
> - do not set tval blindly for every breakpoint exception,
>   handle current specific case under consideration
>
>  target/riscv/cpu_helper.c | 6 ++++++
>  target/riscv/debug.c      | 1 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng@tinylab.org>