From 2a87db8cfdfb468e6cc64a5c718dd58a5cd3f9bc Mon Sep 17 00:00:00 2001 From: James Curtis Date: Wed, 28 Aug 2024 08:34:00 +0000 Subject: [PATCH 1/3] Change hot-plug API endpoint Change from `add` to `target`. The user now supplies the number of vCPUs they would like to reach, rather than the number of vCPUs to add to the system. Obviously to make this work, hot-unplug functionality is needed as well. Examples: - Currently 1 vCPU, would like 16 vCPUs: target = 16 - Currently 32 vCPUs, would like 2 vCPUs: target = 2 Signed-off-by: James Curtis --- src/vmm/src/lib.rs | 22 +++++++++------------- src/vmm/src/rpc_interface.rs | 16 ++++++++-------- src/vmm/src/vmm_config/hotplug.rs | 4 ++-- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 6d6445686a2..0ed613527af 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -615,23 +615,15 @@ impl Vmm { config: HotplugVcpuConfig, ) -> Result { use crate::logger::IncMetric; - if config.add < 1 { - return Err(HotplugVcpuError::VcpuCountTooLow); - } else if self - .vcpus_handles - .len() - .checked_add(config.add.into()) - .ok_or(HotplugVcpuError::VcpuCountTooHigh)? - > MAX_SUPPORTED_VCPUS.into() - { + if config.target > MAX_SUPPORTED_VCPUS.into() { return Err(HotplugVcpuError::VcpuCountTooHigh); } if let Some(kvm_config) = self.vcpu_config.as_mut() { - kvm_config.vcpu_count += config.add; + kvm_config.vcpu_count = config.target; } // Create and start new vcpus - let mut vcpus = Vec::with_capacity(config.add.into()); + let mut vcpus = Vec::with_capacity(config.target.into()); #[allow(clippy::cast_possible_truncation)] let start_idx = self.vcpus_handles.len().try_into().unwrap(); @@ -639,7 +631,7 @@ impl Vmm { self.get_bus_device(DeviceType::CpuContainer, "CpuContainer") { let mut locked_container = cont.lock().expect("Poisoned lock"); - for cpu_idx in start_idx..(start_idx + config.add) { + for cpu_idx in start_idx..config.target { let exit_evt = self .vcpus_exit_evt .try_clone() @@ -666,7 +658,10 @@ impl Vmm { .map_err(HotplugVcpuError::VcpuStart)?; #[allow(clippy::cast_lossless)] - METRICS.hotplug.vcpus_added.add(config.add.into()); + METRICS + .hotplug + .vcpus_added + .add(self.vcpus_handles.len() as u64 - config.target as u64); // Update VM config to reflect new CPUs added #[allow(clippy::cast_possible_truncation)] @@ -686,6 +681,7 @@ impl Vmm { Ok(new_machine_config) } + /// Removes vCPUs from VMM. /// Retrieves the KVM dirty bitmap for each of the guest's memory regions. pub fn reset_dirty_bitmap(&self) { self.guest_memory diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 92db9a61e18..ae09a18f571 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -1914,7 +1914,7 @@ mod tests { #[cfg(target_arch = "x86_64")] check_preboot_request_err( - VmmAction::HotplugRequest(HotplugRequestConfig::Vcpu(HotplugVcpuConfig { add: 4 })), + VmmAction::HotplugRequest(HotplugRequestConfig::Vcpu(HotplugVcpuConfig { target: 4 })), VmmActionError::OperationNotSupportedPreBoot, ); } @@ -2166,7 +2166,7 @@ mod tests { }; vmm.attach_vcpu_config(vcpu_config.clone()); - let config = HotplugVcpuConfig { add: 4 }; + let config = HotplugVcpuConfig { target: 4 }; let result = vmm.hotplug_vcpus(config); assert_eq!(vmm.vcpus_handles.len(), 4); result.unwrap(); @@ -2174,9 +2174,9 @@ mod tests { // Case 2. Vcpu count too low let mut vmm = default_vmm(); vmm.attach_vcpu_config(vcpu_config.clone()); - vmm.hotplug_vcpus(HotplugVcpuConfig { add: 1 }).unwrap(); + vmm.hotplug_vcpus(HotplugVcpuConfig { target: 1 }).unwrap(); assert_eq!(vmm.vcpus_handles.len(), 1); - let config = HotplugVcpuConfig { add: 0 }; + let config = HotplugVcpuConfig { target: 0 }; let result = vmm.hotplug_vcpus(config); result.unwrap_err(); assert_eq!(vmm.vcpus_handles.len(), 1); @@ -2184,9 +2184,9 @@ mod tests { // Case 3. Vcpu count too high let mut vmm = default_vmm(); vmm.attach_vcpu_config(vcpu_config.clone()); - vmm.hotplug_vcpus(HotplugVcpuConfig { add: 1 }).unwrap(); + vmm.hotplug_vcpus(HotplugVcpuConfig { target: 1 }).unwrap(); assert_eq!(vmm.vcpus_handles.len(), 1); - let config = HotplugVcpuConfig { add: 33 }; + let config = HotplugVcpuConfig { target: 33 }; let result = vmm.hotplug_vcpus(config); result.unwrap_err(); assert_eq!(vmm.vcpus_handles.len(), 1); @@ -2194,9 +2194,9 @@ mod tests { // Case 4. Attempted overflow of vcpus let mut vmm = default_vmm(); vmm.attach_vcpu_config(vcpu_config.clone()); - vmm.hotplug_vcpus(HotplugVcpuConfig { add: 2 }).unwrap(); + vmm.hotplug_vcpus(HotplugVcpuConfig { target: 2 }).unwrap(); assert_eq!(vmm.vcpus_handles.len(), 2); - let config = HotplugVcpuConfig { add: 255 }; + let config = HotplugVcpuConfig { target: 255 }; let result = vmm.hotplug_vcpus(config); result.unwrap_err(); assert_eq!(vmm.vcpus_handles.len(), 2); diff --git a/src/vmm/src/vmm_config/hotplug.rs b/src/vmm/src/vmm_config/hotplug.rs index d497dda1a0e..b35a6f10e13 100644 --- a/src/vmm/src/vmm_config/hotplug.rs +++ b/src/vmm/src/vmm_config/hotplug.rs @@ -50,6 +50,6 @@ pub enum HotplugVcpuError { #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct HotplugVcpuConfig { - /// Number of vcpus to start. - pub add: u8, + /// Number of vcpus after operation. + pub target: u8, } From 0bb130afa680597f8d9188cf496feb406bd13bbd Mon Sep 17 00:00:00 2001 From: James Curtis Date: Wed, 28 Aug 2024 08:38:35 +0000 Subject: [PATCH 2/3] Implement hot-unplug on the Guest side Guest side hot-unplug implementation. This means that the API call can be made, and the vCPUs are successfully removed from the guest, but the backing vCPU threads are not removed from the host VMM. As a result of this, hot-plugs that occur after hot-unplugs do not work correctly right now. Once the total thread count in the VMM exceeds 32, there is no effect of hot-plugging. To complete this, a refactor is needed of the VMM, such that the CpuContainer can somehow remove the threads when the guest kernel calls _EJ0. Signed-off-by: James Curtis --- src/vmm/src/devices/acpi/cpu_container.rs | 52 ++++++++++++++++++++++- src/vmm/src/lib.rs | 48 ++++++++++++++++++++- src/vmm/src/rpc_interface.rs | 32 +++++++++++--- 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/src/vmm/src/devices/acpi/cpu_container.rs b/src/vmm/src/devices/acpi/cpu_container.rs index 54c86639d55..429ceb9d073 100644 --- a/src/vmm/src/devices/acpi/cpu_container.rs +++ b/src/vmm/src/devices/acpi/cpu_container.rs @@ -51,6 +51,8 @@ pub const CPU_CONTAINER_ACPI_SIZE: usize = 0xC; const CPU_ENABLE_BIT: u8 = 1 << 0; const CPU_INSERTING_BIT: u8 = 1 << 1; +const CPU_REMOVING_BIT: u8 = 1 << 2; +const CPU_EJECT_BIT: u8 = 1 << 3; const CPU_SELECTION_OFFSET: u64 = 0; const CPU_STATUS_OFFSET: u64 = 4; @@ -94,6 +96,7 @@ impl CpuContainer { cpu_id: i, active: i < boot_count, inserting: false, + removing: false, }) } @@ -123,6 +126,9 @@ impl CpuContainer { if cpu_device.inserting { data[0] |= CPU_INSERTING_BIT; } + if cpu_device.removing { + data[0] |= CPU_REMOVING_BIT; + } } else { error!("Out of range vCPU id: {}", self.selected_cpu) } @@ -143,6 +149,10 @@ impl CpuContainer { if data[0] & CPU_ENABLE_BIT != 0 { cpu_device.active = true; } + if data[0] & CPU_EJECT_BIT != 0 { + cpu_device.active = false; + // TODO: Remove vCPU handle from VMM + } } else { error!("Out of range vCPU id: {}", self.selected_cpu) } @@ -215,7 +225,9 @@ impl Aml for CpuContainer { aml::FieldEntry::Reserved(32), aml::FieldEntry::Named(*b"CPEN", 1), aml::FieldEntry::Named(*b"CINS", 1), - aml::FieldEntry::Reserved(6), + aml::FieldEntry::Named(*b"CRMV", 1), + aml::FieldEntry::Named(*b"CEJ0", 1), + aml::FieldEntry::Reserved(4), aml::FieldEntry::Named(*b"CCMD", 8), ], ), @@ -270,6 +282,8 @@ pub struct CpuDevice { pub active: bool, /// Whether this CPU is in the process of being inserted pub inserting: bool, + /// Whether this CPU is in the process of being removed + pub removing: bool, } impl CpuDevice { @@ -305,6 +319,16 @@ impl Aml for CpuDevice { ))], ), &aml::Name::new("_MAT".into(), &aml::Buffer::new(mat_data)), + &aml::Method::new( + "_EJ0".into(), + 1, + false, + // Call into CEJ0 method which will actually eject device + vec![&aml::MethodCall::new( + "\\_SB_.CPUS.CEJ0".into(), + vec![&self.cpu_id], + )], + ), ], ) .append_aml_bytes(v) @@ -320,7 +344,7 @@ impl Aml for CpuNotify { let object = aml::Path::new(&format!("C{:03X}", self.cpu_id)); aml::If::new( &aml::Equal::new(&aml::Arg(0), &self.cpu_id), - vec![&aml::Notify::new(&object, &1u8)], + vec![&aml::Notify::new(&object, &aml::Arg(1))], ) .append_aml_bytes(v) } @@ -369,6 +393,21 @@ impl Aml for CpuMethods { aml::Method::new("CTFY".into(), 2, true, cpu_notifies_refs).append_aml_bytes(v); + aml::Method::new( + "CEJ0".into(), + 1, + true, + vec![ + &aml::Acquire::new("\\_SB_.PRES.CPLK".into(), 0xffff), + // Write CPU number (in first argument) to I/O port via field + &aml::Store::new(&aml::Path::new("\\_SB_.PRES.CSEL"), &aml::Arg(0)), + // Set CEJ0 bit + &aml::Store::new(&aml::Path::new("\\_SB_.PRES.CEJ0"), &aml::ONE), + &aml::Release::new("\\_SB_.PRES.CPLK".into()), + ], + ) + .append_aml_bytes(v); + aml::Method::new( "CSCN".into(), 0, @@ -396,6 +435,15 @@ impl Aml for CpuMethods { &aml::Store::new(&aml::Path::new("\\_SB_.PRES.CINS"), &aml::ONE), ], ), + &aml::If::new( + &aml::Equal::new(&aml::Path::new("\\_SB_.PRES.CRMV"), &aml::ONE), + // Notify device if it is (with the eject constant 0x3) + vec![ + &aml::MethodCall::new("CTFY".into(), vec![&aml::Local(0), &3u8]), + // Reset CRMV bit + &aml::Store::new(&aml::Path::new("\\_SB_.PRES.CRMV"), &aml::ONE), + ], + ), &aml::Add::new(&aml::Local(0), &aml::Local(0), &aml::ONE), ], ), diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 0ed613527af..d11fa27f849 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -615,7 +615,7 @@ impl Vmm { config: HotplugVcpuConfig, ) -> Result { use crate::logger::IncMetric; - if config.target > MAX_SUPPORTED_VCPUS.into() { + if config.target > MAX_SUPPORTED_VCPUS { return Err(HotplugVcpuError::VcpuCountTooHigh); } @@ -682,6 +682,52 @@ impl Vmm { } /// Removes vCPUs from VMM. + #[cfg(target_arch = "x86_64")] + pub fn hotunplug_vcpus( + &mut self, + config: HotplugVcpuConfig, + ) -> Result { + use crate::logger::IncMetric; + if config.target < 1 { + return Err(HotplugVcpuError::VcpuCountTooLow); + } + if let Some(kvm_config) = self.vcpu_config.as_mut() { + kvm_config.vcpu_count = config.target; + } + + #[allow(clippy::cast_possible_truncation)] + let start_idx: u8 = config.target; + if let Some(devices::BusDevice::CpuContainer(cont)) = + self.get_bus_device(DeviceType::CpuContainer, "CpuContainer") + { + let mut locked_container = cont.lock().expect("Poisoned lock"); + for cpu_idx in start_idx..u8::try_from(self.vcpus_handles.len()).unwrap() { + locked_container.cpu_devices[cpu_idx as usize].removing = true; + } + } + + #[allow(clippy::cast_lossless)] + METRICS + .hotplug + .vcpus_added + .add(self.vcpus_handles.len() as u64 - config.target as u64); + + // Update VM config to reflect new CPUs added + #[allow(clippy::cast_possible_truncation)] + let new_machine_config = MachineConfigUpdate { + vcpu_count: Some(self.vcpus_handles.len() as u8), + mem_size_mib: None, + smt: None, + cpu_template: None, + track_dirty_pages: None, + huge_pages: None, + }; + + self.acpi_device_manager.notify_cpu_container()?; + + Ok(new_machine_config) + } + /// Retrieves the KVM dirty bitmap for each of the guest's memory regions. pub fn reset_dirty_bitmap(&self) { self.guest_memory diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index ae09a18f571..ae55c5e4567 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -670,7 +670,17 @@ impl RuntimeApiController { self.vmm.lock().expect("Poisoned lock").version(), )), #[cfg(target_arch = "x86_64")] - HotplugRequest(request_type) => self.handle_hotplug_request(request_type), + HotplugRequest(request_type) => { + let curr_vcpus: u8 = self + .vmm + .lock() + .expect("Poisoned lock") + .vcpus_handles + .len() + .try_into() + .unwrap(); + self.handle_hotplug_request(request_type, curr_vcpus) + } PatchMMDS(value) => self.patch_mmds(value), Pause => self.pause(), PutMMDS(value) => self.put_mmds(value), @@ -872,13 +882,25 @@ impl RuntimeApiController { fn handle_hotplug_request( &mut self, cfg: HotplugRequestConfig, + curr_vcpus: u8, ) -> Result { match cfg { HotplugRequestConfig::Vcpu(cfg) => { - let result = self.vmm.lock().expect("Poisoned lock").hotplug_vcpus(cfg); - result - .map_err(|err| VmmActionError::HotplugRequest(HotplugRequestError::Vcpu(err))) - .and_then(|machine_cfg_update| self.update_vm_config(machine_cfg_update)) + if cfg.target > curr_vcpus { + let result = self.vmm.lock().expect("Poisoned lock").hotplug_vcpus(cfg); + result + .map_err(|err| { + VmmActionError::HotplugRequest(HotplugRequestError::Vcpu(err)) + }) + .and_then(|machine_cfg_update| self.update_vm_config(machine_cfg_update)) + } else { + let result = self.vmm.lock().expect("Poisoned lock").hotunplug_vcpus(cfg); + result + .map_err(|err| { + VmmActionError::HotplugRequest(HotplugRequestError::Vcpu(err)) + }) + .and_then(|machine_cfg_update| self.update_vm_config(machine_cfg_update)) + } } } } From 99858de966afb46eb47806a5d7b42d2130f96d6d Mon Sep 17 00:00:00 2001 From: James Curtis Date: Wed, 28 Aug 2024 16:21:30 +0000 Subject: [PATCH 3/3] Update metrics and swagger Update metrics and swagger to reflect the changes made to the API. Signed-off-by: James Curtis --- src/firecracker/swagger/firecracker.yaml | 6 ++++-- src/vmm/src/logger/metrics.rs | 2 ++ tests/host_tools/fcmetrics.py | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/firecracker/swagger/firecracker.yaml b/src/firecracker/swagger/firecracker.yaml index a7f1c49695f..f33ecb65952 100644 --- a/src/firecracker/swagger/firecracker.yaml +++ b/src/firecracker/swagger/firecracker.yaml @@ -976,10 +976,12 @@ definitions: type: object description: Hotplug requests relating to vCPU cores. properties: - add: - description: Number of vCPUs cores to be added to the VM + target: + description: The new number of vCPUs the system should have. type: integer minimum: 1 + maximum: 32 + InstanceActionInfo: diff --git a/src/vmm/src/logger/metrics.rs b/src/vmm/src/logger/metrics.rs index 89ea8abd38b..623557fd1e8 100644 --- a/src/vmm/src/logger/metrics.rs +++ b/src/vmm/src/logger/metrics.rs @@ -837,6 +837,7 @@ pub struct HotplugMetrics { pub hotplug_request_fails: SharedIncMetric, pub vcpu_hotplug_request_fails: SharedIncMetric, pub vcpus_added: SharedIncMetric, + pub vcpus_removed: SharedIncMetric, } impl HotplugMetrics { @@ -848,6 +849,7 @@ impl HotplugMetrics { hotplug_request_fails: SharedIncMetric::new(), vcpu_hotplug_request_fails: SharedIncMetric::new(), vcpus_added: SharedIncMetric::new(), + vcpus_removed: SharedIncMetric::new(), } } } diff --git a/tests/host_tools/fcmetrics.py b/tests/host_tools/fcmetrics.py index 74f4cba904e..01789a12f02 100644 --- a/tests/host_tools/fcmetrics.py +++ b/tests/host_tools/fcmetrics.py @@ -157,6 +157,7 @@ def validate_fc_metrics(metrics): "hotplug_request_fails", "vcpu_hotplug_request_fails", "vcpus_added", + "vcpus_removed", ], "i8042": [ "error_count",