From a879102d827ea105b45bb84f76c93f4f43601cec Mon Sep 17 00:00:00 2001 From: Egor Lazarchuk Date: Fri, 15 Nov 2024 12:07:06 +0000 Subject: [PATCH] feat: update IovDeque to support arbitrary size and host page size Remove restriction on size and host page size. Signed-off-by: Egor Lazarchuk --- src/vmm/src/devices/virtio/iov_deque.rs | 55 ++++++++++++++++--------- src/vmm/src/devices/virtio/iovec.rs | 1 + 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/vmm/src/devices/virtio/iov_deque.rs b/src/vmm/src/devices/virtio/iov_deque.rs index 994ba5cd3204..46c08b26bf3a 100644 --- a/src/vmm/src/devices/virtio/iov_deque.rs +++ b/src/vmm/src/devices/virtio/iov_deque.rs @@ -77,13 +77,11 @@ pub enum IovDequeError { // pub iov_len: ::size_t, // } // ``` -// -// This value must be a multiple of 256 because this is the maximum number of `iovec` can fit into -// 1 memory page: 256 * sizeof(iovec) == 4096 == HOST_PAGE_SIZE. IovDeque only operates with -// `HOST_PAGE_SIZE` granularity. + #[derive(Debug)] pub struct IovDeque { pub iov: *mut libc::iovec, + pub bytes: u32, pub start: u16, pub len: u16, } @@ -92,18 +90,15 @@ pub struct IovDeque { unsafe impl Send for IovDeque {} impl IovDeque { - const BYTES: usize = L as usize * std::mem::size_of::(); - const _ASSERT: () = assert!(Self::BYTES % HOST_PAGE_SIZE == 0); - /// Create a [`memfd`] object that represents a single physical page - fn create_memfd() -> Result { + fn create_memfd(pages_bytes: usize) -> Result { // Create a sealable memfd. let opts = memfd::MemfdOptions::default().allow_sealing(true); let mfd = opts.create("iov_deque")?; // Resize to system page size. mfd.as_file() - .set_len(Self::BYTES.try_into().unwrap()) + .set_len(pages_bytes.try_into().unwrap()) .map_err(IovDequeError::MemfdResize)?; // Add seals to prevent further resizing. @@ -136,13 +131,13 @@ impl IovDeque { /// Allocate memory for our ring buffer /// - /// This will allocate 2 * `Self::BYTES` bytes of virtual memory. - fn allocate_ring_buffer_memory() -> Result<*mut c_void, IovDequeError> { + /// This will allocate 2 * `pages_bytes` bytes of virtual memory. + fn allocate_ring_buffer_memory(pages_bytes: usize) -> Result<*mut c_void, IovDequeError> { // SAFETY: We are calling the system call with valid arguments unsafe { Self::mmap( std::ptr::null_mut(), - Self::BYTES * 2, + pages_bytes * 2, libc::PROT_NONE, libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, -1, @@ -153,16 +148,23 @@ impl IovDeque { /// Create a new [`IovDeque`] that can hold memory described by a single VirtIO queue. pub fn new() -> Result { - let memfd = Self::create_memfd()?; + // # Safety: + // Host page size is always set to correct value. + let host_page_size = unsafe { HOST_PAGE_SIZE }; + let bytes = L as usize * std::mem::size_of::(); + let num_host_pages = bytes.div_ceil(host_page_size); + let pages_bytes = num_host_pages * host_page_size; + + let memfd = Self::create_memfd(pages_bytes)?; let raw_memfd = memfd.as_file().as_raw_fd(); - let buffer = Self::allocate_ring_buffer_memory()?; + let buffer = Self::allocate_ring_buffer_memory(pages_bytes)?; // Map the first page of virtual memory to the physical page described by the memfd object // SAFETY: We are calling the system call with valid arguments let _ = unsafe { Self::mmap( buffer, - Self::BYTES, + pages_bytes, libc::PROT_READ | libc::PROT_WRITE, libc::MAP_SHARED | libc::MAP_FIXED, raw_memfd, @@ -173,17 +175,17 @@ impl IovDeque { // Map the second page of virtual memory to the physical page described by the memfd object // // SAFETY: This is safe because: - // * Both `buffer` and the result of `buffer.add(Self::BYTES)` are within bounds of the + // * Both `buffer` and the result of `buffer.add(pages_bytes)` are within bounds of the // allocation we got from `Self::allocate_ring_buffer_memory`. // * The resulting pointer is the beginning of the second page of our allocation, so it // doesn't wrap around the address space. - let next_page = unsafe { buffer.add(Self::BYTES) }; + let next_page = unsafe { buffer.add(pages_bytes) }; // SAFETY: We are calling the system call with valid arguments let _ = unsafe { Self::mmap( next_page, - Self::BYTES, + pages_bytes, libc::PROT_READ | libc::PROT_WRITE, libc::MAP_SHARED | libc::MAP_FIXED, raw_memfd, @@ -193,6 +195,7 @@ impl IovDeque { Ok(Self { iov: buffer.cast(), + bytes: u32::try_from(pages_bytes).unwrap(), start: 0, len: 0, }) @@ -312,8 +315,8 @@ impl IovDeque { impl Drop for IovDeque { fn drop(&mut self) { // SAFETY: We are passing an address that we got from a previous allocation of `2 * - // Self::BYTES` bytes by calling mmap - let _ = unsafe { libc::munmap(self.iov.cast(), Self::BYTES * 2) }; + // self.bytes` by calling mmap + let _ = unsafe { libc::munmap(self.iov.cast(), usize::try_from(self.bytes).unwrap() * 2) }; } } @@ -331,6 +334,18 @@ mod tests { assert_eq!(deque.len(), 0); } + #[test] + fn test_new_less_than_page() { + let deque = super::IovDeque::<128>::new().unwrap(); + assert_eq!(deque.len(), 0); + } + + #[test] + fn test_new_more_than_page() { + let deque = super::IovDeque::<512>::new().unwrap(); + assert_eq!(deque.len(), 0); + } + fn make_iovec(id: u16, len: u16) -> iovec { iovec { iov_base: id as *mut libc::c_void, diff --git a/src/vmm/src/devices/virtio/iovec.rs b/src/vmm/src/devices/virtio/iovec.rs index e8451827bb9d..f6460088e891 100644 --- a/src/vmm/src/devices/virtio/iovec.rs +++ b/src/vmm/src/devices/virtio/iovec.rs @@ -918,6 +918,7 @@ mod verification { }; IovDequeDefault { iov: mem.cast(), + bytes: 2 * u32::try_from(GUEST_PAGE_SIZE).unwrap(), start: kani::any_where(|&start| start < FIRECRACKER_MAX_QUEUE_SIZE), len: 0, }