| Message ID | 20240219032559.79665-4-alvinga@andestech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Modularize common match conditions for trigger | expand |
On 2/19/24 00:25, Alvin Chang wrote: > We have implemented trigger_common_match(), which checks if the enabled > privilege levels of the trigger match CPU's current privilege level. > Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke > trigger_common_match() to check the privilege levels of the type 2 and > type 6 triggers for the watchpoints. > > Only load/store bits and loaded/stored address should be further checked > in riscv_cpu_debug_check_watchpoint(). > > Signed-off-by: Alvin Chang <alvinga@andestech.com> > --- > target/riscv/debug.c | 26 ++++++-------------------- > 1 file changed, 6 insertions(+), 20 deletions(-) > > diff --git a/target/riscv/debug.c b/target/riscv/debug.c > index b971ed5d7a..67ba19c966 100644 > --- a/target/riscv/debug.c > +++ b/target/riscv/debug.c > @@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) > for (i = 0; i < RV_MAX_TRIGGERS; i++) { > trigger_type = get_trigger_type(env, i); > > + if (!trigger_common_match(env, trigger_type, i)) { > + continue; > + } > + The same comments I made in patch 2 also applies here. It's ok to change how the function behaves as long as we're doing it on purpose and explaining why in the commit message. Otherwise this if (!trigger_common_match()" conditional should "return false" instead of keep looping to maintain the existing behavior. Thanks, Daniel > switch (trigger_type) { > case TRIGGER_TYPE_AD_MATCH: > - /* type 2 trigger cannot be fired in VU/VS mode */ > - if (env->virt_enabled) { > - return false; > - } > - > ctrl = env->tdata1[i]; > addr = env->tdata2[i]; > flags = 0; > @@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) > } > > if ((wp->flags & flags) && (wp->vaddr == addr)) { > - /* check U/S/M bit against current privilege level */ > - if ((ctrl >> 3) & BIT(env->priv)) { > - return true; > - } > + return true; > } > break; > case TRIGGER_TYPE_AD_MATCH6: > @@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) > } > > if ((wp->flags & flags) && (wp->vaddr == addr)) { > - if (env->virt_enabled) { > - /* check VU/VS bit against current privilege level */ > - if ((ctrl >> 23) & BIT(env->priv)) { > - return true; > - } > - } else { > - /* check U/S/M bit against current privilege level */ > - if ((ctrl >> 3) & BIT(env->priv)) { > - return true; > - } > - } > + return true; > } > break; > default:
diff --git a/target/riscv/debug.c b/target/riscv/debug.c index b971ed5d7a..67ba19c966 100644 --- a/target/riscv/debug.c +++ b/target/riscv/debug.c @@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) for (i = 0; i < RV_MAX_TRIGGERS; i++) { trigger_type = get_trigger_type(env, i); + if (!trigger_common_match(env, trigger_type, i)) { + continue; + } + switch (trigger_type) { case TRIGGER_TYPE_AD_MATCH: - /* type 2 trigger cannot be fired in VU/VS mode */ - if (env->virt_enabled) { - return false; - } - ctrl = env->tdata1[i]; addr = env->tdata2[i]; flags = 0; @@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) } if ((wp->flags & flags) && (wp->vaddr == addr)) { - /* check U/S/M bit against current privilege level */ - if ((ctrl >> 3) & BIT(env->priv)) { - return true; - } + return true; } break; case TRIGGER_TYPE_AD_MATCH6: @@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) } if ((wp->flags & flags) && (wp->vaddr == addr)) { - if (env->virt_enabled) { - /* check VU/VS bit against current privilege level */ - if ((ctrl >> 23) & BIT(env->priv)) { - return true; - } - } else { - /* check U/S/M bit against current privilege level */ - if ((ctrl >> 3) & BIT(env->priv)) { - return true; - } - } + return true; } break; default:
We have implemented trigger_common_match(), which checks if the enabled privilege levels of the trigger match CPU's current privilege level. Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke trigger_common_match() to check the privilege levels of the type 2 and type 6 triggers for the watchpoints. Only load/store bits and loaded/stored address should be further checked in riscv_cpu_debug_check_watchpoint(). Signed-off-by: Alvin Chang <alvinga@andestech.com> --- target/riscv/debug.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-)