refactor: move initrd code into separate module

Initrd code was spread across multiple modules, move common part of it
into one and make freestanding `load_initrd_*` functions into methods
for the `InitrdConfig` struct.

Also change `initrd_load_addr` method to return an option instead of an
error to remove extra error nesting.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/arch/aarch64/fdt.rs

@@ -12,11 +12,12 @@ use std::fmt::Debug;
 use vm_fdt::{Error as VmFdtError, FdtWriter, FdtWriterNode};
 use vm_memory::GuestMemoryError;
 
-use super::super::{DeviceType, InitrdConfig};
+use super::super::DeviceType;
 use super::cache_info::{CacheEntry, read_cache_config};
 use super::gic::GICDevice;
 use crate::device_manager::mmio::MMIODeviceInfo;
 use crate::devices::acpi::vmgenid::{VMGENID_MEM_SIZE, VmGenId};
+use crate::initrd::InitrdConfig;
 use crate::vstate::memory::{Address, GuestMemory, GuestMemoryMmap};
 
 // This is a value for uniquely identifying the FDT node declaring the interrupt controller.

src/vmm/src/arch/aarch64/mod.rs

@@ -27,15 +27,14 @@ use self::gic::GICDevice;
 use crate::arch::DeviceType;
 use crate::device_manager::mmio::MMIODeviceInfo;
 use crate::devices::acpi::vmgenid::VmGenId;
+use crate::initrd::InitrdConfig;
 use crate::vstate::memory::{Address, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap};
 
 /// Errors thrown while configuring aarch64 system.
 #[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum ConfigurationError {
     /// Failed to create a Flattened Device Tree for this aarch64 microVM: {0}
     SetupFDT(#[from] fdt::FdtError),
-    /// Failed to compute the initrd address.
-    InitrdAddress,
     /// Failed to write to guest memory.
     MemoryError(GuestMemoryError),
 }
@@ -70,7 +69,7 @@ pub fn configure_system(
     device_info: &HashMap<(DeviceType, String), MMIODeviceInfo>,
     gic_device: &GICDevice,
     vmgenid: &Option<VmGenId>,
-    initrd: &Option<super::InitrdConfig>,
+    initrd: &Option<InitrdConfig>,
 ) -> Result<(), ConfigurationError> {
     let fdt = fdt::create_fdt(
         guest_mem,
@@ -94,21 +93,18 @@ pub fn get_kernel_start() -> u64 {
 }
 
 /// Returns the memory address where the initrd could be loaded.
-pub fn initrd_load_addr(
-    guest_mem: &GuestMemoryMmap,
-    initrd_size: usize,
-) -> Result<u64, ConfigurationError> {
+pub fn initrd_load_addr(guest_mem: &GuestMemoryMmap, initrd_size: usize) -> Option<u64> {
     let round_to_pagesize =
         |size| (size + (super::GUEST_PAGE_SIZE - 1)) & !(super::GUEST_PAGE_SIZE - 1);
     match GuestAddress(get_fdt_addr(guest_mem)).checked_sub(round_to_pagesize(initrd_size) as u64) {
         Some(offset) => {
             if guest_mem.address_in_range(offset) {
-                Ok(offset.raw_value())
+                Some(offset.raw_value())
             } else {
-                Err(ConfigurationError::InitrdAddress)
+                None
             }
         }
-        None => Err(ConfigurationError::InitrdAddress),
+        None => None,
     }
 }
 

src/vmm/src/arch/mod.rs

@@ -58,15 +58,6 @@ pub enum DeviceType {
     BootTimer,
 }
 
-/// Type for passing information about the initrd in the guest memory.
-#[derive(Debug)]
-pub struct InitrdConfig {
-    /// Load address of initrd in guest memory
-    pub address: crate::vstate::memory::GuestAddress,
-    /// Size of initrd in guest memory
-    pub size: usize,
-}
-
 /// Default page size for the guest OS.
 pub const GUEST_PAGE_SIZE: usize = 4096;
 

src/vmm/src/arch/x86_64/mod.rs

@@ -39,8 +39,9 @@ use linux_loader::loader::elf::start_info::{
     hvm_memmap_table_entry, hvm_modlist_entry, hvm_start_info,
 };
 
-use crate::arch::{BootProtocol, InitrdConfig, SYSTEM_MEM_SIZE, SYSTEM_MEM_START};
+use crate::arch::{BootProtocol, SYSTEM_MEM_SIZE, SYSTEM_MEM_START};
 use crate::device_manager::resources::ResourceAllocator;
+use crate::initrd::InitrdConfig;
 use crate::utils::{mib_to_bytes, u64_to_usize};
 use crate::vstate::memory::{
     Address, GuestAddress, GuestMemory, GuestMemoryMmap, GuestMemoryRegion,
@@ -63,8 +64,6 @@ pub enum ConfigurationError {
     MpTableSetup(#[from] mptable::MptableError),
     /// Error writing the zero page of guest memory.
     ZeroPageSetup,
-    /// Failed to compute initrd address.
-    InitrdAddress,
     /// Error writing module entry to guest memory.
     ModlistSetup,
     /// Error writing memory map table to guest memory.
@@ -107,21 +106,16 @@ pub fn get_kernel_start() -> u64 {
 }
 
 /// Returns the memory address where the initrd could be loaded.
-pub fn initrd_load_addr(
-    guest_mem: &GuestMemoryMmap,
-    initrd_size: usize,
-) -> Result<u64, ConfigurationError> {
-    let first_region = guest_mem
-        .find_region(GuestAddress::new(0))
-        .ok_or(ConfigurationError::InitrdAddress)?;
+pub fn initrd_load_addr(guest_mem: &GuestMemoryMmap, initrd_size: usize) -> Option<u64> {
+    let first_region = guest_mem.find_region(GuestAddress::new(0))?;
     let lowmem_size = u64_to_usize(first_region.len());
 
     if lowmem_size < initrd_size {
-        return Err(ConfigurationError::InitrdAddress);
+        return None;
     }
 
     let align_to_pagesize = |address| address & !(super::GUEST_PAGE_SIZE - 1);
-    Ok(align_to_pagesize(lowmem_size - initrd_size) as u64)
+    Some(align_to_pagesize(lowmem_size - initrd_size) as u64)
 }
 
 /// Configures the system and should be called once per vm before starting vcpu threads.

src/vmm/src/builder.rs

@@ -6,7 +6,7 @@
 #[cfg(target_arch = "x86_64")]
 use std::convert::TryFrom;
 use std::fmt::Debug;
-use std::io::{self, Seek, SeekFrom};
+use std::io;
 #[cfg(feature = "gdb")]
 use std::sync::mpsc;
 use std::sync::{Arc, Mutex};
@@ -23,16 +23,14 @@ use linux_loader::loader::elf::PvhBootCapability;
 use linux_loader::loader::pe::PE as Loader;
 use userfaultfd::Uffd;
 use utils::time::TimestampUs;
-use vm_memory::ReadVolatile;
 #[cfg(target_arch = "aarch64")]
 use vm_superio::Rtc;
 use vm_superio::Serial;
 use vmm_sys_util::eventfd::EventFd;
 
 #[cfg(target_arch = "x86_64")]
 use crate::acpi;
-use crate::arch::{BootProtocol, EntryPoint, InitrdConfig};
-use crate::builder::StartMicrovmError::Internal;
+use crate::arch::{BootProtocol, EntryPoint};
 #[cfg(target_arch = "aarch64")]
 use crate::construct_kvm_mpidrs;
 use crate::cpu_config::templates::{
@@ -62,17 +60,17 @@ use crate::devices::virtio::rng::Entropy;
 use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend};
 #[cfg(feature = "gdb")]
 use crate::gdb;
+use crate::initrd::{InitrdConfig, InitrdError};
 use crate::logger::{debug, error};
 use crate::persist::{MicrovmState, MicrovmStateError};
 use crate::resources::VmResources;
 use crate::seccomp::BpfThreadMap;
 use crate::snapshot::Persist;
-use crate::utils::u64_to_usize;
 use crate::vmm_config::boot_source::BootConfig;
 use crate::vmm_config::instance_info::InstanceInfo;
 use crate::vmm_config::machine_config::{MachineConfig, MachineConfigError};
 use crate::vstate::kvm::Kvm;
-use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
+use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
 use crate::vstate::vcpu::{Vcpu, VcpuConfig, VcpuError};
 use crate::vstate::vm::Vm;
 use crate::{EventManager, Vmm, VmmError, device_manager};
@@ -99,10 +97,8 @@ pub enum StartMicrovmError {
     CreateVMGenID(VmGenIdError),
     /// Invalid Memory Configuration: {0}
     GuestMemory(crate::vstate::memory::MemoryError),
-    /// Cannot load initrd due to an invalid memory configuration.
-    InitrdLoad,
-    /// Cannot load initrd due to an invalid image: {0}
-    InitrdRead(io::Error),
+    /// Error with initrd initialization: {0}.
+    Initrd(#[from] InitrdError),
     /// Internal error while starting microVM: {0}
     Internal(#[from] VmmError),
     /// Failed to get CPU template: {0}
@@ -241,7 +237,7 @@ pub fn build_microvm_for_boot(
         .map_err(StartMicrovmError::GuestMemory)?;
 
     let entry_point = load_kernel(boot_config, &guest_memory)?;
-    let initrd = load_initrd_from_config(boot_config, &guest_memory)?;
+    let initrd = InitrdConfig::from_config(boot_config, &guest_memory)?;
     // Clone the command-line so that a failed boot doesn't pollute the original.
     #[allow(unused_mut)]
     let mut boot_cmdline = boot_config.cmdline.clone();
@@ -449,7 +445,7 @@ pub fn build_microvm_from_snapshot(
         vm_resources.machine_config.vcpu_count,
         microvm_state.kvm_state.kvm_cap_modifiers.clone(),
     )
-    .map_err(Internal)?;
+    .map_err(StartMicrovmError::Internal)?;
 
     #[cfg(target_arch = "x86_64")]
     {
@@ -603,68 +599,6 @@ fn load_kernel(
     })
 }
 
-fn load_initrd_from_config(
-    boot_cfg: &BootConfig,
-    vm_memory: &GuestMemoryMmap,
-) -> Result<Option<InitrdConfig>, StartMicrovmError> {
-    use self::StartMicrovmError::InitrdRead;
-
-    Ok(match &boot_cfg.initrd_file {
-        Some(f) => Some(load_initrd(
-            vm_memory,
-            &mut f.try_clone().map_err(InitrdRead)?,
-        )?),
-        None => None,
-    })
-}
-
-/// Loads the initrd from a file into the given memory slice.
-///
-/// * `vm_memory` - The guest memory the initrd is written to.
-/// * `image` - The initrd image.
-///
-/// Returns the result of initrd loading
-fn load_initrd<F>(
-    vm_memory: &GuestMemoryMmap,
-    image: &mut F,
-) -> Result<InitrdConfig, StartMicrovmError>
-where
-    F: ReadVolatile + Seek + Debug,
-{
-    use self::StartMicrovmError::{InitrdLoad, InitrdRead};
-
-    // Get the image size
-    let size = match image.seek(SeekFrom::End(0)) {
-        Err(err) => return Err(InitrdRead(err)),
-        Ok(0) => {
-            return Err(InitrdRead(io::Error::new(
-                io::ErrorKind::InvalidData,
-                "Initrd image seek returned a size of zero",
-            )));
-        }
-        Ok(s) => u64_to_usize(s),
-    };
-    // Go back to the image start
-    image.seek(SeekFrom::Start(0)).map_err(InitrdRead)?;
-
-    // Get the target address
-    let address = crate::arch::initrd_load_addr(vm_memory, size).map_err(|_| InitrdLoad)?;
-
-    // Load the image into memory
-    let mut slice = vm_memory
-        .get_slice(GuestAddress(address), size)
-        .map_err(|_| InitrdLoad)?;
-
-    image
-        .read_exact_volatile(&mut slice)
-        .map_err(|_| InitrdLoad)?;
-
-    Ok(InitrdConfig {
-        address: GuestAddress(address),
-        size,
-    })
-}
-
 /// Sets up the serial device.
 pub fn setup_serial_device(
     event_manager: &mut EventManager,
@@ -1010,7 +944,6 @@ pub(crate) fn set_stdout_nonblocking() {
 
 #[cfg(test)]
 pub(crate) mod tests {
-    use std::io::Write;
 
     use linux_loader::cmdline::Cmdline;
     use vmm_sys_util::tempfile::TempFile;
@@ -1024,7 +957,6 @@ pub(crate) mod tests {
     use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_RNG};
     use crate::mmds::data_store::{Mmds, MmdsVersion};
     use crate::mmds::ns::MmdsNetworkStack;
-    use crate::test_utils::{single_region_mem, single_region_mem_at};
     use crate::utils::mib_to_bytes;
     use crate::vmm_config::balloon::{BALLOON_DEV_ID, BalloonBuilder, BalloonDeviceConfig};
     use crate::vmm_config::boot_source::DEFAULT_KERNEL_CMDLINE;
@@ -1270,67 +1202,6 @@ pub(crate) mod tests {
         );
     }
 
-    fn make_test_bin() -> Vec<u8> {
-        let mut fake_bin = Vec::new();
-        fake_bin.resize(1_000_000, 0xAA);
-        fake_bin
-    }
-
-    #[test]
-    // Test that loading the initrd is successful on different archs.
-    fn test_load_initrd() {
-        use crate::vstate::memory::GuestMemory;
-        let image = make_test_bin();
-
-        let mem_size: usize = image.len() * 2 + crate::arch::GUEST_PAGE_SIZE;
-
-        let tempfile = TempFile::new().unwrap();
-        let mut tempfile = tempfile.into_file();
-        tempfile.write_all(&image).unwrap();
-
-        #[cfg(target_arch = "x86_64")]
-        let gm = single_region_mem(mem_size);
-
-        #[cfg(target_arch = "aarch64")]
-        let gm = single_region_mem(mem_size + crate::arch::aarch64::layout::FDT_MAX_SIZE);
-
-        let res = load_initrd(&gm, &mut tempfile);
-        let initrd = res.unwrap();
-        assert!(gm.address_in_range(initrd.address));
-        assert_eq!(initrd.size, image.len());
-    }
-
-    #[test]
-    fn test_load_initrd_no_memory() {
-        let gm = single_region_mem(79);
-        let image = make_test_bin();
-        let tempfile = TempFile::new().unwrap();
-        let mut tempfile = tempfile.into_file();
-        tempfile.write_all(&image).unwrap();
-        let res = load_initrd(&gm, &mut tempfile);
-        assert!(
-            matches!(res, Err(StartMicrovmError::InitrdLoad)),
-            "{:?}",
-            res
-        );
-    }
-
-    #[test]
-    fn test_load_initrd_unaligned() {
-        let image = vec![1, 2, 3, 4];
-        let tempfile = TempFile::new().unwrap();
-        let mut tempfile = tempfile.into_file();
-        tempfile.write_all(&image).unwrap();
-        let gm = single_region_mem_at(crate::arch::GUEST_PAGE_SIZE as u64 + 1, image.len() * 2);
-
-        let res = load_initrd(&gm, &mut tempfile);
-        assert!(
-            matches!(res, Err(StartMicrovmError::InitrdLoad)),
-            "{:?}",
-            res
-        );
-    }
-
     #[test]
     fn test_attach_net_devices() {
         let mut event_manager = EventManager::new().expect("Unable to create EventManager");