Re: [PATCH 06/12] xenpaging: update machine_to_phys_mapping[] during page deallocation

From: Keir Fraser <keir@xen.org>
To: Olaf Hering <olaf@aepfle.de>, xen-devel@lists.xensource.com
Subject: Re: [PATCH 06/12] xenpaging: update machine_to_phys_mapping[] during page deallocation
Date: Tue, 11 Jan 2011 10:37:08 +0000	[thread overview]
Message-ID: <C951E354.29FA4%keir@xen.org> (raw)
In-Reply-To: <20110110164347.726339637@aepfle.de>

Could we do this in free_heap_pages() instead? That definitely catches
everything that gets placed in Xen's free pool.

 -- Keir

On 10/01/2011 16:43, "Olaf Hering" <olaf@aepfle.de> wrote:

> The machine_to_phys_mapping[] array needs updating during page
> deallocation.  If that page is allocated again, a call to
> get_gpfn_from_mfn() will still return an old gfn from another guest.
> This will cause trouble because this gfn number has no or different
> meaning in the context of the current guest.
> 
> This happens when the entire guest ram is paged-out before
> xen_vga_populate_vram() runs.  Then XENMEM_populate_physmap is called
> with gfn 0xff000.  A new page is allocated with alloc_domheap_pages.
> This new page does not have a gfn yet.  However, in
> guest_physmap_add_entry() the passed mfn maps still to an old gfn
> (perhaps from another old guest).  This old gfn is in paged-out state in
> this guests context and has no mfn anymore.  As a result, the ASSERT()
> triggers because p2m_is_ram() is true for p2m_ram_paging* types.
> If the machine_to_phys_mapping[] array is updated properly, both loops
> in guest_physmap_add_entry() turn into no-ops for the new page and the
> mfn/gfn mapping will be done at the end of the function.
> 
> If XENMEM_add_to_physmap is used with XENMAPSPACE_gmfn,
> get_gpfn_from_mfn() will return an appearently valid gfn.  As a result,
> guest_physmap_remove_page() is called.  The ASSERT in p2m_remove_page
> triggers because the passed mfn does not match the old mfn for the
> passed gfn.
> 
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  xen/common/page_alloc.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- xen-unstable.hg-4.1.22571.orig/xen/common/page_alloc.c
> +++ xen-unstable.hg-4.1.22571/xen/common/page_alloc.c
> @@ -1200,9 +1200,15 @@ void free_domheap_pages(struct page_info
>  {
>      int            i, drop_dom_ref;
>      struct domain *d = page_get_owner(pg);
> +    unsigned long mfn;
>  
>      ASSERT(!in_irq());
>  
> +    /* this page is not a gfn anymore */
> +    mfn = page_to_mfn(pg);
> +    for ( i = 0; i < (1 << order); i++ )
> +        set_gpfn_from_mfn(mfn + i, INVALID_M2P_ENTRY);
> +
>      if ( unlikely(is_xen_heap_page(pg)) )
>      {
>          /* NB. May recursively lock from relinquish_memory(). */
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel