Re: [Qemu-devel] [PATCH] TCG: Fix TB invalidation after breakpoint insertion/deletion

[Max Filippov]

On Mon, May 28, 2012 at 1:34 PM, Avi Kivity <address@hidden> wrote:
> On 05/24/2012 10:58 PM, Max Filippov wrote:
>> On Thu, May 24, 2012 at 6:26 PM, Avi Kivity <address@hidden> wrote:
>>> On 05/24/2012 05:11 PM, Max Filippov wrote:
>>>>>
>>>>> Not in breakpoint_invalidate as the missing offset was compensated
>>>>> before your commit (well, starting with c2f07f81a2 in fact).
>>>>
>>>> I'd say that compensation that you mention
>>>>
>>>>     ram_addr = (memory_region_get_ram_addr(section.mr)
>>>>                 + section.offset_within_region) & TARGET_PAGE_MASK;
>>>> this >>>>    ram_addr |= (pc & ~TARGET_PAGE_MASK);
>>>>     tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
>>>>
>>>> was removed by f3705d53296d, not by 1e7855a558
>>>
>>> Indeed.  Note how the |= cleverly accommodates both truncating and
>>> non-truncating cpu_get_phys_page_debug().
>>
>> Right. If the fix is going to be checked in then TeLeMan's original version
>> with '|' is preferable for this reason.
>
> I disagree.  Whatever we call cpu_get_phys_page_debug() has to either
> mask out the low bits, or not (I prefer the latter, since it's
> unambiguous for large pages), but it has to be consistent.  Once it's
> consistent, there's no reason to use clever tricks.

I meant a one line fix for the 1.1. I suspect that fixing entire
cpu_get_phys_page_debug thing for 1.1 is too risky.

-- 
Thanks.
-- Max