Merge branch 'main' into fix-4547

refactor(builder): address comments

address comments

Signed-off-by: tommady <[email protected]>

Author: tommady

src/vmm/src/builder.rs

@@ -274,8 +274,6 @@ pub mod aarch64 {
             vcpus.push(vcpu);
         }
 
-        setup_interrupt_controller(vm, vcpu_count)?;
-
         Ok(vcpus)
     }
 
@@ -317,6 +315,9 @@ pub mod aarch64 {
         let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
             .map_err(StartMicrovmError::Internal)?;
 
+        setup_interrupt_controller(&mut vmm.vm, vm_config.vcpu_count)
+            .map_err(StartMicrovmError::Internal)?;
+
         Ok((vmm, vcpus))
     }
 }
@@ -459,6 +460,11 @@ pub mod x86_64 {
             kvm_capabilities,
         )?;
 
+        setup_interrupt_controller(&mut vmm.vm).map_err(StartMicrovmError::Internal)?;
+        vmm.pio_device_manager
+            .register_devices(vmm.vm.fd())
+            .unwrap();
+
         let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
             .map_err(StartMicrovmError::Internal)?;
 
@@ -478,9 +484,7 @@ fn build_vmm(
     // Set up Kvm Vm and register memory regions.
     // Build custom CPU config if a custom template is provided.
     //
-    // allow unused_mut for the aarch64 platform.
-    #[allow(unused_mut)]
-    let mut vm = Vm::new(kvm_capabilities)
+    let vm = Vm::new(kvm_capabilities)
         .map_err(VmmError::Vm)
         .map_err(Internal)?;
     vm.memory_init(&guest_memory, vm_config.track_dirty_pages)
@@ -499,8 +503,6 @@ fn build_vmm(
     // Instantiate ACPI device manager.
     let acpi_device_manager = ACPIDeviceManager::new();
 
-    // For x86_64 we need to create the interrupt controller before calling `KVM_CREATE_VCPUS`
-    // while on aarch64 we need to do it the other way around.
     #[cfg(target_arch = "x86_64")]
     let pio_device_manager = {
         // Serial device setup.
@@ -513,17 +515,9 @@ fn build_vmm(
             .map_err(VmmError::EventFd)
             .map_err(Internal)?;
 
-        x86_64::setup_interrupt_controller(&mut vm).map_err(Internal)?;
-
         // create pio dev manager with legacy devices
-        let pio_device_manager = {
-            // TODO Remove these unwraps.
-            let mut pio_dev_mgr = PortIODeviceManager::new(serial_device, reset_evt).unwrap();
-            pio_dev_mgr.register_devices(vm.fd()).unwrap();
-            pio_dev_mgr
-        };
-
-        pio_device_manager
+        // TODO Remove this unwraps.
+        PortIODeviceManager::new(serial_device, reset_evt).unwrap()
     };
 
     Ok(Vmm {
@@ -802,6 +796,7 @@ pub fn build_microvm_from_snapshot(
     )?;
     #[cfg(target_arch = "x86_64")]
     event_manager.add_subscriber(vmm.pio_device_manager.stdio_serial.clone());
+
     #[cfg(target_arch = "aarch64")]
     let (mut vmm, mut vcpus) = aarch64::create_vmm_and_vcpus(
         instance_info,

src/vmm/src/device_manager/persist.rs

@@ -423,6 +423,9 @@ impl<'a> Persist<'a> for MMIODeviceManager {
             for state in &state.legacy_devices {
                 if state.type_ == DeviceType::Serial {
                     let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
+                    constructor_args
+                        .event_manager
+                        .add_subscriber(serial.clone());
 
                     constructor_args
                         .resource_allocator

tools/devtool

@@ -418,6 +418,10 @@ cmd_help() {
     echo "        Builds the rootfs and guest kernel artifacts we use for our CI."
     echo "        Run './tools/devtool build_ci_artifacts help' for more details about the available commands."
     echo ""
+    echo "    download_ci_artifacts [--force]"
+    echo "        Downloads the CI artifacts used for testing from our S3 bucket. If --force is passed, purges any existing"
+    echo "        artifacts first. Useful for refreshing local artifacts after an update, or if something got messed up."
+    echo ""
 
     cat <<EOF
     test_debug [-- [<pytest args>]]
@@ -555,14 +559,23 @@ cmd_distclean() {
     fi
 }
 
+cmd_download_ci_artifacts() {
+    if [ "$1" = "--force" ]; then
+        rm -rf $FC_BUILD_DIR/img
+    fi
+
+    ensure_ci_artifacts
+}
+
 ensure_ci_artifacts() {
     if ! command -v aws >/dev/null; then
       die "AWS CLI not installed, which is required for downloading artifacts for integration tests."
     fi
 
     # Fetch all the artifacts so they are local
     say "Fetching CI artifacts from S3"
-    S3_URL=s3://spec.ccfc.min/firecracker-ci/v1.11/$(uname -m)
+    FC_VERSION=$(cmd_sh "cd src/firecracker/src; cargo pkgid | cut -d# -f2 | cut -d. -f1-2")
+    S3_URL=s3://spec.ccfc.min/firecracker-ci/v$FC_VERSION/$(uname -m)
     ARTIFACTS=$MICROVM_IMAGES_DIR/$(uname -m)
     if [ ! -d "$ARTIFACTS" ]; then
         mkdir -pv $ARTIFACTS
@@ -878,7 +891,6 @@ cmd_shell() {
 
 cmd_sh() {
     ensure_build_dir
-    ensure_ci_artifacts
     run_devctr \
         --privileged \
         --ulimit nofile=4096:4096 \
@@ -916,6 +928,7 @@ cmd_sandbox_native() {
 }
 
 cmd_test_debug() {
+    ensure_ci_artifacts
     cmd_sh "tmux new ./tools/test.sh --pdb $@"
 }