diff --git a/.buildkite/pipeline_pr.py b/.buildkite/pipeline_pr.py index 75dbf9e7e6e..3750ea9338a 100755 --- a/.buildkite/pipeline_pr.py +++ b/.buildkite/pipeline_pr.py @@ -33,7 +33,7 @@ def get_changed_files(branch): "platforms": DEFAULT_PLATFORMS, # buildkite step parameters "priority": DEFAULT_PRIORITY, - "timeout_in_minutes": 30, + "timeout_in_minutes": 45, } build_grp = group( diff --git a/CHANGELOG.md b/CHANGELOG.md index 30d810e84d1..11db03c400e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [Unreleased] + +### Fixes + +- Fixed the T2S CPU template to set the RRSBA bit of the IA32_ARCH_CAPABILITIES + MSR to 1 in accordance with an Intel microcode update. +- Fixed the T2CL CPU template to pass through the RSBA and RRSBA bits of the + IA32_ARCH_CAPABILITIES MSR from the host in accordance with an Intel microcode + update. + ## [1.3.3] ### Fixed diff --git a/src/arch/src/x86_64/msr.rs b/src/arch/src/x86_64/msr.rs index 23204b21c49..9772d5ba720 100644 --- a/src/arch/src/x86_64/msr.rs +++ b/src/arch/src/x86_64/msr.rs @@ -331,6 +331,36 @@ pub fn supported_guest_msrs(kvm_fd: &Kvm) -> Result { Ok(msr_list) } +/// Error type for [`get_msrs`]. +#[derive(Debug, thiserror::Error)] +pub enum GetMsrError { + /// Failed to create `Msrs`. + #[error("Could not create `Msrs`")] + Fam(utils::fam::Error), + /// Getting a MSR resulted in an error. + #[error("Getting a MSR resulted in an error: {0}")] + Get(#[from] kvm_ioctls::Error), +} + +/// Get a value for the given MSR index. +/// +/// # Arguments +/// +/// * `vcpu_fd` - Structure for the vCPU that holds the vCPU's fd. +/// * `index` - MSR index to query +pub fn get_msr(vcpu_fd: &VcpuFd, index: u32) -> std::result::Result { + let mut msrs = Msrs::from_entries(&[kvm_msr_entry { + index, + ..Default::default() + }]) + .map_err(GetMsrError::Fam)?; + + vcpu_fd.get_msrs(&mut msrs).map_err(GetMsrError::Get)?; + + // The access to the 0-th item is safe, since the size of `msrs` is 1. + Ok(msrs.as_slice()[0].data) +} + #[cfg(test)] mod tests { use kvm_ioctls::Kvm; diff --git a/src/cpuid/src/template/intel/t2cl.rs b/src/cpuid/src/template/intel/t2cl.rs index 0ae7ab86eda..c36a7f1ff26 100644 --- a/src/cpuid/src/template/intel/t2cl.rs +++ b/src/cpuid/src/template/intel/t2cl.rs @@ -73,16 +73,20 @@ pub fn set_cpuid_entries(kvm_cpuid: &mut CpuId, vm_spec: &VmSpec) -> Result<(), } /// Add the MSR entries speciffic to this T2CL template. -pub fn update_msr_entries(msr_entries: &mut Vec) { - let capabilities = ArchCapaMSRFlags::RDCL_NO +pub fn update_msr_entries(msr_entries: &mut Vec, default_arch_cap: u64) { + let arch_cap = ArchCapaMSRFlags::RDCL_NO | ArchCapaMSRFlags::IBRS_ALL | ArchCapaMSRFlags::SKIP_L1DFL_VMENTRY | ArchCapaMSRFlags::MDS_NO | ArchCapaMSRFlags::IF_PSCHANGE_MC_NO | ArchCapaMSRFlags::TSX_CTRL; + + // Pass through RSBA and RRSBA bits if they are set. + let rsba_rrbsa = default_arch_cap & (ArchCapaMSRFlags::RSBA | ArchCapaMSRFlags::RRSBA).bits(); + msr_entries.push(kvm_msr_entry { index: MSR_IA32_ARCH_CAPABILITIES, - data: capabilities.bits(), + data: arch_cap.bits() | rsba_rrbsa, ..Default::default() }); } @@ -220,8 +224,10 @@ mod tests { #[test] fn test_update_msr_entries() { + // Case 1: The default IA32_ARCH_CAPABILITIES MSR does not enumerate RSBA and RRSBA. let mut msrs = Vec::::new(); - update_msr_entries(&mut msrs); + let default_arch_cap = 0; + update_msr_entries(&mut msrs, default_arch_cap); let arch_cap = msrs[0]; assert_eq!(arch_cap.index, MSR_IA32_ARCH_CAPABILITIES); @@ -235,5 +241,25 @@ mod tests { | ArchCapaMSRFlags::TSX_CTRL) .bits() ); + + // Case 2: The default IA32_ARCH_CAPABILITIES MSR enumerates both RSBA and RRSBA. + let mut msrs = Vec::::new(); + let default_arch_cap = (ArchCapaMSRFlags::RSBA | ArchCapaMSRFlags::RRSBA).bits(); + update_msr_entries(&mut msrs, default_arch_cap); + let arch_cap = msrs[0]; + + assert_eq!(arch_cap.index, MSR_IA32_ARCH_CAPABILITIES); + assert_eq!( + arch_cap.data, + (ArchCapaMSRFlags::RDCL_NO + | ArchCapaMSRFlags::IBRS_ALL + | ArchCapaMSRFlags::RSBA + | ArchCapaMSRFlags::SKIP_L1DFL_VMENTRY + | ArchCapaMSRFlags::MDS_NO + | ArchCapaMSRFlags::IF_PSCHANGE_MC_NO + | ArchCapaMSRFlags::TSX_CTRL + | ArchCapaMSRFlags::RRSBA) + .bits() + ); } } diff --git a/src/cpuid/src/template/intel/t2s.rs b/src/cpuid/src/template/intel/t2s.rs index 20e845741a9..167374a21f7 100644 --- a/src/cpuid/src/template/intel/t2s.rs +++ b/src/cpuid/src/template/intel/t2s.rs @@ -35,7 +35,8 @@ pub fn update_msr_entries(msr_entries: &mut Vec) { | ArchCapaMSRFlags::SKIP_L1DFL_VMENTRY | ArchCapaMSRFlags::IF_PSCHANGE_MC_NO | ArchCapaMSRFlags::MISC_PACKAGE_CTRLS - | ArchCapaMSRFlags::ENERGY_FILTERING_CTL; + | ArchCapaMSRFlags::ENERGY_FILTERING_CTL + | ArchCapaMSRFlags::RRSBA; msr_entries.push(kvm_msr_entry { index: MSR_IA32_ARCH_CAPABILITIES, data: capabilities.bits(), diff --git a/src/vmm/src/vstate/vcpu/x86_64.rs b/src/vmm/src/vstate/vcpu/x86_64.rs index f578f6e089c..0bd02f57e22 100644 --- a/src/vmm/src/vstate/vcpu/x86_64.rs +++ b/src/vmm/src/vstate/vcpu/x86_64.rs @@ -10,7 +10,7 @@ use std::fmt::{Display, Formatter}; use std::{fmt, result}; use arch::x86_64::interrupts; -use arch::x86_64::msr::SetMSRsError; +use arch::x86_64::msr::{GetMsrError, SetMSRsError, MSR_IA32_ARCH_CAPABILITIES}; use arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError}; use cpuid::{c3, filter_cpuid, msrs_to_save_by_cpuid, t2, t2a, t2cl, t2s, VmSpec}; use kvm_bindings::{ @@ -191,6 +191,8 @@ pub enum KvmVcpuConfigureError { VmSpec(cpuid::Error), #[error("Failed to filter CPUID: {0}")] FilterCpuid(cpuid::Error), + #[error("Failed to get a MSR: {0}")] + GetMsr(#[from] GetMsrError), #[error("Failed to set CPUID entries: {0}")] SetCpuidEntries(cpuid::Error), #[error("Failed to set CPUID: {0}")] @@ -317,7 +319,9 @@ impl KvmVcpu { } CpuFeaturesTemplate::T2CL => { self.msr_list.extend(t2cl::msr_entries_to_save()); - t2cl::update_msr_entries(&mut msr_boot_entries); + let default_arch_cap = + arch::x86_64::msr::get_msr(&self.fd, MSR_IA32_ARCH_CAPABILITIES)?; + t2cl::update_msr_entries(&mut msr_boot_entries, default_arch_cap); } _ => (), } diff --git a/tests/integration_tests/build/test_coverage.py b/tests/integration_tests/build/test_coverage.py index 1a9aae6a7d8..ddee25fd1f0 100644 --- a/tests/integration_tests/build/test_coverage.py +++ b/tests/integration_tests/build/test_coverage.py @@ -17,7 +17,7 @@ # Checkout the cpuid crate. In the future other # differences may appear. if utils.is_io_uring_supported(): - COVERAGE_DICT = {"Intel": 82.98, "AMD": 82.14, "ARM": 82.43} + COVERAGE_DICT = {"Intel": 82.95, "AMD": 82.14, "ARM": 82.43} else: COVERAGE_DICT = {"Intel": 80.13, "AMD": 79.28, "ARM": 79.34} diff --git a/tests/integration_tests/functional/test_feat_parity.py b/tests/integration_tests/functional/test_feat_parity.py index 7fe00632e29..85fd887c5c9 100644 --- a/tests/integration_tests/functional/test_feat_parity.py +++ b/tests/integration_tests/functional/test_feat_parity.py @@ -281,6 +281,8 @@ def test_feat_parity_msr_arch_cap(vm): (1 << 6) | # IF_PSCHANGE_MC_NO (1 << 7) # TSX_CTRL ) + if global_props.cpu_model == "Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz": + expected |= (1 << 19) # RRSBA # fmt: on assert actual == expected, f"{actual=:#x} != {expected=:#x}" elif cpu_template == "T2A": diff --git a/tests/integration_tests/performance/test_versioned_serialization_benchmark.py b/tests/integration_tests/performance/test_versioned_serialization_benchmark.py deleted file mode 100644 index fb2723ea079..00000000000 --- a/tests/integration_tests/performance/test_versioned_serialization_benchmark.py +++ /dev/null @@ -1,121 +0,0 @@ -# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 -"""A test that ensures that all unit tests pass at integration time.""" - -import json -import logging -import os -import platform -import shutil -from pathlib import Path - -import pytest - -from framework import utils -from framework.defs import FC_WORKSPACE_DIR -from host_tools import proc - -BENCHMARK_DIRECTORY = "{}/src/vmm".format(FC_WORKSPACE_DIR) -DEFAULT_BUILD_TARGET = "{}-unknown-linux-musl".format(platform.machine()) - -PROC_MODEL = proc.proc_type() - -NSEC_IN_MSEC = 1000000 - -BASELINES = { - "Intel": { - "serialize": { - "no-crc": {"target": 0.205, "delta": 0.050}, # milliseconds # milliseconds - "crc": {"target": 0.244, "delta": 0.44}, # milliseconds # milliseconds - }, - "deserialize": { - "no-crc": {"target": 0.056, "delta": 0.02}, # milliseconds # milliseconds - "crc": {"target": 0.075, "delta": 0.030}, # milliseconds # milliseconds - }, - }, - "AMD": { - "serialize": { - "no-crc": {"target": 0.084, "delta": 0.05}, # milliseconds # milliseconds - "crc": {"target": 0.108, "delta": 0.025}, # milliseconds # milliseconds - }, - "deserialize": { - "no-crc": {"target": 0.030, "delta": 0.02}, # milliseconds # milliseconds - "crc": {"target": 0.052, "delta": 0.04}, # milliseconds # milliseconds - }, - }, - "ARM": { - "serialize": { - "no-crc": {"target": 0.050, "delta": 0.03}, # milliseconds # milliseconds - "crc": {"target": 0.075, "delta": 0.025}, # milliseconds # milliseconds - }, - "deserialize": { - "no-crc": {"target": 0.060, "delta": 0.02}, # milliseconds # milliseconds - "crc": {"target": 0.065, "delta": 0.02}, # milliseconds # milliseconds - }, - }, -} - - -def _check_statistics(directory, mean): - proc_model = [item for item in BASELINES if item in PROC_MODEL] - assert len(proc_model) == 1, "Could not get processor model!" - - if "deserialize" in directory.lower(): - bench = "deserialize" - else: - bench = "serialize" - - if "crc" in directory.lower(): - attribute = "crc" - else: - attribute = "no-crc" - - measure = BASELINES[proc_model[0]][bench][attribute] - target, delta = measure["target"], measure["delta"] - assert target == pytest.approx( - mean, abs=delta, rel=1e-6 - ), f"Benchmark result {directory} has changed!" - - return f"{target - delta} <= result <= {target + delta}" - - -def test_serialization_benchmark(monkeypatch, record_property): - """ - Benchmark test for MicrovmState serialization/deserialization. - - @type: performance - """ - logger = logging.getLogger("serialization_benchmark") - - # Move into the benchmark directory - monkeypatch.chdir(BENCHMARK_DIRECTORY) - - # Run benchmark test - cmd = "cargo bench --target {}".format(DEFAULT_BUILD_TARGET) - result = utils.run_cmd_sync(cmd) - assert result.returncode == 0 - - # Parse each Criterion benchmark from the result folder and - # check the results against a baseline - results_dir = Path(FC_WORKSPACE_DIR) / "build/vmm_benchmark" - for directory in os.listdir(results_dir): - # Ignore the 'report' directory as it is of no use to us - if directory == "report": - continue - - logger.info("Benchmark: %s", directory) - - # Retrieve the 'estimates.json' file content - json_file = results_dir / directory / "base/estimates.json" - estimates = json.loads(json_file.read_text()) - - # Save the Mean measurement(nanoseconds) and transform it(milliseconds) - mean = estimates["mean"]["point_estimate"] / NSEC_IN_MSEC - logger.info("Mean: %f", mean) - - criteria = _check_statistics(directory, mean) - record_property(f"{directory}_ms", mean) - record_property(f"{directory}_criteria", criteria) - - # Cleanup the Target directory - shutil.rmtree(results_dir)