Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

[Peter Maydell]

On 11 January 2016 at 09:01,  <address@hidden> wrote:
> From: Chen Gang <address@hidden>
>
> mmap() size in mmap_frag() is qemu_host_page_size, but the outside calls
> page_set_flags() may be not with qemu_host_page_size. So after mmap(),
> call page_set_flags() in time.
>
> After this fix,  for the next call for the same region, prot1 will be
> PAGE_VALID (not 0), so can avoid to enter "if (prot1 == 0)" case, again.
>
> Signed-off-by: Chen Gang <address@hidden>
> ---
>  linux-user/mmap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 445e8c6..7807ed0 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -162,6 +162,8 @@ static int mmap_frag(abi_ulong real_start,
>                         flags | MAP_ANONYMOUS, -1, 0);
>          if (p == MAP_FAILED)
>              return -1;
> +        page_set_flags(real_start, real_start + qemu_host_page_size,
> +                       PAGE_VALID);
>          prot1 = prot;
>      }
>      prot1 &= PAGE_BITS;

I'm confused about this change, because as far as I can see
page_set_flags/page_get_flags work on guest pages, not host pages.
So setting the flags for the whole of the host page to PAGE_VALID
doesn't seem right -- the other guest pages within this host page
are not valid. And the VALID bit should be set for the guest page
that is within the mapping as part of the call to page_set_flags()
at the bottom of target_mmap().

It would help if you could explain what the failure case is that
you're trying to fix, and how the current code fails.

thanks
-- PMM