From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Colp Subject: Re: [PATCH] Paging and memory sharing for HVM guests Date: Fri, 18 Dec 2009 09:16:59 -0800 Message-ID: <4B2BB90B.60801@cs.ubc.ca> References: <4B2BB5180200007800026AC4@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------040507040302080400060207" Return-path: In-Reply-To: <4B2BB5180200007800026AC4@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: xen-devel@lists.xensource.com, Keir Fraser , Grzegorz Milos , Andrew Peace List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------040507040302080400060207 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Jan Beulich wrote: >>>> Grzegorz Milos 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: > > #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31) > #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28) > > 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). Patrick --------------040507040302080400060207 Content-Type: text/x-patch; name="mem_paging_gfn_to_mfn_check.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="mem_paging_gfn_to_mfn_check.patch" 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 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); --------------040507040302080400060207 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------040507040302080400060207--