On Wed, Mar 20, 2024 at 7:33 PM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
> We're not setting (s/m)tval when triggering breakpoints of type 2
> (mcontrol) and 6 (mcontrol6). According to the debug spec section
> 5.7.12, "Match Control Type 6":
>
> "The Privileged Spec says that breakpoint exceptions that occur on
> instruction fetches, loads, or stores update the tval CSR with either
> zero or the faulting virtual address. The faulting virtual address for
> an mcontrol6 trigger with action = 0 is the address being accessed and
> which caused that trigger to fire."
Setting zero is valid though. I agree it's probably worth setting a
value, but I don't think we need this for 9.0
Alistair
>
> A similar text is also found in the Debug spec section 5.7.11 w.r.t.
> mcontrol.
>
> Given that we always use action = 0, save the faulting address for the
> mcontrol and mcontrol6 trigger breakpoints into env->badaddr, which is
> used as as scratch area for traps with address information. 'tval' is
> then set during riscv_cpu_do_interrupt().
>
> Reported-by: Joseph Chan <jc...@ventanamicro.com>
> Fixes: b5f6379d13 ("target/riscv: debug: Implement debug related TCGCPUOps")
> Fixes: c472c142a7 ("target/riscv: debug: Add initial support of type 6
> trigger")
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> ---
> target/riscv/cpu_helper.c | 1 +
> target/riscv/debug.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index ce7322011d..492ca63b1a 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1717,6 +1717,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> tval = env->bins;
> break;
> case RISCV_EXCP_BREAKPOINT:
> + tval = env->badaddr;
> if (cs->watchpoint_hit) {
> tval = cs->watchpoint_hit->hitaddr;
> cs->watchpoint_hit = NULL;
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index e30d99cc2f..b110370ea6 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -798,6 +798,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> /* check U/S/M bit against current privilege level */
> if ((ctrl >> 3) & BIT(env->priv)) {
> + env->badaddr = pc;
> return true;
> }
> }
> @@ -810,11 +811,13 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> if (env->virt_enabled) {
> /* check VU/VS bit against current privilege level */
> if ((ctrl >> 23) & BIT(env->priv)) {
> + env->badaddr = pc;
> return true;
> }
> } else {
> /* check U/S/M bit against current privilege level */
> if ((ctrl >> 3) & BIT(env->priv)) {
> + env->badaddr = pc;
> return true;
> }
> }
> --
> 2.44.0
>
>
