crosvm: finish up flexible array wrapper

We added the file, but forgot the flexible array. Fix this.

BUG=chromium:892806
TEST=kvm/hypervisor tests pass, atlas tryjobs pass

Change-Id: Ic52d3c9a9e67f1619151a08d3dd0e879de658066
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2347068
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Gurchetan Singh <gurchetansingh@chromium.org>

Cargo.lock

@@ -444,6 +444,7 @@ name = "kvm_sys"
 version = "0.1.0"
 dependencies = [
  "base 0.1.0",
+ "data_model 0.1.0",
  "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

data_model/src/flexible_array.rs

@@ -4,6 +4,7 @@
 
 //! A wrapper for structures that contain flexible arrays.
 
+use std::marker::PhantomData;
 use std::mem::size_of;
 
 // Returns a `Vec<T>` with a size in bytes at least as large as `size_in_bytes`.
@@ -37,3 +38,108 @@ pub fn vec_with_array_field<T: Default, F>(count: usize) -> Vec<T> {
     let vec_size_bytes = size_of::<T>() + element_space;
     vec_with_size_in_bytes(vec_size_bytes)
 }
+
+/// The following code provides generic helpers for creating and accessing flexible array structs.
+/// A complete definition of flexible array structs is found in the ISO 9899 specification
+/// (http://www.iso-9899.info/n1570.html). A flexible array struct is of the form:
+///
+/// ```
+/// #[repr(C)]
+/// struct T {
+///    some_data: u32,
+///    nent: u32
+///    entries: __IncompleteArrayField<S>,
+/// }
+/// ```
+/// where:
+///
+/// - `T` is the flexible array struct type
+/// - `S` is the flexible array type
+/// - `nent` is the flexible array length
+/// - `entries` is the flexible array member
+///
+/// These structures are used by the kernel API.
+
+/// A collection of methods that are required by the FlexibleArrayWrapper type.
+///
+/// When implemented for `T`, this trait allows the caller to set number of `S` entries and
+/// retrieve a slice of `S` entries.  Trait methods must only be called by the FlexibleArrayWrapper
+/// type.
+pub trait FlexibleArray<S> {
+    /// Implementations must set flexible array length in the flexible array struct to the value
+    /// specified by `len`. Appropriate conversions (i.e, usize to u32) are allowed so long as
+    /// they don't overflow or underflow.
+    fn set_len(&mut self, len: usize);
+    /// Implementations must return the length of the flexible array member.  Appropriate
+    /// conversions (i.e, usize to u32) are allowed so long as they don't overflow or underflow.
+    fn get_len(&self) -> usize;
+    /// Implementations must return a slice of flexible array member of length `len`.
+    fn get_slice(&self, len: usize) -> &[S];
+    /// Implementations must return a mutable slice of flexible array member of length `len`.
+    fn get_mut_slice(&mut self, len: usize) -> &mut [S];
+}
+
+pub struct FlexibleArrayWrapper<T, S> {
+    entries: Vec<T>,
+    phantom: PhantomData<S>,
+    allocated_len: usize,
+}
+
+/// Convenience wrapper for flexible array structs.
+///
+/// The FlexibleArray trait must be implemented for the flexible array struct before using this
+/// wrapper.
+impl<T, S> FlexibleArrayWrapper<T, S>
+where
+    T: FlexibleArray<S> + Default,
+{
+    /// Creates a new FlexibleArrayWrapper for the given flexible array struct type and flexible
+    /// array type. The flexible array length is set to `array_len`. vec_with_array_field is used
+    /// to make sure the resultant wrapper is appropriately sized.
+    pub fn new(array_len: usize) -> FlexibleArrayWrapper<T, S> {
+        let mut entries = vec_with_array_field::<T, S>(array_len);
+        entries[0].set_len(array_len);
+
+        FlexibleArrayWrapper {
+            entries,
+            phantom: PhantomData,
+            allocated_len: array_len,
+        }
+    }
+
+    /// 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.
+    fn get_valid_len(&self) -> usize {
+        if self.entries[0].get_len() > self.allocated_len {
+            self.allocated_len
+        } else {
+            self.entries[0].get_len()
+        }
+    }
+
+    /// Returns a slice of the flexible array member, for inspecting. To modify, use
+    /// mut_entries_slice instead.
+    pub fn entries_slice(&self) -> &[S] {
+        let valid_length = self.get_valid_len();
+        self.entries[0].get_slice(valid_length)
+    }
+
+    /// Returns a mutable slice of the flexible array member, for modifying.
+    pub fn mut_entries_slice(&mut self) -> &mut [S] {
+        let valid_length = self.get_valid_len();
+        self.entries[0].set_len(valid_length);
+        self.entries[0].get_mut_slice(valid_length)
+    }
+
+    /// Get a pointer so it can be passed to the kernel. Callers must not access the flexible
+    /// array member.  Using this pointer is unsafe.
+    pub fn as_ptr(&self) -> *const T {
+        &self.entries[0]
+    }
+
+    /// Get a mutable pointer so it can be passed to the kernel. Callers must not access the
+    /// flexible array member.  Using this pointer is unsafe.
+    pub fn as_mut_ptr(&mut self) -> *mut T {
+        &mut self.entries[0]
+    }
+}

data_model/src/lib.rs

@@ -182,6 +182,6 @@ pub mod volatile_memory;
 pub use crate::volatile_memory::*;
 
 mod flexible_array;
-pub use flexible_array::vec_with_array_field;
+pub use flexible_array::{vec_with_array_field, FlexibleArray, FlexibleArrayWrapper};
 
 mod sys;

kvm/src/lib.rs

@@ -21,6 +21,9 @@ use sync::Mutex;
 use base::{AsRawDescriptor, RawDescriptor};
 use data_model::vec_with_array_field;
 
+#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+use data_model::FlexibleArrayWrapper;
+
 use libc::sigset_t;
 use libc::{open, EBUSY, EINVAL, ENOENT, ENOSPC, EOVERFLOW, O_CLOEXEC, O_RDWR};
 
@@ -1648,56 +1651,7 @@ impl Drop for RunnableVcpu {
 /// 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 {
-    kvm_cpuid: Vec<kvm_cpuid2>,
-    allocated_len: usize, // Number of kvm_cpuid_entry2 structs at the end of kvm_cpuid2.
-}
-
-#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-impl CpuId {
-    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,
-        }
-    }
-
-    /// Get the 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) }
-    }
-
-    /// Get the entries slice, for inspecting. To modify, use mut_entries_slice instead.
-    pub fn entries_slice(&self) -> &[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.
-        let slice_size = if self.kvm_cpuid[0].nent as usize > self.allocated_len {
-            self.allocated_len
-        } else {
-            self.kvm_cpuid[0].nent as usize
-        };
-        unsafe { self.kvm_cpuid[0].entries.as_slice(slice_size) }
-    }
-
-    /// 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]
-    }
-
-    /// 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]
-    }
-}
+pub type CpuId = FlexibleArrayWrapper<kvm_cpuid2, kvm_cpuid_entry2>;
 
 // Represents a temporarily blocked signal. It will unblock the signal when dropped.
 struct BlockedSignal {

kvm_sys/Cargo.toml

@@ -5,5 +5,6 @@ authors = ["The Chromium OS Authors"]
 edition = "2018"
 
 [dependencies]
+data_model = { path = "../data_model" }
 libc = "*"
 base = { path = "../base" }

kvm_sys/src/lib.rs

@@ -7,7 +7,7 @@
 #![allow(non_snake_case)]
 
 use base::{ioctl_io_nr, ioctl_ior_nr, ioctl_iow_nr, ioctl_iowr_nr};
-
+use data_model::FlexibleArray;
 // Each of the below modules defines ioctls specific to their platform.
 
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
@@ -155,5 +155,24 @@ ioctl_io_nr!(KVM_SMI, KVMIO, 0xb7);
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
 pub use crate::x86::*;
 
+#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+impl FlexibleArray<kvm_cpuid_entry2> for kvm_cpuid2 {
+    fn set_len(&mut self, len: usize) {
+        self.nent = len as u32;
+    }
+
+    fn get_len(&self) -> usize {
+        self.nent as usize
+    }
+
+    fn get_slice(&self, len: usize) -> &[kvm_cpuid_entry2] {
+        unsafe { self.entries.as_slice(len) }
+    }
+
+    fn get_mut_slice(&mut self, len: usize) -> &mut [kvm_cpuid_entry2] {
+        unsafe { self.entries.as_mut_slice(len) }
+    }
+}
+
 #[cfg(any(target_arch = "arm", target_arch = "aarch64"))]
 pub use aarch64::*;