feat: only reset SYS_CNTPCT_EL0 if KVM_CAP_COUNTER_OFFSET is present

Now when KVM is a separate object, we can query optional capabilities
when configuring vcpus on aarch64 and only reset the SYS_CNTPCT_EL0
if KVM_CAP_COUNTER_OFFSET is present.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/arch/aarch64/vcpu.rs

@@ -14,6 +14,7 @@ use log::warn;
 
 use super::get_fdt_addr;
 use super::regs::*;
+use crate::vstate::kvm::OptionalCapabilities;
 use crate::vstate::memory::GuestMemoryMmap;
 
 /// Errors thrown while setting aarch64 registers.
@@ -79,6 +80,7 @@ pub fn setup_boot_regs(
     cpu_id: u8,
     boot_ip: u64,
     mem: &GuestMemoryMmap,
+    optional_capabilities: &OptionalCapabilities,
 ) -> Result<(), VcpuError> {
     let kreg_off = offset_of!(kvm_regs, regs);
 
@@ -113,17 +115,16 @@ pub fn setup_boot_regs(
         // Resetting KVM_REG_ARM_PTIMER_CNT for single vcpu is enough because there is only
         // one timer struct with offsets per VM.
         // Because the access to KVM_REG_ARM_PTIMER_CNT is only present starting 6.4 kernel,
-        // we don't fail if ioctl returns an error.
+        // we only do the reset if KVM_CAP_COUNTER_OFFSET is present as it was added
+        // in the same patch series as the ability to set the KVM_REG_ARM_PTIMER_CNT register.
         // Path series which introduced the needed changes:
         // https://lore.kernel.org/all/[email protected]/
         // Note: the value observed by the guest will still be above 0, because there is a delta
         // time between this resetting and first call to KVM_RUN.
-        let zero: u64 = 0;
-        if vcpufd
-            .set_one_reg(KVM_REG_ARM_PTIMER_CNT, &zero.to_le_bytes())
-            .is_err()
-        {
-            warn!("Unable to reset VM physical counter. VM will use host value instead.");
+        if optional_capabilities.counter_offset {
+            vcpufd
+                .set_one_reg(KVM_REG_ARM_PTIMER_CNT, &[0; 8])
+                .map_err(|err| VcpuError::SetOneReg(id, err))?;
         }
     }
     Ok(())
@@ -245,8 +246,9 @@ mod tests {
         let vm = kvm.fd.create_vm().unwrap();
         let vcpu = vm.create_vcpu(0).unwrap();
         let mem = arch_mem(layout::FDT_MAX_SIZE + 0x1000);
+        let optional_capabilities = kvm.optional_capabilities();
 
-        let res = setup_boot_regs(&vcpu, 0, 0x0, &mem);
+        let res = setup_boot_regs(&vcpu, 0, 0x0, &mem, &optional_capabilities);
         assert!(matches!(
             res.unwrap_err(),
             VcpuError::SetOneReg(0x6030000000100042, _)
@@ -256,14 +258,17 @@ mod tests {
         vm.get_preferred_target(&mut kvi).unwrap();
         vcpu.vcpu_init(&kvi).unwrap();
 
-        setup_boot_regs(&vcpu, 0, 0x0, &mem).unwrap();
+        setup_boot_regs(&vcpu, 0, 0x0, &mem, &optional_capabilities).unwrap();
 
         // Check that the register is reset on compatible kernels.
         // Because there is a delta in time between we reset the register and time we
         // read it, we cannot compare with 0. Instead we read CNTVNT_EL0 (Virtual counter)
         // from the host. The host value should be much bigger than the VM physical counter.
-        let mut reg_bytes = [0_u8; 8];
-        if vcpu.get_one_reg(SYS_CNTPCT_EL0, &mut reg_bytes).is_ok() {
+        if optional_capabilities.counter_offset {
+            let mut reg_bytes = [0_u8; 8];
+            vcpu.get_one_reg(SYS_CNTPCT_EL0, &mut reg_bytes).unwrap();
+            let counter_value = u64::from_le_bytes(reg_bytes);
+
             let virt_count = unsafe {
                 // Rust is so smart, it cannot figure out that `virt_count`
                 // is read later in the assert!, so it complains about unused assignment.
@@ -276,7 +281,7 @@ mod tests {
                 );
                 virt_count
             };
-            let counter_value = u64::from_le_bytes(reg_bytes);
+
             assert!(counter_value < virt_count);
         }
     }

src/vmm/src/builder.rs

@@ -811,16 +811,16 @@ pub fn configure_system_for_boot(
         cpu_config,
     };
 
-    // Configure vCPUs with normalizing and setting the generated CPU configuration.
-    for vcpu in vcpus.iter_mut() {
-        vcpu.kvm_vcpu
-            .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
-            .map_err(VmmError::VcpuConfigure)
-            .map_err(Internal)?;
-    }
-
     #[cfg(target_arch = "x86_64")]
     {
+        // Configure vCPUs with normalizing and setting the generated CPU configuration.
+        for vcpu in vcpus.iter_mut() {
+            vcpu.kvm_vcpu
+                .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
+                .map_err(VmmError::VcpuConfigure)
+                .map_err(Internal)?;
+        }
+
         // Write the kernel command line to guest memory. This is x86_64 specific, since on
         // aarch64 the command line will be specified through the FDT.
         let cmdline_size = boot_cmdline
@@ -855,6 +855,19 @@ pub fn configure_system_for_boot(
     }
     #[cfg(target_arch = "aarch64")]
     {
+        let optional_capabilities = vmm.kvm.optional_capabilities();
+        // Configure vCPUs with normalizing and setting the generated CPU configuration.
+        for vcpu in vcpus.iter_mut() {
+            vcpu.kvm_vcpu
+                .configure(
+                    vmm.guest_memory(),
+                    entry_addr,
+                    &vcpu_config,
+                    &optional_capabilities,
+                )
+                .map_err(VmmError::VcpuConfigure)
+                .map_err(Internal)?;
+        }
         let vcpu_mpidr = vcpus
             .iter_mut()
             .map(|cpu| cpu.kvm_vcpu.get_mpidr())

src/vmm/src/vstate/kvm.rs

@@ -131,7 +131,13 @@ impl Kvm {
         Ok(())
     }
 }
-
+#[cfg(target_arch = "aarch64")]
+/// Optional capabilities.
+#[derive(Debug, Default)]
+pub struct OptionalCapabilities {
+    // KVM_CAP_COUNTER_OFFSET
+    pub counter_offset: bool,
+}
 #[cfg(target_arch = "aarch64")]
 impl Kvm {
     const DEFAULT_CAPABILITIES: [u32; 7] = [
@@ -144,6 +150,16 @@ impl Kvm {
         kvm_bindings::KVM_CAP_ONE_REG,
     ];
 
+    /// Returns struct with optional capabilities statuses.
+    pub fn optional_capabilities(&self) -> OptionalCapabilities {
+        OptionalCapabilities {
+            counter_offset: self
+                .fd
+                .check_extension_raw(kvm_bindings::KVM_CAP_COUNTER_OFFSET.into())
+                != 0,
+        }
+    }
+
     /// Saves and returns the Kvm Vm state.
     pub fn save_state(&self) -> KvmState {
         KvmState {

src/vmm/src/vstate/vcpu/aarch64.rs

@@ -22,7 +22,7 @@ use crate::cpu_config::aarch64::custom_cpu_template::VcpuFeatures;
 use crate::cpu_config::templates::CpuConfiguration;
 use crate::logger::{error, IncMetric, METRICS};
 use crate::vcpu::{VcpuConfig, VcpuError};
-use crate::vstate::kvm::Kvm;
+use crate::vstate::kvm::{Kvm, OptionalCapabilities};
 use crate::vstate::memory::{Address, GuestAddress, GuestMemoryMmap};
 use crate::vstate::vcpu::VcpuEmulation;
 use crate::vstate::vm::Vm;
@@ -116,6 +116,7 @@ impl KvmVcpu {
         guest_mem: &GuestMemoryMmap,
         kernel_load_addr: GuestAddress,
         vcpu_config: &VcpuConfig,
+        optional_capabilities: &OptionalCapabilities,
     ) -> Result<(), KvmVcpuError> {
         for reg in vcpu_config.cpu_config.regs.iter() {
             self.fd
@@ -128,6 +129,7 @@ impl KvmVcpu {
             self.index,
             kernel_load_addr.raw_value(),
             guest_mem,
+            optional_capabilities,
         )
         .map_err(KvmVcpuError::ConfigureRegisters)?;
 
@@ -322,11 +324,11 @@ mod tests {
 
     #[test]
     fn test_create_vcpu() {
-        let (_, vm, _) = setup_vm_with_memory(0x1000);
+        let (kvm, vm, _) = setup_vm_with_memory(0x1000);
 
         unsafe { libc::close(vm.fd().as_raw_fd()) };
 
-        let err = KvmVcpu::new(0, &vm);
+        let err = KvmVcpu::new(0, &vm, &kvm);
         assert_eq!(
             err.err().unwrap().to_string(),
             "Error creating vcpu: Bad file descriptor (os error 9)".to_string()
@@ -338,7 +340,8 @@ mod tests {
 
     #[test]
     fn test_configure_vcpu() {
-        let (_, _, mut vcpu, vm_mem) = setup_vcpu(0x10000);
+        let (kvm, _, mut vcpu, vm_mem) = setup_vcpu(0x10000);
+        let optional_capabilities = kvm.optional_capabilities();
 
         let vcpu_config = VcpuConfig {
             vcpu_count: 1,
@@ -349,6 +352,7 @@ mod tests {
             &vm_mem,
             GuestAddress(crate::arch::get_kernel_start()),
             &vcpu_config,
+            &optional_capabilities,
         )
         .unwrap();
 
@@ -358,6 +362,7 @@ mod tests {
             &vm_mem,
             GuestAddress(crate::arch::get_kernel_start()),
             &vcpu_config,
+            &optional_capabilities,
         );
         assert_eq!(
             err.unwrap_err(),
@@ -392,8 +397,8 @@ mod tests {
 
     #[test]
     fn test_vcpu_save_restore_state() {
-        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let (kvm, mut vm, _) = setup_vm_with_memory(0x1000);
+        let mut vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         // Calling KVM_GET_REGLIST before KVM_VCPU_INIT will result in error.

src/vmm/src/vstate/vcpu/mod.rs

@@ -1008,6 +1008,7 @@ pub(crate) mod tests {
                     smt: false,
                     cpu_config: crate::cpu_config::aarch64::CpuConfiguration::default(),
                 },
+                &kvm.optional_capabilities(),
             )
             .expect("failed to configure vcpu");