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

[Max Filippov]

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.

>>> But it looks like cpu_get_phys_page_debug was broken for quite a while.
>>> Let's fix those archs to return more than page-aligned addresses.
>>
>> You mean make them all return full physical address?
>> I'd propose to rename the function then as well.
>
> Agree to both.  cpu_translate_virtual_address() or similar would be more
> explanatory IMO.

Looks like Jan has an opposite opinion.

> btw, how does the thing work for soft-tlb cpus?  It looks like this
> thing should trigger on tlb loads, not when the breakpoint is set.  This
> is true for hardware page tables as well, as the mapping can change,
> though it's less likely.

I guess it works by chance. And chances high that breakpoints are
removed in the same virtual memory context where they got triggered,
so that right TBs are invalidated.

> --
> error compiling committee.c: too many arguments to function

-- 
Thanks.
-- Max