Rather than use arm_adjust_watchpoint_address, delay the interpretation of BAS until the check_watchpoint hook so that we use the current value of SCTLR.B.
Signed-off-by: Richard Henderson <[email protected]> --- target/arm/internals.h | 5 -- target/arm/cpu.c | 1 - target/arm/debug_helper.c | 12 +++ target/arm/tcg/cpu-v7m.c | 1 - target/arm/tcg/debug.c | 159 ++++++++++++++++++++++++-------------- 5 files changed, 114 insertions(+), 64 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index 9deb7a6e22..4446adb896 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -694,11 +694,6 @@ bool arm_debug_check_breakpoint(CPUState *cs, CPUBreakpoint *bp); /* Callback function for checking if a watchpoint should trigger. */ bool arm_debug_check_watchpoint(CPUState *cs, CPUBreakpoint *wp); -/* Adjust addresses (in BE32 mode) before testing against watchpoint - * addresses. - */ -vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len); - /* Callback function for when a watchpoint or breakpoint triggers. */ void arm_debug_excp_handler(CPUState *cs, CPUBreakpoint *hit); diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 787e4dc7ab..e06fbb903a 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2609,7 +2609,6 @@ static const TCGCPUOps arm_tcg_ops = { .do_interrupt = arm_cpu_do_interrupt, .do_transaction_failed = arm_cpu_do_transaction_failed, .do_unaligned_access = arm_cpu_do_unaligned_access, - .adjust_watchpoint_address = arm_adjust_watchpoint_address, .debug_check_watchpoint = arm_debug_check_watchpoint, .debug_check_breakpoint = arm_debug_check_breakpoint, #endif /* !CONFIG_USER_ONLY */ diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index 2c3a56c7d5..086043cf5e 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -362,6 +362,18 @@ static void dbgwcr_write(CPUARMState *env, const ARMCPRegInfo *ri, ARMCPU *cpu = env_archcpu(env); int i = ri->crm; + /* + * Bits [7:4] of BAS are RAZ/WI if an 8-bit field is not supported. + * This is always true of v6 debug. + * As an IMPLEMENTATION DEFINED choice, we always implement the 8-bit + * field with ARMv7 cpus. + * This is never true of v8 debug. + * The Linux kernel will probe for 4 or 8-bit by writing all 1's. + */ + if (!arm_feature(env, ARM_FEATURE_V7)) { + value &= ~FIELD_DP64(0, DBGWCR, BAS, 0xf0); + } + raw_write(env, ri, value); if (tcg_enabled()) { hw_watchpoint_update(cpu, i); diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c index dc249ce1f1..b7e5623c59 100644 --- a/target/arm/tcg/cpu-v7m.c +++ b/target/arm/tcg/cpu-v7m.c @@ -262,7 +262,6 @@ static const TCGCPUOps arm_v7m_tcg_ops = { .do_interrupt = arm_v7m_cpu_do_interrupt, .do_transaction_failed = arm_cpu_do_transaction_failed, .do_unaligned_access = arm_cpu_do_unaligned_access, - .adjust_watchpoint_address = arm_adjust_watchpoint_address, .debug_check_watchpoint = arm_debug_check_watchpoint, .debug_check_breakpoint = arm_debug_check_breakpoint, #endif /* !CONFIG_USER_ONLY */ diff --git a/target/arm/tcg/debug.c b/target/arm/tcg/debug.c index d705f7aef2..1dec39c0bf 100644 --- a/target/arm/tcg/debug.c +++ b/target/arm/tcg/debug.c @@ -365,6 +365,31 @@ bool arm_debug_check_breakpoint(CPUState *cs, CPUBreakpoint *bp) return bp_wp_matches(cpu, env->cp15.dbgbcr[bp->id], arm_current_el(env)); } +static bool arm_debug_check_watchpoint_v6(CPUARMState *env, CPUBreakpoint *wp) +{ + uint32_t wcr = env->cp15.dbgwcr[wp->id]; + uint32_t bas = FIELD_EX64(wcr, DBGWCR, BAS) & 0xf; + + /* Create a mask of the bytes touched by the access. */ + uint32_t hitlen = wp->hitlast - wp->hitaddr + 1; + uint32_t hitmask = MAKE_64BIT_MASK(wp->hitaddr & 3, hitlen); + + /* With BE-32, the interpretation of BAS is inverted. */ + if (arm_sctlr_b(env)) { + bas = revbit8(bas) >> 4; + } + + hitmask &= bas; + if (!hitmask) { + return false; + } + + /* Update hit to the first byte matched. */ + wp->hitaddr = wp->itree.start + ctz32(hitmask); + wp->hitlast = wp->hitaddr; + return true; +} + bool arm_debug_check_watchpoint(CPUState *cs, CPUBreakpoint *wp) { /* @@ -380,7 +405,13 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUBreakpoint *wp) */ int access_el = wp->hitattrs.user ? 0 : arm_current_el(env); - return bp_wp_matches(cpu, env->cp15.dbgwcr[wp->id], access_el); + /* + * For ARM v6, we need to validate the address match. + * After that, everyone needs to validate linked watchpoints. + */ + return ((arm_feature(env, ARM_FEATURE_V7) || + arm_debug_check_watchpoint_v6(env, wp)) && + bp_wp_matches(cpu, env->cp15.dbgwcr[wp->id], access_el)); } /* @@ -481,46 +512,44 @@ void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome) raise_exception_debug(env, EXCP_UDEF, syndrome); } -void hw_watchpoint_update(ARMCPU *cpu, int n) +static void hw_watchpoint_update_v6(ARMCPU *cpu, int n, BreakpointFlags flags) +{ + /* + * With v6: + * - MASK does not exist, + * - BAS is 4 bits, + * - non-consecutive BAS bits are not yet deprecated, + * - BE-32 is supported, affecting interpretation of BAS. + * Install a 4 byte aligned watchpoint now and defer checking + * of the BAS bits until breakpoint hit. + */ + CPUARMState *env = &cpu->env; + uint32_t wvr = env->cp15.dbgwvr[n] & ~3; + uint32_t wcr = env->cp15.dbgwcr[n]; + uint32_t bas = FIELD_EX64(wcr, DBGWCR, BAS) & 0xf; + + if (bas == 0) { + /* This must act as if the watchpoint is disabled */ + return; + } + + env->cpu_watchpoint[n] = cpu_watchpoint_insert(CPU(cpu), wvr, 4, + flags, n); +} + +static void hw_watchpoint_update_v7(ARMCPU *cpu, int n, BreakpointFlags flags) { CPUARMState *env = &cpu->env; vaddr len = 0; vaddr wvr = env->cp15.dbgwvr[n]; uint64_t wcr = env->cp15.dbgwcr[n]; - int mask; - BreakpointFlags flags = BP_CPU | BP_STOP_BEFORE_ACCESS; - - if (env->cpu_watchpoint[n]) { - cpu_watchpoint_remove_by_ref(CPU(cpu), env->cpu_watchpoint[n]); - env->cpu_watchpoint[n] = NULL; - } - - if (!FIELD_EX64(wcr, DBGWCR, E)) { - /* E bit clear : watchpoint disabled */ - return; - } - - switch (FIELD_EX64(wcr, DBGWCR, LSC)) { - case 0: - /* LSC 00 is reserved and must behave as if the wp is disabled */ - return; - case 1: - flags |= BP_MEM_READ; - break; - case 2: - flags |= BP_MEM_WRITE; - break; - case 3: - flags |= BP_MEM_ACCESS; - break; - } + int mask = FIELD_EX64(wcr, DBGWCR, MASK); /* * Attempts to use both MASK and BAS fields simultaneously are * CONSTRAINED UNPREDICTABLE; we opt to ignore BAS in this case, * thus generating a watchpoint for every byte in the masked region. */ - mask = FIELD_EX64(wcr, DBGWCR, MASK); if (mask == 1 || mask == 2) { /* * Reserved values of MASK; we must act as if the mask value was @@ -570,6 +599,49 @@ void hw_watchpoint_update(ARMCPU *cpu, int n) flags, n); } +void hw_watchpoint_update(ARMCPU *cpu, int n) +{ + CPUARMState *env = &cpu->env; + uint64_t wcr = env->cp15.dbgwcr[n]; + int flags = BP_CPU | BP_STOP_BEFORE_ACCESS; + + if (env->cpu_watchpoint[n]) { + cpu_watchpoint_remove_by_ref(CPU(cpu), env->cpu_watchpoint[n]); + env->cpu_watchpoint[n] = NULL; + } + + if (!FIELD_EX64(wcr, DBGWCR, E)) { + /* E bit clear : watchpoint disabled */ + return; + } + + switch (FIELD_EX64(wcr, DBGWCR, LSC)) { + case 0: + /* LSC 00 is reserved and must behave as if the wp is disabled */ + return; + case 1: + flags |= BP_MEM_READ; + break; + case 2: + flags |= BP_MEM_WRITE; + break; + case 3: + flags |= BP_MEM_ACCESS; + break; + } + + /* + * ARM v6 debug. The AArch32 DBGDIDR register is deprecated + * under certain v8 conditions, and ID_DFR0 doesn't exist pre-v7. + * Cut the proverbial knot by checking cpu features. + */ + if (arm_feature(env, ARM_FEATURE_V7)) { + hw_watchpoint_update_v7(cpu, n, flags); + } else { + hw_watchpoint_update_v6(cpu, n, flags); + } +} + void hw_watchpoint_update_all(ARMCPU *cpu) { int i; @@ -689,30 +761,3 @@ void hw_breakpoint_update_all(ARMCPU *cpu) hw_breakpoint_update(cpu, i); } } - -#if !defined(CONFIG_USER_ONLY) - -vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) -{ - ARMCPU *cpu = ARM_CPU(cs); - CPUARMState *env = &cpu->env; - - /* - * In BE32 system mode, target memory is stored byteswapped (on a - * little-endian host system), and by the time we reach here (via an - * opcode helper) the addresses of subword accesses have been adjusted - * to account for that, which means that watchpoints will not match. - * Undo the adjustment here. - */ - if (arm_sctlr_b(env)) { - if (len == 1) { - addr ^= 3; - } else if (len == 2) { - addr ^= 2; - } - } - - return addr; -} - -#endif /* !CONFIG_USER_ONLY */ -- 2.43.0
