Memwalks run CBs on all PTEs, not just PTE_Ps
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 10 Jan 2014 20:02:50 +0000 (12:02 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 17 Jan 2014 22:57:12 +0000 (14:57 -0800)
The PTE states need a lot of work, and something is likely to be screwed
up here.

I needed to make a distinction between !PTE_P and completely empty,
since the upcoming page map removals will have PTEs !P, but still
'readable'.  And I'll need to memwalk those.

kern/arch/riscv/env.c
kern/arch/x86/pmap32.c
kern/arch/x86/pmap64.c
kern/src/env.c
kern/src/mm.c

index 4933168..2b7d32d 100644 (file)
@@ -135,7 +135,7 @@ user_mem_walk_recursive(env_t* e, uintptr_t start, size_t len,
                        if(pt_callback != NULL && (ret = pt_callback(e, pte, (void*)pgaddr, arg)))
                                goto out;
                }
-               else if(callback != NULL && !PAGE_UNMAPPED(*pte))
+               else if(callback != NULL)
                        if((ret = callback(e, pte, (void*)pgaddr, arg)))
                                goto out;
        }
@@ -157,6 +157,8 @@ env_pagetable_free(env_t* e)
 {
        int pt_free(env_t* e, pte_t* pte, void* va, void* arg)
        {
+               if (!PAGE_PRESENT(pte))
+                       return 0;
                page_decref(pa2page(PTD_ADDR(*pte)));
                return 0;
        }
index 4a42fc2..1fc9068 100644 (file)
@@ -666,9 +666,8 @@ int env_user_mem_walk(env_t* e, void* start, size_t len,
                                      PTX(end) : NPTENTRIES );
                int ret;
                for (pteno = pteno_start; pteno < pteno_end; pteno++) {
-                       if (!PAGE_UNMAPPED(pt[pteno]))
-                               if((ret = callback(e, &pt[pteno], PGADDR(pdeno, pteno, 0), arg)))
-                                       return ret;
+                       if((ret = callback(e, &pt[pteno], PGADDR(pdeno, pteno, 0), arg)))
+                               return ret;
                }
        }
        return 0;
index 775f228..87d4ac1 100644 (file)
@@ -256,8 +256,8 @@ void map_segment(pde_t *pgdir, uintptr_t va, size_t size, physaddr_t pa,
        __map_segment(pgdir, va, size, pa, perm, pml_shift);
 }
 
-/* For every present PTE in [start, start + len), call callback(pte, shift,
- * etc).  pml_shift is the shift/size of pml.
+/* For every PTE in [start, start + len), call callback(pte, shift,
+ * etc), including the not present PTEs.  pml_shift is the shift/size of pml.
  *
  * This will recurse down into sub PMLs, and perform the CB in a
  * depth-first-search.  The CB will be told which level of the paging it is at,
@@ -288,11 +288,10 @@ static int __pml_for_each(pte_t *pml,  uintptr_t start, size_t len,
               start, PMLx(start, pml_shift), start + len - 1,
               PMLx(start + len - 1, pml_shift), pml_shift, kva);
        for (pte_i = pte_s; pte_i <= pte_e; pte_i++, kva += pgsize) {
-               if (!(*pte_i & PTE_P))
-                       continue;
                visited_all_subs = FALSE;
                /* Complete only on the last level (PML1_SHIFT) or on a jumbo */
-               if (!walk_is_complete(pte_i, pml_shift, PML1_SHIFT)) {
+               if ((*pte_i & PTE_P) &&
+                   (!walk_is_complete(pte_i, pml_shift, PML1_SHIFT))) {
                        /* only pass truncated end points (e.g. start may not be page
                         * aligned) when we're on the first (or last) item.  For the middle
                         * entries, we want the subpmls to process the full range they are
@@ -328,8 +327,10 @@ int unmap_segment(pde_t *pgdir, uintptr_t va, size_t size)
        int pt_free_cb(pte_t *pte, uintptr_t kva, int shift, bool visited_subs,
                       void *data)
        {
+               if (!(*pte & PTE_P))
+                       return 0;
                if ((shift == PML1_SHIFT) || (*pte * PTE_PS)) {
-                       *pte = 0;       /* helps with debugging */
+                       *pte = 0;
                        return 0;
                }
                /* If we haven't visited all of our subs, we might still have some
@@ -338,7 +339,7 @@ int unmap_segment(pde_t *pgdir, uintptr_t va, size_t size)
                        pte_t *pte_i = pte2pml(*pte);   /* first pte == pml */
                        /* make sure we have no PTEs in use */
                        for (int i = 0; i < NPTENTRIES; i++, pte_i++) {
-                               if (*pte_i & PTE_P)
+                               if (*pte_i)
                                        return 0;
                        }
                }
@@ -506,6 +507,8 @@ void env_pagetable_free(struct proc *p)
        int pt_free_cb(pte_t *pte, uintptr_t kva, int shift, bool visited_subs,
                       void *data)
        {
+               if (!(*pte & PTE_P))
+                       return 0;
                if ((shift == PML1_SHIFT) || (*pte * PTE_PS))
                        return 0;
                page_decref(ppn2page(LA2PPN(pte)));
@@ -534,6 +537,8 @@ void page_check(void)
 static int print_pte(pte_t *pte, uintptr_t kva, int shift, bool visited_subs,
                      void *data)
 {
+       if (!(*pte & PTE_P))
+               return 0;
        switch (shift) {
                case (PML1_SHIFT):
                        printk("\t");
index 4a3930c..5187e9d 100644 (file)
@@ -137,17 +137,14 @@ void env_user_mem_free(env_t* e, void* start, size_t len)
        assert((uintptr_t)start + len <= UVPT); //since this keeps fucking happening
        int user_page_free(env_t* e, pte_t* pte, void* va, void* arg)
        {
-               if(PAGE_PRESENT(*pte))
-               {
-                       page_t* page = ppn2page(PTE2PPN(*pte));
-                       *pte = 0;
-                       page_decref(page);
-               } else {
-                       assert(PAGE_PAGED_OUT(*pte));
-                       /* TODO: (SWAP) deal with this */
-                       panic("Swapping not supported!");
-                       *pte = 0;
-               }
+               if (!PAGE_PRESENT(*pte))
+                       return 0;
+               page_t *page = ppn2page(PTE2PPN(*pte));
+               *pte = 0;
+               page_decref(page);
+               /* TODO: consider other states here (like !P, yet still tracking a page,
+                * for VM tricks, page map stuff, etc.  Should be okay: once we're
+                * freeing, everything else about this proc is dead. */
                return 0;
        }
 
index 35a447d..52cbc2a 100644 (file)
@@ -262,6 +262,10 @@ static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
        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))
+                       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)) {
                        /* TODO: check for jumbos */
                        if (upage_alloc(new_p, &pp, 0))