[Qemu-devel] [PATCH v2 13/14] target-arm: Use attribute info to handle user-only watchpoints

[Peter Maydell <peter.maydell@linaro.org>]

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>
---
 target-arm/op_helper.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 7713022..4a8c4e0 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) {
-- 
1.9.1