Re: [Xen-devel] [PATCH] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap

[Tim Deegan <tim@xxxxxxx>]

At 04:36 -0700 on 17 May (1337229382), Andres Lagar-Cavilla wrote:
> I believe the fix we'll converge to is keep paging_in in the "hole" set of
> types, and remove the check for valid mfn. Something along the lines of
> what I've pasted below.

OK.  Please send it as a diff against what's already applied - I think
what we have now is more correct (if less useful) than what we had
before.

Do you have to worry about freeing the page as well?  Will it otherwise
be leaked into a state where it's allocated but not in the p2m?  I see
that guest_physmap_add_entry() doesn't free paging_in pages but maybe
that's wrong too?

Cheers,

Tim.

> # HG changeset patch
> # Parent 9fc0252536f0a4ddf48b7ec9cd7a7243271545f8
> x86/mem_sharing: Rectify test for "empty" physmap entry in
> sharing_add_to_physmap.
> 
> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> 
> diff -r 9fc0252536f0 xen/arch/x86/mm/mem_sharing.c
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1073,9 +1073,10 @@ int mem_sharing_add_to_physmap(struct do
>      if ( spage->sharing->handle != sh )
>          goto err_unlock;
> 
> -    /* Make sure the target page is a hole in the physmap */
> -    if ( mfn_valid(cmfn) ||
> -         (!(p2m_is_ram(cmfn_type))) )
> +    /* Make sure the target page is a hole in the physmap. These are
> typically
> +     * p2m_mmio_dm, but also accept p2m_invalid and paged out pages. See the
> +     * definition of p2m_is_hole in p2m.h. */
> +    if ( !p2m_is_hole(cmfn_type) )
>      {
>          ret = XENMEM_SHARING_OP_C_HANDLE_INVALID;
>          goto err_unlock;
> diff -r 9fc0252536f0 xen/include/asm-x86/p2m.h
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -133,6 +133,13 @@ typedef unsigned int p2m_query_t;
>                         | p2m_to_mask(p2m_ram_paging_in)       \
>                         | p2m_to_mask(p2m_ram_shared))
> 
> +/* Types that represent a physmap hole that is ok to replace with a shared
> + * entry */
> +#define P2M_HOLE_TYPES (p2m_to_mask(p2m_mmio_dm)        \
> +                       | p2m_to_mask(p2m_invalid)       \
> +                       | p2m_to_mask(p2m_ram_paging_in) \
> +                       | p2m_to_mask(p2m_ram_paged))
> +
>  /* Grant mapping types, which map to a real machine frame in another
>   * VM */
>  #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw)  \
> @@ -173,6 +180,7 @@ typedef unsigned int p2m_query_t;
> 
>  /* Useful predicates */
>  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> +#define p2m_is_hole(_t) (p2m_to_mask(_t) & P2M_HOLE_TYPES)
>  #define p2m_is_mmio(_t) (p2m_to_mask(_t) & P2M_MMIO_TYPES)
>  #define p2m_is_readonly(_t) (p2m_to_mask(_t) & P2M_RO_TYPES)
>  #define p2m_is_magic(_t) (p2m_to_mask(_t) & P2M_MAGIC_TYPES)
> 
> >
> > Cheers,
> >
> > Tim.
> >
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel