From cc217b0c90af4c635b7149d7eaa66bb67228e10e Mon Sep 17 00:00:00 2001 From: Julian Brown Date: Fri, 2 Mar 2018 00:24:19 -0500 Subject: [PATCH] arm: Correctly handle watchpoints for BE32 CPUs In BE32 mode, sub-word size watchpoints can fail to trigger because the address of the access is adjusted in the opcode helpers before being compared with the watchpoint registers. This patch reverses the address adjustment before performing the comparison with the help of a new CPUClass hook. This version of the patch augments and tidies up comments a little. Backports commit 40612000599e52e792d23c998377a0fa429c4036 from qemu --- qemu/aarch64.h | 1 + qemu/aarch64eb.h | 1 + qemu/arm.h | 1 + qemu/armeb.h | 1 + qemu/header_gen.py | 1 + qemu/include/qom/cpu.h | 3 +++ qemu/m68k.h | 1 + qemu/mips.h | 1 + qemu/mips64.h | 1 + qemu/mips64el.h | 1 + qemu/mipsel.h | 1 + qemu/powerpc.h | 1 + qemu/qom/cpu.c | 6 ++++++ qemu/sparc.h | 1 + qemu/sparc64.h | 1 + qemu/target/arm/cpu.c | 3 +++ qemu/target/arm/internals.h | 5 +++++ qemu/target/arm/op_helper.c | 22 ++++++++++++++++++++++ qemu/x86_64.h | 1 + 19 files changed, 53 insertions(+) diff --git a/qemu/aarch64.h b/qemu/aarch64.h index acade424..9fbbbc20 100644 --- a/qemu/aarch64.h +++ b/qemu/aarch64.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_aarch64 #define arm926_initfn arm926_initfn_aarch64 #define arm946_initfn arm946_initfn_aarch64 +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_aarch64 #define arm_ccnt_enabled arm_ccnt_enabled_aarch64 #define arm_cp_read_zero arm_cp_read_zero_aarch64 #define arm_cp_reset_ignore arm_cp_reset_ignore_aarch64 diff --git a/qemu/aarch64eb.h b/qemu/aarch64eb.h index 6e3119c0..22cef871 100644 --- a/qemu/aarch64eb.h +++ b/qemu/aarch64eb.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_aarch64eb #define arm926_initfn arm926_initfn_aarch64eb #define arm946_initfn arm946_initfn_aarch64eb +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_aarch64eb #define arm_ccnt_enabled arm_ccnt_enabled_aarch64eb #define arm_cp_read_zero arm_cp_read_zero_aarch64eb #define arm_cp_reset_ignore arm_cp_reset_ignore_aarch64eb diff --git a/qemu/arm.h b/qemu/arm.h index 046e3a4c..42dd2428 100644 --- a/qemu/arm.h +++ b/qemu/arm.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_arm #define arm926_initfn arm926_initfn_arm #define arm946_initfn arm946_initfn_arm +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_arm #define arm_ccnt_enabled arm_ccnt_enabled_arm #define arm_cp_read_zero arm_cp_read_zero_arm #define arm_cp_reset_ignore arm_cp_reset_ignore_arm diff --git a/qemu/armeb.h b/qemu/armeb.h index 61b6bf20..1e09a52b 100644 --- a/qemu/armeb.h +++ b/qemu/armeb.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_armeb #define arm926_initfn arm926_initfn_armeb #define arm946_initfn arm946_initfn_armeb +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_armeb #define arm_ccnt_enabled arm_ccnt_enabled_armeb #define arm_cp_read_zero arm_cp_read_zero_armeb #define arm_cp_reset_ignore arm_cp_reset_ignore_armeb diff --git a/qemu/header_gen.py b/qemu/header_gen.py index d032df50..494bbfc2 100644 --- a/qemu/header_gen.py +++ b/qemu/header_gen.py @@ -140,6 +140,7 @@ symbols = ( 'arm11mpcore_initfn', 'arm926_initfn', 'arm946_initfn', + 'arm_adjust_watchpoint_address', 'arm_ccnt_enabled', 'arm_cp_read_zero', 'arm_cp_reset_ignore', diff --git a/qemu/include/qom/cpu.h b/qemu/include/qom/cpu.h index d3af9f49..89c3d04a 100644 --- a/qemu/include/qom/cpu.h +++ b/qemu/include/qom/cpu.h @@ -110,6 +110,8 @@ struct TranslationBlock; * @cpu_exec_enter: Callback for cpu_exec preparation. * @cpu_exec_exit: Callback for cpu_exec cleanup. * @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec. + * @adjust_watchpoint_address: Perform a target-specific adjustment to an + * address before attempting to match it against watchpoints. * * Represents a CPU family or model. */ @@ -155,6 +157,7 @@ typedef struct CPUClass { void (*cpu_exec_enter)(CPUState *cpu); void (*cpu_exec_exit)(CPUState *cpu); bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); + vaddr (*adjust_watchpoint_address)(CPUState *cpu, vaddr addr, int len); } CPUClass; #ifdef HOST_WORDS_BIGENDIAN diff --git a/qemu/m68k.h b/qemu/m68k.h index 986b2648..f382ad95 100644 --- a/qemu/m68k.h +++ b/qemu/m68k.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_m68k #define arm926_initfn arm926_initfn_m68k #define arm946_initfn arm946_initfn_m68k +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_m68k #define arm_ccnt_enabled arm_ccnt_enabled_m68k #define arm_cp_read_zero arm_cp_read_zero_m68k #define arm_cp_reset_ignore arm_cp_reset_ignore_m68k diff --git a/qemu/mips.h b/qemu/mips.h index 5244987d..0ee7dc58 100644 --- a/qemu/mips.h +++ b/qemu/mips.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_mips #define arm926_initfn arm926_initfn_mips #define arm946_initfn arm946_initfn_mips +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_mips #define arm_ccnt_enabled arm_ccnt_enabled_mips #define arm_cp_read_zero arm_cp_read_zero_mips #define arm_cp_reset_ignore arm_cp_reset_ignore_mips diff --git a/qemu/mips64.h b/qemu/mips64.h index 89dfa38d..fe45132f 100644 --- a/qemu/mips64.h +++ b/qemu/mips64.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_mips64 #define arm926_initfn arm926_initfn_mips64 #define arm946_initfn arm946_initfn_mips64 +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_mips64 #define arm_ccnt_enabled arm_ccnt_enabled_mips64 #define arm_cp_read_zero arm_cp_read_zero_mips64 #define arm_cp_reset_ignore arm_cp_reset_ignore_mips64 diff --git a/qemu/mips64el.h b/qemu/mips64el.h index 8b1a201b..8f7a51f1 100644 --- a/qemu/mips64el.h +++ b/qemu/mips64el.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_mips64el #define arm926_initfn arm926_initfn_mips64el #define arm946_initfn arm946_initfn_mips64el +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_mips64el #define arm_ccnt_enabled arm_ccnt_enabled_mips64el #define arm_cp_read_zero arm_cp_read_zero_mips64el #define arm_cp_reset_ignore arm_cp_reset_ignore_mips64el diff --git a/qemu/mipsel.h b/qemu/mipsel.h index 97ddf1b1..a407328b 100644 --- a/qemu/mipsel.h +++ b/qemu/mipsel.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_mipsel #define arm926_initfn arm926_initfn_mipsel #define arm946_initfn arm946_initfn_mipsel +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_mipsel #define arm_ccnt_enabled arm_ccnt_enabled_mipsel #define arm_cp_read_zero arm_cp_read_zero_mipsel #define arm_cp_reset_ignore arm_cp_reset_ignore_mipsel diff --git a/qemu/powerpc.h b/qemu/powerpc.h index c2bfa9f1..3b8a3c11 100644 --- a/qemu/powerpc.h +++ b/qemu/powerpc.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_powerpc #define arm926_initfn arm926_initfn_powerpc #define arm946_initfn arm946_initfn_powerpc +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_powerpc #define arm_ccnt_enabled arm_ccnt_enabled_powerpc #define arm_cp_read_zero arm_cp_read_zero_powerpc #define arm_cp_reset_ignore arm_cp_reset_ignore_powerpc diff --git a/qemu/qom/cpu.c b/qemu/qom/cpu.c index 1c4a2581..ceee8ebe 100644 --- a/qemu/qom/cpu.c +++ b/qemu/qom/cpu.c @@ -263,6 +263,11 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu) return cpu->cpu_index; } +static vaddr cpu_adjust_watchpoint_address(CPUState *cpu, vaddr addr, int len) +{ + return addr; +} + static void cpu_class_init(struct uc_struct *uc, ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(uc, klass); @@ -280,6 +285,7 @@ static void cpu_class_init(struct uc_struct *uc, ObjectClass *klass, void *data) k->cpu_exec_enter = cpu_common_noop; k->cpu_exec_exit = cpu_common_noop; k->cpu_exec_interrupt = cpu_common_exec_interrupt; + k->adjust_watchpoint_address = cpu_adjust_watchpoint_address; dc->realize = cpu_common_realizefn; /* * Reason: CPUs still need special care by board code: wiring up diff --git a/qemu/sparc.h b/qemu/sparc.h index 735b8ff3..b5828ae9 100644 --- a/qemu/sparc.h +++ b/qemu/sparc.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_sparc #define arm926_initfn arm926_initfn_sparc #define arm946_initfn arm946_initfn_sparc +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_sparc #define arm_ccnt_enabled arm_ccnt_enabled_sparc #define arm_cp_read_zero arm_cp_read_zero_sparc #define arm_cp_reset_ignore arm_cp_reset_ignore_sparc diff --git a/qemu/sparc64.h b/qemu/sparc64.h index f1546036..48a1cc86 100644 --- a/qemu/sparc64.h +++ b/qemu/sparc64.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_sparc64 #define arm926_initfn arm926_initfn_sparc64 #define arm946_initfn arm946_initfn_sparc64 +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_sparc64 #define arm_ccnt_enabled arm_ccnt_enabled_sparc64 #define arm_cp_read_zero arm_cp_read_zero_sparc64 #define arm_cp_reset_ignore arm_cp_reset_ignore_sparc64 diff --git a/qemu/target/arm/cpu.c b/qemu/target/arm/cpu.c index 3a0dcb84..777bad9c 100644 --- a/qemu/target/arm/cpu.c +++ b/qemu/target/arm/cpu.c @@ -1419,6 +1419,9 @@ static void arm_cpu_class_init(struct uc_struct *uc, ObjectClass *oc, void *data #endif cc->debug_excp_handler = arm_debug_excp_handler; cc->debug_check_watchpoint = arm_debug_check_watchpoint; +#if !defined(CONFIG_USER_ONLY) + cc->adjust_watchpoint_address = arm_adjust_watchpoint_address; +#endif } static void cpu_register(struct uc_struct *uc, const ARMCPUInfo *info) diff --git a/qemu/target/arm/internals.h b/qemu/target/arm/internals.h index e9da1da7..c6b65e11 100644 --- a/qemu/target/arm/internals.h +++ b/qemu/target/arm/internals.h @@ -441,6 +441,11 @@ void hw_breakpoint_update_all(ARMCPU *cpu); /* Callback function for checking if a watchpoint should trigger. */ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *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); diff --git a/qemu/target/arm/op_helper.c b/qemu/target/arm/op_helper.c index 7bfcaafc..ba6e45b2 100644 --- a/qemu/target/arm/op_helper.c +++ b/qemu/target/arm/op_helper.c @@ -1230,6 +1230,28 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) return check_watchpoints(cpu); } +vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) +{ + ARMCPU *cpu = ARM_CPU(cs->uc, 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; +} + void arm_debug_excp_handler(CPUState *cs) { /* Called by core code when a watchpoint or breakpoint fires; diff --git a/qemu/x86_64.h b/qemu/x86_64.h index d54b552f..5b1733db 100644 --- a/qemu/x86_64.h +++ b/qemu/x86_64.h @@ -134,6 +134,7 @@ #define arm11mpcore_initfn arm11mpcore_initfn_x86_64 #define arm926_initfn arm926_initfn_x86_64 #define arm946_initfn arm946_initfn_x86_64 +#define arm_adjust_watchpoint_address arm_adjust_watchpoint_address_x86_64 #define arm_ccnt_enabled arm_ccnt_enabled_x86_64 #define arm_cp_read_zero arm_cp_read_zero_x86_64 #define arm_cp_reset_ignore arm_cp_reset_ignore_x86_64