configurator: BootParams struct

Added a new struct that encapsulates boot parameters expressed as bytes.
* header: section of the parameters that is necessary regardless
  of boot protocol. For Linux, it's the boot_params struct; for
  PVH, it's the start_info; for ARM+FDT, it's the device tree
  blob.
* sections: vector of additional boot parameters written at a
  different address than the header. Unused for Linux & FDT. For
  PVH, it's the memory map table.
* modules: vector of module boot configurations, written at a
  different address than the header and sections. Unused for
  Linux & FDT. For PVH, it's (optionally) a vector of 1
  modlist_entry containing the initrd configuration, if any.

Signed-off-by: Alexandra Iordache <aghecen@amazon.com>

Author: Alexandra Iordache

coverage_config_aarch64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 75.7,
+  "coverage_score": 77.2,
   "exclude_path": "",
   "crate_features": ""
 }

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 74.8,
+  "coverage_score": 77.8,
   "exclude_path": "",
   "crate_features": ""
 }

src/configurator/aarch64/fdt.rs

@@ -4,12 +4,12 @@
 
 //! Traits and structs for loading the device tree.
 
-use vm_memory::{ByteValued, Bytes, GuestAddress, GuestMemory};
+use vm_memory::{Bytes, GuestMemory};
 
 use std::error::Error as StdError;
 use std::fmt;
 
-use super::super::{BootConfigurator, Error as BootConfiguratorError, Result};
+use crate::configurator::{BootConfigurator, BootParams, Error as BootConfiguratorError, Result};
 
 /// Errors specific to the device tree boot protocol configuration.
 #[derive(Debug, PartialEq)]
@@ -51,22 +51,70 @@ impl BootConfigurator for FdtBootConfigurator {
     ///
     /// # Arguments
     ///
-    /// * `fdt` - flattened device tree.
-    /// * `sections` - unused.
+    /// * `params` - boot parameters containing the FDT.
     /// * `guest_memory` - guest's physical memory.
-    fn write_bootparams<T, S, M>(
-        fdt: (T, GuestAddress),
-        _sections: Option<(Vec<S>, GuestAddress)>,
-        guest_memory: &M,
-    ) -> Result<()>
+    fn write_bootparams<M>(params: BootParams, guest_memory: &M) -> Result<()>
     where
-        T: ByteValued,
-        S: ByteValued,
         M: GuestMemory,
     {
         // The VMM has filled an FDT and passed it as a `ByteValued` object.
         guest_memory
-            .write_obj(fdt.0, fdt.1)
+            .write_slice(params.header.0.as_slice(), params.header.1)
             .map_err(|_| Error::WriteFDTToMemory.into())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use vm_memory::{Address, ByteValued, GuestAddress, GuestMemoryMmap};
+
+    const FDT_MAX_SIZE: usize = 0x20;
+    const MEM_SIZE: u64 = 0x100_0000;
+
+    fn create_guest_mem() -> GuestMemoryMmap {
+        GuestMemoryMmap::from_ranges(&[(GuestAddress(0x0), (MEM_SIZE as usize))]).unwrap()
+    }
+
+    #[derive(Clone, Copy, Default)]
+    struct FdtPlaceholder([u8; FDT_MAX_SIZE]);
+    unsafe impl ByteValued for FdtPlaceholder {}
+
+    #[test]
+    fn test_configure_fdt_boot() {
+        let fdt = FdtPlaceholder([0u8; FDT_MAX_SIZE]);
+        let guest_memory = create_guest_mem();
+
+        // Error case: FDT doesn't fit in guest memory.
+        let fdt_addr = guest_memory
+            .last_addr()
+            .checked_sub(FDT_MAX_SIZE as u64 - 2)
+            .unwrap();
+        assert_eq!(
+            FdtBootConfigurator::write_bootparams::<GuestMemoryMmap>(
+                BootParams::new::<FdtPlaceholder>(&fdt, fdt_addr),
+                &guest_memory,
+            )
+            .err(),
+            Some(Error::WriteFDTToMemory.into())
+        );
+
+        let fdt_addr = guest_memory
+            .last_addr()
+            .checked_sub(FDT_MAX_SIZE as u64 - 1)
+            .unwrap();
+        assert!(FdtBootConfigurator::write_bootparams::<GuestMemoryMmap>(
+            BootParams::new::<FdtPlaceholder>(&fdt, fdt_addr),
+            &guest_memory,
+        )
+        .is_ok());
+    }
+
+    #[test]
+    fn test_error_messages() {
+        assert_eq!(
+            format!("{}", Error::WriteFDTToMemory),
+            "Device Tree Boot Configurator Error: Error writing FDT in guest memory."
+        )
+    }
+}

src/configurator/mod.rs

@@ -90,27 +90,137 @@ pub trait BootConfigurator {
     ///
     /// # Arguments
     ///
-    /// * `header` - header section of the boot parameters and address where to write it in guest
-    ///              memory. The first element must be a POD struct that implements [`ByteValued`].
-    ///              For the Linux protocol it's the [`boot_params`] struct, and for PVH the
-    ///             [`hvm_start_info`] struct.
-    /// * `sections` - vector of sections that compose the boot parameters and address where to
-    ///                write them in guest memory. Unused for the Linux protocol. For PVH, it's the
-    ///                memory map table represented as a vector of [`hvm_memmap_table_entry`]. Must
-    ///                be a `Vec` of POD data structs that implement [`ByteValued`].
+    /// * `params` - struct containing the header section of the boot parameters, additional
+    ///              sections and modules, and their associated addresses in guest memory. These
+    ///              vary with the boot protocol used.
     /// * `guest_memory` - guest's physical memory.
+    fn write_bootparams<M>(params: BootParams, guest_memory: &M) -> Result<()>
+    where
+        M: GuestMemory;
+}
+
+/// Boot parameters to be written in guest memory.
+#[derive(Clone)]
+pub struct BootParams {
+    /// "Header section", always written in guest memory irrespective of boot protocol.
+    pub header: (Vec<u8>, GuestAddress),
+    /// Optional sections containing boot configurations (e.g. E820 map).
+    pub sections: Option<(Vec<u8>, GuestAddress)>,
+    /// Optional modules specified at boot configuration time.
+    pub modules: Option<(Vec<u8>, GuestAddress)>,
+}
+
+impl BootParams {
+    /// Creates a new [`BootParams`](struct.BootParams.html) struct with the specified header.
+    ///
+    /// # Arguments
+    ///
+    /// * `header` - [`ByteValued`] representation of mandatory boot parameters.
+    /// * `header_addr` - address in guest memory where `header` will be written.
+    ///
+    /// [`ByteValued`]: https://docs.rs/vm-memory/latest/vm_memory/bytes/trait.ByteValued.html
+    pub fn new<T: ByteValued>(header: &T, header_addr: GuestAddress) -> Self {
+        BootParams {
+            header: (header.as_slice().to_vec(), header_addr),
+            sections: None,
+            modules: None,
+        }
+    }
+
+    /// Adds or overwrites the boot sections and associated memory address.
+    ///
+    /// Unused on `aarch64` and for the Linux boot protocol.
+    /// For the PVH boot protocol, the sections specify the memory map table in
+    /// [`hvm_memmap_table_entry`] structs.
+    ///
+    /// # Arguments
+    ///
+    /// * `sections` - vector of [`ByteValued`] boot configurations.
+    /// * `sections_addr` - address where the sections will be written in guest memory.
     ///
-    /// [`boot_params`]: ../loader/bootparam/struct.boot_e820_entry.html
+    /// [`ByteValued`]: https://docs.rs/vm-memory/latest/vm_memory/bytes/trait.ByteValued.html
     /// [`hvm_memmap_table_entry`]: ../loader/elf/start_info/struct.hvm_memmap_table_entry.html
-    /// [`hvm_start_info`]: ../loader/elf/start_info/struct.hvm_start_info.html
+    pub fn add_sections<T: ByteValued>(&mut self, sections: &[T], sections_addr: GuestAddress) {
+        self.sections = Some((
+            sections
+                .iter()
+                .flat_map(|section| section.as_slice().to_vec())
+                .collect(),
+            sections_addr,
+        ));
+    }
+
+    /// Adds or overwrites the boot modules and associated memory address.
+    ///
+    /// Unused on `aarch64` and for the Linux boot protocol.
+    /// For the PVH boot protocol, the modules are specified in [`hvm_modlist_entry`] structs.
+    ///
+    /// # Arguments
+    ///
+    /// * `modules` - vector of [`ByteValued`] boot configurations.
+    /// * `modules_addr` - address where the modules will be written in guest memory.
+    ///
     /// [`ByteValued`]: https://docs.rs/vm-memory/latest/vm_memory/bytes/trait.ByteValued.html
-    fn write_bootparams<T, S, M>(
-        header: (T, GuestAddress),
-        sections: Option<(Vec<S>, GuestAddress)>,
-        guest_memory: &M,
-    ) -> Result<()>
-    where
-        T: ByteValued,
-        S: ByteValued,
-        M: GuestMemory;
+    /// [`hvm_modlist_entry`]: ../loader/elf/start_info/struct.hvm_modlist_entry.html
+    pub fn add_modules<T: ByteValued>(&mut self, modules: &[T], modules_addr: GuestAddress) {
+        self.modules = Some((
+            modules
+                .iter()
+                .flat_map(|module| module.as_slice().to_vec())
+                .collect(),
+            modules_addr,
+        ));
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_error_messages() {
+        #[cfg(target_arch = "x86_64")]
+        {
+            // Linux
+            assert_eq!(
+                format!("{}", Error::Linux(linux::Error::ZeroPagePastRamEnd)),
+                "Boot Configurator Error: The zero page extends past the end of guest memory."
+            );
+            assert_eq!(
+                format!("{}", Error::Linux(linux::Error::ZeroPageSetup)),
+                "Boot Configurator Error: Error writing to the zero page of guest memory."
+            );
+
+            // PVH
+            assert_eq!(
+                format!("{}", Error::Pvh(pvh::Error::MemmapTableMissing)),
+                "Boot Configurator Error: No memory map was passed to the boot configurator."
+            );
+            assert_eq!(
+                format!("{}", Error::Pvh(pvh::Error::MemmapTablePastRamEnd)),
+                "Boot Configurator Error: \
+                 The memory map table extends past the end of guest memory."
+            );
+            assert_eq!(
+                format!("{}", Error::Pvh(pvh::Error::MemmapTableSetup)),
+                "Boot Configurator Error: Error writing memory map table to guest memory."
+            );
+            assert_eq!(
+                format!("{}", Error::Pvh(pvh::Error::StartInfoPastRamEnd)),
+                "Boot Configurator Error: \
+                 The hvm_start_info structure extends past the end of guest memory."
+            );
+            assert_eq!(
+                format!("{}", Error::Pvh(pvh::Error::StartInfoSetup)),
+                "Boot Configurator Error: Error writing hvm_start_info to guest memory."
+            );
+        }
+
+        #[cfg(target_arch = "aarch64")]
+        // FDT
+        assert_eq!(
+            format!("{}", Error::Fdt(fdt::Error::WriteFDTToMemory)),
+            "Boot Configurator Error: Error writing FDT in guest memory."
+        );
+    }
 }

src/configurator/x86_64/linux.rs

@@ -12,14 +12,12 @@
 //! Traits and structs for configuring and loading boot parameters on `x86_64` using the Linux
 //! boot protocol.
 
-use vm_memory::{ByteValued, Bytes, GuestAddress, GuestMemory};
+use vm_memory::{Bytes, GuestMemory};
 
-use super::super::{BootConfigurator, Error as BootConfiguratorError, Result};
-use crate::loader::bootparam::boot_params;
+use crate::configurator::{BootConfigurator, BootParams, Error as BootConfiguratorError, Result};
 
 use std::error::Error as StdError;
 use std::fmt;
-use std::mem;
 
 /// Boot configurator for the Linux boot protocol.
 pub struct LinuxBootConfigurator {}
@@ -64,28 +62,22 @@ impl BootConfigurator for LinuxBootConfigurator {
     ///
     /// # Arguments
     ///
-    /// * `header` - boot parameters encapsulated in a [`boot_params`] struct.
-    /// * `sections` - unused.
+    /// * `params` - boot parameters. The header contains a [`boot_params`] struct. The `sections`
+    ///              and `modules` are unused.
     /// * `guest_memory` - guest's physical memory.
     ///
-    /// [`boot_params`]: ../loader/bootparam/struct.boot_e820_entry.html
-    fn write_bootparams<T, S, M>(
-        header: (T, GuestAddress),
-        _sections: Option<(Vec<S>, GuestAddress)>,
-        guest_memory: &M,
-    ) -> Result<()>
+    /// [`boot_params`]: ../loader/bootparam/struct.boot_params.html
+    fn write_bootparams<M>(params: BootParams, guest_memory: &M) -> Result<()>
     where
-        T: ByteValued,
-        S: ByteValued,
         M: GuestMemory,
     {
         // The VMM has filled a `boot_params` struct and its e820 map.
         // This will be written in guest memory at the zero page.
         guest_memory
-            .checked_offset(header.1, mem::size_of::<boot_params>())
+            .checked_offset(params.header.1, params.header.0.len())
             .ok_or(Error::ZeroPagePastRamEnd)?;
         guest_memory
-            .write_obj(header.0, header.1)
+            .write_slice(params.header.0.as_slice(), params.header.1)
             .map_err(|_| Error::ZeroPageSetup)?;
 
         Ok(())
@@ -95,14 +87,15 @@ impl BootConfigurator for LinuxBootConfigurator {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::loader::bootparam::boot_params;
     use std::mem;
     use vm_memory::{Address, GuestAddress, GuestMemoryMmap};
 
     const KERNEL_BOOT_FLAG_MAGIC: u16 = 0xaa55;
     const KERNEL_HDR_MAGIC: u32 = 0x53726448;
     const KERNEL_LOADER_OTHER: u8 = 0xff;
     const KERNEL_MIN_ALIGNMENT_BYTES: u32 = 0x1000000;
-    const MEM_SIZE: u64 = 0x1000000;
+    const MEM_SIZE: u64 = 0x100_0000;
 
     fn create_guest_mem() -> GuestMemoryMmap {
         GuestMemoryMmap::from_ranges(&[(GuestAddress(0x0), (MEM_SIZE as usize))]).unwrap()
@@ -130,24 +123,34 @@ mod tests {
         let bad_zeropg_addr = GuestAddress(
             guest_memory.last_addr().raw_value() - mem::size_of::<boot_params>() as u64 + 1,
         );
+        let mut bootparams = BootParams::new::<boot_params>(&params, bad_zeropg_addr);
         assert_eq!(
-            LinuxBootConfigurator::write_bootparams::<boot_params, boot_params, GuestMemoryMmap>(
-                (params, bad_zeropg_addr),
-                None,
+            LinuxBootConfigurator::write_bootparams::<GuestMemoryMmap>(
+                bootparams.clone(),
                 &guest_memory,
             )
             .err(),
             Some(Error::ZeroPagePastRamEnd.into()),
         );
 
         // Success case.
-        assert!(
-            LinuxBootConfigurator::write_bootparams::<boot_params, boot_params, GuestMemoryMmap>(
-                (params, zero_page_addr),
-                None,
-                &guest_memory
-            )
-            .is_ok()
+        bootparams.header.1 = zero_page_addr;
+        assert!(LinuxBootConfigurator::write_bootparams::<GuestMemoryMmap>(
+            bootparams,
+            &guest_memory,
+        )
+        .is_ok());
+    }
+
+    #[test]
+    fn test_error_messages() {
+        assert_eq!(
+            format!("{}", Error::ZeroPagePastRamEnd),
+            "Linux Boot Configurator Error: The zero page extends past the end of guest memory."
+        );
+        assert_eq!(
+            format!("{}", Error::ZeroPageSetup),
+            "Linux Boot Configurator Error: Error writing to the zero page of guest memory."
         );
     }
 }