Merge branch 'main' into block-latency-test

roypat · web-flow · commit fa0b2ddde916 · 2025-04-23T16:44:45.000Z
diff --git a/src/vmm/src/arch/aarch64/regs.rs b/src/vmm/src/arch/aarch64/regs.rs
@@ -5,6 +5,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
+use std::fmt::Write;
 use std::mem::offset_of;
 
 use kvm_bindings::*;
@@ -404,6 +405,15 @@ impl<'a> Aarch64RegisterRef<'a> {
         T::from_slice(self.data)
     }
 
+    /// Returns a string with hex formatted value of the register.
+    pub fn value_str(&self) -> String {
+        let hex = self.data.iter().rev().fold(String::new(), |mut acc, byte| {
+            write!(&mut acc, "{:02x}", byte).unwrap();
+            acc
+        });
+        format!("0x{hex}")
+    }
+
     /// Returns register data as a byte slice
     pub fn as_slice(&self) -> &[u8] {
         self.data
@@ -695,6 +705,32 @@ mod tests {
         assert_eq!(reg_ref.value::<u64, 8>(), 69);
     }
 
+    #[test]
+    fn test_reg_ref_value_str() {
+        let bytes = 0x10_u8.to_le_bytes();
+        let reg_ref = Aarch64RegisterRef::new(KVM_REG_SIZE_U8 as u64, &bytes);
+        assert_eq!(reg_ref.value_str(), "0x10");
+
+        let bytes = 0x1020_u16.to_le_bytes();
+        let reg_ref = Aarch64RegisterRef::new(KVM_REG_SIZE_U16, &bytes);
+        assert_eq!(reg_ref.value_str(), "0x1020");
+
+        let bytes = 0x10203040_u32.to_le_bytes();
+        let reg_ref = Aarch64RegisterRef::new(KVM_REG_SIZE_U32, &bytes);
+        assert_eq!(reg_ref.value_str(), "0x10203040");
+
+        let bytes = 0x1020304050607080_u64.to_le_bytes();
+        let reg_ref = Aarch64RegisterRef::new(KVM_REG_SIZE_U64, &bytes);
+        assert_eq!(reg_ref.value_str(), "0x1020304050607080");
+
+        let bytes = [
+            0x71, 0x61, 0x51, 0x41, 0x31, 0x21, 0x11, 0x90, 0x80, 0x70, 0x60, 0x50, 0x40, 0x30,
+            0x20, 0x10,
+        ];
+        let reg_ref = Aarch64RegisterRef::new(KVM_REG_SIZE_U128, &bytes);
+        assert_eq!(reg_ref.value_str(), "0x10203040506070809011213141516171");
+    }
+
     /// Should panic because ID has different size from a slice length.
     /// - Size in ID: 128
     /// - Length of slice: 1
diff --git a/src/vmm/src/arch/aarch64/vcpu.rs b/src/vmm/src/arch/aarch64/vcpu.rs
@@ -7,7 +7,6 @@
 
 use std::fmt::{Debug, Write};
 use std::mem::offset_of;
-use std::path::PathBuf;
 
 use kvm_bindings::*;
 use kvm_ioctls::{VcpuExit, VcpuFd, VmFd};
@@ -31,8 +30,8 @@ use crate::vstate::vm::Vm;
 pub enum VcpuArchError {
     /// Failed to get register {0}: {1}
     GetOneReg(u64, kvm_ioctls::Error),
-    /// Failed to set register {0}: {1}
-    SetOneReg(u64, kvm_ioctls::Error),
+    /// Failed to set register {0:#x} to value {1}: {2}
+    SetOneReg(u64, String, kvm_ioctls::Error),
     /// Failed to retrieve list of registers: {0}
     GetRegList(kvm_ioctls::Error),
     /// Failed to get multiprocessor state: {0}
@@ -48,9 +47,7 @@ pub enum VcpuArchError {
 /// Extract the Manufacturer ID from the host.
 /// The ID is found between bits 24-31 of MIDR_EL1 register.
 pub fn get_manufacturer_id_from_host() -> Result<u32, VcpuArchError> {
-    let midr_el1_path =
-        &PathBuf::from("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1".to_string());
-
+    let midr_el1_path = "/sys/devices/system/cpu/cpu0/regs/identification/midr_el1";
     let midr_el1 = std::fs::read_to_string(midr_el1_path).map_err(|err| {
         VcpuArchError::GetMidrEl1(format!("Failed to get MIDR_EL1 from host path: {err}"))
     })?;
@@ -181,7 +178,11 @@ impl KvmVcpu {
     ) -> Result<(), KvmVcpuError> {
         for reg in vcpu_config.cpu_config.regs.iter() {
             self.fd.set_one_reg(reg.id, reg.as_slice()).map_err(|err| {
-                KvmVcpuError::ApplyCpuTemplate(VcpuArchError::SetOneReg(reg.id, err))
+                KvmVcpuError::ApplyCpuTemplate(VcpuArchError::SetOneReg(
+                    reg.id,
+                    reg.value_str(),
+                    err,
+                ))
             })?;
         }
 
@@ -325,7 +326,9 @@ impl KvmVcpu {
         let id = arm64_core_reg_id!(KVM_REG_SIZE_U64, pstate);
         self.fd
             .set_one_reg(id, &PSTATE_FAULT_BITS_64.to_le_bytes())
-            .map_err(|err| VcpuArchError::SetOneReg(id, err))?;
+            .map_err(|err| {
+                VcpuArchError::SetOneReg(id, format!("{PSTATE_FAULT_BITS_64:#x}"), err)
+            })?;
 
         // Other vCPUs are powered off initially awaiting PSCI wakeup.
         if self.index == 0 {
@@ -334,17 +337,18 @@ impl KvmVcpu {
             let id = arm64_core_reg_id!(KVM_REG_SIZE_U64, pc);
             self.fd
                 .set_one_reg(id, &boot_ip.to_le_bytes())
-                .map_err(|err| VcpuArchError::SetOneReg(id, err))?;
+                .map_err(|err| VcpuArchError::SetOneReg(id, format!("{boot_ip:#x}"), err))?;
 
             // Last mandatory thing to set -> the address pointing to the FDT (also called DTB).
             // "The device tree blob (dtb) must be placed on an 8-byte boundary and must
             // not exceed 2 megabytes in size." -> https://www.kernel.org/doc/Documentation/arm64/booting.txt.
             // We are choosing to place it the end of DRAM. See `get_fdt_addr`.
             let regs0 = offset_of!(user_pt_regs, regs) + kreg_off;
             let id = arm64_core_reg_id!(KVM_REG_SIZE_U64, regs0);
+            let fdt_addr = get_fdt_addr(mem);
             self.fd
-                .set_one_reg(id, &get_fdt_addr(mem).to_le_bytes())
-                .map_err(|err| VcpuArchError::SetOneReg(id, err))?;
+                .set_one_reg(id, &fdt_addr.to_le_bytes())
+                .map_err(|err| VcpuArchError::SetOneReg(id, format!("{fdt_addr:#x}"), err))?;
 
             // Reset the physical counter for the guest. This way we avoid guest reading
             // host physical counter.
@@ -360,7 +364,9 @@ impl KvmVcpu {
             if optional_capabilities.counter_offset {
                 self.fd
                     .set_one_reg(KVM_REG_ARM_PTIMER_CNT, &[0; 8])
-                    .map_err(|err| VcpuArchError::SetOneReg(id, err))?;
+                    .map_err(|err| {
+                        VcpuArchError::SetOneReg(id, format!("{KVM_REG_ARM_PTIMER_CNT:#x}"), err)
+                    })?;
             }
         }
         Ok(())
@@ -412,7 +418,7 @@ impl KvmVcpu {
     pub fn set_register(&self, reg: Aarch64RegisterRef) -> Result<(), VcpuArchError> {
         self.fd
             .set_one_reg(reg.id, reg.as_slice())
-            .map_err(|e| VcpuArchError::SetOneReg(reg.id, e))?;
+            .map_err(|e| VcpuArchError::SetOneReg(reg.id, reg.value_str(), e))?;
         Ok(())
     }
 
@@ -526,13 +532,16 @@ mod tests {
         unsafe { libc::close(vm.fd().as_raw_fd()) };
 
         let err = KvmVcpu::new(0, &vm);
+
+        // dropping vm would double close the gic fd, so leak it
+        // do the drop before assertion. Otherwise if assert fails,
+        // we get IO runtime error instead of assert error.
+        std::mem::forget(vm);
+
         assert_eq!(
             err.err().unwrap().to_string(),
             "Error creating vcpu: Bad file descriptor (os error 9)".to_string()
         );
-
-        // dropping vm would double close the gic fd, so leak it
-        std::mem::forget(vm);
     }
 
     #[test]
@@ -568,16 +577,20 @@ mod tests {
             &vcpu_config,
             &optional_capabilities,
         );
+
+        // dropping vcpu would double close the gic fd, so leak it
+        // do the drop before assertion. Otherwise if assert fails,
+        // we get IO runtime error instead of assert error.
+        std::mem::forget(vcpu);
+
         assert_eq!(
             err.unwrap_err(),
             KvmVcpuError::ConfigureRegisters(VcpuArchError::SetOneReg(
                 0x6030000000100042,
+                "0x3c5".to_string(),
                 kvm_ioctls::Error::new(9)
             ))
         );
-
-        // dropping vcpu would double close the gic fd, so leak it
-        std::mem::forget(vcpu);
     }
 
     #[test]
@@ -629,7 +642,7 @@ mod tests {
         let res = vcpu.restore_state(&faulty_vcpu_state);
         assert!(matches!(
             res.unwrap_err(),
-            KvmVcpuError::RestoreState(VcpuArchError::SetOneReg(0, _))
+            KvmVcpuError::RestoreState(VcpuArchError::SetOneReg(0, _, _))
         ));
 
         vcpu.init(&[]).unwrap();
@@ -699,7 +712,7 @@ mod tests {
         let res = vcpu.setup_boot_regs(0x0, &mem, &optional_capabilities);
         assert!(matches!(
             res.unwrap_err(),
-            VcpuArchError::SetOneReg(0x6030000000100042, _)
+            VcpuArchError::SetOneReg(0x6030000000100042, _, _)
         ));
 
         vcpu.init_vcpu().unwrap();
@@ -773,9 +786,12 @@ mod tests {
         assert!(matches!(res, Err(VcpuArchError::GetMp(_))), "{:?}", res);
 
         let res = vcpu.set_mpstate(kvm_mp_state::default());
-        assert!(matches!(res, Err(VcpuArchError::SetMp(_))), "{:?}", res);
 
         // dropping vcpu would double close the fd, so leak it
+        // do the drop before assertion. Otherwise if assert fails,
+        // we get IO runtime error instead of assert error.
         std::mem::forget(vcpu);
+
+        assert!(matches!(res, Err(VcpuArchError::SetMp(_))), "{:?}", res);
     }
 }
