fix mmap_windows

Signed-off-by: Alexandru Agache <[email protected]>

Author: alexandruag

Cargo.toml

@@ -22,7 +22,7 @@ arc-swap = { version = ">=1.0.0", optional = true }
 
 [target.'cfg(windows)'.dependencies.winapi]
 version = ">=0.3"
-features = ["errhandlingapi"]
+features = ["errhandlingapi", "sysinfoapi"]
 
 [dev-dependencies]
 criterion = "0.3.0"

src/bitmap/backend/atomic_bitmap.rs

@@ -137,8 +137,28 @@ impl NewBitmap for AtomicBitmap {
     fn with_len(len: usize) -> Self {
         use std::convert::TryFrom;
 
-        // There's no unsafe potential in calling this function.
-        let page_size = unsafe { libc::sysconf(libc::_SC_PAGE_SIZE) };
+        let page_size;
+
+        #[cfg(unix)]
+        {
+            // There's no unsafe potential in calling this function.
+            page_size = unsafe { libc::sysconf(libc::_SC_PAGE_SIZE) };
+        }
+
+        #[cfg(windows)]
+        {
+            use winapi::um::sysinfoapi::{GetSystemInfo, LPSYSTEM_INFO, SYSTEM_INFO};
+
+            // It's safe to initialize this object from a zeroed memory region.
+            let mut sysinfo: SYSTEM_INFO = unsafe { std::mem::zeroed() };
+
+            // It's safe to call this method as the pointer is based on the address
+            // of the previously initialized `sysinfo` object.
+            unsafe { GetSystemInfo(&mut sysinfo as LPSYSTEM_INFO) };
+
+            page_size = sysinfo.dwPageSize;
+        }
+
         // The `unwrap` is safe to use because the above call should always succeed on the
         // supported platforms, and the size of a page will always fit within a `usize`.
         AtomicBitmap::new(len, usize::try_from(page_size).unwrap())

src/mmap_windows.rs

@@ -12,8 +12,9 @@ use libc::{c_void, size_t};
 
 use winapi::um::errhandlingapi::GetLastError;
 
+use crate::bitmap::{Bitmap, BS};
 use crate::guest_memory::FileOffset;
-use crate::mmap::AsSlice;
+use crate::mmap::{AsSlice, NewBitmap};
 use crate::volatile_memory::{self, compute_offset, VolatileMemory, VolatileSlice};
 
 #[allow(non_snake_case)]
@@ -70,20 +71,21 @@ pub const ERROR_INVALID_PARAMETER: i32 = 87;
 /// physical memory may be mapped into the current process due to the limited virtual address
 /// space size of the process.
 #[derive(Debug)]
-pub struct MmapRegion {
+pub struct MmapRegion<B> {
     addr: *mut u8,
     size: usize,
+    bitmap: B,
     file_offset: Option<FileOffset>,
 }
 
 // Send and Sync aren't automatically inherited for the raw address pointer.
 // Accessing that pointer is only done through the stateless interface which
 // allows the object to be shared by multiple threads without a decrease in
 // safety.
-unsafe impl Send for MmapRegion {}
-unsafe impl Sync for MmapRegion {}
+unsafe impl<B: Send> Send for MmapRegion<B> {}
+unsafe impl<B: Sync> Sync for MmapRegion<B> {}
 
-impl MmapRegion {
+impl<B: NewBitmap> MmapRegion<B> {
     /// Creates a shared anonymous mapping of `size` bytes.
     ///
     /// # Arguments
@@ -101,6 +103,7 @@ impl MmapRegion {
         Ok(Self {
             addr: addr as *mut u8,
             size,
+            bitmap: B::with_len(size),
             file_offset: None,
         })
     }
@@ -155,11 +158,16 @@ impl MmapRegion {
         Ok(Self {
             addr: addr as *mut u8,
             size,
+            bitmap: B::with_len(size),
             file_offset: Some(file_offset),
         })
     }
+}
 
-    /// Returns a pointer to the beginning of the memory region.
+impl<B: Bitmap> MmapRegion<B> {
+    /// Returns a pointer to the beginning of the memory region. Mutable accesses performed
+    /// using the resulting pointer are not automatically accounted for by the dirty bitmap
+    /// tracking functionality.
     ///
     /// Should only be used for passing this region to ioctls for setting guest memory.
     pub fn as_ptr(&self) -> *mut u8 {
@@ -175,9 +183,14 @@ impl MmapRegion {
     pub fn file_offset(&self) -> Option<&FileOffset> {
         self.file_offset.as_ref()
     }
+
+    /// Returns a reference to the inner bitmap object.
+    pub fn bitmap(&self) -> &B {
+        &self.bitmap
+    }
 }
 
-impl AsSlice for MmapRegion {
+impl<B> AsSlice for MmapRegion<B> {
     unsafe fn as_slice(&self) -> &[u8] {
         // This is safe because we mapped the area at addr ourselves, so this slice will not
         // overflow. However, it is possible to alias.
@@ -192,24 +205,36 @@ impl AsSlice for MmapRegion {
     }
 }
 
-impl VolatileMemory for MmapRegion {
+impl<B: Bitmap> VolatileMemory for MmapRegion<B> {
+    type B = B;
+
     fn len(&self) -> usize {
         self.size
     }
 
-    fn get_slice(&self, offset: usize, count: usize) -> volatile_memory::Result<VolatileSlice> {
+    fn get_slice(
+        &self,
+        offset: usize,
+        count: usize,
+    ) -> volatile_memory::Result<VolatileSlice<BS<Self::B>>> {
         let end = compute_offset(offset, count)?;
         if end > self.size {
             return Err(volatile_memory::Error::OutOfBounds { addr: end });
         }
 
         // Safe because we checked that offset + count was within our range and we only ever hand
         // out volatile accessors.
-        Ok(unsafe { VolatileSlice::new((self.addr as usize + offset) as *mut _, count) })
+        Ok(unsafe {
+            VolatileSlice::with_bitmap(
+                (self.addr as usize + offset) as *mut _,
+                count,
+                self.bitmap.slice_at(offset),
+            )
+        })
     }
 }
 
-impl Drop for MmapRegion {
+impl<B> Drop for MmapRegion<B> {
     fn drop(&mut self) {
         // This is safe because we mmap the area at addr ourselves, and nobody
         // else is holding a reference to it.
@@ -233,15 +258,27 @@ impl Drop for MmapRegion {
 
 #[cfg(test)]
 mod tests {
-    use crate::guest_memory::FileOffset;
-    use crate::mmap_windows::{MmapRegion, INVALID_HANDLE_VALUE};
     use std::os::windows::io::FromRawHandle;
 
+    use crate::bitmap::AtomicBitmap;
+    use crate::guest_memory::FileOffset;
+    use crate::mmap_windows::INVALID_HANDLE_VALUE;
+
+    type MmapRegion = super::MmapRegion<()>;
+
     #[test]
     fn map_invalid_handle() {
         let file = unsafe { std::fs::File::from_raw_handle(INVALID_HANDLE_VALUE) };
         let file_offset = FileOffset::new(file, 0);
         let e = MmapRegion::from_file(file_offset, 1024).unwrap_err();
         assert_eq!(e.raw_os_error(), Some(libc::EBADF));
     }
+
+    #[test]
+    fn test_dirty_tracking() {
+        // Using the `crate` prefix because we aliased `MmapRegion` to `MmapRegion<()>` for
+        // the rest of the unit tests above.
+        let m = crate::MmapRegion::<AtomicBitmap>::new(0x1_0000).unwrap();
+        crate::bitmap::tests::test_volatile_memory(&m);
+    }
 }