Implemented unmap with checks. Fixed unmap_region to unmap each frame individually.
authorSimon Gerber <simugerber@student.ethz.ch>
Thu, 22 Nov 2012 14:53:17 +0000 (15:53 +0100)
committerSimon Gerber <simon.gerber@inf.ethz.ch>
Tue, 29 Jan 2013 10:31:01 +0000 (11:31 +0100)
include/arch/x86_64/barrelfish/invocations_arch.h
include/barrelfish/capabilities.h
kernel/arch/x86_64/page_mappings_arch.c
kernel/arch/x86_64/syscall.c
kernel/include/capabilities.h
kernel/include/paging_generic.h
lib/barrelfish/target/x86_64/pmap_target.c
lib/barrelfish/vspace/memobj_anon.c

index 8a38beb..1bda951 100644 (file)
@@ -196,9 +196,9 @@ static inline errval_t invoke_cnode_revoke(struct capref root, capaddr_t cap,
     return cap_invoke3(root, CNodeCmd_Revoke, cap, bits).error;
 }
 
-static inline errval_t invoke_vnode_unmap(struct capref cap, size_t entry)
+static inline errval_t invoke_vnode_unmap(struct capref cap, size_t entry, size_t num_pages)
 {
-    return cap_invoke2(cap, VNodeCmd_Unmap, entry).error;
+    return cap_invoke3(cap, VNodeCmd_Unmap, entry, num_pages).error;
 }
 
 static inline errval_t
index 49adb23..d7aa324 100644 (file)
@@ -102,9 +102,9 @@ vnode_map(struct capref dest, struct capref src, capaddr_t slot,
                              attr, off);
 }
 
-static inline errval_t vnode_unmap(struct capref pgtl, size_t entry)
+static inline errval_t vnode_unmap(struct capref pgtl, size_t entry, size_t num_pages)
 {
-    return invoke_vnode_unmap(pgtl, entry);
+    return invoke_vnode_unmap(pgtl, entry, num_pages);
 }
 
 static inline errval_t
index 0cffc3c..06029a2 100644 (file)
@@ -18,6 +18,7 @@
 #include <target/x86_64/offsets_target.h>
 #include <mdb/mdb_tree.h>
 #include <string.h>
+#include <barrelfish_kpi/init.h>
 
 /// Map within a x86_64 pml4
 static errval_t x86_64_pml4(struct capability *dest, cslot_t slot,
@@ -248,31 +249,25 @@ errval_t caps_copy_to_vnode(struct cte *dest_vnode_cte, cslot_t dest_slot,
         // set current pte as mapping pte if not set already
         if (src_cte->mapping_info.pte == 0) {
             src_cte->mapping_info.pte = pte;
-            src_cte->mapping_info.pt_slot = dest_slot;
-        }
-        else if (dest_slot == 0 && src_cte->mapping_info.pt2 == 0) {
-            // if dest_slot zero and pte set, assume we crossed leaf ptable
-            // boundaries, and store pte in mapping_info.pte2 (if not set already)
-            src_cte->mapping_info.pt2 = pte;
-        }
-        else if (dest_slot == 0) {
-            printf("mapping uses more than 3 leaf ptables, expect unmap badness\n");
         }
         src_cte->mapping_info.mapped_pages += 1;
     }
     return r;
 }
 
-errval_t page_mappings_unmap(struct capability *pgtable, size_t slot)
+static inline void read_pt_entry(struct capability *pgtable, size_t slot,
+                                 genpaddr_t *mapped_addr, lpaddr_t *pte,
+                                 void **entry)
 {
     assert(type_is_vnode(pgtable->type));
 
+    genpaddr_t paddr;
+    lpaddr_t pte_;
+    void *entry_;
+
     genpaddr_t gp = get_address(pgtable);
     lpaddr_t lp = gen_phys_to_local_phys(gp);
     lvaddr_t lv = local_phys_to_mem(lp);
-    genpaddr_t paddr;
-    lpaddr_t pte;
-    void *entry;
 
     // get paddr
     switch (pgtable->type) {
@@ -282,42 +277,47 @@ errval_t page_mappings_unmap(struct capability *pgtable, size_t slot)
         union x86_64_pdir_entry *e =
             (union x86_64_pdir_entry *)lv + slot;
         paddr = e->d.base_addr << BASE_PAGE_BITS;
-        entry = e;
-        pte = lp + slot * sizeof(union x86_64_pdir_entry);
+        entry_ = e;
+        pte_ = lp + slot * sizeof(union x86_64_pdir_entry);
         break;
     }
     case ObjType_VNode_x86_64_ptable: {
         union x86_64_ptable_entry *e =
             (union x86_64_ptable_entry *)lv + slot;
         paddr = e->base.base_addr << BASE_PAGE_BITS;
-        entry = e;
-        pte = lp + slot * sizeof(union x86_64_ptable_entry);
+        entry_ = e;
+        pte_ = lp + slot * sizeof(union x86_64_ptable_entry);
         break;
     }
     default:
         assert(!"Should not get here");
     }
 
+    if (mapped_addr) {
+        *mapped_addr = paddr;
+    }
+    if (pte) {
+        *pte = pte_;
+    }
+    if (entry) {
+        *entry = entry_;
+    }
+}
+
+static inline errval_t lookup_cap_for_mapping(genpaddr_t paddr, lvaddr_t pte, struct cte **retcte)
+{
     // lookup matching cap
-    // TODO: fix lookup to choose correct cap
     struct cte *mem, *last, *orig;
+    // find a cap for paddr
     errval_t err = mdb_find_cap_for_address(paddr, &mem);
     if (err_is_fail(err)) {
         printf("could not find a cap for 0x%"PRIxGENPADDR" (%ld)\n", paddr, err);
         return err;
     }
-    printf("unmap request = 0x%"PRIxGENPADDR"\n", paddr);
-    printf("has_copies(mem) = %d\n", has_copies(mem));
-
-    // select correct pt base address and calculate correct offset
-    lvaddr_t base_pte = mem->mapping_info.pt2 ? mem->mapping_info.pt2 : mem->mapping_info.pte;
-    size_t entries = mem->mapping_info.mapped_pages;
-    if (mem->mapping_info.pt2) {
-        entries -= (X86_64_PTABLE_SIZE - mem->mapping_info.pt_slot);
-    }
+    //printf("lookup request = 0x%"PRIxGENPADDR"\n", paddr);
+    //printf("has_copies(mem) = %d\n", has_copies(mem));
 
-    // search matching capability in mdb tree
-    err = SYS_ERR_VM_MAP_OFFSET;
+    // look at all copies of mem
     last = mem;
     orig = mem;
     // search backwards in tree
@@ -325,93 +325,141 @@ errval_t page_mappings_unmap(struct capability *pgtable, size_t slot)
         struct capability *cap = &mem->cap;
         struct mapping_info *map = &mem->mapping_info;
         genpaddr_t base = get_address(cap);
-        genpaddr_t last_mapped_page = base + map->offset + (map->mapped_pages - 1) * BASE_PAGE_SIZE;
-        lpaddr_t last_used_pte = base_pte + (entries - 1) * sizeof(union x86_64_pdir_entry);
-        printf("looking at mapping @0x%"PRIxGENPADDR", last mapped page @0x%"PRIxGENPADDR"\n"
-               "last used pte = 0x%"PRIxLPADDR", pte = 0x%"PRIxLPADDR"\n",
-                base + map->offset, last_mapped_page, last_used_pte, pte);
-        printf("map->pte = 0x%"PRIxLPADDR", mapped_pages = %zd\n", map->pte, map->mapped_pages);
-        printf("base_pte = 0x%"PRIxLPADDR", entries      = %zd\n", base_pte, entries);
-        // only allow unmaps at end of mapped region
-        if (last_mapped_page == paddr && last_used_pte == pte)
+        // only match mappings that start where we want to unmap
+        if (base + map->offset == paddr && map->pte == pte)
         {
             // found matching cap
-            err = SYS_ERR_OK;
-            goto found;
+            *retcte = mem;
+            return SYS_ERR_OK;
         }
         last = mem;
         mem = mdb_predecessor(mem);
     }
     last = orig;
+    // search forward in tree
     mem = mdb_successor(orig);
     while (is_copy(&mem->cap, &last->cap)) {
         struct capability *cap = &mem->cap;
         struct mapping_info *map = &mem->mapping_info;
         genpaddr_t base = get_address(cap);
-        genpaddr_t last_mapped_page = base + map->offset + (map->mapped_pages - 1) * BASE_PAGE_SIZE;
-        lpaddr_t last_used_pte = base_pte + (entries - 1) * sizeof(union x86_64_pdir_entry);
-        printf("looking at mapping @0x%"PRIxGENPADDR", last mapped page @0x%"PRIxGENPADDR"\n"
-               "last used pte = 0x%"PRIxLPADDR", pte = 0x%"PRIxLPADDR"\n",
-                base + map->offset, last_mapped_page, last_used_pte, pte);
-        printf("map->pte = 0x%"PRIxLPADDR", mapped_pages = %zd\n", map->pte, map->mapped_pages);
-        printf("base_pte = 0x%"PRIxLPADDR", entries      = %zd\n", base_pte, entries);
-        // only allow unmaps at end of mapped region
-        if (last_mapped_page == paddr && last_used_pte == pte)
+        // only match mappings that start where we want to unmap
+        if (base + map->offset == paddr && map->pte == pte)
         {
             // found matching cap
-            err = SYS_ERR_OK;
-            goto found;
+            *retcte = mem;
+            return SYS_ERR_OK;
         }
         last = mem;
         mem = mdb_successor(mem);
     }
 
-    if (err_is_fail(err)) {
-        // return error if no matching cap was found
-        printf("\n\n");
-        return err;
-    }
+    // if we get here, we have not found a matching cap
+    return SYS_ERR_CAP_NOT_FOUND;
+}
 
-found:
-    printf("found cap: 0x%"PRIxGENPADDR", mapping->pte = 0x%"PRIxLPADDR", pte = 0x%"PRIxLPADDR"\n",
-            get_address(&mem->cap), mem->mapping_info.pte, pte);
+static inline void clear_pt_entry(lvaddr_t pte) {
+    ((union x86_64_pdir_entry *)pte)->raw = 0;
+}
 
-    printf("state before unmap: mapped_pages = %zd\n", mem->mapping_info.mapped_pages);
 
+static inline struct cte *cte_for_cap(struct capability *cap)
+{
+    return (struct cte *) (cap - offsetof(struct cte, cap));
+}
 
-    // decrease count of mapped pages to reflect unmap
-    mem->mapping_info.mapped_pages -= 1;
+static inline lvaddr_t get_leaf_ptable_for_vaddr(genvaddr_t vaddr)
+{
+    lvaddr_t root_pt = local_phys_to_mem(dcb_current->vspace);
+
+    // get pdpt
+    union x86_64_pdir_entry *pdpt = (union x86_64_pdir_entry *)root_pt + X86_64_PML4_BASE(vaddr);
+    if (!pdpt->raw) { return 0; }
+    genpaddr_t pdpt_gp = pdpt->d.base_addr << BASE_PAGE_BITS;
+    lvaddr_t pdpt_lv = local_phys_to_mem(gen_phys_to_local_phys(pdpt_gp));
+    // get pdir
+    union x86_64_pdir_entry *pdir = (union x86_64_pdir_entry *)pdpt_lv + X86_64_PDPT_BASE(vaddr);
+    if (!pdir->raw) { return 0; }
+    genpaddr_t pdir_gp = pdir->d.base_addr << BASE_PAGE_BITS;
+    lvaddr_t pdir_lv = local_phys_to_mem(gen_phys_to_local_phys(pdir_gp));
+    // get ptable
+    union x86_64_ptable_entry *ptable = (union x86_64_ptable_entry *)pdir_lv + X86_64_PDIR_BASE(vaddr);
+    if (!ptable->raw) { return 0; }
+    genpaddr_t ptable_gp = ptable->base.base_addr << BASE_PAGE_BITS;
+    lvaddr_t ptable_lv = local_phys_to_mem(gen_phys_to_local_phys(ptable_gp));
+
+    return ptable_lv;
+}
 
-    if (slot == 0 && mem->mapping_info.pt2) {
-        // unmapped all entries in pt2
-        mem->mapping_info.pt2 = 0;
-    }
+errval_t page_mappings_unmap(struct capability *pgtable, size_t slot, size_t num_pages)
+{
+    assert(type_is_vnode(pgtable->type));
+    //printf("page_mappings_unmap(%zd pages)\n", num_pages);
 
-    if (mem->mapping_info.mapped_pages == 0) {
-        // all pages unmapped, delete mapping
-        printf("mapped_pages == 0 --> clearing mapping\n");
-        memset(&mem->mapping_info, 0, sizeof(struct mapping_info));
+    // get page table entry data
+    genpaddr_t paddr;
+    lvaddr_t pte;
+    read_pt_entry(pgtable, slot, &paddr, &pte, NULL);
+    lvaddr_t pt = local_phys_to_mem(gen_phys_to_local_phys(get_address(pgtable)));
+
+    // get virtual address of first page
+    // TODO: error checking
+    genvaddr_t vaddr;
+    struct cte *leaf_pt = cte_for_cap(pgtable);
+    compile_vaddr(leaf_pt, slot, &vaddr);
+
+    // get cap for mapping
+    struct cte *mem;
+    errval_t err = lookup_cap_for_mapping(paddr, pte, &mem);
+    if (err_is_fail(err)) {
+        return err;
     }
+    //printf("state before unmap: mapped_pages = %zd\n", mem->mapping_info.mapped_pages);
+    //printf("state before unmap: num_pages    = %zd\n", num_pages);
 
-    // clear page table entry
-    switch (pgtable->type) {
-    case ObjType_VNode_x86_64_pml4:
-    case ObjType_VNode_x86_64_pdpt:
-    case ObjType_VNode_x86_64_pdir: {
-        union x86_64_pdir_entry *e = entry;
-        e->raw = 0;
-        break;
+    if (num_pages > mem->mapping_info.mapped_pages) {
+        // want to unmap too many pages
+        return SYS_ERR_VM_MAP_SIZE;
     }
-    case ObjType_VNode_x86_64_ptable: {
-        union x86_64_ptable_entry *e = entry;
-        e->raw = 0;
-        break;
+
+    // iterate over affected leaf ptables
+    size_t unmapped_pages = 0;
+    union x86_64_ptable_entry *ptentry = (union x86_64_ptable_entry *)pt + slot;
+    do {
+        size_t target = (num_pages - unmapped_pages) < (X86_64_PTABLE_SIZE - slot) ? slot + (num_pages - unmapped_pages) : X86_64_PTABLE_SIZE;
+        //printf("slot   = %zd\ntarget = %zd\n", slot, target);
+        int i;
+        size_t old_unmapped = unmapped_pages;
+        for (i = slot; i < target; i++) {
+            ptentry++->raw = 0;
+            unmapped_pages++;
+        }
+        //printf("%zd\n", unmapped_pages);
+        if (i == X86_64_PTABLE_SIZE && unmapped_pages < num_pages) {
+            // get next leaf pt
+            vaddr += (unmapped_pages - old_unmapped) * X86_64_BASE_PAGE_SIZE;
+            while (!(pt = get_leaf_ptable_for_vaddr(vaddr)) && unmapped_pages < num_pages) {
+                // no leaf page table for this address
+                unmapped_pages += X86_64_PTABLE_SIZE * X86_64_BASE_PAGE_SIZE;
+                vaddr += X86_64_PTABLE_SIZE * X86_64_BASE_PAGE_SIZE;
+            }
+            slot = 0;
+            ptentry = (union x86_64_ptable_entry *)pt;
+        }
+    } while(unmapped_pages < num_pages);
+
+    if (unmapped_pages > num_pages) { unmapped_pages = num_pages; }
+
+    // update mapping info
+    mem->mapping_info.mapped_pages -= unmapped_pages;
+    if (mem->mapping_info.mapped_pages == 0) {
+        memset(&mem->mapping_info, 0, sizeof(struct mapping_info));
     }
-    default:
-        assert(!"Should not get here");
+    else {
+        // set first still mapped entry as mapping info pte
+        mem->mapping_info.pte = pt;
     }
 
-    printf("state after unmap: mapped_pages = %zd\n", mem->mapping_info.mapped_pages);
+    //printf("state after unmap: mapped_pages = %zd\n", mem->mapping_info.mapped_pages);
 
     // XXX: FIXME: Going to reload cr3 to flush the entire TLB.
     // This is inefficient.
@@ -422,6 +470,5 @@ found:
     __asm__ __volatile__("mov %%cr3,%0" : "=a" (cr3) : );
     __asm__ __volatile__("mov %0,%%cr3" :  : "a" (cr3));
 
-    printf("returning 0K\n");
     return SYS_ERR_OK;
 }
index 76f7a90..2dc36b5 100644 (file)
@@ -174,7 +174,8 @@ static struct sysret handle_unmap(struct capability *pgtable,
                                   int cmd, uintptr_t *args)
 {
     size_t entry = args[0];
-    errval_t err = page_mappings_unmap(pgtable, entry);
+    size_t pages = args[1];
+    errval_t err = page_mappings_unmap(pgtable, entry, pages);
     return SYSRET(err);
 }
 
index 6729778..c8211f0 100644 (file)
@@ -59,7 +59,7 @@ errval_t caps_copy_to_cte(struct cte *dest_cte, struct cte *src_cte, bool mint,
 errval_t caps_copy_to_vnode(struct cte *dest_vnode_cte, cslot_t dest_slot,
                             struct cte *src_cte, uintptr_t param1,
                             uintptr_t param2);
-errval_t page_mappings_unmap(struct capability *pgtable, size_t entry);
+errval_t page_mappings_unmap(struct capability *pgtable, size_t entry, size_t num_pages);
 
 errval_t caps_retype(enum objtype type, size_t objbits,
                      struct capability *dest_cnode,
index 8442a54..030dd91 100644 (file)
@@ -20,8 +20,6 @@
 
 struct mapping_info {
     lvaddr_t pte;           ///< where the capability is mapped
-    uint16_t pt_slot;       ///< the first slot number for the mapping in the page table corresponding to pte
-    lvaddr_t pt2;           ///< base address of 2nd leaf level ptable
     size_t mapped_pages;    ///< the amount of currently mapped pages
     // target meta data
     size_t page_count;      ///< the targeted amount of mapped pages
index 07f417b..130c764 100644 (file)
@@ -245,6 +245,7 @@ static errval_t do_map(struct pmap_x86 *pmap, genvaddr_t vaddr,
         // Create user level datastructure for the mapping
         struct vnode *page = find_vnode(ptable, X86_64_PTABLE_BASE(vaddr));
         if (page != NULL) {
+            printf("page already exists for 0x%"PRIxGENVADDR"\n", vaddr);
             return LIB_ERR_PMAP_EXISTING_MAPPING;
         }
         page = slab_alloc(&pmap->slab);
@@ -421,29 +422,40 @@ static errval_t unmap(struct pmap *pmap, genvaddr_t vaddr, size_t size,
     struct pmap_x86 *x86 = (struct pmap_x86*)pmap;
     size = ROUND_UP(size, X86_64_BASE_PAGE_SIZE);
 
-    for (size_t i = X86_64_BASE_PAGE_SIZE; i <= size; i+=X86_64_BASE_PAGE_SIZE) {
+
+    //printf("size  = %zd\n", size);
+    size_t pages = size / X86_64_BASE_PAGE_SIZE;
+    //printf("pages = %zd\n", pages);
+    // Unmap it in the kernel
+    struct vnode *pt= find_ptable(x86, vaddr);
+    if (pt) {
+        struct vnode *page = find_vnode(pt, X86_64_PTABLE_BASE(vaddr));
+        if (page) {
+            err = vnode_unmap(pt->u.vnode.cap, page->entry, pages);
+            if (err_is_fail(err)) {
+                printf("vnode_unmap returned error: %s (%"PRIuERRV")\n", err_getstring(err), err);
+                ret = err_push(err, LIB_ERR_VNODE_UNMAP);
+                return ret;
+            }
+        }
+    }
+
+    // cleanup user space vnodes
+    for (size_t i = 0; i < size; i+=X86_64_BASE_PAGE_SIZE) {
         // Find the page table
         struct vnode *ptable;
-        genvaddr_t vaddr_ = vaddr + size - i;
-        ptable = find_ptable(x86, vaddr_);
+        ptable = find_ptable(x86, vaddr + i);
         if (ptable == NULL) {
+            printf("no pt for 0x%"PRIxGENVADDR"\n",vaddr +i);
             continue; // not mapped
         }
-
         // Find the page
-        struct vnode *page = find_vnode(ptable, X86_64_PTABLE_BASE(vaddr_));
+        struct vnode *page = find_vnode(ptable, X86_64_PTABLE_BASE(vaddr + i));
         if (!page) {
+            printf("no page for 0x%"PRIxGENVADDR"\n", vaddr + i);
             continue; // not mapped
         }
 
-        // Unmap it in the kernel
-        err = vnode_unmap(ptable->u.vnode.cap, page->entry);
-        if (err_is_fail(err)) {
-            printf("vnode_unmap returned error: %s (%"PRIuERRV")\n", err_getstring(err), err);
-            ret = err_push(err, LIB_ERR_VNODE_UNMAP);
-            continue; // try to unmap the rest anyway
-        }
-
         // Free up the resources
         remove_vnode(ptable, page);
         slab_free(&x86->slab, page);
@@ -470,10 +482,27 @@ static errval_t modify_flags(struct pmap *pmap, genvaddr_t vaddr, size_t size,
     errval_t err, ret;
     struct pmap_x86 *x86 = (struct pmap_x86 *)pmap;
     size = ROUND_UP(size, X86_64_BASE_PAGE_SIZE);
+    size_t pages = size / X86_64_BASE_PAGE_SIZE;
 
     // TODO: reset mapping info
     // XXX: need new copy of cap?
 
+    // Unmap it in the kernel
+    struct vnode *pt= find_ptable(x86, vaddr);
+    if (pt) {
+        struct vnode *page = find_vnode(pt, X86_64_PTABLE_BASE(vaddr));
+        if (page) {
+            err = vnode_unmap(pt->u.vnode.cap, page->entry, pages);
+            if (err_is_fail(err)) {
+                printf("vnode_unmap returned error: %s (%"PRIuERRV")\n", err_getstring(err), err);
+                ret = err_push(err, LIB_ERR_VNODE_UNMAP);
+                return ret;
+            }
+            vm_modify_mapping(page->u.frame.cap, pages, page->u.frame.offset);
+        }
+    }
+
+
     for (size_t i = 0; i < size; i += X86_64_BASE_PAGE_SIZE) {
         // Find the page table
         struct vnode *ptable = find_ptable(x86, vaddr + i);
@@ -489,12 +518,6 @@ static errval_t modify_flags(struct pmap *pmap, genvaddr_t vaddr, size_t size,
             continue;
         }
 
-        // Unmap it in the kernel
-        err = vnode_unmap(ptable->u.vnode.cap, vn->entry);
-        if (err_is_fail(err)) {
-            return err_push(err, LIB_ERR_VNODE_UNMAP);
-        }
-
         // Remap with changed flags
         paging_x86_64_flags_t pmap_flags = vregion_to_pmap_flag(flags);
         //printf("\tmodify_flags calling vnode_map()\n");
index 7ee53e0..08920fe 100644 (file)
@@ -76,30 +76,54 @@ static errval_t unmap_region(struct memobj *memobj, struct vregion *vregion)
     struct pmap *pmap     = vspace_get_pmap(vspace);
     genvaddr_t vregion_base  = vregion_get_base_addr(vregion);
     genvaddr_t vregion_off   = vregion_get_offset(vregion);
+    size_t vregion_size = vregion_get_size(vregion);
+    genvaddr_t vregion_end = vregion_off + vregion_size;
 
-    err = pmap->f.unmap(pmap, vregion_base + vregion_off, memobj->size, NULL);
-    if (err_is_fail(err)) {
-        return err_push(err, LIB_ERR_PMAP_UNMAP);
-    }
+    //printf("(%s:%d) unmap(0x%"PRIxGENVADDR", memobj->size = %zd) vregion size = %zd\n", __FILE__, __LINE__, vregion_base + vregion_off, memobj->size, vregion_size);
 
-    /* Remove the vregion from the list */
-    struct vregion_list *prev = NULL;
-    for (struct vregion_list *elt = anon->vregion_list; elt != NULL;
-         elt = elt->next) {
-        if (elt->region == vregion) {
-            if (prev == NULL) {
-                assert(elt == anon->vregion_list);
-                anon->vregion_list = elt->next;
-            } else {
-                assert(prev->next == elt);
-                prev->next = elt->next;
+    // unmap all affected frames
+    struct memobj_frame_list *fwalk = anon->frame_list;
+    struct memobj_frame_list *fprev = NULL;
+    //printf("vregion_off = 0x%"PRIxGENVADDR"\n", vregion_off);
+    //printf("vregion_end = 0x%"PRIxGENVADDR"\n", vregion_end);
+    err = LIB_ERR_VSPACE_VREGION_NOT_FOUND;
+    while (fwalk) {
+        //printf("fwalk->offset = %zd\n", fwalk->offset);
+        //printf("fwalk->next   = %p\n", fwalk->next);
+        if (fwalk->offset < vregion_off) {
+            fprev = fwalk;
+            fwalk = fwalk->next;
+            continue;
+        }
+        else if (fwalk->offset < vregion_end) {
+            err = pmap->f.unmap(pmap, vregion_base + vregion_off, fwalk->size, NULL);
+            if (err_is_fail(err)) {
+                return err_push(err, LIB_ERR_PMAP_UNMAP);
             }
-            slab_free(&anon->vregion_slab, elt);
-            return SYS_ERR_OK;
+
+            /* Remove the vregion from the list */
+            struct vregion_list *prev = NULL;
+            for (struct vregion_list *elt = anon->vregion_list; elt != NULL;
+                 elt = elt->next) {
+                if (elt->region == vregion) {
+                    if (prev == NULL) {
+                        assert(elt == anon->vregion_list);
+                        anon->vregion_list = elt->next;
+                    } else {
+                        assert(prev->next == elt);
+                        prev->next = elt->next;
+                    }
+                    slab_free(&anon->vregion_slab, elt);
+                    err = SYS_ERR_OK;
+                }
+            }
+            vregion_off += fwalk->size;
+            fprev = fwalk;
+            fwalk = fwalk->next;
         }
     }
 
-    return LIB_ERR_VSPACE_VREGION_NOT_FOUND; // XXX: not quite the right error
+    return err; // XXX: not quite the right error
 }
 
 /**
@@ -280,6 +304,7 @@ static errval_t unfill(struct memobj *memobj, genvaddr_t offset,
             size_t retsize;
 
             assert((vregion_base + fwalk->offset) % BASE_PAGE_SIZE == 0);
+            //printf("(%s:%d) unmap(0x%"PRIxGENVADDR", %zd)\n", __FILE__, __LINE__, vregion_base + fwalk->offset, fwalk->size);
             err = pmap->f.unmap(pmap, vregion_base + fwalk->offset, fwalk->size,
                                 &retsize);
             if (err_is_fail(err)) {