From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgAmJ-0000CL-23 for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:37:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YgAmF-0002yc-Ot for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:37:46 -0400 Received: from mail-pd0-x22d.google.com ([2607:f8b0:400e:c02::22d]:35332) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgAmF-0002yW-Hg for qemu-devel@nongnu.org; Thu, 09 Apr 2015 07:37:43 -0400 Received: by pddn5 with SMTP id n5so150203971pdd.2 for ; Thu, 09 Apr 2015 04:37:42 -0700 (PDT) Date: Thu, 9 Apr 2015 21:37:37 +1000 From: "Edgar E. Iglesias" Message-ID: <20150409113737.GM30629@toto> References: <1428437400-8474-1-git-send-email-peter.maydell@linaro.org> <1428437400-8474-14-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1428437400-8474-14-git-send-email-peter.maydell@linaro.org> Subject: Re: [Qemu-devel] [PATCH 13/14] target-arm: Use attribute info to handle user-only watchpoints List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Peter Crosthwaite , patches@linaro.org, qemu-devel@nongnu.org, Greg Bellows , Paolo Bonzini , Alex =?iso-8859-1?Q?Benn=E9e?= , Richard Henderson 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 Reviewed-by: Edgar E. Iglesias > --- > 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 >