Backport T2S/T2CL template fix to v1.3

.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(

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

src/arch/src/x86_64/msr.rs

@@ -331,6 +331,36 @@ pub fn supported_guest_msrs(kvm_fd: &Kvm) -> Result<MsrList> {
     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<u64, GetMsrError> {
+    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;

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<kvm_msr_entry>) {
-    let capabilities = ArchCapaMSRFlags::RDCL_NO
+pub fn update_msr_entries(msr_entries: &mut Vec<kvm_msr_entry>, 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::<kvm_msr_entry>::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::<kvm_msr_entry>::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()
+        );
     }
 }

src/cpuid/src/template/intel/t2s.rs

@@ -35,7 +35,8 @@ pub fn update_msr_entries(msr_entries: &mut Vec<kvm_msr_entry>) {
         | 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(),

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);
             }
             _ => (),
         }

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}
 

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":