target-arm: Use attribute info to handle user-only watchpoints

Now that we have memory access attribute information in the watchpoint
checking code, we can correctly implement handling of watchpoints
which should match only on userspace accesses, where LDRT/STRT/LDT/STT
from EL1 are treated as userspace accesses.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

1 files changed, 12 insertions, 11 deletions

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 7713022752..4a8c4e000d 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -602,13 +602,22 @@ static bool bp_wp_matches(ARMCPU *cpu, int n, bool is_wp)
     int pac, hmc, ssc, wt, lbn;
     /* TODO: check against CPU security state when we implement TrustZone */
     bool is_secure = false;
+    int access_el = arm_current_el(env);
 
     if (is_wp) {
-        if (!env->cpu_watchpoint[n]
-            || !(env->cpu_watchpoint[n]->flags & BP_WATCHPOINT_HIT)) {
+        CPUWatchpoint *wp = env->cpu_watchpoint[n];
+
+        if (!wp || !(wp->flags & BP_WATCHPOINT_HIT)) {
             return false;
         }
         cr = env->cp15.dbgwcr[n];
+        if (wp->hitattrs.user) {
+            /* The LDRT/STRT/LDT/STT "unprivileged access" instructions should
+             * match watchpoints as if they were accesses done at EL0, even if
+             * the CPU is at EL1 or higher.
+             */
+            access_el = 0;
+        }
     } else {
         uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
 
@@ -649,15 +658,7 @@ static bool bp_wp_matches(ARMCPU *cpu, int n, bool is_wp)
         break;
     }
 
-    /* TODO: this is not strictly correct because the LDRT/STRT/LDT/STT
-     * "unprivileged access" instructions should match watchpoints as if
-     * they were accesses done at EL0, even if the CPU is at EL1 or higher.
-     * Implementing this would require reworking the core watchpoint code
-     * to plumb the mmu_idx through to this point. Luckily Linux does not
-     * rely on this behaviour currently.
-     * For breakpoints we do want to use the current CPU state.
-     */
-    switch (arm_current_el(env)) {
+    switch (access_el) {
     case 3:
     case 2:
         if (!hmc) {