Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/guest_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,26 @@ pub trait GuestMemoryRegion: Bytes<MemoryRegionAddress, E = Error> {
fn as_volatile_slice(&self) -> Result<volatile_memory::VolatileSlice> {
self.get_slice(MemoryRegionAddress(0), self.len() as usize)
}

/// Show if the region is based on the `HugeTLBFS`.
/// Returns Some(true) if the region is backed by hugetlbfs.
/// None represents that no information is available.
///
/// # Examples (uses the `backend-mmap` feature)
///
/// ```
/// # #[cfg(feature = "backend-mmap")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just merged a PR that updated all the code examples that needed features. Can you please update this example to follow that template?

Changes that would be needed:

  • specify in the header that this example is based on the the mmap feature. Here is an example:
    /// # Examples (uses the `backend-mmap` feature)
  • declare the [cfg(feature = "backend-mmap")] only one at the begginging of the example, and get rid of the test_guest_memory_mmap_is_hugetlbfs function. The code can sit directly in the example.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test_guest_memory_mmap_is_hugetlbs function is still there. To get rid of it, and also declare the #[cfg(feature = "backend-mmap")], we would need to start a code block (and have it all conditionally compiled only for that feature).

    /// # #[cfg(feature = "backend-mmap")]
    /// # {
    /// # use vm_memory::{GuestAddress, GuestMemory, GuestMemoryMmap, GuestRegionMmap};
    ///
    /// let addr = GuestAddress(0x1000);
    /// let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
    /// let r = mem.find_region(addr).unwrap();
    /// assert_eq!(r.is_hugetlbfs(), None);
    /// # }

With this approach, you no longer need the function test_guest_memory_mmap_is_hugetlbfs, and all the cfg macros.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo fmt fails because of alignment. The code needs to be aligned as in my previous comment. We can fix it in a subsequent PR.

/// # {
/// # use vm_memory::{GuestAddress, GuestMemory, GuestMemoryMmap, GuestRegionMmap};
/// let addr = GuestAddress(0x1000);
/// let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
/// let r = mem.find_region(addr).unwrap();
/// assert_eq!(r.is_hugetlbfs(), None);
/// # }
/// ```
fn is_hugetlbfs(&self) -> Option<bool> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late comment, but can we actually just use conditional compilation here and make it available only on Linux, instead of having a blank implementation for Windows?

None
}
}

/// `GuestAddressSpace` provides a way to retrieve a `GuestMemory` object.
Expand Down Expand Up @@ -1199,4 +1219,13 @@ mod tests {

crate::bytes::tests::check_atomic_accesses(mem, addr, bad_addr);
}

#[cfg(feature = "backend-mmap")]
#[test]
fn test_guest_memory_mmap_is_hugetlbfs() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicates the test in comment. (as pointed out before)

let addr = GuestAddress(0x1000);
let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
let r = mem.find_region(addr).unwrap();
assert_eq!(r.is_hugetlbfs(), None);
}
}
4 changes: 4 additions & 0 deletions src/mmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,10 @@ impl GuestMemoryRegion for GuestRegionMmap {
let slice = self.mapping.get_slice(offset.raw_value() as usize, count)?;
Ok(slice)
}

fn is_hugetlbfs(&self) -> Option<bool> {
self.mapping.is_hugetlbfs()
}
}

/// [`GuestMemory`](trait.GuestMemory.html) implementation that mmaps the guest's memory
Expand Down
52 changes: 52 additions & 0 deletions src/mmap_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub struct MmapRegion {
prot: i32,
flags: i32,
owned: bool,
hugetlbfs: Option<bool>,
}

// Send and Sync aren't automatically inherited for the raw address pointer.
Expand Down Expand Up @@ -172,6 +173,7 @@ impl MmapRegion {
prot,
flags,
owned: true,
hugetlbfs: None,
})
}

Expand Down Expand Up @@ -213,6 +215,7 @@ impl MmapRegion {
prot,
flags,
owned: false,
hugetlbfs: None,
})
}

Expand Down Expand Up @@ -272,6 +275,16 @@ impl MmapRegion {
}
false
}

/// Set the hugetlbfs of the region
pub fn set_hugetlbfs(&mut self, hugetlbfs: bool) {
self.hugetlbfs = Some(hugetlbfs)
}

/// Returns `true` if the region is hugetlbfs
pub fn is_hugetlbfs(&self) -> Option<bool> {
self.hugetlbfs
}
}

impl AsSlice for MmapRegion {
Expand Down Expand Up @@ -354,6 +367,45 @@ mod tests {
);
}

#[test]
fn test_mmap_region_set_hugetlbfs() {
assert!(MmapRegion::new(0).is_err());

let size = 4096;

let r = MmapRegion::new(size).unwrap();
assert_eq!(r.size(), size);
assert!(r.file_offset().is_none());
assert_eq!(r.prot(), libc::PROT_READ | libc::PROT_WRITE);
assert_eq!(
r.flags(),
libc::MAP_ANONYMOUS | libc::MAP_NORESERVE | libc::MAP_PRIVATE
);
assert_eq!(r.is_hugetlbfs(), None);

let mut r = MmapRegion::new(size).unwrap();
r.set_hugetlbfs(false);
assert_eq!(r.size(), size);
assert!(r.file_offset().is_none());
assert_eq!(r.prot(), libc::PROT_READ | libc::PROT_WRITE);
assert_eq!(
r.flags(),
libc::MAP_ANONYMOUS | libc::MAP_NORESERVE | libc::MAP_PRIVATE
);
assert_eq!(r.is_hugetlbfs(), Some(false));

let mut r = MmapRegion::new(size).unwrap();
r.set_hugetlbfs(true);
assert_eq!(r.size(), size);
assert!(r.file_offset().is_none());
assert_eq!(r.prot(), libc::PROT_READ | libc::PROT_WRITE);
assert_eq!(
r.flags(),
libc::MAP_ANONYMOUS | libc::MAP_NORESERVE | libc::MAP_PRIVATE
);
assert_eq!(r.is_hugetlbfs(), Some(true));
}

#[test]
fn test_mmap_region_from_file() {
let mut f = TempFile::new().unwrap().into_file();
Expand Down
5 changes: 5 additions & 0 deletions src/mmap_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ impl MmapRegion {
pub fn file_offset(&self) -> Option<&FileOffset> {
self.file_offset.as_ref()
}

/// This information is not available on Windows platforms.
pub fn is_hugetlbfs(&self) -> Option<bool> {
None
}
}

impl AsSlice for MmapRegion {
Expand Down