Re: [Qemu-devel] [PATCH 4/5] ARM BE32 watchpoint fix.

[Julian Brown]

On Fri, 4 Nov 2016 09:55:17 +0100
Paolo Bonzini <address@hidden> wrote:

> On 04/11/2016 00:20, Julian Brown wrote:
> > On Thu, 3 Nov 2016 23:14:05 +0000
> > Peter Maydell <address@hidden> wrote:
> >   
> >> On 3 November 2016 at 17:30, Julian Brown <address@hidden>
> >> 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 <address@hidden>
> >>> ---
> >>>  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.

How's this? There's still some grubbiness, but it's mostly confined to
the ARM backend code.

Thanks,

Julian

0003-ARM-BE32-watchpoint-fix.patch
Description: Text Data