vmm: protect vcpu states in CpuManager with a mutex

This is a prerequisite for the next commit where we need shared access.

On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
This commit is contained in:
Philipp Schuster
2026-04-09 22:24:37 +02:00
committed by Rob Bradford
parent c657ea6e23
commit 6d0d4bc5e2

View File

@@ -682,7 +682,7 @@ pub struct CpuManager {
reset_evt: EventFd, reset_evt: EventFd,
#[cfg(feature = "guest_debug")] #[cfg(feature = "guest_debug")]
vm_debug_evt: EventFd, vm_debug_evt: EventFd,
vcpu_states: Vec<VcpuState>, vcpu_states: Arc<Mutex<Vec<VcpuState>>>,
selected_cpu: u32, selected_cpu: u32,
vcpus: Vec<Arc<Mutex<Vcpu>>>, vcpus: Vec<Arc<Mutex<Vcpu>>>,
seccomp_action: SeccompAction, seccomp_action: SeccompAction,
@@ -741,6 +741,7 @@ impl BusDevice for CpuManager {
fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) { fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) {
// The Linux kernel, quite reasonably, doesn't zero the memory it gives us. // The Linux kernel, quite reasonably, doesn't zero the memory it gives us.
data.fill(0); data.fill(0);
let vcpu_states = self.vcpu_states.lock().unwrap();
match offset { match offset {
CPU_SELECTION_OFFSET => { CPU_SELECTION_OFFSET => {
@@ -750,7 +751,7 @@ impl BusDevice for CpuManager {
} }
CPU_STATUS_OFFSET => { CPU_STATUS_OFFSET => {
if self.selected_cpu < self.max_vcpus() { if self.selected_cpu < self.max_vcpus() {
let state = &self.vcpu_states[usize::try_from(self.selected_cpu).unwrap()]; let state = &vcpu_states[usize::try_from(self.selected_cpu).unwrap()];
if state.active() { if state.active() {
data[0] |= 1 << CPU_ENABLE_FLAG; data[0] |= 1 << CPU_ENABLE_FLAG;
} }
@@ -779,23 +780,28 @@ impl BusDevice for CpuManager {
} }
CPU_STATUS_OFFSET => { CPU_STATUS_OFFSET => {
if self.selected_cpu < self.max_vcpus() { if self.selected_cpu < self.max_vcpus() {
let state = &mut self.vcpu_states[usize::try_from(self.selected_cpu).unwrap()]; let eject = {
// The ACPI code writes back a 1 to acknowledge the insertion // This structure is not shared with the vCPU thread, therefore, holding the
if (data[0] & (1 << CPU_INSERTING_FLAG) == 1 << CPU_INSERTING_FLAG) // lock for the entire function doesn't cause any deadlock.
&& state.inserting let mut vcpu_states = self.vcpu_states.lock().unwrap();
{ let state = &mut vcpu_states[usize::try_from(self.selected_cpu).unwrap()];
state.inserting = false;
} if (data[0] & (1 << CPU_INSERTING_FLAG) == 1 << CPU_INSERTING_FLAG)
// Ditto for removal && state.inserting
if (data[0] & (1 << CPU_REMOVING_FLAG) == 1 << CPU_REMOVING_FLAG) {
&& state.removing state.inserting = false;
{ }
state.removing = false;
} if (data[0] & (1 << CPU_REMOVING_FLAG) == 1 << CPU_REMOVING_FLAG)
// Trigger removal of vCPU && state.removing
if data[0] & (1 << CPU_EJECT_FLAG) == 1 << CPU_EJECT_FLAG {
&& let Err(e) = self.remove_vcpu(self.selected_cpu) state.removing = false;
{ }
data[0] & (1 << CPU_EJECT_FLAG) == 1 << CPU_EJECT_FLAG
};
if eject && let Err(e) = self.remove_vcpu(self.selected_cpu) {
error!("Error removing vCPU: {e:?}"); error!("Error removing vCPU: {e:?}");
} }
} else { } else {
@@ -907,6 +913,7 @@ impl CpuManager {
let max_vcpus = usize::try_from(config.max_vcpus).unwrap(); let max_vcpus = usize::try_from(config.max_vcpus).unwrap();
let mut vcpu_states = Vec::with_capacity(max_vcpus); let mut vcpu_states = Vec::with_capacity(max_vcpus);
vcpu_states.resize_with(max_vcpus, VcpuState::default); vcpu_states.resize_with(max_vcpus, VcpuState::default);
let vcpu_states = Arc::new(Mutex::new(vcpu_states));
let hypervisor_type = hypervisor.hypervisor_type(); let hypervisor_type = hypervisor.hypervisor_type();
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
let cpu_vendor = hypervisor.get_cpu_vendor(); let cpu_vendor = hypervisor.get_cpu_vendor();
@@ -1176,14 +1183,14 @@ impl CpuManager {
let vcpus_pause_signalled = self.vcpus_pause_signalled.clone(); let vcpus_pause_signalled = self.vcpus_pause_signalled.clone();
let vcpus_kick_signalled = self.vcpus_kick_signalled.clone(); let vcpus_kick_signalled = self.vcpus_kick_signalled.clone();
let vcpu_kill = self.vcpu_states[usize::try_from(vcpu_id).unwrap()] let mut vcpu_states = self.vcpu_states.lock().unwrap();
.kill
.clone(); let vcpu_kill = vcpu_states[usize::try_from(vcpu_id).unwrap()].kill.clone();
let vcpu_run_interrupted = self.vcpu_states[usize::try_from(vcpu_id).unwrap()] let vcpu_run_interrupted = vcpu_states[usize::try_from(vcpu_id).unwrap()]
.vcpu_run_interrupted .vcpu_run_interrupted
.clone(); .clone();
let panic_vcpu_run_interrupted = vcpu_run_interrupted.clone(); let panic_vcpu_run_interrupted = vcpu_run_interrupted.clone();
let vcpu_paused = self.vcpu_states[usize::try_from(vcpu_id).unwrap()] let vcpu_paused = vcpu_states[usize::try_from(vcpu_id).unwrap()]
.paused .paused
.clone(); .clone();
@@ -1470,8 +1477,8 @@ impl CpuManager {
// On hot plug calls into this function entry_point is None. It is for // On hot plug calls into this function entry_point is None. It is for
// those hotplug CPU additions that we need to set the inserting flag. // those hotplug CPU additions that we need to set the inserting flag.
self.vcpu_states[usize::try_from(vcpu_id).unwrap()].handle = handle; vcpu_states[usize::try_from(vcpu_id).unwrap()].handle = handle;
self.vcpu_states[usize::try_from(vcpu_id).unwrap()].inserting = inserting; vcpu_states[usize::try_from(vcpu_id).unwrap()].inserting = inserting;
Ok(()) Ok(())
} }
@@ -1515,17 +1522,20 @@ impl CpuManager {
} }
fn mark_vcpus_for_removal(&mut self, desired_vcpus: u32) { fn mark_vcpus_for_removal(&mut self, desired_vcpus: u32) {
let mut vcpu_states = self.vcpu_states.lock().unwrap();
let present_vcpus = Self::active_vcpus(&vcpu_states);
// Mark vCPUs for removal, actual removal happens on ejection // Mark vCPUs for removal, actual removal happens on ejection
for cpu_id in desired_vcpus..self.present_vcpus() { for cpu_id in desired_vcpus..present_vcpus {
self.vcpu_states[usize::try_from(cpu_id).unwrap()].removing = true; vcpu_states[usize::try_from(cpu_id).unwrap()].removing = true;
self.vcpu_states[usize::try_from(cpu_id).unwrap()] vcpu_states[usize::try_from(cpu_id).unwrap()]
.pending_removal .pending_removal
.store(true, Ordering::SeqCst); .store(true, Ordering::SeqCst);
} }
} }
pub fn check_pending_removed_vcpu(&mut self) -> bool { pub fn check_pending_removed_vcpu(&mut self) -> bool {
for state in self.vcpu_states.iter() { for state in self.vcpu_states.lock().unwrap().iter() {
if state.active() && state.pending_removal.load(Ordering::SeqCst) { if state.active() && state.pending_removal.load(Ordering::SeqCst) {
return true; return true;
} }
@@ -1535,7 +1545,8 @@ impl CpuManager {
fn remove_vcpu(&mut self, cpu_id: u32) -> Result<()> { fn remove_vcpu(&mut self, cpu_id: u32) -> Result<()> {
info!("Removing vCPU: cpu_id = {cpu_id}"); info!("Removing vCPU: cpu_id = {cpu_id}");
let state = &mut self.vcpu_states[usize::try_from(cpu_id).unwrap()]; let mut vcpu_states = self.vcpu_states.lock().unwrap();
let state = &mut vcpu_states[usize::try_from(cpu_id).unwrap()];
state.kill.store(true, Ordering::SeqCst); state.kill.store(true, Ordering::SeqCst);
state.signal_thread(); state.signal_thread();
state.wait_until_signal_acknowledged()?; state.wait_until_signal_acknowledged()?;
@@ -1631,12 +1642,15 @@ impl CpuManager {
/// For the vCPU threads this will interrupt the KVM_RUN ioctl() allowing /// For the vCPU threads this will interrupt the KVM_RUN ioctl() allowing
/// the loop to check the shared state booleans. /// the loop to check the shared state booleans.
fn signal_vcpus(&mut self) -> Result<()> { fn signal_vcpus(&mut self) -> Result<()> {
// Holding the lock for the whole operation is correct:
let vcpu_states = self.vcpu_states.lock().unwrap();
// Splitting this into two loops reduced the time to pause many vCPUs // Splitting this into two loops reduced the time to pause many vCPUs
// massively. Example: 254 vCPUs. >254ms -> ~4ms. // massively. Example: 254 vCPUs. >254ms -> ~4ms.
for state in self.vcpu_states.iter() { for state in vcpu_states.iter() {
state.signal_thread(); state.signal_thread();
} }
for state in self.vcpu_states.iter() { for state in vcpu_states.iter() {
state.wait_until_signal_acknowledged()?; state.wait_until_signal_acknowledged()?;
} }
@@ -1651,14 +1665,14 @@ impl CpuManager {
self.vcpus_pause_signalled.store(false, Ordering::SeqCst); self.vcpus_pause_signalled.store(false, Ordering::SeqCst);
// Unpark all the VCPU threads. // Unpark all the VCPU threads.
for state in self.vcpu_states.iter() { for state in self.vcpu_states.lock().unwrap().iter() {
state.unpark_thread(); state.unpark_thread();
} }
self.signal_vcpus()?; self.signal_vcpus()?;
// Wait for all the threads to finish. This removes the state from the vector. // Wait for all the threads to finish. This removes the state from the vector.
for mut state in self.vcpu_states.drain(..) { for mut state in self.vcpu_states.lock().unwrap().drain(..) {
state.join_thread()?; state.join_thread()?;
} }
@@ -1691,8 +1705,15 @@ impl CpuManager {
self.cpuid.clone() self.cpuid.clone()
} }
/// Locks the vCPU states and calls [`Self::active_vcpus`].
fn present_vcpus(&self) -> u32 { fn present_vcpus(&self) -> u32 {
self.vcpu_states let lock = self.vcpu_states.lock().unwrap();
Self::active_vcpus(&lock)
}
/// Counts the number of active vCPUs (running vCPU threads).
fn active_vcpus(vcpu_states: &[VcpuState]) -> u32 {
vcpu_states
.iter() .iter()
.fold(0, |acc, state| acc + state.active() as u32) .fold(0, |acc, state| acc + state.active() as u32)
} }
@@ -2651,7 +2672,7 @@ impl Pausable for CpuManager {
// The vCPU thread will change its paused state before parking, wait here for each // The vCPU thread will change its paused state before parking, wait here for each
// activated vCPU change their state to ensure they have parked. // activated vCPU change their state to ensure they have parked.
for state in self.vcpu_states.iter() { for state in self.vcpu_states.lock().unwrap().iter() {
if state.active() { if state.active() {
// wait for vCPU to update state // wait for vCPU to update state
while !state.paused.load(Ordering::SeqCst) { while !state.paused.load(Ordering::SeqCst) {
@@ -2669,16 +2690,18 @@ impl Pausable for CpuManager {
// their run vCPU loop. // their run vCPU loop.
self.vcpus_pause_signalled.store(false, Ordering::SeqCst); self.vcpus_pause_signalled.store(false, Ordering::SeqCst);
let vcpu_states = self.vcpu_states.lock().unwrap();
// Unpark all the vCPU threads. // Unpark all the vCPU threads.
// Step 1/2: signal each thread // Step 1/2: signal each thread
{ {
for state in self.vcpu_states.iter() { for state in vcpu_states.iter() {
state.unpark_thread(); state.unpark_thread();
} }
} }
// Step 2/2: wait for state ACK // Step 2/2: wait for state ACK
{ {
for state in self.vcpu_states.iter() { for state in vcpu_states.iter() {
// wait for vCPU to update state // wait for vCPU to update state
while state.paused.load(Ordering::SeqCst) { while state.paused.load(Ordering::SeqCst) {
// To avoid a priority inversion with the vCPU thread // To avoid a priority inversion with the vCPU thread