WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [RFC/PATCH v1] xen-gntdev: Use XENMEM_populate_physmap on un

To: Ian.Campbell@xxxxxxxxxx
Subject: [Xen-devel] [RFC/PATCH v1] xen-gntdev: Use XENMEM_populate_physmap on unmapped pages in HVM
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Fri, 4 Mar 2011 15:53:37 -0500
Cc: Tim.Deegan@xxxxxxxxxxxxx, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 04 Mar 2011 12:54:27 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D713798.4050907@xxxxxxxxxxxxx>
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: <4D713798.4050907@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>> Is there some reason the gntdev driver can't just balloon down
>> (XENMEM_decrease_reservation) to make itself a space to map and then
>> balloon back up (XENMEM_increase_reservation) after unmap when running
>> HVM?
> 
> I recall having problems with doing this last time, but I think other changes
> to make the balloon driver work in HVM may have fixed the issue. I'll try it
> again; I think this would be the best solution.

Using the balloon hypercalls does work in my test. Below is a patch that
uses them directly in gntdev, which is simpler but doesn't handle errors
in re-ballooning and doesn't take advantage of already-ballooned pages
from the balloon device. Taking advantage of the balloon device is also
possible and would be more efficient, but adds another dependency to
gntdev. Any opinions on which method is preferred? If not, I'll try
making a patch to implement the balloon-assisted version.

8<-------------------------------------------------------------------------

Grant mappings cause the PFN<->MFN mapping to be lost on the pages
used for the mapping. Instead of leaking memory, explicitly free it
to the hypervisor and request the memory back after unmapping. This
removes the need for the bad-page leak workaround.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
 drivers/xen/gntdev.c |   58 +++++++++++++++++++++++++++++++++----------------
 1 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index d43ff30..104098a 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -41,6 +41,7 @@
 #include <asm/xen/hypervisor.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/page.h>
+#include <xen/interface/memory.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Derek G. Murray <Derek.Murray@xxxxxxxxxxxx>, "
@@ -203,27 +204,9 @@ static void gntdev_put_map(struct grant_map *map)
                        unmap_grant_pages(map, 0, map->count);
 
                for (i = 0; i < map->count; i++) {
-                       uint32_t check, *tmp;
                        if (!map->pages[i])
                                continue;
-                       /* XXX When unmapping in an HVM domain, Xen will
-                        * sometimes end up mapping the GFN to an invalid MFN.
-                        * In this case, writes will be discarded and reads will
-                        * return all 0xFF bytes.  Leak these unusable GFNs
-                        * until Xen supports fixing their p2m mapping.
-                        *
-                        * Confirmed present in Xen 4.1-RC3 with HVM source
-                        */
-                       tmp = kmap(map->pages[i]);
-                       *tmp = 0xdeaddead;
-                       mb();
-                       check = *tmp;
-                       kunmap(map->pages[i]);
-                       if (check == 0xdeaddead)
-                               __free_page(map->pages[i]);
-                       else
-                               pr_debug("Discard page %d=%ld\n", i,
-                                       page_to_pfn(map->pages[i]));
+                       __free_page(map->pages[i]);
                }
        }
        kfree(map->pages);
@@ -258,11 +241,19 @@ static int map_grant_pages(struct grant_map *map)
 {
        int i, err = 0;
        phys_addr_t addr;
+       unsigned long frame;
 
        if (!use_ptemod) {
+               struct xen_memory_reservation reservation = {
+                       .address_bits = 0,
+                       .extent_order = 0,
+                       .nr_extents   = 1,
+                       .domid        = DOMID_SELF
+               };
                /* Note: it could already be mapped */
                if (map->map_ops[0].handle != -1)
                        return 0;
+               set_xen_guest_handle(reservation.extent_start, &frame);
                for (i = 0; i < map->count; i++) {
                        addr = (phys_addr_t)
                                pfn_to_kaddr(page_to_pfn(map->pages[i]));
@@ -271,6 +262,12 @@ static int map_grant_pages(struct grant_map *map)
                                map->grants[i].domid);
                        gnttab_set_unmap_op(&map->unmap_ops[i], addr,
                                map->flags, -1 /* handle */);
+                       /* TODO batch hypercall, needs buffer */
+                       frame = page_to_pfn(map->pages[i]);
+                       err = HYPERVISOR_memory_op(XENMEM_decrease_reservation,
+                               &reservation);
+                       if (WARN_ON(err <= 0))
+                               printk(KERN_INFO "memop returned %d\n", err);
                }
        }
 
@@ -324,6 +321,29 @@ static int __unmap_grant_pages(struct grant_map *map, int 
offset, int pages)
                        map->unmap_ops[offset+i].status);
                map->unmap_ops[offset+i].handle = -1;
        }
+
+       if (!use_ptemod) {
+               struct xen_memory_reservation reservation = {
+                       .address_bits = 0,
+                       .extent_order = 0,
+                       .nr_extents   = 1,
+                       .domid        = DOMID_SELF
+               };
+               int rc;
+               unsigned long frame;
+               set_xen_guest_handle(reservation.extent_start, &frame);
+               /* TODO batch hypercall, needs buffer */
+               for (i = offset; i < pages + offset; i++) {
+                       frame = page_to_pfn(map->pages[i]);
+                       rc = HYPERVISOR_memory_op(XENMEM_populate_physmap,
+                               &reservation);
+                       if (WARN_ON(rc <= 0)) {
+                               map->pages[i] = NULL;
+                               continue;
+                       }
+               }
+       }
+
        return err;
 }
 
-- 
1.7.3.4


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