Re: [Xen-devel] [PATCHv2 2/3] mm: don't free pages until mm locks are released

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: David Vrabel [mailto:david.vrabel@xxxxxxxxxx]
> Sent: Saturday, November 14, 2015 2:50 AM
> 
> If a page is freed without translations being invalidated, and the page is
> subsequently allocated to another domain, a guest with a cached
> translation will still be able to access the page.
> 
> Currently translations are invalidated before releasing the page ref, but
> while still holding the mm locks.  To allow translations to be invalidated
> without holding the mm locks, we need to keep a reference to the page
> for a bit longer in some cases.
> 
> [ This seems difficult to a) verify as correct; and b) difficult to get
> correct in the future.  A better suggestion would be useful.  Perhaps
> using something like pg->tlbflush_needed mechanism that already exists
> for pages from PV guests? ]

Per-page flag looks clean in general, but not an expert here. Tim might
have a better idea.

> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
>  xen/arch/x86/mm/p2m.c | 9 +++++++--
>  xen/common/memory.c   | 2 +-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index ed0bbd7..e2c82b1 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2758,6 +2758,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long 
> fgfn,
>      p2m_type_t p2mt, p2mt_prev;
>      unsigned long prev_mfn, mfn;
>      struct page_info *page;
> +    struct page_info *prev_page = NULL;
>      int rc;
>      struct domain *fdom;
> 
> @@ -2805,6 +2806,9 @@ int p2m_add_foreign(struct domain *tdom, unsigned long 
> fgfn,
>      prev_mfn = mfn_x(get_gfn(tdom, gpfn, &p2mt_prev));
>      if ( mfn_valid(_mfn(prev_mfn)) )
>      {
> +        prev_page = mfn_to_page(_mfn(prev_mfn));
> +        get_page(prev_page, tdom);
> +
>          if ( is_xen_heap_mfn(prev_mfn) )
>              /* Xen heap frames are simply unhooked from this phys slot */
>              guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
> @@ -2823,14 +2827,15 @@ int p2m_add_foreign(struct domain *tdom, unsigned long
> fgfn,
>                   "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
>                   gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id);
> 
> -    put_page(page);
> -
>      /*
>       * This put_gfn for the above get_gfn for prev_mfn.  We must do this
>       * after set_foreign_p2m_entry so another cpu doesn't populate the gpfn
>       * before us.
>       */
>      put_gfn(tdom, gpfn);
> +    if ( prev_page )
> +        put_page(prev_page);
> +    put_page(page);
> 
>  out:
>      if ( fdom )
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index a3bffb7..571c754 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -272,8 +272,8 @@ int guest_remove_page(struct domain *d, unsigned long 
> gmfn)
> 
>      guest_physmap_remove_page(d, gmfn, mfn, 0);
> 
> -    put_page(page);
>      put_gfn(d, gmfn);
> +    put_page(page);
> 
>      return 1;
>  }
> --
> 2.1.4


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