Re: [Qemu-devel] [PULL 07/23] exec.c: Relax restrictions on watchpoint length and alignment

[Peter Maydell]

On 17 September 2014 21:48, Max Filippov <address@hidden> wrote:
> Hi Peter,
>
> On Fri, Sep 12, 2014 at 6:23 AM, Peter Maydell <address@hidden> wrote:
>> The current implementation of watchpoints requires that they
>> have a power of 2 length which is not greater than TARGET_PAGE_SIZE
>> and that their address is a multiple of their length. Watchpoints
>> on ARM don't fit these restrictions, so change the implementation
>> so they can be relaxed.
>>
>> Signed-off-by: Peter Maydell <address@hidden>
>> Reviewed-by: Richard Henderson <address@hidden>
>> ---
>>  exec.c            | 44 +++++++++++++++++++++++++++++++-------------
>>  include/qom/cpu.h |  2 +-
>>  linux-user/main.c |  3 +--
>>  3 files changed, 33 insertions(+), 16 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 7dddcc8..f3e7fb6 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -582,12 +582,10 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, 
>> vaddr len,
>>  int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
>>                            int flags, CPUWatchpoint **watchpoint)
>>  {
>> -    vaddr len_mask = ~(len - 1);
>>      CPUWatchpoint *wp;
>>
>> -    /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints 
>> */
>> -    if ((len & (len - 1)) || (addr & ~len_mask) ||
>> -            len == 0 || len > TARGET_PAGE_SIZE) {
>> +    /* forbid ranges which are empty or run off the end of the address 
>> space */
>> +    if (len == 0 || (addr + len - 1) <= addr) {
>
> With this change it's no longer possible to set one-byte-long watchpoint,
> xtensa testsuite fails because of that.
> I guess you meant 'len == 0 || (addr + len - 1) < addr' ?

Oops, yes.

thanks
-- PMM