On Tue, Apr 07, 2015 at 09:09:59PM +0100, Peter Maydell wrote:
> 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.mayd...@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.igles...@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..ce09ab3 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 & MEMTXATTRS_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
>
