| Message ID | 1478194258-75276-5-git-send-email-julian@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On 3 November 2016 at 17:30, Julian Brown <julian@codesourcery.com> wrote: > 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 reversed the address > adjustment before performing the comparison. > > Signed-off-by: Julian Brown <julian@codesourcery.com> > --- > exec.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/exec.c b/exec.c > index 4c84389..eadab54 100644 > --- a/exec.c > +++ b/exec.c > @@ -2047,6 +2047,19 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) > return; > } > vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; > +#if defined(TARGET_ARM) && !defined(CONFIG_USER_ONLY) > + /* In BE32 system mode, target memory is stored byteswapped (FIXME: > + relative to 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) > + vaddr ^= 3; > + else if (len == 2) > + vaddr ^= 2; > + } > +#endif No target-CPU specific code in exec.c, please... thanks -- PMM
On Thu, 3 Nov 2016 23:14:05 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On 3 November 2016 at 17:30, Julian Brown <julian@codesourcery.com> > wrote: > > 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 reversed > > the address adjustment before performing the comparison. > > > > Signed-off-by: Julian Brown <julian@codesourcery.com> > > --- > > exec.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/exec.c b/exec.c > > index 4c84389..eadab54 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -2047,6 +2047,19 @@ static void check_watchpoint(int offset, int > > len, MemTxAttrs attrs, int flags) return; > > } > > vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; > > +#if defined(TARGET_ARM) && !defined(CONFIG_USER_ONLY) > > + /* In BE32 system mode, target memory is stored byteswapped > > (FIXME: > > + relative to 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) > > + vaddr ^= 3; > > + else if (len == 2) > > + vaddr ^= 2; > > + } > > +#endif > > No target-CPU specific code in exec.c, please... Yeah, I'd imagine not. I struggled with this one. Any suggestions for a better way to do this? Thanks, Julian
On 04/11/2016 00:20, Julian Brown wrote: > On Thu, 3 Nov 2016 23:14:05 +0000 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On 3 November 2016 at 17:30, Julian Brown <julian@codesourcery.com> >> wrote: >>> 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 reversed >>> the address adjustment before performing the comparison. >>> >>> Signed-off-by: Julian Brown <julian@codesourcery.com> >>> --- >>> exec.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/exec.c b/exec.c >>> index 4c84389..eadab54 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -2047,6 +2047,19 @@ static void check_watchpoint(int offset, int >>> len, MemTxAttrs attrs, int flags) return; >>> } >>> vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; >>> +#if defined(TARGET_ARM) && !defined(CONFIG_USER_ONLY) >>> + /* In BE32 system mode, target memory is stored byteswapped >>> (FIXME: >>> + relative to 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) >>> + vaddr ^= 3; >>> + else if (len == 2) >>> + vaddr ^= 2; >>> + } >>> +#endif >> >> No target-CPU specific code in exec.c, please... > > Yeah, I'd imagine not. I struggled with this one. Any suggestions for a > better way to do this? You can add a function pointer to CPUClass and call it from here. It's how cc->debug_check_watchpoint is being called already. Paolo
diff --git a/exec.c b/exec.c index 4c84389..eadab54 100644 --- a/exec.c +++ b/exec.c @@ -2047,6 +2047,19 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags) return; } vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; +#if defined(TARGET_ARM) && !defined(CONFIG_USER_ONLY) + /* In BE32 system mode, target memory is stored byteswapped (FIXME: + relative to 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) + vaddr ^= 3; + else if (len == 2) + vaddr ^= 2; + } +#endif QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { if (cpu_watchpoint_address_matches(wp, vaddr, len) && (wp->flags & flags)) {
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 reversed the address adjustment before performing the comparison. Signed-off-by: Julian Brown <julian@codesourcery.com> --- exec.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)