Redefines PTE present vs mapped

Mapped now means it points to a physical page in all cases.  Present is
mapped and with bits set for some valid PTE walk.  For instance, a user
read works on x86 with PTE_U and PTE_P.

PTE_P shouldn't be used from the arch-indep code now.

I might have missed something with this commit, like leaking memory or
dirty bits.  Feel free to check out the usages of is_present, is_mapped,
and is_unmapped.

kern/arch/riscv/ros/mmu.h

diff --git a/kern/arch/riscv/ros/mmu.h b/kern/arch/riscv/ros/mmu.h
index c1de48e..a393b91 100644
--- a/kern/arch/riscv/ros/mmu.h
+++ b/kern/arch/riscv/ros/mmu.h
@@ -110,11 +110,18 @@
 #define PTE_NOCACHE	0 // PTE bits to turn off caching, if possible
 
 // commly used access modes
+
+#warning "Review RISCV PTEs.  Maybe want PTE_E/PTE_R?"
+	/* arch-indep code doesn't set PTE_P, it just sets a perm */
+
+
 #define PTE_KERN_RW	(PTE_SR | PTE_SW | PTE_SX)
 #define PTE_KERN_RO	(PTE_SR | PTE_SX)
 #define PTE_USER_RW	(PTE_SR | PTE_SW | PTE_UR | PTE_UW | PTE_UX)
 #define PTE_USER_RO	(PTE_SR | PTE_UR | PTE_UX)
+#define PTE_NONE	0
 
+#warning "probably remove this"
 // x86 equivalencies
 #define PTE_P      PTE_E
 

kern/arch/x86/pmap64.c

diff --git a/kern/arch/x86/pmap64.c b/kern/arch/x86/pmap64.c
index 658df50..901a9af 100644
--- a/kern/arch/x86/pmap64.c
+++ b/kern/arch/x86/pmap64.c
@@ -163,7 +163,7 @@
 	     pa += pgsize) {
 		kpte = get_next_pte(kpte, pgdir, va, PG_WALK_CREATE | pml_shift);
 		assert(kpte);
-		*kpte = PTE_ADDR(pa) | PTE_P | perm |
+		*kpte = PTE_ADDR(pa) | perm |
 		        (pml_shift != PML1_SHIFT ? PTE_PS : 0);
 		printd("Wrote *kpte %p, for va %p to pa %p tried to cover %p\n",
 		       *kpte, va, pa, amt_mapped);
@@ -230,8 +230,8 @@
 
 /* Map [va, va+size) of virtual (linear) address space to physical [pa, pa+size)
  * in the page table rooted at pgdir.  Size is a multiple of PGSIZE.  Use
- * permission bits perm|PTE_P for the entries.  Set pml_shift to the shift of
- * the largest page size you're willing to use.
+ * permission bits perm for the entries.  Set pml_shift to the shift of the
+ * largest page size you're willing to use.
  *
  * Doesn't handle having pages currently mapped yet, and while supporting that
  * is relatively easy, doing an insertion of small pages into an existing jumbo
@@ -383,7 +383,7 @@
 	kpte = &pml[PMLx(va, pml_shift)];
 	if (!(*kpte & PTE_P))
 		return 0;
-	perms_here = *kpte & (PTE_PERM | PTE_P);
+	perms_here = *kpte & PTE_PERM;
 	if (walk_is_complete(kpte, pml_shift, PML1_SHIFT))
 		return perms_here;
 	return pml_perm_walk(kpte2pml(*kpte), va, pml_shift - BITS_PER_PML) &
@@ -450,9 +450,9 @@
 	/* For the LAPIC and IOAPIC, we use PAT (but not *the* PAT flag) to make
 	 * these type UC */
 	map_segment(boot_pgdir, LAPIC_BASE, APIC_SIZE, LAPIC_PBASE,
-	            PTE_PCD | PTE_PWT | PTE_W | PTE_G, max_jumbo_shift);
+	            PTE_PCD | PTE_PWT | PTE_KERN_RW | PTE_G, max_jumbo_shift);
 	map_segment(boot_pgdir, IOAPIC_BASE, APIC_SIZE, IOAPIC_PBASE,
-	            PTE_PCD | PTE_PWT | PTE_W | PTE_G, max_jumbo_shift);
+	            PTE_PCD | PTE_PWT | PTE_KERN_RW | PTE_G, max_jumbo_shift);
 	/* VPT mapping: recursive PTE inserted at the VPT spot */
 	boot_kpt[PML4(VPT)] = PADDR(boot_kpt) | PTE_W | PTE_P;
 	/* same for UVPT, accessible by userspace (RO). */
@@ -594,8 +594,8 @@
 	memset(ept, 0, PGSIZE);
 
 	/* VPT and UVPT map the proc's page table, with different permissions. */
-	kpt[PML4(VPT)]  = PTE(LA2PPN(PADDR(kpt)), PTE_P | PTE_KERN_RW);
-	kpt[PML4(UVPT)] = PTE(LA2PPN(PADDR(kpt)), PTE_P | PTE_USER_RO);
+	kpt[PML4(VPT)]  = PTE(LA2PPN(PADDR(kpt)), PTE_KERN_RW);
+	kpt[PML4(UVPT)] = PTE(LA2PPN(PADDR(kpt)), PTE_USER_RO);
 
 	new_pd->kpte = kpt;
 	/* Processes do not have EPTs by default, only once they are VMMs */
@@ -627,7 +627,7 @@
 static int print_pte(kpte_t *kpte, uintptr_t kva, int shift, bool visited_subs,
                      void *data)
 {
-	if (!(*kpte & PTE_P))
+	if (kpte_is_unmapped(kpte))
 		return 0;
 	switch (shift) {
 		case (PML1_SHIFT):

kern/arch/x86/pmap_ops.h

diff --git a/kern/arch/x86/pmap_ops.h b/kern/arch/x86/pmap_ops.h
index 7a2293e..78c05fb 100644
--- a/kern/arch/x86/pmap_ops.h
+++ b/kern/arch/x86/pmap_ops.h
@@ -24,13 +24,21 @@
 }
 
 /* PTE states:
- *  - present: the PTE is involved in a valid page table walk, with the physaddr
- *  part pointing to a physical page.
+ *  - present: the PTE is involved in a valid page table walk, can be used
+ *  for some form of hardware access (read, write, user, etc), and 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.
+ * 	storing something in the PTE, but it is isn't necessarily present.
+ * 	Currently, all mapped pages should point to an actual physical page.
+ * 	All present are mapped, but not vice versa.  Mapped pages can point to a
+ * 	real page, but with no access permissions, which is the main distinction
+ * 	between present and mapped.
+ *
+ * 	- paged_out: we don't actually use this yet.  Since mapped vs present is
+ * 	based on the PTE present bits, we'd need to use reserved bits in the PTE to
+ * 	differentiate between other states.  Right now, paged_out == mapped, as far
+ * 	as the code is concerned.
  *
  * 	- unmapped: completely unused. (0 value) */
 static inline bool pte_is_present(pte_t pte)

kern/arch/x86/ros/mmu64.h

diff --git a/kern/arch/x86/ros/mmu64.h b/kern/arch/x86/ros/mmu64.h
index ab97a02..38b8361 100644
--- a/kern/arch/x86/ros/mmu64.h
+++ b/kern/arch/x86/ros/mmu64.h
@@ -286,11 +286,12 @@
 
 /* Permissions fields and common access modes.  These should be read as 'just
  * kernel or user too' and 'RO or RW'.  USER_RO means read-only for everyone. */
-#define PTE_PERM		(PTE_W | PTE_U)
-#define PTE_KERN_RW		PTE_W		// Kernel Read/Write
-#define PTE_KERN_RO		0		// Kernel Read-Only
-#define PTE_USER_RW		(PTE_W | PTE_U)	// Kernel/User Read/Write
-#define PTE_USER_RO		PTE_U		// Kernel/User Read-Only
+#define PTE_PERM		(PTE_W | PTE_U | PTE_P)
+#define PTE_KERN_RW		(PTE_W | PTE_P)
+#define PTE_KERN_RO		PTE_P
+#define PTE_USER_RW		(PTE_W | PTE_U | PTE_P)
+#define PTE_USER_RO		(PTE_U | PTE_P)
+#define PTE_NONE		0
 
 /* The PTE/translation part of a PTE/virtual(linear) address.  It's used
  * frequently to be the page address of a virtual address.  Note this doesn't
@@ -303,7 +304,7 @@
 /* we must guarantee that for any PTE, exactly one of the following is true */
 #define PAGE_PRESENT(pte) ((pte) & PTE_P)
 #define PAGE_UNMAPPED(pte) ((pte) == 0)
-#define PAGE_PAGED_OUT(pte) (!PAGE_PRESENT(pte) && !PAGE_UNMAPPED(pte))
+#define PAGE_PAGED_OUT(pte) (0)	/* Need to use an OS reserved PTE bit */
 
 
 /* **************************************** */

kern/src/env.c

diff --git a/kern/src/env.c b/kern/src/env.c
index d7a58c7..f2c49c1 100644
--- a/kern/src/env.c
+++ b/kern/src/env.c
@@ -103,7 +103,7 @@
 	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 (!pte_is_present(pte))
+		if (!pte_is_mapped(pte))
 			return 0;
 		page_t *page = pa2page(pte_get_paddr(pte));
 		pte_clear(pte);

kern/src/mm.c

diff --git a/kern/src/mm.c b/kern/src/mm.c
index 94daa68..2b10536 100644
--- a/kern/src/mm.c
+++ b/kern/src/mm.c
@@ -288,7 +288,7 @@
 			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 (pte_is_present(pte)) {
+		if (pte_is_mapped(pte)) {
 			/* TODO: check for jumbos */
 			if (upage_alloc(new_p, &pp, 0))
 				return -ENOMEM;
@@ -459,7 +459,7 @@
 	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,
@@ -483,10 +483,19 @@
 		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_is_dirty(pte) ? PTE_D : 0);
 	/* We have a ref to page, which we are storing in the PTE */
-	pte_write(pte, page2pa(page), PTE_P | prot);
+	pte_write(pte, page2pa(page), prot);
 	spin_unlock(&p->pte_lock);
 	return 0;
 }
@@ -705,7 +714,7 @@
 	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. */
@@ -720,10 +729,14 @@
 		}
 		vmr->vm_prot = prot;
 		spin_lock(&p->pte_lock);	/* walking and changing PTEs */
-		/* TODO: use a memwalk */
+		/* 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_walk_okay(pte) && pte_is_present(pte)) {
+			if (pte_walk_okay(pte) && pte_is_mapped(pte)) {
 				pte_replace_perm(pte, pte_prot);
 				shootdown_needed = TRUE;
 			}
@@ -765,11 +778,9 @@
                                      void *arg)
 {
 	bool *shootdown_needed = (bool*)arg;
-	struct page *page;
 	/* could put in some checks here for !P and also !0 */
 	if (!pte_is_present(pte))	/* unmapped (== 0) *ptes are also not PTE_P */
 		return 0;
-	page = pa2page(pte_get_paddr(pte));
 	pte_clear_present(pte);
 	*shootdown_needed = TRUE;
 	return 0;
@@ -1164,7 +1175,7 @@
 	/* 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_P | PTE_KERN_RW | flags)) {
+	                     PTE_KERN_RW | flags)) {
 		warn("Unable to map a vmap segment");	/* probably a bug */
 		return 0;
 	}

kern/src/pagemap.c

diff --git a/kern/src/pagemap.c b/kern/src/pagemap.c
index 8c7e5a2..6cf169c 100644
--- a/kern/src/pagemap.c
+++ b/kern/src/pagemap.c
@@ -306,7 +306,11 @@
 static int __pm_mark_not_present(struct proc *p, pte_t pte, void *va, void *arg)
 {
 	struct page *page;
-	if (!pte_is_present(pte))
+	/* mapped includes present.  Any PTE pointing to a page (mapped) will get
+	 * flagged for removal and have its access prots revoked.  We need to deal
+	 * with mapped-but-maybe-not-present in case of a dirtied file that was
+	 * mprotected to PROT_NONE (which is not present) */
+	if (pte_is_unmapped(pte))
 		return 0;
 	page = pa2page(pte_get_paddr(pte));
 	if (atomic_read(&page->pg_flags) & PG_REMOVAL)

kern/src/pmap.c

diff --git a/kern/src/pmap.c b/kern/src/pmap.c
index cb8351f..b16020f 100644
--- a/kern/src/pmap.c
+++ b/kern/src/pmap.c
@@ -223,7 +223,7 @@
 	 * any other state. (TODO: review all other states, maybe rm only for P) */
 	if (pte_is_mapped(pte))
 		page_remove(pgdir, va);
-	pte_write(pte, page2pa(page), PTE_P | perm);
+	pte_write(pte, page2pa(page), perm);
 	return 0;
 }
 
@@ -247,7 +247,7 @@
 page_t *page_lookup(pgdir_t pgdir, void *va, pte_t *pte_store)
 {
 	pte_t pte = pgdir_walk(pgdir, va, 0);
-	if (!pte_walk_okay(pte) || !pte_is_present(pte))
+	if (!pte_walk_okay(pte) || !pte_is_mapped(pte))
 		return 0;
 	if (pte_store)
 		*pte_store = pte;
@@ -285,7 +285,7 @@
 	if (!pte_walk_okay(pte) || pte_is_unmapped(pte))
 		return;
 
-	if (pte_is_present(pte)) {
+	if (pte_is_mapped(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. */

kern/src/umem.c

diff --git a/kern/src/umem.c b/kern/src/umem.c
index 38b4d92..87f4e3a 100644
--- a/kern/src/umem.c
+++ b/kern/src/umem.c
@@ -34,7 +34,7 @@
 	const void *start, *end;
 	size_t num_pages, i;
 	pte_t pte;
-	uintptr_t perm = PTE_P | PTE_USER_RO;
+	uintptr_t perm = PTE_USER_RO;
 	size_t bytes_copied = 0;
 
 	static_assert(ULIM % PGSIZE == 0 && ULIM != 0); // prevent wrap-around
@@ -100,7 +100,7 @@
 	const void *start, *end;
 	size_t num_pages, i;
 	pte_t pte;
-	uintptr_t perm = PTE_P | PTE_USER_RW;
+	uintptr_t perm = PTE_USER_RW;
 	size_t bytes_copied = 0;
 
 	static_assert(ULIM % PGSIZE == 0 && ULIM != 0); // prevent wrap-around