This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] Paging and memory sharing for HVM guests

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] Paging and memory sharing for HVM guests
From: Patrick Colp <pjcolp@xxxxxxxxx>
Date: Fri, 18 Dec 2009 09:16:59 -0800
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Grzegorz Milos <gm281@xxxxxxxxx>, Andrew Peace <Andrew.Peace@xxxxxxxxxxxxx>
Delivery-date: Fri, 18 Dec 2009 09:17:31 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4B2BB5180200007800026AC4@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <db8ce2bd0912161514s7a162546gf7f5909db22e274c@xxxxxxxxxxxxxx> <4B2BB5180200007800026AC4@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (X11/20090817)
Jan Beulich wrote:
Grzegorz Milos <gm281@xxxxxxxxx> 17.12.09 00:14 >>>
The series of 46 patches attached to this email contain the initial
implementation of memory paging and sharing for Xen. Patrick Colp
leads the work on the pager, and I am mostly responsible for memory
sharing. We would be grateful for any comments/suggestions you might
have. Individual patches are labeled with comments describing their
purpose and a sign-off footnote. Of course we are happy to discuss
them in more detail, as required. Assuming that there are no major
objections against including them in the mainstream xen-unstable tree,
we would like to move future development to that tree.

So as far as I can tell we now have to colliding values in domctl.h:


Was it determined that this is not (going to be) a problem? It's just
the tools interface, but it's a public header...

XEN_DOMCTL_PFINFO_LPINTAB is currently only used by suspend/restore and offline page (which says the domain should be suspended first). Paging hasn't been fully tested with suspend yet. However, in xc_domain_save(), xc_map_foreign_batch() is called before xc_get_pfn_type_batch(), and xc_map_foreign_batch() ensures that all the pages in the specified range are paged in. So as far as I can tell, there should be no conflict here right now. But, that doesn't mean this couldn't cause problems in the future. Coming up with a non-conflicting solution would ultimately be preferred.

Also there are several places were gmfn_to_mfn() was replaced by
mfn_x(gfn_to_mfn()), but the p2m_is_valid() check that the former
does was lost. Again - has it been determined that this will never
cause any problem?

It's been awhile since I made that decision, but I seem to recall it making sense at the time. That could have been for EPT only, though (which is what paging/mem_event was originally designed on). However, I can see no reason to not have the p2m_is_valid() check put in below the gfn_to_mfn() calls. I've attached a patch which does this (except for in hvm_set_ioreq_page, since there's already a check for !p2m_is_ram() which will cause the function to return an error).

Add checks for p2m_is_valid() after calls to gfn_to_mfn() that replace calls
to gmfn_to_mfn(), which does the check internally.

Signed-off-by: Patrick Colp <Patrick.Colp@xxxxxxxxxx>

diff -r 1a911fd65e52 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Fri Dec 18 07:53:27 2009 +0000
+++ b/xen/arch/x86/mm.c Fri Dec 18 09:01:05 2009 -0800
@@ -3105,6 +3105,8 @@
             req.ptr -= cmd;
             gmfn = req.ptr >> PAGE_SHIFT;
             mfn = mfn_x(gfn_to_mfn(pt_owner, gmfn, &p2mt));
+            if ( !p2m_is_valid(p2mt) )
+              mfn = INVALID_MFN;
             if ( p2m_is_paged(p2mt) )
diff -r 1a911fd65e52 xen/common/grant_table.c
--- a/xen/common/grant_table.c  Fri Dec 18 07:53:27 2009 +0000
+++ b/xen/common/grant_table.c  Fri Dec 18 09:01:05 2009 -0800
@@ -1888,6 +1888,8 @@
         p2m_type_t p2mt;
         s_frame = mfn_x(gfn_to_mfn(sd, op->source.u.gmfn, &p2mt));
+        if ( !p2m_is_valid(p2mt) )
+          s_frame = INVALID_MFN;
         if ( p2m_is_paging(p2mt) )
             p2m_mem_paging_populate(sd, op->source.u.gmfn);
@@ -1929,6 +1931,8 @@
         p2m_type_t p2mt;
         d_frame = gfn_to_mfn_private(dd, op->dest.u.gmfn, &p2mt);
+        if ( !p2m_is_valid(p2mt) )
+          d_frame = INVALID_MFN;
         if ( p2m_is_paging(p2mt) )
             p2m_mem_paging_populate(dd, op->dest.u.gmfn);
Xen-devel mailing list