Makes pte_t an opaque type
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 16 Mar 2015 18:16:48 +0000 (14:16 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 17 Mar 2015 14:56:00 +0000 (10:56 -0400)
The nature of pte_t will be arch-dependent.  It could be a struct or the
old pte_t (kpte_t) and have its own tricks for modification.  This patch
changes all of the accesses that implicitly treated pte_t as something
that could be read or written in a certain manner.

It also catches a bug in the unused (and warned) unmap_vmap_segment().

kern/arch/x86/cpuinfo.c
kern/include/env.h
kern/include/pmap.h
kern/src/arsc.c
kern/src/elf.c
kern/src/env.c
kern/src/mm.c
kern/src/pagemap.c
kern/src/pmap.c
kern/src/process.c
kern/src/umem.c

index 521b6dd..f9dbf09 100644 (file)
@@ -176,8 +176,8 @@ void print_cpuinfo(void)
 
 void show_mapping(pde_t *pgdir, uintptr_t start, size_t size)
 {
-       pte_t *pte;
-       pte_t *pde;
+       pte_t pte;
+       int perm;
        page_t *page;
        uintptr_t i;
 
@@ -187,14 +187,26 @@ void show_mapping(pde_t *pgdir, uintptr_t start, size_t size)
        for(i = 0; i < size; i += PGSIZE, start += PGSIZE) {
                pte = pgdir_walk(pgdir, (void*)start, 0);
                printk("%p  ", start);
-               if (pte) {
-                       pde = &pgdir[PDX(start)];
-                       /* for a jumbo, pde = pte and PTE_PS (better be) = 1 */
+               if (pte_walk_okay(pte)) {
+                       /* A note on PTE perms.  If you look at just the PTE, you don't get
+                        * the full picture for W and U.  Those are the intersection of all
+                        * bits.  In Akaros, we do U or not at the earliest point (PML4
+                        * entries).  All other PTEs have U set.  For W, it's the opposite.
+                        * The PTE for the actual page has W or not, and all others has W
+                        * set.  W needs to be more fine-grained, but U doesn't.  Plus the
+                        * UVPT mapping requires the U to see interior pages (but have W
+                        * off). */
+                       perm = get_va_perms(pgdir, (void*)start);
                        printk("%p  %1d  %1d  %1d  %1d  %1d  %1d %1d %1d\n",
-                              PTE_ADDR(*pte), (*pte & PTE_PS) >> 7, (*pte & PTE_D) >> 6,
-                              (*pte & PTE_A) >> 5, (*pte & PTE_PCD) >> 4,
-                              (*pte & PTE_PWT) >> 3, (*pte & *pde & PTE_U) >> 2,
-                              (*pte & *pde & PTE_W) >> 1, (*pte & PTE_P));
+                              pte_get_paddr(pte),
+                              pte_is_jumbo(pte),
+                              pte_is_dirty(pte),
+                              pte_is_accessed(pte),
+                              (pte_print(pte) & PTE_PCD) / PTE_PCD,
+                              (pte_print(pte) & PTE_PWT) / PTE_PWT,
+                              (perm & PTE_U) / PTE_U,
+                              (perm & PTE_W) / PTE_W,
+                              pte_is_present(pte));
                } else {
                        printk("%p\n", 0);
                }
index 5a2e84f..91cd3ce 100644 (file)
@@ -122,7 +122,7 @@ int         env_setup_vm(env_t *e);
 void   env_user_mem_free(env_t* e, void* start, size_t len);
 void   env_pagetable_free(env_t* e);
 
-typedef int (*mem_walk_callback_t)(env_t* e, pte_t* pte, void* va, void* arg);
+typedef int (*mem_walk_callback_t)(env_t* e, pte_t pte, void* va, void* arg);
 int            env_user_mem_walk(env_t* e, void* start, size_t len, mem_walk_callback_t callback, void* arg);
 
 #endif // !ROS_KERN_ENV_H
index a441680..9d9d7a9 100644 (file)
@@ -85,7 +85,7 @@ void *boot_zalloc(size_t amt, size_t align);
 void page_check(void);
 int     page_insert(pde_t *pgdir, struct page *page, void *SNT va, int perm);
 void page_remove(pde_t *COUNT(NPDENTRIES) pgdir, void *SNT va);
-page_t*COUNT(1) page_lookup(pde_t SSOMELOCK*COUNT(NPDENTRIES) pgdir, void *SNT va, pte_t **pte_store);
+page_t* page_lookup(pde_t *pgdir, void *va, pte_t *pte_store);
 error_t        pagetable_remove(pde_t *COUNT(NPDENTRIES) pgdir, void *SNT va);
 void   page_decref(page_t *COUNT(1) pp);
 
@@ -95,8 +95,8 @@ bool regions_collide_unsafe(uintptr_t start1, uintptr_t end1,
                             uintptr_t start2, uintptr_t end2);
 
 /* Arch specific implementations for these */
-pte_t *pgdir_walk(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va, int create);
-int get_va_perms(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va);
+pte_t pgdir_walk(pde_t *pgdir, const void *va, int create);
+int get_va_perms(pde_t *pgdir, const void *va);
 
 static inline page_t *SAFE ppn2page(size_t ppn)
 {
@@ -157,4 +157,109 @@ static inline unsigned long nr_pages(size_t nr_bytes)
        return (nr_bytes >> PGSHIFT) + (PGOFF(nr_bytes) ? 1 : 0);
 }
 
+static inline bool pte_walk_okay(pte_t pte)
+{
+       return pte ? TRUE : FALSE;
+}
+
+/* PTE states:
+ *  - present: the PTE is involved in a valid page table walk, with the physaddr
+ *  part pointing to a physical page.
+ *
+ *     - mapped: the PTE is involved in some sort of mapping, e.g. a VMR.  We're
+ *     storing something in the PTE, but it is isn't necessarily present and
+ *     pointing to an actual physical page.  All present are mapped, but not vice
+ *     versa.  Mapped could also include paged-out, if we support that later.
+ *
+ *     - unmapped: completely unused. (0 value) */
+static inline bool pte_is_present(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_P ? TRUE : FALSE;
+}
+
+static inline bool pte_is_unmapped(pte_t pte)
+{
+       return PAGE_UNMAPPED(*(kpte_t*)pte);
+}
+
+static inline bool pte_is_mapped(pte_t pte)
+{
+       return !PAGE_UNMAPPED(*(kpte_t*)pte);
+}
+
+static inline bool pte_is_paged_out(pte_t pte)
+{
+       return PAGE_PAGED_OUT(*(kpte_t*)pte);
+}
+
+static inline bool pte_is_dirty(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_D ? TRUE : FALSE;
+}
+
+static inline bool pte_is_accessed(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_A ? TRUE : FALSE;
+}
+
+/* Used in debugging code - want something better involving the walk */
+static inline bool pte_is_jumbo(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_PS ? TRUE : FALSE;
+}
+
+static inline physaddr_t pte_get_paddr(pte_t pte)
+{
+       return PTE_ADDR(*(kpte_t*)pte);
+}
+
+/* Returns the PTE in an unsigned long, for debugging mostly. */
+static inline unsigned long pte_print(pte_t pte)
+{
+       return *(kpte_t*)pte;
+}
+
+static inline void pte_write(pte_t pte, physaddr_t pa, int perm)
+{
+       *(kpte_t*)pte = PTE(pa2ppn(pa), perm);
+}
+
+static inline void pte_clear_present(pte_t pte)
+{
+       *(kpte_t*)pte &= ~PTE_P;
+}
+
+static inline void pte_clear(pte_t pte)
+{
+       *(kpte_t*)pte = 0;
+}
+
+/* These are used by memcpy_*_user, but are very dangerous (and possibly used
+ * incorrectly there).  These aren't the overall perms for a VA.  For U and W,
+ * we need the intersection of the PTEs along the walk and not just the last
+ * one.  It just so happens that the W is only cleared on the last PTE, so the
+ * check works for that.  But if there was a page under ULIM that wasn't U due
+ * to an intermediate PTE, we'd miss that. */
+static inline bool pte_has_perm_ur(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_USER_RO ? TRUE : FALSE;
+}
+
+static inline bool pte_has_perm_urw(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_USER_RW ? TRUE : FALSE;
+}
+
+/* return the arch-independent format for prots - whatever you'd expect to
+ * receive for pte_write.  Careful with the ret, since a valid type is 0. */
+static inline int pte_get_perm(pte_t pte)
+{
+       return *(kpte_t*)pte & PTE_PERM;
+}
+
+static inline void pte_replace_perm(pte_t pte, int perm)
+{
+       *(kpte_t*)pte = (*(kpte_t*)pte & ~PTE_PERM) | perm;
+}
+
 #endif /* !ROS_KERN_PMAP_H */
index acda958..441197f 100644 (file)
@@ -41,9 +41,9 @@ syscall_sring_t* sys_init_arsc(struct proc *p)
        // TODO: need to pin this page in the future when swapping happens
        va = do_mmap(p,MMAP_LOWEST_VA, SYSCALLRINGSIZE, PROT_READ | PROT_WRITE,
                     MAP_ANONYMOUS | MAP_POPULATE, NULL, 0);
-       pte_t *pte = pgdir_walk(p->env_pgdir, (void*)va, 0);
-       assert(pte);
-       sring = (syscall_sring_t*) (ppn2kva(PTE2PPN(*pte)));
+       pte_t pte = pgdir_walk(p->env_pgdir, (void*)va, 0);
+       assert(pte_walk_okay(pte));
+       sring = (syscall_sring_t*) KADDR(pte_get_paddr(pte));
        /*make sure we are able to allocate the shared ring */
        assert(sring != NULL);
        p->procdata->syscallring = sring;
index eb06184..245b0b1 100644 (file)
@@ -206,7 +206,7 @@ static int load_one_elf(struct proc *p, struct file *f, uintptr_t pgoffset,
                                         * filesystems should zero out the last bits of a page if
                                         * the file doesn't fill the last page.  But we're dealing
                                         * with windows into otherwise complete files. */
-                                       pte_t *pte = pgdir_walk(p->env_pgdir, (void*)last_page, 0);
+                                       pte_t pte = pgdir_walk(p->env_pgdir, (void*)last_page, 0);
                                        /* if we were able to get a PTE, then there is a real page
                                         * backing the VMR, and we need to zero the excess.  if
                                         * there isn't, then the page fault code should handle it.
@@ -215,8 +215,8 @@ static int load_one_elf(struct proc *p, struct file *f, uintptr_t pgoffset,
                                         * size.  in this case, we let them mmap it, but didn't
                                         * populate it.  there will be a PF right away if someone
                                         * tries to use this.  check out do_mmap for more info. */
-                                       if (pte) {
-                                               void* last_page_kva = ppn2kva(PTE2PPN(*pte));
+                                       if (pte_walk_okay(pte)) {
+                                               void* last_page_kva = KADDR(pte_get_paddr(pte));
                                                memset(last_page_kva + partial, 0, PGSIZE - partial);
                                        }
 
index 5187e9d..b2f02d6 100644 (file)
@@ -135,12 +135,12 @@ type SLOCKED(name##_lock) *\
 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)
+       int user_page_free(env_t* e, pte_t pte, void* va, void* arg)
        {
-               if (!PAGE_PRESENT(*pte))
+               if (!pte_is_present(pte))
                        return 0;
-               page_t *page = ppn2page(PTE2PPN(*pte));
-               *pte = 0;
+               page_t *page = pa2page(pte_get_paddr(pte));
+               pte_clear(pte);
                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
index 59ccec2..94daa68 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)
 {
@@ -267,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)
 {
@@ -281,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_present(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_perm(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;
        }
@@ -465,28 +465,28 @@ out_error:        /* for debugging */
 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;
        }
        /* 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), PTE_P | prot);
        spin_unlock(&p->pte_lock);
        return 0;
 }
@@ -702,7 +702,7 @@ 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;
@@ -723,8 +723,8 @@ int __do_mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
                /* TODO: use a memwalk */
                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_present(pte)) {
+                               pte_replace_perm(pte, pte_prot);
                                shootdown_needed = TRUE;
                        }
                }
@@ -761,16 +761,16 @@ int munmap(struct proc *p, uintptr_t addr, size_t len)
        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;
+       page = pa2page(pte_get_paddr(pte));
+       pte_clear_present(pte);
        *shootdown_needed = TRUE;
        return 0;
 }
@@ -788,13 +788,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;
@@ -803,7 +803,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
@@ -913,7 +912,6 @@ 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);
@@ -1109,20 +1107,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;
@@ -1135,10 +1133,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
index 4a28841..8c7e5a2 100644 (file)
@@ -303,45 +303,45 @@ static void vmr_for_each(struct vm_region *vmr, unsigned long pg_idx,
  * at to determine if the underlying page is dirty.  We need to make sure no one
  * clears dirty bits unless they handle the WB (or discard).  HPF preserves the
  * dirty bit for this reason. */
-static int __pm_mark_not_present(struct proc *p, pte_t *pte, void *va, void *arg)
+static int __pm_mark_not_present(struct proc *p, pte_t pte, void *va, void *arg)
 {
        struct page *page;
-       if (!PAGE_PRESENT(*pte))
+       if (!pte_is_present(pte))
                return 0;
-       page = ppn2page(PTE2PPN(*pte));
+       page = pa2page(pte_get_paddr(pte));
        if (atomic_read(&page->pg_flags) & PG_REMOVAL)
-               *pte &= ~PTE_P;
+               pte_clear_present(pte);
        return 0;
 }
 
-static int __pm_mark_dirty_pgs_unmap(struct proc *p, pte_t *pte, void *va,
+static int __pm_mark_dirty_pgs_unmap(struct proc *p, pte_t pte, void *va,
                                      void *arg)
 {
        struct page *page;
        /* we're not checking for 'present' or not, since we marked them !P earlier.
         * but the CB is still called on everything in the range.  we can tell the
         * formerly-valid PTEs from the completely unmapped, since the latter are
-        * == 0, while the former have other things in them, but just are !P. */
-       if (!(*pte))
+        * unmapped, while the former have other things in them, but just are !P. */
+       if (pte_is_unmapped(pte))
                return 0;
-       page = ppn2page(PTE2PPN(*pte));
+       page = pa2page(pte_get_paddr(pte));
        /* need to check for removal again, just like in mark_not_present */
        if (atomic_read(&page->pg_flags) & PG_REMOVAL) {
-               if (*pte & PTE_D)
+               if (pte_is_dirty(pte))
                        atomic_or(&page->pg_flags, PG_DIRTY);
-               *pte = 0;
+               pte_clear(pte);
        }
        return 0;
 }
 
-static int __pm_mark_unmap(struct proc *p, pte_t *pte, void *va, void *arg)
+static int __pm_mark_unmap(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));
+       page = pa2page(pte_get_paddr(pte));
        if (atomic_read(&page->pg_flags) & PG_REMOVAL)
-               *pte = 0;
+               pte_clear(pte);
        return 0;
 }
 
index 2cd8286..8659c38 100644 (file)
@@ -211,18 +211,19 @@ void *boot_zalloc(size_t amt, size_t align)
  */
 int page_insert(pde_t *pgdir, struct page *page, void *va, int perm) 
 {
-       pte_t* pte = pgdir_walk(pgdir, va, 1);
-       if (!pte)
+       pte_t pte = pgdir_walk(pgdir, va, 1);
+       if (!pte_walk_okay(pte))
                return -ENOMEM;
        /* Two things here:  First, we need to up the ref count of the page we want
         * to insert in case it is already mapped at va.  In that case we don't want
         * page_remove to ultimately free it, and then for us to continue as if pp
         * wasn't freed. (moral = up the ref asap) */
        kref_get(&page->pg_kref, 1);
-       /* Careful, page remove handles the cases where the page is PAGED_OUT. */
-       if (!PAGE_UNMAPPED(*pte))
+       /* Careful, page remove handles the cases where the page is PAGED_OUT and
+        * any other state. (TODO: review all other states, maybe rm only for P) */
+       if (pte_is_mapped(pte))
                page_remove(pgdir, va);
-       *pte = PTE(page2ppn(page), PTE_P | perm);
+       pte_write(pte, page2pa(page), PTE_P | perm);
        return 0;
 }
 
@@ -243,14 +244,14 @@ int page_insert(pde_t *pgdir, struct page *page, void *va, int perm)
  * @return PAGE the page mapped at virtual address 'va'
  * @return NULL No mapping exists at virtual address 'va', or it's paged out
  */
-page_t *page_lookup(pde_t *pgdir, void *va, pte_t **pte_store)
+page_t *page_lookup(pde_t *pgdir, void *va, pte_t *pte_store)
 {
-       pte_t* pte = pgdir_walk(pgdir, va, 0);
-       if (!pte || !PAGE_PRESENT(*pte))
+       pte_t pte = pgdir_walk(pgdir, va, 0);
+       if (!pte_walk_okay(pte) || !pte_is_present(pte))
                return 0;
        if (pte_store)
                *pte_store = pte;
-       return pa2page(PTE_ADDR(*pte));
+       return pa2page(pte_get_paddr(pte));
 }
 
 /**
@@ -277,25 +278,25 @@ page_t *page_lookup(pde_t *pgdir, void *va, pte_t **pte_store)
  * in env_user_mem_free, minus the walk. */
 void page_remove(pde_t *pgdir, void *va)
 {
-       pte_t *pte;
+       pte_t pte;
        page_t *page;
 
        pte = pgdir_walk(pgdir,va,0);
-       if (!pte || PAGE_UNMAPPED(*pte))
+       if (!pte_walk_okay(pte) || pte_is_unmapped(pte))
                return;
 
-       if (PAGE_PRESENT(*pte)) {
+       if (pte_is_present(pte)) {
                /* TODO: (TLB) need to do a shootdown, inval sucks.  And might want to
                 * manage the TLB / free pages differently. (like by the caller).
                 * Careful about the proc/memory lock here. */
-               page = ppn2page(PTE2PPN(*pte));
-               *pte = 0;
+               page = pa2page(pte_get_paddr(pte));
+               pte_clear(pte);
                tlb_invalidate(pgdir, va);
                page_decref(page);
-       } else if (PAGE_PAGED_OUT(*pte)) {
+       } else if (pte_is_paged_out(pte)) {
                /* TODO: (SWAP) need to free this from the swap */
                panic("Swapping not supported!");
-               *pte = 0;
+               pte_clear(pte);
        }
 }
 
index 41ecfe0..2332652 100644 (file)
@@ -445,9 +445,9 @@ struct proc *proc_create(struct file *prog, char **argv, char **envp)
        return p;
 }
 
-static int __cb_assert_no_pg(struct proc *p, pte_t *pte, void *va, void *arg)
+static int __cb_assert_no_pg(struct proc *p, pte_t pte, void *va, void *arg)
 {
-       assert(!*pte);
+       assert(pte_is_unmapped(pte));
        return 0;
 }
 
index c7ae3d1..a6357f3 100644 (file)
@@ -33,7 +33,7 @@ int memcpy_from_user(struct proc *p, void *dest, const void *DANGEROUS va,
 {
        const void *DANGEROUS start, *DANGEROUS end;
        size_t num_pages, i;
-       pte_t *pte;
+       pte_t pte;
        uintptr_t perm = PTE_P | PTE_USER_RO;
        size_t bytes_copied = 0;
 
@@ -48,15 +48,15 @@ int memcpy_from_user(struct proc *p, void *dest, const void *DANGEROUS va,
        num_pages = LA2PPN(end - start);
        for (i = 0; i < num_pages; i++) {
                pte = pgdir_walk(p->env_pgdir, start + i * PGSIZE, 0);
-               if (!pte)
+               if (!pte_walk_okay(pte))
                        return -EFAULT;
-               if ((*pte & PTE_P) && (*pte & PTE_USER_RO) != PTE_USER_RO)
+               if (pte_is_present(pte) && !pte_has_perm_ur(pte))
                        return -EFAULT;
-               if (!(*pte & PTE_P))
+               if (!pte_is_present(pte))
                        if (handle_page_fault(p, (uintptr_t)start + i * PGSIZE, PROT_READ))
                                return -EFAULT;
 
-               void *kpage = KADDR(PTE_ADDR(*pte));
+               void *kpage = KADDR(pte_get_paddr(pte));
                const void *src_start = i > 0 ? kpage : kpage + (va - start);
                void *dst_start = dest + bytes_copied;
                size_t copy_len = PGSIZE;
@@ -99,7 +99,7 @@ int memcpy_to_user(struct proc *p, void *va, const void *src, size_t len)
 {
        const void *DANGEROUS start, *DANGEROUS end;
        size_t num_pages, i;
-       pte_t *pte;
+       pte_t pte;
        uintptr_t perm = PTE_P | PTE_USER_RW;
        size_t bytes_copied = 0;
 
@@ -114,14 +114,14 @@ int memcpy_to_user(struct proc *p, void *va, const void *src, size_t len)
        num_pages = LA2PPN(end - start);
        for (i = 0; i < num_pages; i++) {
                pte = pgdir_walk(p->env_pgdir, start + i * PGSIZE, 0);
-               if (!pte)
+               if (!pte_walk_okay(pte))
                        return -EFAULT;
-               if ((*pte & PTE_P) && (*pte & PTE_USER_RW) != PTE_USER_RW)
+               if (pte_is_present(pte) && !pte_has_perm_urw(pte))
                        return -EFAULT;
-               if (!(*pte & PTE_P))
+               if (!pte_is_present(pte))
                        if (handle_page_fault(p, (uintptr_t)start + i * PGSIZE, PROT_WRITE))
                                return -EFAULT;
-               void *kpage = KADDR(PTE_ADDR(*pte));
+               void *kpage = KADDR(pte_get_paddr(pte));
                void *dst_start = i > 0 ? kpage : kpage + (va - start);
                const void *src_start = src + bytes_copied;
                size_t copy_len = PGSIZE;