[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] exec: flush the whole TLB if a watchpoint crosses a page
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2] exec: flush the whole TLB if a watchpoint crosses a page boundary |
Date: |
Fri, 5 Jun 2020 18:11:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/3/20 2:46 PM, Philippe Mathieu-Daudé wrote:
> On 6/3/20 1:24 PM, Alex Bennée wrote:
>> There is no particular reason why you can't have a watchpoint in TCG
>> that covers a large chunk of the address space. We could be clever
>> about it but these cases are pretty rare and we can assume the user
>> will expect a little performance degradation.
>>
>> NB: In my testing gdb will silently squash a watchpoint like:
>>
>> watch (char[0x7fffffffff]) *0x0
>>
>> to a 4 byte watchpoint. Practically it will limit the maximum size
>> based on max-value-size. However given enough of a tweak the sky is
>> the limit.
>>
>> Reported-by: Alexander Bulekov <alxndr@bu.edu>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v2
>> - use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth
>
> Can we have a macro for this?
> Maybe QEMU_IN_PAGE_OFFSET(addr, TARGET_PAGE_MASK)?
> or QEMU_OFFSET_IN_PAGE()...
As this is queued, I suppose the implicit answer is "no."
>
>> ---
>> exec.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 5162f0d12f9..65a4376df37 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1036,6 +1036,7 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr,
>> vaddr len,
>> int flags, CPUWatchpoint **watchpoint)
>> {
>> CPUWatchpoint *wp;
>> + vaddr in_page;
>>
>> /* forbid ranges which are empty or run off the end of the address
>> space */
>> if (len == 0 || (addr + len - 1) < addr) {
>> @@ -1056,7 +1057,12 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr,
>> vaddr len,
>> QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);
>> }
>>
>> - tlb_flush_page(cpu, addr);
>> + in_page = -(addr | TARGET_PAGE_MASK);
>> + if (len <= in_page) {
>> + tlb_flush_page(cpu, addr);
>> + } else {
>> + tlb_flush(cpu);
>> + }
>>
>> if (watchpoint)
>> *watchpoint = wp;
>>
>