diff mbox series

[v4,22/28] target/arm: Restrict watchpoint code to system emulation

Message ID 20210303214708.1727801-23-f4bug@amsat.org
State New
Headers show
Series cpu: Introduce SysemuCPUOps structure, remove watchpoints from usermode | expand

Commit Message

Philippe Mathieu-Daudé March 3, 2021, 9:47 p.m. UTC
We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. Add #ifdef'ry around it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/arm/internals.h    |  2 ++
 target/arm/cpu.c          |  4 ++--
 target/arm/debug_helper.c |  8 ++++++++
 target/arm/helper.c       |  4 ++++
 target/arm/sve_helper.c   | 12 ++++++++++++
 5 files changed, 28 insertions(+), 2 deletions(-)

Comments

Richard Henderson March 3, 2021, 10:40 p.m. UTC | #1
On 3/3/21 1:47 PM, Philippe Mathieu-Daudé wrote:
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 844db08bd57..ed3f22d78a5 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -4849,6 +4849,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
>                   /* Some page is MMIO, see below. */
>                   goto do_fault;
>               }
> +#ifndef CONFIG_USER_ONLY
>               if (unlikely(flags & TLB_WATCHPOINT) &&
>                   (cpu_watchpoint_address_matches
>                    (env_cpu(env), addr + mem_off, 1 << msz)
> @@ -4856,6 +4857,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
>                   /* Watchpoint hit, see below. */
>                   goto do_fault;
>               }
> +#endif
>   

Nack.  These are eliminated via TLB_WATCHPOINT 0 in exec/cpu-all.h, and an 
empty inline function in hw/core/cpu.h.


r~
diff mbox series

Patch

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 3fb295431ae..8fa0a244d59 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -263,6 +263,7 @@  static inline bool extended_addresses_enabled(CPUARMState *env)
            (arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
 }
 
+#ifndef CONFIG_USER_ONLY
 /*
  * Update a QEMU watchpoint based on the information the guest has set in the
  * DBGWCR<n>_EL1 and DBGWVR<n>_EL1 registers.
@@ -286,6 +287,7 @@  bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
  * addresses.
  */
 vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len);
+#endif /* !CONFIG_USER_ONLY */
 
 void hw_breakpoint_update(ARMCPU *cpu, int n);
 /*
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 09566a535e5..efc338b24eb 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -432,10 +432,10 @@  static void arm_cpu_reset(DeviceState *dev)
     if (kvm_enabled()) {
         kvm_arm_reset_vcpu(cpu);
     }
-#endif
 
-    hw_breakpoint_update_all(cpu);
     hw_watchpoint_update_all(cpu);
+#endif
+    hw_breakpoint_update_all(cpu);
     arm_rebuild_hflags(env);
 }
 
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 980110e1328..b8b7d81762d 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -11,6 +11,8 @@ 
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 
+#ifndef CONFIG_USER_ONLY
+
 /* Return true if the linked breakpoint entry lbn passes its checks */
 static bool linked_bp_matches(ARMCPU *cpu, int lbn)
 {
@@ -227,6 +229,8 @@  bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
     return check_watchpoints(cpu);
 }
 
+#endif /* !CONFIG_USER_ONLY */
+
 static bool check_breakpoints(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -240,11 +244,13 @@  static bool check_breakpoints(ARMCPU *cpu)
         return false;
     }
 
+#ifndef CONFIG_USER_ONLY
     for (int n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) {
         if (bp_wp_matches(cpu, n, false)) {
             return true;
         }
     }
+#endif /* !CONFIG_USER_ONLY */
     return false;
 }
 
@@ -267,6 +273,7 @@  void arm_debug_excp_handler(CPUState *cs)
     CPUARMState *env = &cpu->env;
     uint64_t pc;
     bool same_el;
+#ifndef CONFIG_USER_ONLY
     CPUWatchpoint *wp_hit = cs->watchpoint_hit;
 
     if (wp_hit && (wp_hit->flags & BP_CPU)) {
@@ -282,6 +289,7 @@  void arm_debug_excp_handler(CPUState *cs)
                         arm_debug_target_el(env));
         return;
     }
+#endif /* !CONFIG_USER_ONLY */
 
     pc = is_a64(env) ? env->pc : env->regs[15];
     same_el = (arm_debug_target_el(env) == arm_current_el(env));
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 54648c7fbb6..2e7a6356ae3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6345,6 +6345,7 @@  static const ARMCPRegInfo zcr_el3_reginfo = {
     .writefn = zcr_write, .raw_writefn = raw_write
 };
 
+#ifndef CONFIG_USER_ONLY
 void hw_watchpoint_update(ARMCPU *cpu, int n)
 {
     CPUARMState *env = &cpu->env;
@@ -6470,6 +6471,7 @@  static void dbgwcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     raw_write(env, ri, value);
     hw_watchpoint_update(cpu, i);
 }
+#endif /* !CONFIG_USER_ONLY */
 
 void hw_breakpoint_update(ARMCPU *cpu, int n)
 {
@@ -6643,6 +6645,7 @@  static void define_debug_regs(ARMCPU *cpu)
         define_arm_cp_regs(cpu, dbgregs);
     }
 
+#ifndef CONFIG_USER_ONLY
     for (i = 0; i < arm_num_wrps(cpu); i++) {
         ARMCPRegInfo dbgregs[] = {
             { .name = "DBGWVR", .state = ARM_CP_STATE_BOTH,
@@ -6661,6 +6664,7 @@  static void define_debug_regs(ARMCPU *cpu)
         };
         define_arm_cp_regs(cpu, dbgregs);
     }
+#endif /* !CONFIG_USER_ONLY */
 }
 
 static void define_pmu_regs(ARMCPU *cpu)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 844db08bd57..ed3f22d78a5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4849,6 +4849,7 @@  void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
                 /* Some page is MMIO, see below. */
                 goto do_fault;
             }
+#ifndef CONFIG_USER_ONLY
             if (unlikely(flags & TLB_WATCHPOINT) &&
                 (cpu_watchpoint_address_matches
                  (env_cpu(env), addr + mem_off, 1 << msz)
@@ -4856,6 +4857,7 @@  void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
                 /* Watchpoint hit, see below. */
                 goto do_fault;
             }
+#endif
             if (mtedesc && !mte_probe1(env, mtedesc, addr + mem_off)) {
                 goto do_fault;
             }
@@ -4900,12 +4902,14 @@  void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
         uint64_t pg = *(uint64_t *)(vg + (reg_off >> 3));
         do {
             if ((pg >> (reg_off & 63)) & 1) {
+#ifndef CONFIG_USER_ONLY
                 if (unlikely(flags & TLB_WATCHPOINT) &&
                     (cpu_watchpoint_address_matches
                      (env_cpu(env), addr + mem_off, 1 << msz)
                      & BP_MEM_READ)) {
                     goto do_fault;
                 }
+#endif
                 if (mtedesc && !mte_probe1(env, mtedesc, addr + mem_off)) {
                     goto do_fault;
                 }
@@ -5355,10 +5359,12 @@  void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
                                mmu_idx, retaddr);
 
                 if (likely(in_page >= msize)) {
+#ifndef CONFIG_USER_ONLY
                     if (unlikely(info.flags & TLB_WATCHPOINT)) {
                         cpu_check_watchpoint(env_cpu(env), addr, msize,
                                              info.attrs, BP_MEM_READ, retaddr);
                     }
+#endif
                     if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
                         mte_check1(env, mtedesc, addr, retaddr);
                     }
@@ -5367,11 +5373,13 @@  void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
                     /* Element crosses the page boundary. */
                     sve_probe_page(&info2, false, env, addr + in_page, 0,
                                    MMU_DATA_LOAD, mmu_idx, retaddr);
+#ifndef CONFIG_USER_ONLY
                     if (unlikely((info.flags | info2.flags) & TLB_WATCHPOINT)) {
                         cpu_check_watchpoint(env_cpu(env), addr,
                                              msize, info.attrs,
                                              BP_MEM_READ, retaddr);
                     }
+#endif
                     if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
                         mte_check1(env, mtedesc, addr, retaddr);
                     }
@@ -5568,11 +5576,13 @@  void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
                 if (unlikely(info.flags & (TLB_INVALID_MASK | TLB_MMIO))) {
                     goto fault;
                 }
+#ifndef CONFIG_USER_ONLY
                 if (unlikely(info.flags & TLB_WATCHPOINT) &&
                     (cpu_watchpoint_address_matches
                      (env_cpu(env), addr, msize) & BP_MEM_READ)) {
                     goto fault;
                 }
+#endif
                 if (mtedesc &&
                     arm_tlb_mte_tagged(&info.attrs) &&
                     !mte_probe1(env, mtedesc, addr)) {
@@ -5754,10 +5764,12 @@  void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
                     info.flags |= info2.flags;
                 }
 
+#ifndef CONFIG_USER_ONLY
                 if (unlikely(info.flags & TLB_WATCHPOINT)) {
                     cpu_check_watchpoint(env_cpu(env), addr, msize,
                                          info.attrs, BP_MEM_WRITE, retaddr);
                 }
+#endif
 
                 if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
                     mte_check1(env, mtedesc, addr, retaddr);