increase the number of IRQs on x86_64

CHANGELOG.md

@@ -18,6 +18,9 @@
   replaced by the `--` separator for extra arguments.
 - Changed the output of the `--version` command line parameter to include a list
   of supported snapshot data format versions for the firecracker binary.
+- Increased the maximum number of virtio devices from 11 to 19.
+- Added a new check that prevents creating v0.23 snapshots when more than 11
+  devices are attached.
 
 ### Fixed
 

src/arch/src/x86_64/layout.rs

@@ -18,11 +18,11 @@ pub const CMDLINE_MAX_SIZE: usize = 0x10000;
 /// Start of the high memory.
 pub const HIMEM_START: u64 = 0x0010_0000; //1 MB.
 
-// Typically, on x86 systems 16 IRQs are used (0-15).
+// Typically, on x86 systems 24 IRQs are used (0-23).
 /// First usable IRQ ID for virtio device interrupts on x86_64.
 pub const IRQ_BASE: u32 = 5;
 /// Last usable IRQ ID for virtio device interrupts on x86_64.
-pub const IRQ_MAX: u32 = 15;
+pub const IRQ_MAX: u32 = 23;
 
 /// Address for the TSS setup.
 pub const KVM_TSS_ADDRESS: u64 = 0xfffb_d000;

src/arch/src/x86_64/mptable.rs

@@ -5,6 +5,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
+use std::convert::TryFrom;
 use std::io;
 use std::mem;
 use std::result;
@@ -15,6 +16,8 @@ use libc::c_char;
 use arch_gen::x86::mpspec;
 use vm_memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap};
 
+use crate::IRQ_MAX;
+
 // This is a workaround to the Rust enforcement specifying that any implementation of a foreign
 // trait (in this case `ByteValued`) where:
 // *    the type that is implementing the trait is foreign or
@@ -57,6 +60,8 @@ pub enum Error {
     Clear,
     /// Number of CPUs exceeds the maximum supported CPUs
     TooManyCpus,
+    /// Number of IRQs exceeds the maximum supported IRQs
+    TooManyIrqs,
     /// Failure to write the MP floating pointer.
     WriteMpfIntel,
     /// Failure to write MP CPU entry.
@@ -120,7 +125,7 @@ fn compute_mp_size(num_cpus: u8) -> usize {
         + mem::size_of::<MpcCpuWrapper>() * (num_cpus as usize)
         + mem::size_of::<MpcIoapicWrapper>()
         + mem::size_of::<MpcBusWrapper>()
-        + mem::size_of::<MpcIntsrcWrapper>() * 16
+        + mem::size_of::<MpcIntsrcWrapper>() * (IRQ_MAX as usize + 1)
         + mem::size_of::<MpcLintsrcWrapper>() * 2
 }
 
@@ -215,7 +220,7 @@ pub fn setup_mptable(mem: &GuestMemoryMmap, num_cpus: u8) -> Result<()> {
         checksum = checksum.wrapping_add(compute_checksum(&mpc_ioapic.0));
     }
     // Per kvm_setup_default_irq_routing() in kernel
-    for i in 0..16 {
+    for i in 0..=u8::try_from(IRQ_MAX).map_err(|_| Error::TooManyIrqs)? {
         let size = mem::size_of::<MpcIntsrcWrapper>() as u64;
         let mut mpc_intsrc = MpcIntsrcWrapper(mpspec::mpc_intsrc::default());
         mpc_intsrc.0.type_ = mpspec::MP_INTSRC as u8;

src/vmm/src/device_manager/mmio.rs

@@ -314,6 +314,17 @@ impl MMIODeviceManager {
         &self.id_to_dev_info
     }
 
+    #[cfg(target_arch = "x86_64")]
+    /// Gets the number of interrupts used by the devices registered.
+    pub fn used_irqs_count(&self) -> usize {
+        let mut irq_number = 0;
+        let _: Result<()> = self.for_each_device(|_, _, device_info, _| {
+            irq_number += device_info.irqs.len();
+            Ok(())
+        });
+        irq_number
+    }
+
     /// Gets the the specified device.
     pub fn get_device(
         &self,
@@ -723,6 +734,7 @@ mod tests {
                 Ok(())
             });
             assert_eq!(count, 3);
+            assert_eq!(device_manager.used_irqs_count(), 2);
         }
     }
 

src/vmm/src/persist.rs

@@ -18,11 +18,13 @@ use crate::mem_size_mib;
 use crate::vmm_config::snapshot::{CreateSnapshotParams, LoadSnapshotParams, SnapshotType};
 use crate::vstate::{self, vcpu::VcpuState, vm::VmState};
 
+use crate::device_manager::mmio::MMIODeviceManager;
 use crate::device_manager::persist::DeviceStates;
 use crate::memory_snapshot;
 use crate::memory_snapshot::{GuestMemoryState, SnapshotMemory};
 use crate::version_map::FC_VERSION_TO_SNAP_VERSION;
 use crate::Vmm;
+use arch::IRQ_BASE;
 use cpuid::common::{get_vendor_id_from_cpuid, get_vendor_id_from_host};
 use logger::{error, info};
 use polly::event_manager::EventManager;
@@ -32,6 +34,10 @@ use versionize::{VersionMap, Versionize, VersionizeResult};
 use versionize_derive::Versionize;
 use vm_memory::GuestMemoryMmap;
 
+const FC_V0_23_SNAP_VERSION: u16 = 1;
+const FC_V0_23_IRQ_NUMBER: u32 = 16;
+const FC_V0_23_MAX_DEVICES: u32 = FC_V0_23_IRQ_NUMBER - IRQ_BASE;
+
 /// Holds information related to the VM that is not part of VmState.
 #[derive(Debug, PartialEq, Versionize)]
 // NOTICE: Any changes to this structure require a snapshot version bump.
@@ -115,6 +121,8 @@ pub enum CreateSnapshotError {
     SerializeMicrovmState(snapshot::Error),
     /// Failed to open the snapshot backing file.
     SnapshotBackingFile(io::Error),
+    /// Number of devices exceeds the maximum supported devices for the snapshot data version.
+    TooManyDevices(usize),
 }
 
 impl Display for CreateSnapshotError {
@@ -132,6 +140,12 @@ impl Display for CreateSnapshotError {
             MicrovmState(err) => write!(f, "Cannot save microvm state: {}", err),
             SerializeMicrovmState(err) => write!(f, "Cannot serialize MicrovmState: {:?}", err),
             SnapshotBackingFile(err) => write!(f, "Cannot open snapshot file: {:?}", err),
+            TooManyDevices(val) => write!(
+                f,
+                "Too many devices attached: {}. The maximum number allowed \
+                 for the snapshot data version requested is {}.",
+                val, FC_V0_23_MAX_DEVICES
+            ),
         }
     }
 }
@@ -187,6 +201,7 @@ pub fn create_snapshot(
         &params.snapshot_path,
         &params.version,
         version_map,
+        &vmm.mmio_device_manager,
     )?;
 
     Ok(())
@@ -197,6 +212,7 @@ fn snapshot_state_to_file(
     snapshot_path: &PathBuf,
     version: &Option<String>,
     version_map: VersionMap,
+    device_manager: &MMIODeviceManager,
 ) -> std::result::Result<(), CreateSnapshotError> {
     use self::CreateSnapshotError::*;
     let mut snapshot_file = OpenOptions::new()
@@ -208,6 +224,10 @@ fn snapshot_state_to_file(
     // Translate the microVM version to its corresponding snapshot data format.
     let snapshot_data_version = match version {
         Some(version) => match FC_VERSION_TO_SNAP_VERSION.get(version) {
+            Some(&FC_V0_23_SNAP_VERSION) => {
+                validate_devices_number(device_manager.used_irqs_count())?;
+                Ok(FC_V0_23_SNAP_VERSION)
+            }
             Some(data_version) => Ok(*data_version),
             _ => Err(InvalidVersion),
         },
@@ -330,6 +350,14 @@ fn guest_memory_from_file(
     GuestMemoryMmap::restore(&mem_file, mem_state, track_dirty_pages).map_err(DeserializeMemory)
 }
 
+fn validate_devices_number(device_number: usize) -> std::result::Result<(), CreateSnapshotError> {
+    use self::CreateSnapshotError::TooManyDevices;
+    if device_number > FC_V0_23_MAX_DEVICES as usize {
+        return Err(TooManyDevices(device_number));
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -462,6 +490,9 @@ mod tests {
 
         let err = SnapshotBackingFile(io::Error::from_raw_os_error(0));
         let _ = format!("{}{:?}", err, err);
+
+        let err = TooManyDevices(0);
+        let _ = format!("{}{:?}", err, err);
     }
 
     #[test]

tests/framework/builder.py

@@ -179,6 +179,17 @@ def __init__(self, microvm):
         """Initialize the snapshot builder."""
         self._microvm = microvm
 
+    def create_snapshot_dir(self):
+        """Create dir and files for saving snapshot state and memory."""
+        chroot_path = self._microvm.jailer.chroot_path()
+        snapshot_dir = os.path.join(chroot_path, "snapshot")
+        Path(snapshot_dir).mkdir(parents=True, exist_ok=True)
+        cmd = 'chown {}:{} {}'.format(self._microvm.jailer.uid,
+                                      self._microvm.jailer.gid,
+                                      snapshot_dir)
+        utils.run_cmd(cmd)
+        return snapshot_dir
+
     def create(self,
                disks,
                ssh_key: Artifact,
@@ -190,13 +201,7 @@ def create(self,
         # Disable API timeout as the APIs for snapshot related procedures
         # take longer.
         self._microvm.api_session.untime()
-        chroot_path = self._microvm.jailer.chroot_path()
-        snapshot_dir = os.path.join(chroot_path, "snapshot")
-        Path(snapshot_dir).mkdir(parents=True, exist_ok=True)
-        cmd = 'chown {}:{} {}'.format(self._microvm.jailer.uid,
-                                      self._microvm.jailer.gid,
-                                      snapshot_dir)
-        utils.run_cmd(cmd)
+        snapshot_dir = self.create_snapshot_dir()
         self._microvm.pause_to_snapshot(
             mem_file_path="/snapshot/"+mem_file_name,
             snapshot_path="/snapshot/"+snapshot_name,

tests/integration_tests/functional/test_max_devices.py

@@ -0,0 +1,73 @@
+# Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0
+"""Tests scenario for adding the maximum number of devices to a microVM."""
+
+import platform
+import pytest
+import host_tools.network as net_tools
+
+# IRQs are available from 5 to 23, so the maximum number of devices
+# supported at the same time is 19.
+MAX_DEVICES_ATTACHED = 19
+
+
+@pytest.mark.skipif(
+    platform.machine() != "x86_64",
+    reason="Firecracker supports 24 IRQs on x86_64."
+)
+def test_attach_maximum_devices(test_microvm_with_ssh, network_config):
+    """Test attaching maximum number of devices to the microVM."""
+    test_microvm = test_microvm_with_ssh
+    test_microvm.spawn()
+
+    # Set up a basic microVM.
+    test_microvm.basic_config()
+
+    # Add (`MAX_DEVICES_ATTACHED` - 1) devices because the rootfs
+    # has already been configured in the `basic_config()`function.
+    guest_ips = []
+    for i in range(MAX_DEVICES_ATTACHED - 1):
+        # Create tap before configuring interface.
+        _tap, _host_ip, guest_ip = test_microvm.ssh_network_config(
+            network_config,
+            str(i)
+        )
+        guest_ips.append(guest_ip)
+
+    test_microvm.start()
+
+    # Test that network devices attached are operational.
+    for i in range(MAX_DEVICES_ATTACHED - 1):
+        test_microvm.ssh_config['hostname'] = guest_ips[i]
+        ssh_connection = net_tools.SSHConnection(test_microvm.ssh_config)
+        # Verify if guest can run commands.
+        exit_code, _, _ = ssh_connection.execute_command("sync")
+        assert exit_code == 0
+
+
+@pytest.mark.skipif(
+    platform.machine() != "x86_64",
+    reason="Firecracker supports 24 IRQs on x86_64."
+)
+def test_attach_too_many_devices(test_microvm_with_ssh, network_config):
+    """Test attaching to a microVM more devices than available IRQs."""
+    test_microvm = test_microvm_with_ssh
+    test_microvm.spawn()
+
+    # Set up a basic microVM.
+    test_microvm.basic_config()
+
+    # Add `MAX_DEVICES_ATTACHED` network devices on top of the
+    # already configured rootfs.
+    for i in range(MAX_DEVICES_ATTACHED):
+        # Create tap before configuring interface.
+        _tap, _host_ip, _guest_ip = test_microvm.ssh_network_config(
+            network_config,
+            str(i)
+        )
+
+    # Attempting to start a microVM with more than
+    # `MAX_DEVICES_ATTACHED` devices should fail.
+    response = test_microvm.actions.put(action_type='InstanceStart')
+    assert test_microvm.api_session.is_status_bad_request(response.status_code)
+    assert "no more IRQs are available" in response.text