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

[Paolo Bonzini]

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.

Paolo