From 0f1ab38dedb9184f06f12e56f4d1d54193be7625 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 5 Aug 2020 12:00:56 +0200 Subject: [PATCH] hypervisor: kvm: Make MSRs set/get more flexible Based on the way KVM_GET_MSRS and KVM_SET_MSRS work, both function are very unlikely to fail, as they simply stop looping through the list of MSRs as soon as getting or setting one fails. This is causing some issues with the snapshot/restore feature, as on some platforms, we only save a subset of the list of MSRs, leading to unproper way of saving the VM. The way to address this issue is by checking the number of MSRs get/set matches the expected amount from the list. In case it does not match, we simply ignore the failing MSR and continue getting/setting the rest of the list. By doing this by iterations, we end up getting/setting as many MSRs as the platform can support. Signed-off-by: Sebastien Boeuf --- Cargo.lock | 1 + hypervisor/Cargo.toml | 1 + hypervisor/src/kvm/mod.rs | 82 +++++++++++++++++++++++++++++++++++++-- hypervisor/src/lib.rs | 6 ++- 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21a238d7f..cfd8d8da0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -469,6 +469,7 @@ dependencies = [ "kvm-ioctls", "libc", "linux-loader", + "log 0.4.11", "serde", "serde_derive", "serde_json", diff --git a/hypervisor/Cargo.toml b/hypervisor/Cargo.toml index ebe96a13a..8cb5dc181 100644 --- a/hypervisor/Cargo.toml +++ b/hypervisor/Cargo.toml @@ -11,6 +11,7 @@ kvm = [] anyhow = "1.0" thiserror = "1.0" libc = "0.2.74" +log = "0.4.11" kvm-ioctls = { git = "https://github.com/cloud-hypervisor/kvm-ioctls", branch = "ch" } kvm-bindings = { git = "https://github.com/cloud-hypervisor/kvm-bindings", branch = "ch", features = ["with-serde", "fam-wrappers"] } serde = {version = ">=1.0.27", features = ["rc"] } diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 507042917..de0e7d053 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -774,8 +774,55 @@ impl cpu::Vcpu for KvmVcpu { let xcrs = self.get_xcrs()?; let lapic_state = self.get_lapic()?; let fpu = self.get_fpu()?; - let mut msrs = self.msrs.clone(); - self.get_msrs(&mut msrs)?; + + // Try to get all MSRs based on the list previously retrieved from KVM. + // If the number of MSRs obtained from GET_MSRS is different from the + // expected amount, we fallback onto a slower method by getting MSRs + // by chunks. This is the only way to make sure we try to get as many + // MSRs as possible, even if some MSRs are not supported. + let mut msr_entries = self.msrs.clone(); + let expected_num_msrs = msr_entries.as_fam_struct_ref().nmsrs as usize; + let num_msrs = self.get_msrs(&mut msr_entries)?; + let msrs = if num_msrs != expected_num_msrs { + let mut faulty_msr_index = num_msrs; + let mut msr_entries_tmp = + MsrEntries::from_entries(&msr_entries.as_slice()[..faulty_msr_index]); + + loop { + warn!( + "Detected faulty MSR 0x{:x} while getting MSRs", + msr_entries.as_slice()[faulty_msr_index].index + ); + + let start_pos = faulty_msr_index + 1; + let mut sub_msr_entries = + MsrEntries::from_entries(&msr_entries.as_slice()[start_pos..]); + let expected_num_msrs = sub_msr_entries.as_fam_struct_ref().nmsrs as usize; + let num_msrs = self.get_msrs(&mut sub_msr_entries)?; + + for i in 0..num_msrs { + msr_entries_tmp + .push(sub_msr_entries.as_slice()[i]) + .map_err(|e| { + cpu::HypervisorCpuError::GetMsrEntries(anyhow!( + "Failed adding MSR entries: {:?}", + e + )) + })?; + } + + if num_msrs == expected_num_msrs { + break; + } + + faulty_msr_index = start_pos + num_msrs; + } + + msr_entries_tmp + } else { + msr_entries + }; + let vcpu_events = self.get_vcpu_events()?; Ok(CpuState { @@ -842,7 +889,36 @@ impl cpu::Vcpu for KvmVcpu { self.set_xcrs(&state.xcrs)?; self.set_lapic(&state.lapic_state)?; self.set_fpu(&state.fpu)?; - self.set_msrs(&state.msrs)?; + + // Try to set all MSRs previously stored. + // If the number of MSRs set from SET_MSRS is different from the + // expected amount, we fallback onto a slower method by setting MSRs + // by chunks. This is the only way to make sure we try to set as many + // MSRs as possible, even if some MSRs are not supported. + let expected_num_msrs = state.msrs.as_fam_struct_ref().nmsrs as usize; + let num_msrs = self.set_msrs(&state.msrs)?; + if num_msrs != expected_num_msrs { + let mut faulty_msr_index = num_msrs; + + loop { + warn!( + "Detected faulty MSR 0x{:x} while setting MSRs", + state.msrs.as_slice()[faulty_msr_index].index + ); + + let start_pos = faulty_msr_index + 1; + let sub_msr_entries = MsrEntries::from_entries(&state.msrs.as_slice()[start_pos..]); + let expected_num_msrs = sub_msr_entries.as_fam_struct_ref().nmsrs as usize; + let num_msrs = self.set_msrs(&sub_msr_entries)?; + + if num_msrs == expected_num_msrs { + break; + } + + faulty_msr_index = start_pos + num_msrs; + } + } + self.set_vcpu_events(&state.vcpu_events)?; Ok(()) diff --git a/hypervisor/src/lib.rs b/hypervisor/src/lib.rs index b9065f0d6..ad65d2a5c 100644 --- a/hypervisor/src/lib.rs +++ b/hypervisor/src/lib.rs @@ -18,12 +18,14 @@ //! - arm64 //! +#[macro_use] +extern crate anyhow; +#[macro_use] +extern crate log; extern crate serde; extern crate serde_derive; extern crate serde_json; extern crate thiserror; -#[macro_use] -extern crate anyhow; /// KVM implementation module pub mod kvm;