[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] Fix guest_physmap_add_entry checks


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: "George Dunlap" <George.Dunlap@xxxxxxxxxxxxx>
  • Date: Wed, 24 Sep 2008 16:17:19 +0100
  • Delivery-date: Wed, 24 Sep 2008 08:17:46 -0700
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:sender:to:subject:mime-version:content-type :content-transfer-encoding:content-disposition:x-google-sender-auth; b=pi83ANgC1Nr3f8dvca61ZwUWgK9CmNesme1DAOB5p2PGlraLrl01h4wXgqA0kDA4As 4cCE6zCeOK41zNE+avSvf6s/fpw8Y2aAiHbdddLBPJ2EHv7oDIfLGoWCWhrIqlp7JLtK sJ5HojOXtZFQ6t+faMMQmftrIrnNpetY+6c3o=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

guest_physmap_add_entry() checks to see if the given mfn and gpfn
range in the p2m and m2p tables is already mapped before overwriting
the maps, and attempts to do something reasonable so that we don't
have any "dangling" pointers.

Unfortunately, these checks got broken when the page_order argument
was added.  Each individual p2m and m2p entry needs to be checked, not
just the first page in a page order.

This patch implements that.

One decision that deserves some double-checking: when removing old p2m
mappings to the given MFN range, it now calls p2m_remove_page() with a
page_order of 0 (since we don't know how big the original order was
supposed to be) for each individual mfn.  It didn't look like this
would cause a problem anywhere.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

diff -r b53b02976633 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c     Wed Sep 24 12:47:33 2008 +0100
+++ b/xen/arch/x86/mm/p2m.c     Wed Sep 24 16:22:37 2008 +0100
@@ -953,38 +953,47 @@ guest_physmap_add_entry(struct domain *d

     P2M_DEBUG("adding gfn=%#lx mfn=%#lx\n", gfn, mfn);

-    omfn = gfn_to_mfn(d, gfn, &ot);
-    if ( p2m_is_ram(ot) )
+    /* First, remove m->p mappings for existing p->m mappings */
+    for ( i = 0; i < (1UL << page_order); i++ )
     {
-        ASSERT(mfn_valid(omfn));
-        for ( i = 0; i < (1UL << page_order); i++ )
-            set_gpfn_from_mfn(mfn_x(omfn)+i, INVALID_M2P_ENTRY);
-    }
-
-    ogfn = mfn_to_gfn(d, _mfn(mfn));
-    if (
-#ifdef __x86_64__
-        (ogfn != 0x5555555555555555L)
-#else
-        (ogfn != 0x55555555L)
-#endif
-        && (ogfn != INVALID_M2P_ENTRY)
-        && (ogfn != gfn) )
-    {
-        /* This machine frame is already mapped at another physical address */
-        P2M_DEBUG("aliased! mfn=%#lx, old gfn=%#lx, new gfn=%#lx\n",
-                  mfn, ogfn, gfn);
-        omfn = gfn_to_mfn(d, ogfn, &ot);
+        omfn = gfn_to_mfn(d, gfn, &ot);
         if ( p2m_is_ram(ot) )
         {
             ASSERT(mfn_valid(omfn));
-            P2M_DEBUG("old gfn=%#lx -> mfn %#lx\n",
-                      ogfn , mfn_x(omfn));
-            if ( mfn_x(omfn) == mfn )
-                p2m_remove_page(d, ogfn, mfn, page_order);
+            set_gpfn_from_mfn(mfn_x(omfn)+i, INVALID_M2P_ENTRY);
         }
     }

+    /* Then, look for m->p mappings for this range and deal with them */
+    for ( i = 0; i < (1UL << page_order); i++ )
+    {
+        ogfn = mfn_to_gfn(d, _mfn(mfn));
+        if (
+#ifdef __x86_64__
+            (ogfn != 0x5555555555555555L)
+#else
+            (ogfn != 0x55555555L)
+#endif
+            && (ogfn != INVALID_M2P_ENTRY)
+            && (ogfn != gfn) )
+        {
+            /* This machine frame is already mapped at another physical
+             * address */
+            P2M_DEBUG("aliased! mfn=%#lx, old gfn=%#lx, new gfn=%#lx\n",
+                      mfn, ogfn, gfn);
+            omfn = gfn_to_mfn(d, ogfn, &ot);
+            if ( p2m_is_ram(ot) )
+            {
+                ASSERT(mfn_valid(omfn));
+                P2M_DEBUG("old gfn=%#lx -> mfn %#lx\n",
+                          ogfn , mfn_x(omfn));
+                if ( mfn_x(omfn) == mfn )
+                    p2m_remove_page(d, ogfn, mfn, 0);
+            }
+        }
+    }
+
+    /* Now, actually do the two-way mapping */
     if ( mfn_valid(_mfn(mfn)) )
     {
         if ( !set_p2m_entry(d, gfn, _mfn(mfn), page_order, t) )

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.