implement CpuId and MsrList using KvmVec

Signed-off-by: Serban Iorga <[email protected]>

src/ioctls/mod.rs

@@ -10,10 +10,12 @@ use std::mem::size_of;
 use std::os::unix::io::AsRawFd;
 use std::ptr::null_mut;
 use std::result;
+use MAX_KVM_CPUID_ENTRIES;
+use MAX_KVM_MSR_ENTRIES;
 
 use kvm_bindings::kvm_run;
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-use kvm_bindings::{kvm_cpuid2, kvm_cpuid_entry2};
+use kvm_bindings::{__IncompleteArrayField, kvm_cpuid2, kvm_cpuid_entry2, kvm_msr_list};
 
 /// Helper for dealing with KVM api structures
 mod common;
@@ -34,134 +36,66 @@ use self::common::kvm_vec::*;
 /// is otherwise a direct mapping to Result.
 pub type Result<T> = result::Result<T, io::Error>;
 
-// Returns a `Vec<T>` with a size in bytes at least as large as `size_in_bytes`.
-fn vec_with_size_in_bytes<T: Default>(size_in_bytes: usize) -> Vec<T> {
-    let rounded_size = (size_in_bytes + size_of::<T>() - 1) / size_of::<T>();
-    let mut v = Vec::with_capacity(rounded_size);
-    for _ in 0..rounded_size {
-        v.push(T::default())
-    }
-    v
-}
-
-// The kvm API has many structs that resemble the following `Foo` structure:
-//
-// ```
-// #[repr(C)]
-// struct Foo {
-//    some_data: u32
-//    entries: __IncompleteArrayField<__u32>,
-// }
-// ```
-//
-// In order to allocate such a structure, `size_of::<Foo>()` would be too small because it would not
-// include any space for `entries`. To make the allocation large enough while still being aligned
-// for `Foo`, a `Vec<Foo>` is created. Only the first element of `Vec<Foo>` would actually be used
-// as a `Foo`. The remaining memory in the `Vec<Foo>` is for `entries`, which must be contiguous
-// with `Foo`. This function is used to make the `Vec<Foo>` with enough space for `count` entries.
-fn vec_with_array_field<T: Default, F>(count: usize) -> Vec<T> {
-    let element_space = count * size_of::<F>();
-    let vec_size_bytes = size_of::<T>() + element_space;
-    vec_with_size_in_bytes(vec_size_bytes)
-}
-
-/// Wrapper for `kvm_cpuid2` which has a zero length array at the end.
-/// Hides the zero length array behind a bounds check.
-#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-pub struct CpuId {
-    /// Wrapper over `kvm_cpuid2` from which we only use the first element.
-    kvm_cpuid: Vec<kvm_cpuid2>,
-    // Number of `kvm_cpuid_entry2` structs at the end of kvm_cpuid2.
-    allocated_len: usize,
-}
-
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-impl Clone for CpuId {
-    fn clone(&self) -> Self {
-        let mut kvm_cpuid = Vec::with_capacity(self.kvm_cpuid.len());
-        for _ in 0..self.kvm_cpuid.len() {
-            kvm_cpuid.push(kvm_cpuid2::default());
-        }
+impl KvmArray for kvm_cpuid2 {
+    type Entry = kvm_cpuid_entry2;
 
-        let num_bytes = self.kvm_cpuid.len() * size_of::<kvm_cpuid2>();
+    fn len(&self) -> usize {
+        self.nent as usize
+    }
 
-        let src_byte_slice =
-            unsafe { std::slice::from_raw_parts(self.kvm_cpuid.as_ptr() as *const u8, num_bytes) };
+    fn set_len(&mut self, len: usize) {
+        self.nent = len as u32;
+    }
 
-        let dst_byte_slice =
-            unsafe { std::slice::from_raw_parts_mut(kvm_cpuid.as_mut_ptr() as *mut u8, num_bytes) };
+    fn max_len() -> usize {
+        MAX_KVM_CPUID_ENTRIES
+    }
 
-        dst_byte_slice.copy_from_slice(src_byte_slice);
+    fn entries(&self) -> &__IncompleteArrayField<kvm_cpuid_entry2> {
+        &self.entries
+    }
 
-        CpuId {
-            kvm_cpuid,
-            allocated_len: self.allocated_len,
-        }
+    fn entries_mut(&mut self) -> &mut __IncompleteArrayField<kvm_cpuid_entry2> {
+        &mut self.entries
     }
 }
 
-#[cfg(test)]
+/// Wrapper for `kvm_cpuid2`.
+///
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-impl PartialEq for CpuId {
-    fn eq(&self, other: &CpuId) -> bool {
-        let entries: &[kvm_cpuid_entry2] =
-            unsafe { self.kvm_cpuid[0].entries.as_slice(self.allocated_len) };
-        let other_entries: &[kvm_cpuid_entry2] =
-            unsafe { self.kvm_cpuid[0].entries.as_slice(other.allocated_len) };
-        self.allocated_len == other.allocated_len && entries == other_entries
-    }
-}
+pub type CpuId = KvmVec<kvm_cpuid2>;
 
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-impl CpuId {
-    /// Creates a new `CpuId` structure that can contain at most `array_len` KVM CPUID entries.
-    ///
-    /// # Arguments
-    ///
-    /// * `array_len` - Maximum number of CPUID entries.
-    ///
-    /// # Example
-    ///
-    /// ```
-    /// #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-    /// use kvm_ioctls::CpuId;
-    /// let cpu_id = CpuId::new(32);
-    /// ```
-    pub fn new(array_len: usize) -> CpuId {
-        let mut kvm_cpuid = vec_with_array_field::<kvm_cpuid2, kvm_cpuid_entry2>(array_len);
-        kvm_cpuid[0].nent = array_len as u32;
-
-        CpuId {
-            kvm_cpuid,
-            allocated_len: array_len,
-        }
+impl KvmArray for kvm_msr_list {
+    type Entry = u32;
+
+    fn len(&self) -> usize {
+        self.nmsrs as usize
     }
 
-    /// Get the mutable entries slice so they can be modified before passing to the VCPU.
-    ///
-    pub fn mut_entries_slice(&mut self) -> &mut [kvm_cpuid_entry2] {
-        // Mapping the unsized array to a slice is unsafe because the length isn't known.  Using
-        // the length we originally allocated with eliminates the possibility of overflow.
-        if self.kvm_cpuid[0].nent as usize > self.allocated_len {
-            self.kvm_cpuid[0].nent = self.allocated_len as u32;
-        }
-        let nent = self.kvm_cpuid[0].nent as usize;
-        unsafe { self.kvm_cpuid[0].entries.as_mut_slice(nent) }
+    fn set_len(&mut self, len: usize) {
+        self.nmsrs = len as u32;
     }
 
-    /// Get a  pointer so it can be passed to the kernel. Using this pointer is unsafe.
-    ///
-    pub fn as_ptr(&self) -> *const kvm_cpuid2 {
-        &self.kvm_cpuid[0]
+    fn max_len() -> usize {
+        MAX_KVM_MSR_ENTRIES as usize
     }
 
-    /// Get a mutable pointer so it can be passed to the kernel. Using this pointer is unsafe.
-    ///
-    pub fn as_mut_ptr(&mut self) -> *mut kvm_cpuid2 {
-        &mut self.kvm_cpuid[0]
+    fn entries(&self) -> &__IncompleteArrayField<u32> {
+        &self.indices
+    }
+
+    fn entries_mut(&mut self) -> &mut __IncompleteArrayField<u32> {
+        &mut self.indices
     }
 }
 
+/// Wrapper for `kvm_msr_list`.
+///
+#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+pub type MsrList = KvmVec<kvm_msr_list>;
+
 /// A safe wrapper over the `kvm_run` struct.
 ///
 /// The wrapper is needed for sending the pointer to `kvm_run` between

src/ioctls/system.rs

@@ -14,13 +14,13 @@ use std::os::raw::{c_char, c_ulong};
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
 use cap::Cap;
-use ioctls::vec_with_array_field;
 use ioctls::vm::{new_vmfd, VmFd};
-#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-use ioctls::CpuId;
 use ioctls::Result;
+#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+use ioctls::{CpuId, MsrList};
 use kvm_ioctls::*;
 use sys_ioctl::*;
+use MAX_KVM_MSR_ENTRIES;
 
 /// A wrapper over the `/dev/kvm` file.
 ///
@@ -207,33 +207,19 @@ impl Kvm {
     ///
     #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
     pub fn get_msr_index_list(&self) -> Result<Vec<u32>> {
-        const MAX_KVM_MSR_ENTRIES: usize = 256;
-
-        let mut msr_list = vec_with_array_field::<kvm_msr_list, u32>(MAX_KVM_MSR_ENTRIES);
-        msr_list[0].nmsrs = MAX_KVM_MSR_ENTRIES as u32;
+        let mut msr_list = MsrList::new(MAX_KVM_MSR_ENTRIES);
 
         let ret = unsafe {
             // ioctl is unsafe. The kernel is trusted not to write beyond the bounds of the memory
             // allocated for the struct. The limit is read from nmsrs, which is set to the allocated
             // size (MAX_KVM_MSR_ENTRIES) above.
-            ioctl_with_mut_ref(self, KVM_GET_MSR_INDEX_LIST(), &mut msr_list[0])
+            ioctl_with_mut_ptr(self, KVM_GET_MSR_INDEX_LIST(), msr_list.as_mut_ptr())
         };
         if ret < 0 {
             return Err(io::Error::last_os_error());
         }
 
-        let mut nmsrs = msr_list[0].nmsrs;
-
-        // Mapping the unsized array to a slice is unsafe because the length isn't known.  Using
-        // the length we originally allocated with eliminates the possibility of overflow.
-        let indices: &[u32] = unsafe {
-            if nmsrs > MAX_KVM_MSR_ENTRIES as u32 {
-                nmsrs = MAX_KVM_MSR_ENTRIES as u32;
-            }
-            msr_list[0].indices.as_slice(nmsrs as usize)
-        };
-
-        Ok(indices.to_vec())
+        Ok(msr_list.as_entries_slice().to_vec())
     }
 
     /// Creates a VM fd using the KVM fd.
@@ -308,7 +294,7 @@ mod tests {
     fn test_get_supported_cpuid() {
         let kvm = Kvm::new().unwrap();
         let mut cpuid = kvm.get_supported_cpuid(MAX_KVM_CPUID_ENTRIES).unwrap();
-        let cpuid_entries = cpuid.mut_entries_slice();
+        let cpuid_entries = cpuid.as_mut_entries_slice();
         assert!(cpuid_entries.len() > 0);
         assert!(cpuid_entries.len() <= MAX_KVM_CPUID_ENTRIES);
     }
@@ -318,7 +304,7 @@ mod tests {
     fn test_get_emulated_cpuid() {
         let kvm = Kvm::new().unwrap();
         let mut cpuid = kvm.get_emulated_cpuid(MAX_KVM_CPUID_ENTRIES).unwrap();
-        let cpuid_entries = cpuid.mut_entries_slice();
+        let cpuid_entries = cpuid.as_mut_entries_slice();
         assert!(cpuid_entries.len() > 0);
         assert!(cpuid_entries.len() <= MAX_KVM_CPUID_ENTRIES);
     }

src/ioctls/vcpu.rs

@@ -486,7 +486,7 @@ mod tests {
         if kvm.check_extension(Cap::ExtCpuid) {
             let vm = kvm.create_vm().unwrap();
             let mut cpuid = kvm.get_supported_cpuid(MAX_KVM_CPUID_ENTRIES).unwrap();
-            assert!(cpuid.mut_entries_slice().len() <= MAX_KVM_CPUID_ENTRIES);
+            assert!(cpuid.as_mut_entries_slice().len() <= MAX_KVM_CPUID_ENTRIES);
             let nr_vcpus = kvm.get_nr_vcpus();
             for cpu_id in 0..nr_vcpus {
                 let vcpu = vm.create_vcpu(cpu_id as u8).unwrap();

src/lib.rs

@@ -25,7 +25,7 @@ pub use ioctls::system::Kvm;
 pub use ioctls::vcpu::{VcpuExit, VcpuFd};
 pub use ioctls::vm::VmFd;
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-pub use ioctls::CpuId;
+pub use ioctls::{CpuId, MsrList};
 // The following example is used to verify that our public
 // structures are exported properly.
 /// # Example
@@ -42,3 +42,6 @@ pub use ioctls::{KvmRunWrapper, Result};
 /// It can be used for calls to [get_supported_cpuid](struct.Kvm.html#method.get_supported_cpuid) and
 /// [get_emulated_cpuid](struct.Kvm.html#method.get_emulated_cpuid).
 pub const MAX_KVM_CPUID_ENTRIES: usize = 80;
+
+/// Maximum number of MSR entries that can be returned by a call to KVM ioctls.
+pub const MAX_KVM_MSR_ENTRIES: usize = 256;

tests/coverage

@@ -1 +1 @@
-91.3
+91.4