diff --git a/src/firecracker/src/api_server/request/machine_configuration.rs b/src/firecracker/src/api_server/request/machine_configuration.rs index 4457a9b6b75..2e8addffb74 100644 --- a/src/firecracker/src/api_server/request/machine_configuration.rs +++ b/src/firecracker/src/api_server/request/machine_configuration.rs @@ -123,6 +123,8 @@ mod tests { cpu_template: None, track_dirty_pages: Some(false), huge_pages: Some(expected), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_eq!( vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()), @@ -142,6 +144,8 @@ mod tests { cpu_template: Some(StaticCpuTemplate::None), track_dirty_pages: Some(false), huge_pages: Some(HugePageConfig::None), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_eq!( vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()), @@ -161,6 +165,8 @@ mod tests { cpu_template: None, track_dirty_pages: Some(true), huge_pages: Some(HugePageConfig::None), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_eq!( vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()), @@ -184,6 +190,8 @@ mod tests { cpu_template: Some(StaticCpuTemplate::T2), track_dirty_pages: Some(true), huge_pages: Some(HugePageConfig::None), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_eq!( vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()), @@ -209,6 +217,8 @@ mod tests { cpu_template: None, track_dirty_pages: Some(true), huge_pages: Some(HugePageConfig::None), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_eq!( vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()), diff --git a/src/vmm/src/arch/aarch64/fdt.rs b/src/vmm/src/arch/aarch64/fdt.rs index 2eb154a33b3..c708add44bf 100644 --- a/src/vmm/src/arch/aarch64/fdt.rs +++ b/src/vmm/src/arch/aarch64/fdt.rs @@ -15,6 +15,7 @@ use vm_memory::GuestMemoryError; use super::super::{DeviceType, InitrdConfig}; 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::vstate::memory::{Address, GuestMemory, GuestMemoryMmap}; @@ -42,16 +43,6 @@ const GIC_FDT_IRQ_TYPE_PPI: u32 = 1; const IRQ_TYPE_EDGE_RISING: u32 = 1; const IRQ_TYPE_LEVEL_HI: u32 = 4; -/// Trait for devices to be added to the Flattened Device Tree. -pub trait DeviceInfoForFDT { - /// Returns the address where this device will be loaded. - fn addr(&self) -> u64; - /// Returns the associated interrupt for this device. - fn irq(&self) -> u32; - /// Returns the amount of memory that needs to be reserved for this device. - fn length(&self) -> u64; -} - /// Errors thrown while configuring the Flattened Device Tree for aarch64. #[derive(Debug, thiserror::Error, displaydoc::Display)] pub enum FdtError { @@ -64,11 +55,11 @@ pub enum FdtError { } /// Creates the flattened device tree for this aarch64 microVM. -pub fn create_fdt( +pub fn create_fdt( guest_mem: &GuestMemoryMmap, vcpu_mpidr: Vec, cmdline: CString, - device_info: &HashMap<(DeviceType, String), T>, + device_info: &HashMap<(DeviceType, String), MMIODeviceInfo>, gic_device: &GICDevice, vmgenid: &Option, initrd: &Option, @@ -361,17 +352,18 @@ fn create_psci_node(fdt: &mut FdtWriter) -> Result<(), FdtError> { Ok(()) } -fn create_virtio_node( - fdt: &mut FdtWriter, - dev_info: &T, -) -> Result<(), FdtError> { - let virtio_mmio = fdt.begin_node(&format!("virtio_mmio@{:x}", dev_info.addr()))?; +fn create_virtio_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> { + let virtio_mmio = fdt.begin_node(&format!("virtio_mmio@{:x}", dev_info.addr))?; fdt.property_string("compatible", "virtio,mmio")?; - fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?; + fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?; fdt.property_array_u32( "interrupts", - &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING], + &[ + GIC_FDT_IRQ_TYPE_SPI, + dev_info.irq.unwrap().into(), + IRQ_TYPE_EDGE_RISING, + ], )?; fdt.property_u32("interrupt-parent", GIC_PHANDLE)?; fdt.end_node(virtio_mmio)?; @@ -379,38 +371,36 @@ fn create_virtio_node( Ok(()) } -fn create_serial_node( - fdt: &mut FdtWriter, - dev_info: &T, -) -> Result<(), FdtError> { - let serial = fdt.begin_node(&format!("uart@{:x}", dev_info.addr()))?; +fn create_serial_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> { + let serial = fdt.begin_node(&format!("uart@{:x}", dev_info.addr))?; fdt.property_string("compatible", "ns16550a")?; - fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?; + fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?; fdt.property_u32("clocks", CLOCK_PHANDLE)?; fdt.property_string("clock-names", "apb_pclk")?; fdt.property_array_u32( "interrupts", - &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING], + &[ + GIC_FDT_IRQ_TYPE_SPI, + dev_info.irq.unwrap().into(), + IRQ_TYPE_EDGE_RISING, + ], )?; fdt.end_node(serial)?; Ok(()) } -fn create_rtc_node( - fdt: &mut FdtWriter, - dev_info: &T, -) -> Result<(), FdtError> { +fn create_rtc_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> { // Driver requirements: // https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/rtc/arm,pl031.yaml // We do not offer the `interrupt` property because the device // does not implement interrupt support. let compatible = b"arm,pl031\0arm,primecell\0"; - let rtc = fdt.begin_node(&format!("rtc@{:x}", dev_info.addr()))?; + let rtc = fdt.begin_node(&format!("rtc@{:x}", dev_info.addr))?; fdt.property("compatible", compatible)?; - fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?; + fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?; fdt.property_u32("clocks", CLOCK_PHANDLE)?; fdt.property_string("clock-names", "apb_pclk")?; fdt.end_node(rtc)?; @@ -418,12 +408,12 @@ fn create_rtc_node( Ok(()) } -fn create_devices_node( +fn create_devices_node( fdt: &mut FdtWriter, - dev_info: &HashMap<(DeviceType, String), T, S>, + dev_info: &HashMap<(DeviceType, String), MMIODeviceInfo>, ) -> Result<(), FdtError> { // Create one temp Vec to store all virtio devices - let mut ordered_virtio_device: Vec<&T> = Vec::new(); + let mut ordered_virtio_device: Vec<&MMIODeviceInfo> = Vec::new(); for ((device_type, _device_id), info) in dev_info { match device_type { @@ -437,7 +427,7 @@ fn create_devices_node u64 { - self.addr - } - fn irq(&self) -> u32 { - self.irq - } - fn length(&self) -> u64 { - LEN - } - } // The `load` function from the `device_tree` will mistakenly check the actual size // of the buffer with the allocated size. This works around that. fn set_size(buf: &mut [u8], pos: usize, val: u32) { @@ -493,17 +467,26 @@ mod tests { let dev_info: HashMap<(DeviceType, std::string::String), MMIODeviceInfo> = [ ( (DeviceType::Serial, DeviceType::Serial.to_string()), - MMIODeviceInfo { addr: 0x00, irq: 1 }, + MMIODeviceInfo { + addr: 0x00, + irq: NonZeroU32::new(1), + len: LEN, + }, ), ( (DeviceType::Virtio(1), "virtio".to_string()), - MMIODeviceInfo { addr: LEN, irq: 2 }, + MMIODeviceInfo { + addr: LEN, + irq: NonZeroU32::new(2), + len: LEN, + }, ), ( (DeviceType::Rtc, "rtc".to_string()), MMIODeviceInfo { addr: 2 * LEN, - irq: 3, + irq: NonZeroU32::new(3), + len: LEN, }, ), ] diff --git a/src/vmm/src/arch/aarch64/mod.rs b/src/vmm/src/arch/aarch64/mod.rs index 7ab1685cf69..a09721e1775 100644 --- a/src/vmm/src/arch/aarch64/mod.rs +++ b/src/vmm/src/arch/aarch64/mod.rs @@ -23,9 +23,9 @@ use std::fmt::Debug; use vm_memory::GuestMemoryError; -pub use self::fdt::DeviceInfoForFDT; use self::gic::GICDevice; use crate::arch::DeviceType; +use crate::device_manager::mmio::MMIODeviceInfo; use crate::devices::acpi::vmgenid::VmGenId; use crate::vstate::memory::{Address, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap}; @@ -63,11 +63,11 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> { /// * `device_info` - A hashmap containing the attached devices for building FDT device nodes. /// * `gic_device` - The GIC device. /// * `initrd` - Information about an optional initrd. -pub fn configure_system( +pub fn configure_system( guest_mem: &GuestMemoryMmap, cmdline_cstring: CString, vcpu_mpidr: Vec, - device_info: &HashMap<(DeviceType, String), T>, + device_info: &HashMap<(DeviceType, String), MMIODeviceInfo>, gic_device: &GICDevice, vmgenid: &Option, initrd: &Option, diff --git a/src/vmm/src/arch/x86_64/mod.rs b/src/vmm/src/arch/x86_64/mod.rs index 24a7de7ec8a..ad680d89cff 100644 --- a/src/vmm/src/arch/x86_64/mod.rs +++ b/src/vmm/src/arch/x86_64/mod.rs @@ -41,7 +41,7 @@ use linux_loader::loader::elf::start_info::{ use crate::arch::{BootProtocol, InitrdConfig, SYSTEM_MEM_SIZE, SYSTEM_MEM_START}; use crate::device_manager::resources::ResourceAllocator; -use crate::utils::u64_to_usize; +use crate::utils::{mib_to_bytes, u64_to_usize}; use crate::vstate::memory::{ Address, GuestAddress, GuestMemory, GuestMemoryMmap, GuestMemoryRegion, }; @@ -73,10 +73,11 @@ pub enum ConfigurationError { StartInfoSetup, } -const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32; +/// First address that cannot be addressed using 32 bit anymore. +pub const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32; /// Size of MMIO gap at top of 32-bit address space. -pub const MEM_32BIT_GAP_SIZE: u64 = 768 << 20; +pub const MEM_32BIT_GAP_SIZE: u64 = mib_to_bytes(768) as u64; /// The start of the memory area reserved for MMIO devices. pub const MMIO_MEM_START: u64 = FIRST_ADDR_PAST_32BITS - MEM_32BIT_GAP_SIZE; /// The size of the memory area reserved for MMIO devices. @@ -402,7 +403,7 @@ mod tests { ); // Now assigning some memory that falls before the 32bit memory hole. - let mem_size = 128 << 20; + let mem_size = mib_to_bytes(128); let gm = arch_mem(mem_size); let mut resource_allocator = ResourceAllocator::new().unwrap(); configure_system( @@ -427,7 +428,7 @@ mod tests { .unwrap(); // Now assigning some memory that is equal to the start of the 32bit memory hole. - let mem_size = 3328 << 20; + let mem_size = mib_to_bytes(3328); let gm = arch_mem(mem_size); let mut resource_allocator = ResourceAllocator::new().unwrap(); configure_system( @@ -452,7 +453,7 @@ mod tests { .unwrap(); // Now assigning some memory that falls after the 32bit memory hole. - let mem_size = 3330 << 20; + let mem_size = mib_to_bytes(3330); let gm = arch_mem(mem_size); let mut resource_allocator = ResourceAllocator::new().unwrap(); configure_system( diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index ae9cab9e51c..cd1e063fb40 100644 --- a/src/vmm/src/builder.rs +++ b/src/vmm/src/builder.rs @@ -32,6 +32,7 @@ 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; #[cfg(target_arch = "aarch64")] use crate::construct_kvm_mpidrs; use crate::cpu_config::templates::{ @@ -103,7 +104,7 @@ pub enum StartMicrovmError { /// Cannot load initrd due to an invalid image: {0} InitrdRead(io::Error), /// Internal error while starting microVM: {0} - Internal(VmmError), + Internal(#[from] VmmError), /// Failed to get CPU template: {0} GetCpuTemplate(#[from] GetCpuTemplateError), /// Invalid kernel command line: {0} @@ -130,8 +131,6 @@ pub enum StartMicrovmError { SetVmResources(MachineConfigError), /// Cannot create the entropy device: {0} CreateEntropyDevice(crate::devices::virtio::rng::EntropyError), - /// Failed to allocate guest resource: {0} - AllocateResources(#[from] vm_allocator::Error), /// Error configuring ACPI: {0} #[cfg(target_arch = "x86_64")] Acpi(#[from] crate::acpi::AcpiError), @@ -159,23 +158,13 @@ fn create_vmm_and_vcpus( uffd: Option, vcpu_count: u8, kvm_capabilities: Vec, -) -> Result<(Vmm, Vec), StartMicrovmError> { - use self::StartMicrovmError::*; - - let kvm = Kvm::new(kvm_capabilities) - .map_err(VmmError::Kvm) - .map_err(StartMicrovmError::Internal)?; +) -> Result<(Vmm, Vec), VmmError> { + let kvm = Kvm::new(kvm_capabilities)?; // Set up Kvm Vm and register memory regions. // Build custom CPU config if a custom template is provided. - let mut vm = Vm::new(&kvm) - .map_err(VmmError::Vm) - .map_err(StartMicrovmError::Internal)?; - kvm.check_memory(&guest_memory) - .map_err(VmmError::Kvm) - .map_err(StartMicrovmError::Internal)?; - vm.memory_init(&guest_memory) - .map_err(VmmError::Vm) - .map_err(StartMicrovmError::Internal)?; + let mut vm = Vm::new(&kvm)?; + kvm.check_memory(&guest_memory)?; + vm.memory_init(&guest_memory)?; let resource_allocator = ResourceAllocator::new()?; @@ -185,10 +174,7 @@ fn create_vmm_and_vcpus( // Instantiate ACPI device manager. let acpi_device_manager = ACPIDeviceManager::new(); - let (vcpus, vcpus_exit_evt) = vm - .create_vcpus(vcpu_count) - .map_err(VmmError::Vm) - .map_err(Internal)?; + let (vcpus, vcpus_exit_evt) = vm.create_vcpus(vcpu_count)?; #[cfg(target_arch = "x86_64")] let pio_device_manager = { @@ -196,14 +182,10 @@ fn create_vmm_and_vcpus( set_stdout_nonblocking(); // Serial device setup. - let serial_device = - setup_serial_device(event_manager, std::io::stdin(), io::stdout()).map_err(Internal)?; + let serial_device = setup_serial_device(event_manager, std::io::stdin(), io::stdout())?; // x86_64 uses the i8042 reset event as the Vmm exit event. - let reset_evt = vcpus_exit_evt - .try_clone() - .map_err(VmmError::EventFd) - .map_err(Internal)?; + let reset_evt = vcpus_exit_evt.try_clone().map_err(VmmError::EventFd)?; // create pio dev manager with legacy devices // TODO Remove these unwraps. @@ -323,7 +305,7 @@ pub fn build_microvm_for_boot( } #[cfg(target_arch = "aarch64")] - attach_legacy_devices_aarch64(event_manager, &mut vmm, &mut boot_cmdline).map_err(Internal)?; + attach_legacy_devices_aarch64(event_manager, &mut vmm, &mut boot_cmdline)?; attach_vmgenid_device(&mut vmm)?; @@ -363,8 +345,7 @@ pub fn build_microvm_for_boot( .ok_or_else(|| MissingSeccompFilters("vcpu".to_string()))? .clone(), ) - .map_err(VmmError::VcpuStart) - .map_err(Internal)?; + .map_err(VmmError::VcpuStart)?; // Load seccomp filters for the VMM thread. // Execution panics if filters cannot be loaded, use --no-seccomp if skipping filters @@ -375,8 +356,7 @@ pub fn build_microvm_for_boot( .get("vmm") .ok_or_else(|| MissingSeccompFilters("vmm".to_string()))?, ) - .map_err(VmmError::SeccompFilters) - .map_err(Internal)?; + .map_err(VmmError::SeccompFilters)?; event_manager.add_subscriber(vmm.clone()); @@ -401,10 +381,7 @@ pub fn build_and_boot_microvm( debug!("event_end: build microvm for boot"); // The vcpus start off in the `Paused` state, let them run. debug!("event_start: boot microvm"); - vmm.lock() - .unwrap() - .resume_vm() - .map_err(StartMicrovmError::Internal)?; + vmm.lock().unwrap().resume_vm()?; debug!("event_end: boot microvm"); Ok(vmm) } @@ -471,7 +448,8 @@ pub fn build_microvm_from_snapshot( uffd, vm_resources.machine_config.vcpu_count, microvm_state.kvm_state.kvm_cap_modifiers.clone(), - )?; + ) + .map_err(Internal)?; #[cfg(target_arch = "x86_64")] { @@ -575,7 +553,7 @@ fn load_kernel( let mut kernel_file = boot_config .kernel_file .try_clone() - .map_err(|err| StartMicrovmError::Internal(VmmError::KernelFile(err)))?; + .map_err(VmmError::KernelFile)?; let entry_addr = Loader::load::( guest_memory, @@ -609,7 +587,7 @@ fn load_kernel( let mut kernel_file = boot_config .kernel_file .try_clone() - .map_err(|err| StartMicrovmError::Internal(VmmError::KernelFile(err)))?; + .map_err(VmmError::KernelFile)?; let entry_addr = Loader::load::( guest_memory, @@ -778,8 +756,7 @@ pub fn configure_system_for_boot( for vcpu in vcpus.iter_mut() { vcpu.kvm_vcpu .init(&cpu_template.vcpu_features) - .map_err(VmmError::VcpuInit) - .map_err(Internal)?; + .map_err(VmmError::VcpuInit)?; } let mut regs = Aarch64RegisterVec::default(); @@ -803,8 +780,7 @@ pub fn configure_system_for_boot( for vcpu in vcpus.iter_mut() { vcpu.kvm_vcpu .configure(vmm.guest_memory(), entry_point, &vcpu_config) - .map_err(VmmError::VcpuConfigure) - .map_err(Internal)?; + .map_err(VmmError::VcpuConfigure)?; } // Write the kernel command line to guest memory. This is x86_64 specific, since on @@ -852,8 +828,7 @@ pub fn configure_system_for_boot( &vcpu_config, &optional_capabilities, ) - .map_err(VmmError::VcpuConfigure) - .map_err(Internal)?; + .map_err(VmmError::VcpuConfigure)?; } let vcpu_mpidr = vcpus .iter_mut() @@ -1050,6 +1025,7 @@ pub(crate) mod tests { 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; use crate::vmm_config::drive::{BlockBuilder, BlockDeviceConfig}; @@ -1113,7 +1089,7 @@ pub(crate) mod tests { } pub(crate) fn default_vmm() -> Vmm { - let (kvm, mut vm, guest_memory) = setup_vm_with_memory(128 << 20); + let (kvm, mut vm, guest_memory) = setup_vm_with_memory(mib_to_bytes(128)); let mmio_device_manager = MMIODeviceManager::new(); let acpi_device_manager = ACPIDeviceManager::new(); diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index b4961623a15..20d9b381e1c 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -23,8 +23,6 @@ use vm_allocator::AllocPolicy; use super::resources::ResourceAllocator; use crate::arch::DeviceType; use crate::arch::DeviceType::Virtio; -#[cfg(target_arch = "aarch64")] -use crate::arch::aarch64::DeviceInfoForFDT; use crate::devices::BusDevice; #[cfg(target_arch = "aarch64")] use crate::devices::legacy::RTCDevice; @@ -522,19 +520,6 @@ impl MMIODeviceManager { } } -#[cfg(target_arch = "aarch64")] -impl DeviceInfoForFDT for MMIODeviceInfo { - fn addr(&self) -> u64 { - self.addr - } - fn irq(&self) -> u32 { - self.irq.unwrap().into() - } - fn length(&self) -> u64 { - self.len - } -} - #[cfg(test)] mod tests { diff --git a/src/vmm/src/devices/virtio/vsock/event_handler.rs b/src/vmm/src/devices/virtio/vsock/event_handler.rs index a16cff7dbfc..632148546e5 100755 --- a/src/vmm/src/devices/virtio/vsock/event_handler.rs +++ b/src/vmm/src/devices/virtio/vsock/event_handler.rs @@ -223,10 +223,7 @@ mod tests { use super::super::*; use super::*; - use crate::devices::virtio::vsock::packet::VSOCK_PKT_HDR_SIZE; use crate::devices::virtio::vsock::test_utils::{EventHandlerContext, TestContext}; - use crate::test_utils::multi_region_mem; - use crate::vstate::memory::Bytes; #[test] fn test_txq_event() { @@ -427,8 +424,9 @@ mod tests { // function for testing error cases, so the asserts always expect is_err() to be true. When // desc_idx = 0 we are altering the header (first descriptor in the chain), and when // desc_idx = 1 we are altering the packet buffer. + #[cfg(target_arch = "x86_64")] fn vsock_bof_helper(test_ctx: &mut TestContext, desc_idx: usize, addr: u64, len: u32) { - use crate::vstate::memory::GuestAddress; + use crate::vstate::memory::{Bytes, GuestAddress}; assert!(desc_idx <= 1); @@ -472,19 +470,23 @@ mod tests { } #[test] + #[cfg(target_arch = "x86_64")] + #[allow(clippy::cast_possible_truncation)] /* casting of constants we know fit into u32 */ fn test_vsock_bof() { + use crate::arch::MMIO_MEM_START; + use crate::arch::x86_64::{FIRST_ADDR_PAST_32BITS, MEM_32BIT_GAP_SIZE}; + use crate::devices::virtio::vsock::packet::VSOCK_PKT_HDR_SIZE; + use crate::test_utils::multi_region_mem; + use crate::utils::mib_to_bytes; use crate::vstate::memory::GuestAddress; - const GAP_SIZE: u32 = 768 << 20; - const FIRST_AFTER_GAP: usize = 1 << 32; - const GAP_START_ADDR: usize = FIRST_AFTER_GAP - GAP_SIZE as usize; - const MIB: usize = 1 << 20; + const MIB: usize = mib_to_bytes(1); let mut test_ctx = TestContext::new(); test_ctx.mem = multi_region_mem(&[ (GuestAddress(0), 8 * MIB), - (GuestAddress((GAP_START_ADDR - MIB) as u64), MIB), - (GuestAddress(FIRST_AFTER_GAP as u64), MIB), + (GuestAddress(MMIO_MEM_START - MIB as u64), MIB), + (GuestAddress(FIRST_ADDR_PAST_32BITS), MIB), ]); // The default configured descriptor chains are valid. @@ -506,20 +508,25 @@ mod tests { } // Let's check what happens when the header descriptor is right before the gap. - vsock_bof_helper( - &mut test_ctx, - 0, - GAP_START_ADDR as u64 - 1, - VSOCK_PKT_HDR_SIZE, - ); + vsock_bof_helper(&mut test_ctx, 0, MMIO_MEM_START - 1, VSOCK_PKT_HDR_SIZE); // Let's check what happens when the buffer descriptor crosses into the gap, but does // not go past its right edge. - vsock_bof_helper(&mut test_ctx, 1, GAP_START_ADDR as u64 - 4, GAP_SIZE + 4); + vsock_bof_helper( + &mut test_ctx, + 1, + MMIO_MEM_START - 4, + MEM_32BIT_GAP_SIZE as u32 + 4, + ); // Let's modify the buffer descriptor addr and len such that it crosses over the MMIO gap, // and check we cannot assemble the VsockPkts. - vsock_bof_helper(&mut test_ctx, 1, GAP_START_ADDR as u64 - 4, GAP_SIZE + 100); + vsock_bof_helper( + &mut test_ctx, + 1, + MMIO_MEM_START - 4, + MEM_32BIT_GAP_SIZE as u32 + 100, + ); } #[test] diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 492077df068..993409dc819 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -205,6 +205,8 @@ pub const HTTP_MAX_PAYLOAD_SIZE: usize = 51200; /// have permissions to open the KVM fd). #[derive(Debug, thiserror::Error, displaydoc::Display)] pub enum VmmError { + /// Failed to allocate guest resource: {0} + AllocateResources(#[from] vm_allocator::Error), #[cfg(target_arch = "aarch64")] /// Invalid command line error. Cmdline, @@ -255,9 +257,9 @@ pub enum VmmError { /// Cannot spawn Vcpu thread: {0} VcpuSpawn(io::Error), /// Vm error: {0} - Vm(vstate::vm::VmError), + Vm(#[from] vstate::vm::VmError), /// Kvm error: {0} - Kvm(vstate::kvm::KvmError), + Kvm(#[from] vstate::kvm::KvmError), /// Error thrown by observer object on Vmm initialization: {0} VmmObserverInit(vmm_sys_util::errno::Error), /// Error thrown by observer object on Vmm teardown: {0} diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 113c24cad51..2d472a3f3e2 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -13,6 +13,7 @@ use crate::logger::info; use crate::mmds; use crate::mmds::data_store::{Mmds, MmdsVersion}; use crate::mmds::ns::MmdsNetworkStack; +use crate::utils::mib_to_bytes; use crate::utils::net::ipv4addr::is_link_local_valid; use crate::vmm_config::balloon::*; use crate::vmm_config::boot_source::{ @@ -462,7 +463,8 @@ impl VmResources { self.machine_config.huge_pages, ) } else { - let regions = crate::arch::arch_memory_regions(self.machine_config.mem_size_mib << 20); + let regions = + crate::arch::arch_memory_regions(mib_to_bytes(self.machine_config.mem_size_mib)); GuestMemoryMmap::anonymous( regions.into_iter(), self.machine_config.track_dirty_pages, @@ -1328,6 +1330,8 @@ mod tests { cpu_template: Some(StaticCpuTemplate::V1N1), track_dirty_pages: Some(false), huge_pages: Some(HugePageConfig::None), + #[cfg(feature = "gdb")] + gdb_socket_path: None, }; assert_ne!( diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 8eb7227347a..127b75e594e 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -808,14 +808,12 @@ impl RuntimeApiController { // vhost-user-block updates if new_cfg.path_on_host.is_none() && new_cfg.rate_limiter.is_none() { vmm.update_vhost_user_block_config(&new_cfg.drive_id) - .map(|()| VmmData::Empty) .map_err(DriveError::DeviceUpdate)?; } // virtio-block updates if let Some(new_path) = new_cfg.path_on_host { vmm.update_block_device_path(&new_cfg.drive_id, new_path) - .map(|()| VmmData::Empty) .map_err(DriveError::DeviceUpdate)?; } if new_cfg.rate_limiter.is_some() { @@ -824,7 +822,6 @@ impl RuntimeApiController { RateLimiterUpdate::from(new_cfg.rate_limiter).bandwidth, RateLimiterUpdate::from(new_cfg.rate_limiter).ops, ) - .map(|()| VmmData::Empty) .map_err(DriveError::DeviceUpdate)?; } Ok(VmmData::Empty) diff --git a/src/vmm/src/utils/mod.rs b/src/vmm/src/utils/mod.rs index a0ee2e90b6b..9c81c7a3016 100644 --- a/src/vmm/src/utils/mod.rs +++ b/src/vmm/src/utils/mod.rs @@ -13,6 +13,9 @@ pub mod sm; use std::num::Wrapping; use std::result::Result; +/// How many bits to left-shift by to convert MiB to bytes +const MIB_TO_BYTES_SHIFT: usize = 20; + /// Return the default page size of the platform, in bytes. pub fn get_page_size() -> Result { // SAFETY: Safe because the parameters are valid. @@ -45,3 +48,8 @@ pub const fn usize_to_u64(num: usize) -> u64 { pub const fn wrap_usize_to_u32(num: usize) -> Wrapping { Wrapping(((num as u64) & 0xFFFFFFFF) as u32) } + +/// Converts MiB to Bytes +pub const fn mib_to_bytes(mib: usize) -> usize { + mib << MIB_TO_BYTES_SHIFT +} diff --git a/src/vmm/src/vstate/memory.rs b/src/vmm/src/vstate/memory.rs index e0f47848684..8875791f3a6 100644 --- a/src/vmm/src/vstate/memory.rs +++ b/src/vmm/src/vstate/memory.rs @@ -23,7 +23,7 @@ use vmm_sys_util::errno; use crate::DirtyBitmap; use crate::arch::arch_memory_regions; -use crate::utils::{get_page_size, u64_to_usize}; +use crate::utils::{get_page_size, mib_to_bytes, u64_to_usize}; use crate::vmm_config::machine_config::HugePageConfig; /// Type of GuestMemoryMmap. @@ -72,7 +72,7 @@ where huge_pages: HugePageConfig, ) -> Result { let memfd_file = create_memfd(mem_size_mib, huge_pages.into())?.into_file(); - let regions = arch_memory_regions(mem_size_mib << 20).into_iter(); + let regions = arch_memory_regions(mib_to_bytes(mem_size_mib)).into_iter(); Self::create( regions, @@ -330,7 +330,7 @@ fn create_memfd( size: usize, hugetlb_size: Option, ) -> Result { - let mem_size = size << 20; + let mem_size = mib_to_bytes(size); // Create a memfd. let opts = memfd::MemfdOptions::default() .hugetlb(hugetlb_size) @@ -713,7 +713,7 @@ mod tests { #[test] fn test_create_memfd() { let size = 1; - let size_mb = 1 << 20; + let size_mb = mib_to_bytes(1) as u64; let memfd = create_memfd(size, None).unwrap(); diff --git a/src/vmm/src/vstate/vcpu.rs b/src/vmm/src/vstate/vcpu.rs index d54d7de759b..b7de0017e46 100644 --- a/src/vmm/src/vstate/vcpu.rs +++ b/src/vmm/src/vstate/vcpu.rs @@ -775,6 +775,7 @@ pub(crate) mod tests { use crate::devices::BusDevice; use crate::devices::bus::DummyDevice; use crate::seccomp::get_empty_filters; + use crate::utils::mib_to_bytes; use crate::utils::signal::validate_signal_num; use crate::vstate::kvm::Kvm; use crate::vstate::memory::{GuestAddress, GuestMemoryMmap}; @@ -962,7 +963,7 @@ pub(crate) mod tests { fn vcpu_configured_for_boot() -> (VcpuHandle, EventFd, GuestMemoryMmap) { Vcpu::register_kick_signal_handler(); // Need enough mem to boot linux. - let mem_size = 64 << 20; + let mem_size = mib_to_bytes(64); let (kvm, _, mut vcpu, vm_mem) = setup_vcpu(mem_size); let vcpu_exit_evt = vcpu.exit_evt.try_clone().unwrap(); diff --git a/src/vmm/src/vstate/vm.rs b/src/vmm/src/vstate/vm.rs index 4292c2f2f49..bb13b17ee99 100644 --- a/src/vmm/src/vstate/vm.rs +++ b/src/vmm/src/vstate/vm.rs @@ -135,6 +135,7 @@ impl Vm { pub(crate) mod tests { use super::*; use crate::test_utils::single_region_mem; + use crate::utils::mib_to_bytes; use crate::vstate::kvm::Kvm; use crate::vstate::memory::GuestMemoryMmap; @@ -189,7 +190,7 @@ pub(crate) mod tests { #[test] fn test_create_vcpus() { let vcpu_count = 2; - let (_, mut vm, _) = setup_vm_with_memory(128 << 20); + let (_, mut vm, _) = setup_vm_with_memory(mib_to_bytes(128)); let (vcpu_vec, _) = vm.create_vcpus(vcpu_count).unwrap();