From 36d28400170188a8ef9aeacc6b8a55f0cea01b51 Mon Sep 17 00:00:00 2001
From: Ben Pig Chu <benpichu@gmail.com>
Date: Sun, 21 Jun 2020 22:36:43 +0800
Subject: [PATCH] Various fix and workaround for vmar test

---
 kernel-hal-bare/src/arch/x86_64/mod.rs | 29 ++++++++++++++++++++++----
 kernel-hal/src/user.rs                 |  3 +++
 scripts/testcases.txt                  |  4 +++-
 zircon-object/src/vm/vmar.rs           |  5 ++++-
 zircon-syscall/src/vmar.rs             | 19 ++++++++++++-----
 5 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/kernel-hal-bare/src/arch/x86_64/mod.rs b/kernel-hal-bare/src/arch/x86_64/mod.rs
index 18fe3214..77a3bcce 100644
--- a/kernel-hal-bare/src/arch/x86_64/mod.rs
+++ b/kernel-hal-bare/src/arch/x86_64/mod.rs
@@ -70,7 +70,13 @@ impl PageTableImpl {
             .unwrap()
             .flush();
         };
-        trace!("map: {:x?} -> {:x?}, flags={:?}", vaddr, paddr, flags);
+        trace!(
+            "map: {:x?} -> {:x?}, flags={:?} in {:#x?}",
+            vaddr,
+            paddr,
+            flags,
+            self.root_paddr
+        );
         Ok(())
     }
 
@@ -79,10 +85,25 @@ impl PageTableImpl {
     pub fn unmap(&mut self, vaddr: x86_64::VirtAddr) -> Result<(), ()> {
         let mut pt = self.get();
         let page = Page::<Size4KiB>::from_start_address(vaddr).unwrap();
-        if let Ok((_, flush)) = pt.unmap(page) {
-            flush.flush();
+        // This is a workaround to an issue in the x86-64 crate
+        // A page without PRESENT bit is not unmappable AND mapable
+        // So we add PRESENT bit here
+        unsafe {
+            pt.update_flags(page, PTF::PRESENT | PTF::NO_EXECUTE).ok();
+        }
+        match pt.unmap(page) {
+            Ok((_, flush)) => {
+                flush.flush();
+                trace!("unmap: {:x?} in {:#x?}", vaddr, self.root_paddr);
+            }
+            Err(err) => {
+                debug!(
+                    "unmap failed: {:x?} err={:x?} in {:#x?}",
+                    vaddr, err, self.root_paddr
+                );
+                return Err(());
+            }
         }
-        trace!("unmap: {:x?}", vaddr);
         Ok(())
     }
 
diff --git a/kernel-hal/src/user.rs b/kernel-hal/src/user.rs
index 494d6d88..2d4c481f 100644
--- a/kernel-hal/src/user.rs
+++ b/kernel-hal/src/user.rs
@@ -84,6 +84,9 @@ impl<T, P: Policy> UserPtr<T, P> {
         if self.ptr.is_null() {
             return Err(Error::InvalidPointer);
         }
+        if (self.ptr as usize) % core::mem::align_of::<T>() != 0 {
+            return Err(Error::InvalidPointer);
+        }
         Ok(())
     }
 }
diff --git a/scripts/testcases.txt b/scripts/testcases.txt
index 57bf8117..e4353046 100644
--- a/scripts/testcases.txt
+++ b/scripts/testcases.txt
@@ -20,7 +20,9 @@
 -Threads.Reading*State
 -Threads.WriteReadDebugRegisterState
 -Threads.DebugRegistersValidation
--Vmar.*
+-Vmar.ProtectOverDemandPagedTest
+-Vmar.ProtectLargeUncomittedTest
+-Vmar.UnmapLargeUncommittedTest
 -VmoTestCase.ReadOnlyMap
 -VmoTestCase.NoPermMap
 -VmoTestCase.NoPermProtect
diff --git a/zircon-object/src/vm/vmar.rs b/zircon-object/src/vm/vmar.rs
index f4c6b8ea..0e24a58c 100644
--- a/zircon-object/src/vm/vmar.rs
+++ b/zircon-object/src/vm/vmar.rs
@@ -292,9 +292,12 @@ impl VmAddressRegion {
     fn destroy_internal(&self) -> ZxResult {
         let mut guard = self.inner.lock();
         let inner = guard.as_mut().ok_or(ZxError::BAD_STATE)?;
-        for vmar in inner.children.iter() {
+        for vmar in inner.children.drain(..) {
             vmar.destroy_internal()?;
         }
+        for mapping in inner.mappings.drain(..) {
+            drop(mapping);
+        }
         *guard = None;
         Ok(())
     }
diff --git a/zircon-syscall/src/vmar.rs b/zircon-syscall/src/vmar.rs
index d5ec9859..26423f61 100644
--- a/zircon-syscall/src/vmar.rs
+++ b/zircon-syscall/src/vmar.rs
@@ -1,8 +1,11 @@
 use {super::*, bitflags::bitflags, zircon_object::vm::*};
 
 fn amount_of_alignments(options: u32) -> ZxResult<usize> {
-    let align_pow2 = (options >> 24) as usize;
-    if (align_pow2 < 10 && align_pow2 != 0) || (align_pow2 > 32) {
+    let mut align_pow2 = (options >> 24) as usize;
+    if align_pow2 == 0 {
+        align_pow2 = PAGE_SIZE_LOG2;
+    }
+    if (align_pow2 < PAGE_SIZE_LOG2) || (align_pow2 > 32) {
         Err(ZxError::INVALID_ARGS)
     } else {
         Ok(1 << align_pow2)
@@ -55,11 +58,12 @@ impl Syscall<'_> {
             None
         };
 
+        let size = roundup_pages(size as usize);
         // check `size`
-        if size == 0u64 {
+        if size == 0usize {
             return Err(ZxError::INVALID_ARGS);
         }
-        let child = parent.allocate(offset, size as usize, vmar_flags, align)?;
+        let child = parent.allocate(offset, size, vmar_flags, align)?;
         let child_addr = child.addr();
         let child_handle = proc.add_handle(Handle::new(child, Rights::DEFAULT_VMAR | perm_rights));
         info!("vmar.allocate: at {:#x?}", child_addr);
@@ -181,7 +185,12 @@ impl Syscall<'_> {
         mapping_flags.set(MMUFlags::READ, options.contains(VmOptions::PERM_READ));
         mapping_flags.set(MMUFlags::WRITE, options.contains(VmOptions::PERM_WRITE));
         mapping_flags.set(MMUFlags::EXECUTE, options.contains(VmOptions::PERM_EXECUTE));
-        vmar.protect(addr as usize, len as usize, mapping_flags)?;
+
+        let len = roundup_pages(len as usize);
+        if len == 0usize {
+            return Err(ZxError::INVALID_ARGS);
+        }
+        vmar.protect(addr as usize, len, mapping_flags)?;
         Ok(())
     }