From af0efed7402862e9d2d0fe967388bd8ad04b250b Mon Sep 17 00:00:00 2001 From: Nuno Das Neves Date: Wed, 24 Apr 2024 18:51:26 +0000 Subject: [PATCH] ioctls: Rework get_dirty_log Update to use reworked MSHV_GET_GPAP_ACCESS_BITMAP. Improve the testing with 8GiB mem size, and test setting/clearing the bits. Signed-off-by: Nuno Das Neves --- mshv-ioctls/src/ioctls/vm.rs | 189 ++++++++++++++++++--------------- mshv-ioctls/src/mshv_ioctls.rs | 6 +- 2 files changed, 106 insertions(+), 89 deletions(-) diff --git a/mshv-ioctls/src/ioctls/vm.rs b/mshv-ioctls/src/ioctls/vm.rs index ff1f9fd..13b7bcd 100644 --- a/mshv-ioctls/src/ioctls/vm.rs +++ b/mshv-ioctls/src/ioctls/vm.rs @@ -630,100 +630,72 @@ impl VmFd { ) } - /// Get page access state - /// The data provides each page's access state whether it is dirty or accessed + /// Get page access states as bitmap + /// A bitmap of dirty or accessed bits for a range of guest pages /// Prerequisite: Need to enable page_acess_tracking - /// Flags: - /// bit 1: ClearAccessed - /// bit 2: SetAccessed - /// bit 3: ClearDirty - /// bit 4: SetDirty - /// Number of bits reserved: 60 - pub fn get_gpa_access_state( + /// Args: + /// base_pfn: Guest page number + /// page_count: Number of pages + /// access_type: MSHV_GPAP_ACCESS_TYPE_* + /// access_op: MSHV_GPAP_ACCESS_OP_* to optionally clear or set bits + pub fn get_gpap_access_bitmap( &self, base_pfn: u64, - nr_pfns: u32, - flags: u64, - ) -> Result { - let mut states: Vec = - vec![hv_gpa_page_access_state { as_uint8: 0 }; nr_pfns as usize]; - let mut gpa_pages_access_state: mshv_get_gpa_pages_access_state = - mshv_get_gpa_pages_access_state { - count: nr_pfns, - hv_gpa_page_number: base_pfn, - flags, - states: states.as_mut_ptr(), - }; + page_count: u32, + access_type: u8, + access_op: u8, + ) -> Result> { + let buf_sz = (page_count + 63) / 64; + let mut bitmap: Vec = vec![0u64; buf_sz as usize]; + let mut args = mshv_gpap_access_bitmap { + access_type, + access_op, + page_count, + gpap_base: base_pfn, + bitmap_ptr: bitmap.as_mut_ptr() as u64, + ..Default::default() + }; // SAFETY: IOCTL with correct types - let ret = unsafe { - ioctl_with_mut_ref( - self, - MSHV_GET_GPA_ACCESS_STATES(), - &mut gpa_pages_access_state, - ) - }; + let ret = unsafe { ioctl_with_mut_ref(self, MSHV_GET_GPAP_ACCESS_BITMAP(), &mut args) }; if ret == 0 { - Ok(gpa_pages_access_state) + Ok(bitmap) } else { Err(errno::Error::last().into()) } } /// Gets the bitmap of pages dirtied since the last call of this function - /// - /// Flags: - /// bit 1: ClearAccessed - /// bit 2: SetAccessed - /// bit 3: ClearDirty - /// bit 4: SetDirty - /// Number of bits reserved: 60 - pub fn get_dirty_log(&self, base_pfn: u64, memory_size: usize, flags: u64) -> Result> { - // Compute the length of the bitmap needed for all dirty pages in one memory slot. - // One memory page is `page_size` bytes and `KVM_GET_DIRTY_LOG` returns one dirty bit for - // each page. - // SAFETY: FFI call to libc - let page_size = match unsafe { libc::sysconf(libc::_SC_PAGESIZE) } { - -1 => return Err(errno::Error::last().into()), - ps => ps as usize, - }; - + /// Args: + /// base_pfn: Guest page number + /// memory_size: In bytes + /// access_op: MSHV_GPAP_ACCESS_OP_* to optionally clear or set bits + pub fn get_dirty_log( + &self, + base_pfn: u64, + memory_size: usize, + access_op: u8, + ) -> Result> { // For ease of access we are saving the bitmap in a u64 vector. We are using ceil to // make sure we count all dirty pages even when `memory_size` is not a multiple of // `page_size * 64`. let div_ceil = |dividend, divisor| (dividend + divisor - 1) / divisor; - let bitmap_size = div_ceil(memory_size, page_size * 64); - let mut bitmap = vec![0u64; bitmap_size]; - - let mut processed: usize = 0; - let mut mask; - let mut state: u8; - let mut current_size; - let mut remaining = (memory_size / page_size) as u32; - let mut bit_index = 0; - let mut bitmap_index = 0; - - while remaining != 0 { - current_size = cmp::min(PAGE_ACCESS_STATES_BATCH_SIZE, remaining); - let page_states = - self.get_gpa_access_state(base_pfn + processed as u64, current_size, flags)?; - // SAFETY: we're sure states and count meet the requirements for from_raw_parts - let slices: &[hv_gpa_page_access_state] = unsafe { - std::slice::from_raw_parts(page_states.states, page_states.count as usize) - }; - for item in slices.iter() { - let bits = &mut bitmap[bitmap_index]; - mask = 1 << bit_index; - // SAFETY: access union field - state = unsafe { item.__bindgen_anon_1.dirty() }; - if state == 1 { - *bits |= mask; - } - processed += 1; - bitmap_index = processed / 64; - bit_index = processed % 64; - } - remaining -= page_states.count; + let bitmap_size = div_ceil(memory_size, HV_PAGE_SIZE * 64); + let mut bitmap: Vec = Vec::with_capacity(bitmap_size); + let mut completed = 0; + let total = (memory_size / HV_PAGE_SIZE) as u32; + + while completed < total { + let remaining = total - completed; + let batch_size = cmp::min(PAGE_ACCESS_STATES_BATCH_SIZE, remaining); + let mut bitmap_part = self.get_gpap_access_bitmap( + base_pfn + completed as u64, + batch_size, + MSHV_GPAP_ACCESS_TYPE_DIRTY as u8, + access_op, + )?; + bitmap.append(&mut bitmap_part); + completed += batch_size; } Ok(bitmap) } @@ -953,12 +925,10 @@ mod tests { assert!(vm.set_msi_routing(&msi_routing).is_ok()); } - #[test] - fn test_get_gpa_access_states() { + fn _test_clear_set_get_dirty_log(mem_size: usize) { let hv = Mshv::new().unwrap(); let vm = hv.create_vm().unwrap(); // Try to allocate 32 MB memory - let mem_size = 32 * 1024 * 1024; let load_addr = unsafe { libc::mmap( std::ptr::null_mut(), @@ -976,18 +946,65 @@ mod tests { userspace_addr: load_addr as u64, ..Default::default() }; - // TODO need more real time testing: validating data, - // number of bits returned etc. vm.map_user_memory(mem_region).unwrap(); vm.enable_dirty_page_tracking().unwrap(); - let bitmaps_1: Vec = vm.get_dirty_log(0, mem_size, 0x4).unwrap(); - let bitmaps_2: Vec = vm.get_dirty_log(0, mem_size, 0x8).unwrap(); + + let bitmap_len = ((mem_size + HV_PAGE_SIZE - 1) >> HV_HYP_PAGE_SHIFT) / 64; + { + let bitmap = vm + .get_dirty_log(0, mem_size, MSHV_GPAP_ACCESS_OP_CLEAR as u8) + .unwrap(); + assert!(bitmap.len() == bitmap_len); + } + // get the clear bits and verify cleared, set the bits again + // (not all are really set; due to mmio or overlay pages gaps) + let clear_bitmap = { + let bitmap = vm + .get_dirty_log(0, mem_size, MSHV_GPAP_ACCESS_OP_SET as u8) + .unwrap(); + assert!(bitmap.len() == bitmap_len); + bitmap + }; + for x in clear_bitmap { + assert!(x == 0); + } + // get the set bits, noop + let set_bitmap_0 = { + let bitmap = vm + .get_dirty_log(0, mem_size, MSHV_GPAP_ACCESS_OP_NOOP as u8) + .unwrap(); + assert!(bitmap.len() == bitmap_len); + bitmap + }; + // get the set bits after noop + let set_bitmap_1 = { + let bitmap = vm + .get_dirty_log(0, mem_size, MSHV_GPAP_ACCESS_OP_NOOP as u8) + .unwrap(); + assert!(bitmap.len() == bitmap_len); + bitmap + }; + for i in 0..bitmap_len { + assert!(set_bitmap_0[i] == set_bitmap_1[i]); + } + vm.disable_dirty_page_tracking().unwrap(); - assert!(bitmaps_1.len() == bitmaps_2.len()); vm.unmap_user_memory(mem_region).unwrap(); unsafe { libc::munmap(load_addr as *mut c_void, mem_size) }; } + #[test] + fn test_get_dirty_log_32M() { + let mem_size = 32 * 1024 * 1024; + _test_clear_set_get_dirty_log(mem_size); + } + + #[test] + fn test_get_dirty_log_8G() { + let mem_size = 8 * 1024 * 1024 * 1024; + _test_clear_set_get_dirty_log(mem_size); + } + #[test] #[ignore] fn test_signal_event_direct() { diff --git a/mshv-ioctls/src/mshv_ioctls.rs b/mshv-ioctls/src/mshv_ioctls.rs index 183a791..6bb7755 100644 --- a/mshv-ioctls/src/mshv_ioctls.rs +++ b/mshv-ioctls/src/mshv_ioctls.rs @@ -62,10 +62,10 @@ ioctl_iow_nr!( ); ioctl_iowr_nr!(MSHV_VP_TRANSLATE_GVA, MSHV_IOCTL, 0x0E, mshv_translate_gva); ioctl_iowr_nr!( - MSHV_GET_GPA_ACCESS_STATES, + MSHV_GET_GPAP_ACCESS_BITMAP, MSHV_IOCTL, - 0x12, - mshv_get_gpa_pages_access_state + 0x07, + mshv_gpap_access_bitmap ); ioctl_iowr_nr!(MSHV_CREATE_DEVICE, MSHV_IOCTL, 0x13, mshv_create_device);