Pmap ops: perm->settings
This clarifies some aspects of pte_get_perms and pte_write. In reality,
write takes all sorts of non-permission based flags. What we really
wanted (I think) was the pte settings: the non-paddr lower 12 bits, in
an arch-indep format (based on the #defines).
Still, pte_replace_perm() is very useful as is. pte_get_perm paired
with pte_write and wasn't really related to pte_replace_perm (in usage).
diff --git a/kern/arch/riscv/pmap_ops.h b/kern/arch/riscv/pmap_ops.h
index c5eae53..d6012a4 100644
--- a/kern/arch/riscv/pmap_ops.h
+++ b/kern/arch/riscv/pmap_ops.h
@@ -69,9 +69,9 @@
return *(kpte_t*)pte;
}
-static inline void pte_write(pte_t pte, physaddr_t pa, int perm)
+static inline void pte_write(pte_t pte, physaddr_t pa, int settings)
{
- *(kpte_t*)pte = PTE(pa2ppn(pa), perm);
+ *(kpte_t*)pte = PTE(pa2ppn(pa), settings);
}
static inline void pte_clear_present(pte_t pte)
@@ -100,9 +100,13 @@
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)
+/* Settings includes protection (maskable via PTE_PROT) and other bits, such as
+ * jumbo, dirty, accessed, etc. Whatever this returns can get fed back to
+ * replace_settings or pte_write.
+ *
+ * Arch-indep settings include: PTE_PERM (U, W, P, etc), PTE_D, PTE_A, PTE_PS.
+ * Other OSs (x86) may include others. */
+static inline int pte_get_settings(pte_t pte)
{
return *(kpte_t*)pte & PTE_PERM;
}
diff --git a/kern/arch/x86/kpt.h b/kern/arch/x86/kpt.h
index 3415418..c42e393 100644
--- a/kern/arch/x86/kpt.h
+++ b/kern/arch/x86/kpt.h
@@ -55,10 +55,11 @@
return *kpte;
}
-static inline void kpte_write(kpte_t *kpte, physaddr_t pa, int perm)
+static inline void kpte_write(kpte_t *kpte, physaddr_t pa, int settings)
{
assert(!PGOFF(pa));
- *kpte = pa | perm;
+ /* The arch-bits like PTE_D, PTE_PS, etc are all in the native KPT format */
+ *kpte = pa | settings;
}
static inline void kpte_clear_present(kpte_t *kpte)
@@ -81,9 +82,9 @@
return (*kpte & PTE_USER_RW) == PTE_USER_RW;
}
-static inline int kpte_get_perm(kpte_t *kpte)
+static inline int kpte_get_settings(kpte_t *kpte)
{
- return *kpte & PTE_PERM;
+ return *kpte & 0xfff;
}
static inline void kpte_replace_perm(kpte_t *kpte, int perm)
diff --git a/kern/arch/x86/pmap64.c b/kern/arch/x86/pmap64.c
index 857479a..f8ccc76 100644
--- a/kern/arch/x86/pmap64.c
+++ b/kern/arch/x86/pmap64.c
@@ -681,7 +681,8 @@
reason = "paddr";
goto fail;
}
- if (kpte_get_perm(kpte) != epte_get_perm(epte)) {
+ if ((kpte_get_settings(kpte) & PTE_PERM) !=
+ (epte_get_settings(epte) & PTE_PERM)) {
reason = "permissions";
goto fail;
}
diff --git a/kern/arch/x86/pmap_ops.h b/kern/arch/x86/pmap_ops.h
index 19a0fec..e5e08e1 100644
--- a/kern/arch/x86/pmap_ops.h
+++ b/kern/arch/x86/pmap_ops.h
@@ -90,10 +90,10 @@
return kpte_print(pte);
}
-static inline void pte_write(pte_t pte, physaddr_t pa, int perm)
+static inline void pte_write(pte_t pte, physaddr_t pa, int settings)
{
- kpte_write(pte, pa, perm);
- epte_write(kpte_to_epte(pte), pa, perm);
+ kpte_write(pte, pa, settings);
+ epte_write(kpte_to_epte(pte), pa, settings);
}
static inline void pte_clear_present(pte_t pte)
@@ -124,11 +124,15 @@
return kpte_has_perm_urw(pte);
}
-/* 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)
+/* Settings includes protection (maskable via PTE_PROT) and other bits, such as
+ * jumbo, dirty, accessed, etc. Whatever this returns can get fed back to
+ * pte_write.
+ *
+ * Arch-indep settings include: PTE_PERM (U, W, P, etc), PTE_D, PTE_A, PTE_PS.
+ * Other OSs (x86) may include others. */
+static inline int pte_get_settings(pte_t pte)
{
- return kpte_get_perm(pte);
+ return kpte_get_settings(pte);
}
static inline void pte_replace_perm(pte_t pte, int perm)
diff --git a/kern/arch/x86/vmm/ept.h b/kern/arch/x86/vmm/ept.h
index b2d3da9..4cb39ed 100644
--- a/kern/arch/x86/vmm/ept.h
+++ b/kern/arch/x86/vmm/ept.h
@@ -134,24 +134,26 @@
return (*epte & (EPTE_R | EPTE_W | EPTE_X)) == (EPTE_R | EPTE_W | EPTE_X);
}
-/* We want to know User and Writable, in the 'PTE' sense. All present epte
- * entries are User PTEs. */
-static inline int epte_get_perm(epte_t *epte)
+static inline int epte_get_settings(epte_t *epte)
{
int settings = 0;
if (*epte & EPTE_P) {
+ /* We want to know User and Writable, in the 'PTE' sense. All present
+ * epte entries are User PTEs. */
settings |= PTE_P | PTE_U;
settings |= *epte & EPTE_W ? PTE_W : 0;
}
- //settings |= *epte & EPTE_PS ? PTE_PS : 0; /* TODO */
+ settings |= *epte & EPTE_PS ? PTE_PS : 0;
+ settings |= *epte & EPTE_A ? PTE_A : 0;
+ settings |= *epte & EPTE_D ? PTE_D : 0;
return settings;
}
/* Again, we're replacing the old perms with U and/or W. Any non-U are ignored,
- * as with epte_write. R (and X) are implied. */
-static inline void epte_replace_perm(epte_t *epte, int settings)
+ * as with epte_write. */
+static inline void epte_replace_perm(epte_t *epte, int perm)
{
- *epte = (*epte & ~EPTE_P) | __pte_to_epte_perm(settings & PTE_PERM);
+ *epte = (*epte & ~EPTE_P) | __pte_to_epte_perm(perm & PTE_PERM);
}
/* These ops might be the same for AMD as Intel; in which case we can move the
diff --git a/kern/src/mm.c b/kern/src/mm.c
index bf93513..9270970 100644
--- a/kern/src/mm.c
+++ b/kern/src/mm.c
@@ -292,7 +292,7 @@
/* TODO: check for jumbos */
if (upage_alloc(new_p, &pp, 0))
return -ENOMEM;
- if (page_insert(new_p->env_pgdir, pp, va, pte_get_perm(pte))) {
+ if (page_insert(new_p->env_pgdir, pp, va, pte_get_settings(pte))) {
page_decref(pp);
return -ENOMEM;
}
