arch,pt: implement better vunmap() functionality

Previous naive implementation was creating new pagetable entries when no such mapping had been created. It wasn't also taking into account the order of the mapping, assuming all mappings were 4K. Also, return unmapped MFN and page order to let the caller handle corresponding frame. Signed-off-by: Pawel Wieczorkiewicz <[email protected]>
KernelTestFramework · Nov 14, 2023 · 3c521ad · 3c521ad
1 parent 89601db
commit 3c521ad
Show file tree

Hide file tree

Showing 11 changed files with 130 additions and 31 deletions.
diff --git a/arch/x86/pagetables.c b/arch/x86/pagetables.c
@@ -23,6 +23,7 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 #include <console.h>
+#include <errno.h>
 #include <ktf.h>
 #include <multiboot2.h>
 #include <page.h>
@@ -363,6 +364,88 @@ static void map_tmp_mapping_entry(void) {
     _tmp_mapping_entry = paddr_to_virt_kern(_paddr(entry));
 }
 
+static int _vunmap(cr3_t *cr3_ptr, void *va, mfn_t *mfn, unsigned int *order) {
+    pgentry_t *tab;
+    mfn_t _mfn;
+    unsigned int _order;
+    pgentry_t *entry;
+    bool present;
+
+    if (mfn_invalid(cr3_ptr->mfn))
+        return -EINVAL;
+
+    dprintk("%s: va: 0x%p (cr3: 0x%p)\n", __func__, va, cr3_ptr);
+
+    tab = tmp_map_mfn(cr3_ptr->mfn);
+#if defined(__x86_64__)
+    pml4_t *l4e = l4_table_entry((pml4_t *) tab, va);
+    if (mfn_invalid(l4e->mfn) || !l4e->P)
+        return -ENOENT;
+
+    tab = tmp_map_mfn(l4e->mfn);
+#endif
+    pdpe_t *l3e = l3_table_entry((pdpe_t *) tab, va);
+    if (l3e->PS) {
+        _mfn = l3e->mfn;
+        _order = PAGE_ORDER_1G;
+        entry = &l3e->entry;
+        present = l3e->P;
+        goto done;
+    }
+
+    if (mfn_invalid(l3e->mfn) || !l3e->P)
+        return -ENOENT;
+
+    tab = tmp_map_mfn(l3e->mfn);
+    pde_t *l2e = l2_table_entry((pde_t *) tab, va);
+    if (l2e->PS) {
+        _mfn = l2e->mfn;
+        _order = PAGE_ORDER_2M;
+        entry = &l2e->entry;
+        present = l2e->P;
+        goto done;
+    }
+
+    if (mfn_invalid(l2e->mfn) || !l2e->P)
+        return -ENOENT;
+
+    tab = tmp_map_mfn(l2e->mfn);
+    pte_t *l1e = l1_table_entry((pte_t *) tab, va);
+    _mfn = l1e->mfn;
+    _order = PAGE_ORDER_4K;
+    entry = &l1e->entry;
+    present = l1e->P;
+
+done:
+    if (mfn)
+        *mfn = _mfn;
+    if (order)
+        *order = _order;
+    set_pgentry(entry, MFN_INVALID, PT_NO_FLAGS);
+    if (present)
+        invlpg(va);
+
+    return 0;
+}
+
+int vunmap_kern(void *va, mfn_t *mfn, unsigned int *order) {
+    int err;
+
+    spin_lock(&vmap_lock);
+    err = _vunmap(&cr3, va, mfn, order);
+    spin_unlock(&vmap_lock);
+    return err;
+}
+
+int vunmap_user(void *va, mfn_t *mfn, unsigned int *order) {
+    int err;
+
+    spin_lock(&vmap_lock);
+    err = _vunmap(&user_cr3, va, mfn, order);
+    spin_unlock(&vmap_lock);
+    return err;
+}
+
 static inline void init_cr3(cr3_t *cr3_ptr) {
     memset(cr3_ptr, 0, sizeof(*cr3_ptr));
     cr3_ptr->mfn = MFN_INVALID;

diff --git a/common/acpi.c b/common/acpi.c
@@ -156,7 +156,7 @@ static void acpi_table_unmap_pages(void *addr, unsigned mapped_pages) {
     mfn_t mfn = virt_to_mfn(addr);
 
     for (unsigned i = 0; i < mapped_pages; i++, mfn++) {
-        vunmap_kern(mfn_to_virt_kern(mfn), PAGE_ORDER_4K);
+        vunmap_kern(mfn_to_virt_kern(mfn), NULL, NULL);
     }
 }
 

diff --git a/common/setup.c b/common/setup.c
@@ -117,10 +117,19 @@ static __always_inline void zero_bss(void) {
 void zap_boot_mappings(void) {
     for_each_memory_range (r) {
         if (r->base == VIRT_IDENT_BASE && IS_INIT_SECTION(r->name)) {
+            unsigned int order = PAGE_ORDER_4K;
+
             memset(r->start, 0, r->end - r->start);
-            for (mfn_t mfn = virt_to_mfn(r->start); mfn < virt_to_mfn(r->end); mfn++) {
-                vunmap_kern(mfn_to_virt(mfn), PAGE_ORDER_4K);
-                reclaim_frame(mfn, PAGE_ORDER_4K);
+            for (void *va = r->start; va < r->end; va += ORDER_TO_SIZE(order)) {
+                mfn_t mfn;
+
+                if (vunmap_kern(va, &mfn, &order)) {
+                    /* FIXME: Use warning */
+                    printk("Unable to unmap kernel boot mapping at %p\n", va);
+                    order = PAGE_ORDER_4K;
+                    continue;
+                }
+                reclaim_frame(mfn, order);
             }
         }
     }

diff --git a/common/usermode.c b/common/usermode.c
@@ -64,9 +64,15 @@ long syscall_handler(long syscall_nr, long arg1, long arg2, long arg3, long arg4
 
     case SYSCALL_MUNMAP: {
         void *va = _ptr(arg1);
-        unsigned int order = _u(arg2);
+        mfn_t mfn;
+        unsigned int order;
+        int err;
+
+        err = vunmap_user(va, &mfn, &order);
+        if (err)
+            return err;
 
-        vunmap_user(va, order);
+        put_free_frames(mfn, order);
         return 0;
     }
 
@@ -220,8 +226,8 @@ static inline long __user_text sys_mmap(void *va, unsigned long order) {
     return syscall2(SYSCALL_MMAP, _ul(va), order);
 }
 
-static inline long __user_text sys_munmap(void *va, unsigned long order) {
-    return syscall2(SYSCALL_MUNMAP, _ul(va), order);
+static inline long __user_text sys_munmap(void *va) {
+    return syscall1(SYSCALL_MUNMAP, _ul(va));
 }
 
 void __user_text exit(unsigned long exit_code) {
@@ -240,6 +246,6 @@ void *__user_text mmap(void *va, unsigned long order) {
     return _ptr(sys_mmap(va, order));
 }
 
-void __user_text munmap(void *va, unsigned long order) {
-    sys_munmap(va, order);
+int __user_text munmap(void *va) {
+    return sys_munmap(va);
 }
diff --git a/drivers/acpi/acpica/osl.c b/drivers/acpi/acpica/osl.c
@@ -339,7 +339,7 @@ void AcpiOsUnmapMemory(void *LogicalAddress, ACPI_SIZE Length) {
         if (--frame->refcount > 0)
             continue;
 
-        vunmap_kern(mfn_to_virt_map(mfn), PAGE_ORDER_4K);
+        vunmap_kern(mfn_to_virt_map(mfn), NULL, NULL);
         list_unlink(&frame->list);
         kfree(frame);
     }

diff --git a/include/arch/x86/page.h b/include/arch/x86/page.h
@@ -183,6 +183,8 @@ extern void *vmap_user(void *va, mfn_t mfn, unsigned int order,
                        unsigned long l3_flags, unsigned long l2_flags,
                        unsigned long l1_flags);
 
+extern int vunmap_kern(void *va, mfn_t *mfn, unsigned int *order);
+extern int vunmap_user(void *va, mfn_t *mfn, unsigned int *order);
 extern void pat_set_type(pat_field_t field, pat_memory_type_t type);
 extern pat_memory_type_t pat_get_type(pat_field_t field);
 
@@ -249,14 +251,6 @@ static inline mfn_t virt_to_mfn(const void *va) {
     return paddr_to_mfn(virt_to_paddr(va));
 }
 
-static inline void vunmap_kern(void *va, unsigned int order) {
-    vmap_kern(va, MFN_INVALID, order, PT_NO_FLAGS, PT_NO_FLAGS, PT_NO_FLAGS, PT_NO_FLAGS);
-}
-
-static inline void vunmap_user(void *va, unsigned int order) {
-    vmap_user(va, MFN_INVALID, order, PT_NO_FLAGS, PT_NO_FLAGS, PT_NO_FLAGS, PT_NO_FLAGS);
-}
-
 static inline void *kmap(mfn_t mfn, unsigned int order,
 #if defined(__x86_64__)
                          unsigned long l4_flags,

diff --git a/include/mm/vmm.h b/include/mm/vmm.h
@@ -38,7 +38,7 @@ typedef enum gfp_flags gfp_flags_t;
 /* External definitions */
 
 extern void *get_free_pages(unsigned int order, gfp_flags_t flags);
-extern void put_pages(void *page, unsigned int order);
+extern void put_pages(void *page);
 
 /* Static definitions */
 
@@ -55,11 +55,11 @@ static inline void *get_free_page_top(gfp_flags_t flags) {
 }
 
 static inline void put_page(void *page) {
-    put_pages(page, PAGE_ORDER_4K);
+    put_pages(page);
 }
 
 static inline void put_page_top(void *page) {
-    put_pages(page - PAGE_SIZE, PAGE_ORDER_4K);
+    put_pages(page - PAGE_SIZE);
 }
 
 #endif /* KTF_VMM_H */
diff --git a/include/usermode.h b/include/usermode.h
@@ -64,7 +64,7 @@ extern void init_usermode(percpu_t *percpu);
 extern void __user_text exit(unsigned long exit_code);
 extern void __user_text printf(const char *fmt, ...);
 extern void *__user_text mmap(void *va, unsigned long order);
-extern void __user_text munmap(void *va, unsigned long order);
+extern int __user_text munmap(void *va);
 extern bool __user_text syscall_mode(syscall_mode_t);
 
 #endif /* __ASSEMBLY__ */

diff --git a/mm/slab.c b/mm/slab.c
@@ -159,7 +159,7 @@ meta_slab_t *slab_meta_alloc() {
     ret = initialize_slab(meta_slab_page);
     if (ret != ESUCCESS) {
         dprintk("initialize_slab in slab_meta_alloc failed\n");
-        put_pages(free_page, PAGE_ORDER_4K);
+        put_pages(free_page);
         return NULL;
     }
 
@@ -250,7 +250,7 @@ static void *ktf_alloc(size_t size) {
 
     if (ret != ESUCCESS) {
         dprintk("initialize_slab failed\n");
-        put_pages(free_page, PAGE_ORDER_4K);
+        put_pages(free_page);
         alloc = NULL;
         goto out;
     }
@@ -306,7 +306,7 @@ static void ktf_free(void *ptr) {
                      * meta slab free
                      */
                     list_unlink(&slab->list);
-                    put_pages(slab->slab_base, PAGE_ORDER_4K);
+                    put_pages(slab->slab_base);
                     meta_slab_page = META_SLAB_PAGE_ENTRY(slab);
                     slab_free(meta_slab_page, slab);
                     /*
@@ -320,7 +320,7 @@ static void ktf_free(void *ptr) {
                                 meta_slab_page->slab_base);
                         list_unlink(&meta_slab_page->list);
                         memset(meta_slab_page, 0, PAGE_SIZE);
-                        put_pages(meta_slab_page, PAGE_ORDER_4K);
+                        put_pages(meta_slab_page);
                     }
                 }
                 spin_unlock(&slab_mm_lock);

diff --git a/mm/vmm.c b/mm/vmm.c
@@ -79,10 +79,12 @@ void *get_free_pages(unsigned int order, gfp_flags_t flags) {
     return va;
 }
 
-void put_pages(void *page, unsigned int order) {
-    /* FIXME: unmap all mappings */
+void put_pages(void *page) {
+    unsigned int order;
+    mfn_t mfn;
+
     spin_lock(&mmap_lock);
-    vunmap_kern(page, order);
+    BUG_ON(vunmap_kern(page, &mfn, &order));
     spin_unlock(&mmap_lock);
-    put_free_frames(virt_to_mfn(page), order);
+    put_free_frames(mfn, order);
 }
diff --git a/tests/unittests.c b/tests/unittests.c
@@ -98,7 +98,12 @@ static unsigned long __user_text test_user_task_func2(void *arg) {
 
     va = mmap(_ptr(0xfff80000), PAGE_ORDER_4K);
     printf(USTR("mmap: %lx\n"), _ul(va));
+    if (munmap(va) != 0) {
+        printf(USTR("ERROR: munmap failed\n"));
+        ud2();
+    }
 
+    va = mmap(_ptr(0xfff80000), PAGE_ORDER_4K);
     memset(va, 0xcc, 0x1000);
     ((void (*)(void)) va)();