Re: [Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap

[Alexander Graf]

On 11.05.2012, at 17:46, Peter Maydell wrote:

> On 7 May 2012 12:38, Alexander Graf <address@hidden> wrote:
>> 
>> On 07.05.2012, at 13:32, Alexander Graf wrote:
>> 
>>> 
>>> On 07.05.2012, at 12:37, Peter Maydell wrote:
>>> 
>>>> On 7 May 2012 10:30, Alexander Graf <address@hidden> wrote:
>>>>> @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
>>>>> int prot,
>>>>>    page_dump(stdout);
>>>>>    printf("\n");
>>>>> #endif
>>>>> +    tb_invalidate_phys_page_range(start, start + len, 0);
>>>>>    mmap_unlock();
>>>>>    return start;
>>>> 
>>>> The comment at the top of tb_invalidate_phys_page_range() says
>>>> "start and end must refer to the same physical page" -- is it
>>>> out of date or does that not apply to user-mode?
>>>> 
>>>> Do you need to also invalidate the range on munmap() and
>>>> mprotect-to-not-executable in order to correctly fault on
>>>> the case of:
>>>> map something
>>>> execute it
>>>> unmap it
>>>> try to execute it again
>>>> 
>>>> ? (haven't tested that case but it seems like it might be an issue)
>>> 
>>> Yeah, the issue does exist:
>> 
>> And the below patch on top of my revised patch fixes it.
> 
> I think these two patches look correct (and as you pointed out
> on irc I was wrong about mprotect, which effectively already
> handles flushing the tb if needed). If you can roll them together
> into a single patch with a commit message and signed-off-by
> you can add my Reviewed-by: tag to it.

Well, if we invalidate on unmap, do we need to invalidate on mmap() still? Or 
is only invalidating on unmap enough? Maybe when you use fixed addresses... hrm

Ah whatever, let's just flush everywhere now and then optimize later.


Alex