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:
#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
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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home