Added API to enumerate the VM regions of a process
[akaros.git] / kern / src / mm.c
index 13cb144..022f5ad 100644 (file)
@@ -27,7 +27,7 @@
 
 struct kmem_cache *vmr_kcache;
 
-static int __vmr_free_pgs(struct proc *p, pte_t *pte, void *va, void *arg);
+static int __vmr_free_pgs(struct proc *p, pte_t pte, void *va, void *arg);
 /* minor helper, will ease the file->chan transition */
 static struct page_map *file2pm(struct file *file)
 {
@@ -58,7 +58,7 @@ struct vm_region *create_vmr(struct proc *p, uintptr_t va, size_t len)
        vm_i = TAILQ_FIRST(&p->vm_regions);
        /* This works for now, but if all we have is BRK_END ones, we'll start
         * growing backwards (TODO) */
-       if (!vm_i || (va + len < vm_i->vm_base)) {
+       if (!vm_i || (va + len <= vm_i->vm_base)) {
                vmr = kmem_cache_alloc(vmr_kcache, 0);
                if (!vmr)
                        panic("EOM!");
@@ -250,6 +250,7 @@ void unmap_and_destroy_vmrs(struct proc *p)
        /* this only gets called from __proc_free, so there should be no sync
         * concerns.  still, better safe than sorry. */
        spin_lock(&p->vmr_lock);
+       p->vmr_history++;
        spin_lock(&p->pte_lock);
        TAILQ_FOREACH(vmr_i, &p->vm_regions, vm_link) {
                /* note this CB sets the PTE = 0, regardless of if it was P or not */
@@ -266,7 +267,7 @@ void unmap_and_destroy_vmrs(struct proc *p)
 
 /* Helper: copies the contents of pages from p to new p.  For pages that aren't
  * present, once we support swapping or CoW, we can do something more
- * intelligent.  0 on success, -ERROR on failure. */
+ * intelligent.  0 on success, -ERROR on failure.  Can't handle jumbos. */
 static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
                       uintptr_t va_end)
 {
@@ -280,30 +281,30 @@ static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
                     va_end);
                return -EINVAL;
        }
-       int copy_page(struct proc *p, pte_t *pte, void *va, void *arg) {
+       int copy_page(struct proc *p, pte_t pte, void *va, void *arg) {
                struct proc *new_p = (struct proc*)arg;
                struct page *pp;
-               if (PAGE_UNMAPPED(*pte))
+               if (pte_is_unmapped(pte))
                        return 0;
                /* pages could be !P, but right now that's only for file backed VMRs
                 * undergoing page removal, which isn't the caller of copy_pages. */
-               if (PAGE_PRESENT(*pte)) {
+               if (pte_is_mapped(pte)) {
                        /* TODO: check for jumbos */
                        if (upage_alloc(new_p, &pp, 0))
                                return -ENOMEM;
-                       if (page_insert(new_p->env_pgdir, pp, va, *pte & PTE_PERM)) {
+                       if (page_insert(new_p->env_pgdir, pp, va, pte_get_settings(pte))) {
                                page_decref(pp);
                                return -ENOMEM;
                        }
-                       memcpy(page2kva(pp), ppn2kva(PTE2PPN(*pte)), PGSIZE);
+                       memcpy(page2kva(pp), KADDR(pte_get_paddr(pte)), PGSIZE);
                        page_decref(pp);
-               } else if (PAGE_PAGED_OUT(*pte)) {
+               } else if (pte_is_paged_out(pte)) {
                        /* TODO: (SWAP) will need to either make a copy or CoW/refcnt the
                         * backend store.  For now, this PTE will be the same as the
                         * original PTE */
                        panic("Swapping not supported!");
                } else {
-                       panic("Weird PTE %p in %s!", *pte, __FUNCTION__);
+                       panic("Weird PTE %p in %s!", pte_print(pte), __FUNCTION__);
                }
                return 0;
        }
@@ -358,10 +359,16 @@ void print_vmrs(struct proc *p)
                       vmr->vm_file, vmr->vm_foff);
 }
 
-/* Helper: returns the number of pages required to hold nr_bytes */
-static unsigned long nr_pages(unsigned long nr_bytes)
+void enumerate_vmrs(struct proc *p,
+                                       void (*func)(struct vm_region *vmr, void *opaque),
+                                       void *opaque)
 {
-       return (nr_bytes >> PGSHIFT) + (PGOFF(nr_bytes) ? 1 : 0);
+       struct vm_region *vmr;
+
+       spin_lock(&p->vmr_lock);
+       TAILQ_FOREACH(vmr, &p->vm_regions, vm_link)
+               func(vmr, opaque);
+       spin_unlock(&p->vmr_lock);
 }
 
 /* Error values aren't quite comprehensive - check man mmap() once we do better
@@ -464,32 +471,43 @@ out_error:        /* for debugging */
        return FALSE;
 }
 
-/* Helper, maps in page at addr, but only if nothing is present there.  Returns
+/* Helper, maps in page at addr, but only if nothing is mapped there.  Returns
  * 0 on success.  If this is called by non-PM code, we'll store your ref in the
  * PTE. */
 static int map_page_at_addr(struct proc *p, struct page *page, uintptr_t addr,
                             int prot)
 {
-       pte_t *pte;
+       pte_t pte;
        spin_lock(&p->pte_lock);        /* walking and changing PTEs */
        /* find offending PTE (prob don't read this in).  This might alloc an
         * intermediate page table page. */
        pte = pgdir_walk(p->env_pgdir, (void*)addr, TRUE);
-       if (!pte) {
+       if (!pte_walk_okay(pte)) {
                spin_unlock(&p->pte_lock);
                return -ENOMEM;
        }
        /* a spurious, valid PF is possible due to a legit race: the page might have
         * been faulted in by another core already (and raced on the memory lock),
         * in which case we should just return. */
-       if (PAGE_PRESENT(*pte)) {
+       if (pte_is_present(pte)) {
                spin_unlock(&p->pte_lock);
+               /* callers expect us to eat the ref if we succeed. */
+               page_decref(page);
                return 0;
        }
+       if (pte_is_mapped(pte)) {
+               /* we're clobbering an old entry.  if we're just updating the prot, then
+                * it's no big deal.  o/w, there might be an issue. */
+               if (page2pa(page) != pte_get_paddr(pte)) {
+                       warn_once("Clobbered a PTE mapping (%p -> %p)\n", pte_print(pte),
+                                 page2pa(page) | prot);
+               }
+               page_decref(pa2page(pte_get_paddr(pte)));
+       }
        /* preserve the dirty bit - pm removal could be looking concurrently */
-       prot |= (*pte & PTE_D ? PTE_D : 0);
+       prot |= (pte_is_dirty(pte) ? PTE_D : 0);
        /* We have a ref to page, which we are storing in the PTE */
-       *pte = PTE(page2ppn(page), PTE_P | prot);
+       pte_write(pte, page2pa(page), prot);
        spin_unlock(&p->pte_lock);
        return 0;
 }
@@ -507,6 +525,77 @@ static int __copy_and_swap_pmpg(struct proc *p, struct page **pp)
        return 0;
 }
 
+/* Hold the VMR lock when you call this - it'll assume the entire VA range is
+ * mappable, which isn't true if there are concurrent changes to the VMRs. */
+static int populate_anon_va(struct proc *p, uintptr_t va, unsigned long nr_pgs,
+                            int pte_prot)
+{
+       struct page *page;
+       int ret;
+       for (long i = 0; i < nr_pgs; i++) {
+               if (upage_alloc(p, &page, TRUE))
+                       return -ENOMEM;
+               /* could imagine doing a memwalk instead of a for loop */
+               ret = map_page_at_addr(p, page, va + i * PGSIZE, pte_prot);
+               if (ret) {
+                       page_decref(page);
+                       return ret;
+               }
+       }
+       return 0;
+}
+
+/* This will periodically unlock the vmr lock. */
+static int populate_pm_va(struct proc *p, uintptr_t va, unsigned long nr_pgs,
+                          int pte_prot, struct page_map *pm, size_t offset,
+                          int flags, bool exec)
+{
+       int ret = 0;
+       unsigned long pm_idx0 = offset >> PGSHIFT;
+       int vmr_history = ACCESS_ONCE(p->vmr_history);
+       struct page *page;
+
+       /* locking rules: start the loop holding the vmr lock, enter and exit the
+        * entire func holding the lock. */
+       for (long i = 0; i < nr_pgs; i++) {
+               ret = pm_load_page_nowait(pm, pm_idx0 + i, &page);
+               if (ret) {
+                       if (ret != -EAGAIN)
+                               break;
+                       spin_unlock(&p->vmr_lock);
+                       /* might block here, can't hold the spinlock */
+                       ret = pm_load_page(pm, pm_idx0 + i, &page);
+                       spin_lock(&p->vmr_lock);
+                       if (ret)
+                               break;
+                       /* while we were sleeping, the VMRs could have changed on us. */
+                       if (vmr_history != ACCESS_ONCE(p->vmr_history)) {
+                               pm_put_page(page);
+                               printk("[kernel] FYI: VMR changed during populate\n");
+                               break;
+                       }
+               }
+               if (flags & MAP_PRIVATE) {
+                       ret = __copy_and_swap_pmpg(p, &page);
+                       if (ret) {
+                               pm_put_page(page);
+                               break;
+                       }
+               }
+               /* if this is an executable page, we might have to flush the
+                * instruction cache if our HW requires it.
+                * TODO: is this still needed?  andrew put this in a while ago*/
+               if (exec)
+                       icache_flush_page(0, page2kva(page));
+               ret = map_page_at_addr(p, page, va + i * PGSIZE, pte_prot);
+               if (atomic_read(&page->pg_flags) & PG_PAGEMAP)
+                       pm_put_page(page);
+               if (ret)
+                       break;
+       }
+       return ret;
+}
+
 void *do_mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
               struct file *file, size_t offset)
 {
@@ -515,6 +604,7 @@ void *do_mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
 
        /* read/write vmr lock (will change the tree) */
        spin_lock(&p->vmr_lock);
+       p->vmr_history++;
        /* Sanity check, for callers that bypass mmap().  We want addr for anon
         * memory to start above the break limit (BRK_END), but not 0.  Keep this in
         * sync with BRK_END in mmap(). */
@@ -584,67 +674,15 @@ void *do_mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
        if (flags & MAP_POPULATE && prot != PROT_NONE) {
                int pte_prot = (prot & PROT_WRITE) ? PTE_USER_RW :
                           (prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
-               int num_pages = len / PGSIZE;
+               unsigned long nr_pgs = len >> PGSHIFT;
                int ret = 0;
-               struct page *a_page;
                if (!file) {
-                       for (int i = 0; i < num_pages; i++) {
-                               if (upage_alloc(p, &a_page, TRUE)) {
-                                       ret = -ENOMEM;
-                                       break;
-                               }
-                               a_page->pg_tree_slot = 0;
-                               /* could imagine doing a memwalk instead of a for loop */
-                               ret = map_page_at_addr(p, a_page, addr + i * PGSIZE, pte_prot);
-                               if (ret) {
-                                       page_decref(a_page);
-                                       break;
-                               }
-                       }
+                       ret = populate_anon_va(p, addr, nr_pgs, pte_prot);
                } else {
-                       /* our refcnt on the file keeps the pm alive when we unlock */
-                       struct page_map *pm = file->f_mapping;
-                       unsigned long pm_idx0 = offset >> PGSHIFT;
-                       struct vm_region vmr_copy;
-                       memcpy(&vmr_copy, vmr, sizeof(struct vm_region));
-                       for (int i = 0; i < num_pages; i++) {
-                               ret = pm_load_page_nowait(pm, pm_idx0 + i, &a_page);
-                               if (ret) {
-                                       if (ret != -EAGAIN)
-                                               break;
-                                       spin_unlock(&p->vmr_lock);
-                                       /* might block here, can't hold the spinlock */
-                                       ret = pm_load_page(pm, pm_idx0 + i, &a_page);
-                                       spin_lock(&p->vmr_lock);
-                                       if (ret)
-                                               break;
-                                       /* ugly - while we were sleeping, our VMR could have changed
-                                        * on us.  should be okay with weird ABA races too. */
-                                       vmr = find_vmr(p, addr + i * PGSIZE);
-                                       if (memcmp(&vmr_copy, vmr, sizeof(struct vm_region))) {
-                                               pm_put_page(a_page);
-                                               printk("[kernel] FYI: VMR changed during populate\n");
-                                               break;
-                                       }
-                               }
-                               if (flags & MAP_PRIVATE) {
-                                       ret = __copy_and_swap_pmpg(p, &a_page);
-                                       if (ret) {
-                                               pm_put_page(a_page);
-                                               break;
-                                       }
-                               }
-                               /* if this is an executable page, we might have to flush the
-                                * instruction cache if our HW requires it.
-                                * TODO: is this still needed?  andrew put this in a while ago*/
-                               if (prot & PROT_EXEC)
-                                       icache_flush_page(0, page2kva(a_page));
-                               ret = map_page_at_addr(p, a_page, addr + i * PGSIZE, pte_prot);
-                               if (atomic_read(&a_page->pg_flags) & PG_PAGEMAP)
-                                       pm_put_page(a_page);
-                               if (ret)
-                                       break;
-                       }
+                       /* Note: this will unlock if it blocks.  our refcnt on the file
+                        * keeps the pm alive when we unlock */
+                       ret = populate_pm_va(p, addr, nr_pgs, pte_prot, file->f_mapping,
+                                            offset, flags, prot & PROT_EXEC);
                }
                if (ret == -ENOMEM) {
                        spin_unlock(&p->vmr_lock);
@@ -673,6 +711,7 @@ int mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
        }
        /* read/write lock, will probably change the tree and settings */
        spin_lock(&p->vmr_lock);
+       p->vmr_history++;
        int ret = __do_mprotect(p, addr, len, prot);
        spin_unlock(&p->vmr_lock);
        return ret;
@@ -684,10 +723,10 @@ int mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
 int __do_mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
 {
        struct vm_region *vmr, *next_vmr;
-       pte_t *pte;
+       pte_t pte;
        bool shootdown_needed = FALSE;
        int pte_prot = (prot & PROT_WRITE) ? PTE_USER_RW :
-                      (prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
+                      (prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : PTE_NONE;
        /* TODO: this is aggressively splitting, when we might not need to if the
         * prots are the same as the previous.  Plus, there are three excessive
         * scans.  Finally, we might be able to merge when we are done. */
@@ -702,11 +741,15 @@ int __do_mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
                }
                vmr->vm_prot = prot;
                spin_lock(&p->pte_lock);        /* walking and changing PTEs */
-               /* TODO: use a memwalk */
-               for (uintptr_t va = vmr->vm_base; va < vmr->vm_end; va += PGSIZE) { 
+               /* TODO: use a memwalk.  At a minimum, we need to change every existing
+                * PTE that won't trigger a PF (meaning, present PTEs) to have the new
+                * prot.  The others will fault on access, and we'll change the PTE
+                * then.  In the off chance we have a mapped but not present PTE, we
+                * might as well change it too, since we're already here. */
+               for (uintptr_t va = vmr->vm_base; va < vmr->vm_end; va += PGSIZE) {
                        pte = pgdir_walk(p->env_pgdir, (void*)va, 0);
-                       if (pte && PAGE_PRESENT(*pte)) {
-                               *pte = (*pte & ~PTE_PERM) | pte_prot;
+                       if (pte_walk_okay(pte) && pte_is_mapped(pte)) {
+                               pte_replace_perm(pte, pte_prot);
                                shootdown_needed = TRUE;
                        }
                }
@@ -724,6 +767,8 @@ int munmap(struct proc *p, uintptr_t addr, size_t len)
        printd("munmap(addr %x, len %x)\n", addr, len);
        if (!len)
                return 0;
+       len = ROUNDUP(len, PGSIZE);
+
        if ((addr % PGSIZE) || (addr < MMAP_LOWEST_VA)) {
                set_errno(EINVAL);
                return -1;
@@ -735,21 +780,20 @@ int munmap(struct proc *p, uintptr_t addr, size_t len)
        }
        /* read/write: changing the vmrs (trees, properties, and whatnot) */
        spin_lock(&p->vmr_lock);
+       p->vmr_history++;
        int ret = __do_munmap(p, addr, len);
        spin_unlock(&p->vmr_lock);
        return ret;
 }
 
-static int __munmap_mark_not_present(struct proc *p, pte_t *pte, void *va,
+static int __munmap_mark_not_present(struct proc *p, pte_t pte, void *va,
                                      void *arg)
 {
        bool *shootdown_needed = (bool*)arg;
-       struct page *page;
        /* could put in some checks here for !P and also !0 */
-       if (!PAGE_PRESENT(*pte))        /* unmapped (== 0) *ptes are also not PTE_P */
+       if (!pte_is_present(pte))       /* unmapped (== 0) *ptes are also not PTE_P */
                return 0;
-       page = ppn2page(PTE2PPN(*pte));
-       *pte &= ~PTE_P;
+       pte_clear_present(pte);
        *shootdown_needed = TRUE;
        return 0;
 }
@@ -767,13 +811,13 @@ static int __munmap_mark_not_present(struct proc *p, pte_t *pte, void *va,
  * 0 or not.  If it isn't, then we're still okay to look at the page.  Consider
  * the PTE a weak ref on the page.  So long as you hold the mm lock, you can
  * look at the PTE and know the page isn't being freed. */
-static int __vmr_free_pgs(struct proc *p, pte_t *pte, void *va, void *arg)
+static int __vmr_free_pgs(struct proc *p, pte_t pte, void *va, void *arg)
 {
        struct page *page;
-       if (!*pte)
+       if (pte_is_unmapped(pte))
                return 0;
-       page = ppn2page(PTE2PPN(*pte));
-       *pte = 0;
+       page = pa2page(pte_get_paddr(pte));
+       pte_clear(pte);
        if (!(atomic_read(&page->pg_flags) & PG_PAGEMAP))
                page_decref(page);
        return 0;
@@ -782,7 +826,6 @@ static int __vmr_free_pgs(struct proc *p, pte_t *pte, void *va, void *arg)
 int __do_munmap(struct proc *p, uintptr_t addr, size_t len)
 {
        struct vm_region *vmr, *next_vmr, *first_vmr;
-       pte_t *pte;
        bool shootdown_needed = FALSE;
 
        /* TODO: this will be a bit slow, since we end up doing three linear
@@ -879,7 +922,7 @@ static int __hpf_load_page(struct proc *p, struct page_map *pm,
        return 0;
 }
 
-/* Returns 0 on success, or an appropriate -error code. 
+/* Returns 0 on success, or an appropriate -error code.
  *
  * Notes: if your TLB caches negative results, you'll need to flush the
  * appropriate tlb entry.  Also, you could have a weird race where a present PTE
@@ -892,19 +935,17 @@ int handle_page_fault(struct proc *p, uintptr_t va, int prot)
        struct vm_region *vmr;
        struct page *a_page;
        unsigned int f_idx;     /* index of the missing page in the file */
-       pte_t *pte;
        int ret = 0;
        bool first = TRUE;
        va = ROUNDDOWN(va,PGSIZE);
 
-       if (prot != PROT_READ && prot != PROT_WRITE && prot != PROT_EXEC)
-               panic("bad prot!");
 refault:
        /* read access to the VMRs TODO: RCU */
        spin_lock(&p->vmr_lock);
        /* Check the vmr's protection */
        vmr = find_vmr(p, va);
        if (!vmr) {                                                     /* not mapped at all */
+               printd("fault: %p not mapped\n", va);
                ret = -EFAULT;
                goto out;
        }
@@ -975,6 +1016,9 @@ refault:
        int pte_prot = (vmr->vm_prot & PROT_WRITE) ? PTE_USER_RW :
                       (vmr->vm_prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
        ret = map_page_at_addr(p, a_page, va, pte_prot);
+       if (ret) {
+               printd("map_page_at for %p fails with %d\n", va, ret);
+       }
        /* fall through, even for errors */
 out_put_pg:
        /* the VMR's existence in the PM (via the mmap) allows us to have PTE point
@@ -987,6 +1031,56 @@ out:
        return ret;
 }
 
+/* Attempts to populate the pages, as if there was a page faults.  Bails on
+ * errors, and returns the number of pages populated.  */
+unsigned long populate_va(struct proc *p, uintptr_t va, unsigned long nr_pgs)
+{
+       struct vm_region *vmr, vmr_copy;
+       unsigned long nr_pgs_this_vmr;
+       unsigned long nr_filled = 0;
+       struct page *page;
+       int pte_prot;
+
+       /* we can screw around with ways to limit the find_vmr calls (can do the
+        * next in line if we didn't unlock, etc., but i don't expect us to do this
+        * for more than a single VMR in most cases. */
+       spin_lock(&p->vmr_lock);
+       while (nr_pgs) {
+               vmr = find_vmr(p, va);
+               if (!vmr)
+                       break;
+               if (vmr->vm_prot == PROT_NONE)
+                       break;
+               pte_prot = (vmr->vm_prot & PROT_WRITE) ? PTE_USER_RW :
+                          (vmr->vm_prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
+               nr_pgs_this_vmr = MIN(nr_pgs, (vmr->vm_end - va) >> PGSHIFT);
+               if (!vmr->vm_file) {
+                       if (populate_anon_va(p, va, nr_pgs_this_vmr, pte_prot)) {
+                               /* on any error, we can just bail.  we might be underestimating
+                                * nr_filled. */
+                               break;
+                       }
+               } else {
+                       /* need to keep the file alive in case we unlock/block */
+                       kref_get(&vmr->vm_file->f_kref, 1);
+                       if (populate_pm_va(p, va, nr_pgs_this_vmr, pte_prot,
+                                          vmr->vm_file->f_mapping,
+                                          vmr->vm_foff - (va - vmr->vm_base),
+                                                          vmr->vm_flags, vmr->vm_prot & PROT_EXEC)) {
+                               /* we might have failed if the underlying file doesn't cover the
+                                * mmap window, depending on how we'll deal with truncation. */
+                               break;
+                       }
+                       kref_put(&vmr->vm_file->f_kref);
+               }
+               nr_filled += nr_pgs_this_vmr;
+               va += nr_pgs_this_vmr << PGSHIFT;
+               nr_pgs -= nr_pgs_this_vmr;
+       }
+       spin_unlock(&p->vmr_lock);
+       return nr_filled;
+}
+
 /* Kernel Dynamic Memory Mappings */
 uintptr_t dyn_vmap_llim = KERN_DYN_TOP;
 spinlock_t dyn_vmap_lock = SPINLOCK_INITIALIZER;
@@ -1038,20 +1132,20 @@ int map_vmap_segment(uintptr_t vaddr, uintptr_t paddr, unsigned long num_pages,
         * isn't enough, since there might be a race on outer levels of page tables.
         * For now, we'll just use the dyn_vmap_lock (which technically works). */
        spin_lock(&dyn_vmap_lock);
-       pte_t *pte;
+       pte_t pte;
 #ifdef CONFIG_X86
        perm |= PTE_G;
 #endif
        for (int i = 0; i < num_pages; i++) {
                pte = pgdir_walk(boot_pgdir, (void*)(vaddr + i * PGSIZE), 1);
-               if (!pte) {
+               if (!pte_walk_okay(pte)) {
                        spin_unlock(&dyn_vmap_lock);
                        return -ENOMEM;
                }
                /* You probably should have unmapped first */
-               if (*pte)
-                       warn("Existing PTE value %p\n", *pte);
-               *pte = PTE(pa2ppn(paddr + i * PGSIZE), perm);
+               if (pte_is_mapped(pte))
+                       warn("Existing PTE value %p\n", pte_print(pte));
+               pte_write(pte, paddr + i * PGSIZE, perm);
        }
        spin_unlock(&dyn_vmap_lock);
        return 0;
@@ -1064,10 +1158,11 @@ int unmap_vmap_segment(uintptr_t vaddr, unsigned long num_pages)
        warn("Incomplete, don't call this yet.");
        spin_lock(&dyn_vmap_lock);
        /* TODO: For all pgdirs */
-       pte_t *pte;
+       pte_t pte;
        for (int i = 0; i < num_pages; i++) {
                pte = pgdir_walk(boot_pgdir, (void*)(vaddr + i * PGSIZE), 1);
-               *pte = 0;
+               if (pte_walk_okay(pte))
+                       pte_clear(pte);
        }
        /* TODO: TLB shootdown.  Also note that the global flag is set on the PTE
         * (for x86 for now), which requires a global shootdown.  bigger issue is
@@ -1078,21 +1173,37 @@ int unmap_vmap_segment(uintptr_t vaddr, unsigned long num_pages)
        return 0;
 }
 
-uintptr_t vmap_pmem(uintptr_t paddr, size_t nr_bytes)
+/* This can handle unaligned paddrs */
+static uintptr_t vmap_pmem_flags(uintptr_t paddr, size_t nr_bytes, int flags)
 {
        uintptr_t vaddr;
-       unsigned long nr_pages = ROUNDUP(nr_bytes, PGSIZE) >> PGSHIFT;
+       unsigned long nr_pages;
        assert(nr_bytes && paddr);
+       nr_bytes += PGOFF(paddr);
+       nr_pages = ROUNDUP(nr_bytes, PGSIZE) >> PGSHIFT;
        vaddr = get_vmap_segment(nr_pages);
        if (!vaddr) {
                warn("Unable to get a vmap segment");   /* probably a bug */
                return 0;
        }
-       if (map_vmap_segment(vaddr, paddr, nr_pages, PTE_P | PTE_KERN_RW)) {
+       /* it's not strictly necessary to drop paddr's pgoff, but it might save some
+        * vmap heartache in the future. */
+       if (map_vmap_segment(vaddr, PG_ADDR(paddr), nr_pages,
+                            PTE_KERN_RW | flags)) {
                warn("Unable to map a vmap segment");   /* probably a bug */
                return 0;
        }
-       return vaddr;
+       return vaddr + PGOFF(paddr);
+}
+
+uintptr_t vmap_pmem(uintptr_t paddr, size_t nr_bytes)
+{
+       return vmap_pmem_flags(paddr, nr_bytes, 0);
+}
+
+uintptr_t vmap_pmem_nocache(uintptr_t paddr, size_t nr_bytes)
+{
+       return vmap_pmem_flags(paddr, nr_bytes, PTE_NOCACHE);
 }
 
 int vunmap_vmem(uintptr_t vaddr, size_t nr_bytes)