hcl: Private generalized get_vp_register functions (#1643)

The generalized functions here should not be used unless we know the
register is partition-wide, and not per-VP. This is easy to do
accidentally. Make these functions private so this is harder, and add
specialized accessors for some known good cases. Fix the bad case in the
tlb lock check.

Author: smalis-msft

openhcl/hcl/Cargo.toml

@@ -19,7 +19,7 @@ inspect.workspace = true
 user_driver.workspace = true
 
 anyhow.workspace = true
-bitvec.workspace = true
+bitvec = { workspace = true, features = ["std"] }
 parking_lot.workspace = true
 signal-hook.workspace = true
 thiserror.workspace = true

openhcl/hcl/src/ioctl.rs

@@ -1431,7 +1431,7 @@ impl MshvHvcall {
     /// Get a single VP register for the given VTL via hypercall. Only a select
     /// set of registers are supported; others will cause a panic.
     #[cfg(guest_arch = "x86_64")]
-    pub fn get_vp_register_for_vtl(
+    fn get_vp_register_for_vtl(
         &self,
         vtl: HvInputVtl,
         name: HvX64RegisterName,
@@ -1473,7 +1473,7 @@ impl MshvHvcall {
     /// Get a single VP register for the given VTL via hypercall. Only a select
     /// set of registers are supported; others will cause a panic.
     #[cfg(guest_arch = "aarch64")]
-    pub fn get_vp_register_for_vtl(
+    fn get_vp_register_for_vtl(
         &self,
         vtl: HvInputVtl,
         name: HvArm64RegisterName,
@@ -1796,9 +1796,7 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
             actions.flush();
         }
     }
-}
 
-impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
     // Registers that are shared between VTLs need to be handled by the kernel
     // as they may require special handling there. set_reg and get_reg will
     // handle these registers using a dedicated ioctl, instead of the general-
@@ -1865,7 +1863,7 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
         Ok(())
     }
 
-    fn get_reg(&mut self, vtl: GuestVtl, regs: &mut [HvRegisterAssoc]) -> Result<(), Error> {
+    fn get_reg(&mut self, vtl: Vtl, regs: &mut [HvRegisterAssoc]) -> Result<(), Error> {
         if regs.is_empty() {
             return Ok(());
         }
@@ -2040,9 +2038,7 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
     pub fn is_sidecar(&self) -> bool {
         self.sidecar.is_some()
     }
-}
 
-impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
     fn get_vp_registers_inner<R: Copy + Into<HvRegisterName>>(
         &mut self,
         vtl: GuestVtl,
@@ -2065,7 +2061,7 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
             }
         }
 
-        self.get_reg(vtl, &mut assoc)?;
+        self.get_reg(vtl.into(), &mut assoc)?;
         for (&i, assoc) in offset.iter().zip(&assoc) {
             values[i] = assoc.value;
         }
@@ -2087,6 +2083,24 @@ impl<'a, T: Backing<'a>> ProcessorRunner<'a, T> {
         Ok(value[0])
     }
 
+    /// Get the following register on the current VP for VTL 2.
+    ///
+    /// This will fail for registers that are in the mmapped CPU context, i.e.
+    /// registers that are shared between VTL0 and VTL2.
+    pub fn get_vp_vtl2_register(
+        &mut self,
+        #[cfg(guest_arch = "x86_64")] name: HvX64RegisterName,
+        #[cfg(guest_arch = "aarch64")] name: HvArm64RegisterName,
+    ) -> Result<HvRegisterValue, Error> {
+        let mut assoc = [HvRegisterAssoc {
+            name: name.into(),
+            pad: Default::default(),
+            value: FromZeros::new_zeroed(),
+        }];
+        self.get_reg(Vtl::Vtl2, &mut assoc)?;
+        Ok(assoc[0].value)
+    }
+
     /// Get the following VP registers on the current VP.
     ///
     /// # Panics
@@ -2565,7 +2579,7 @@ impl Hcl {
     /// Get a single VP register for the given VTL via hypercall. Only a select
     /// set of registers are supported; others will cause a panic.
     #[cfg(guest_arch = "x86_64")]
-    pub fn get_vp_register(
+    fn get_vp_register(
         &self,
         name: impl Into<HvX64RegisterName>,
         vtl: HvInputVtl,
@@ -2576,7 +2590,7 @@ impl Hcl {
     /// Get a single VP register for the given VTL via hypercall. Only a select
     /// set of registers are supported; others will cause a panic.
     #[cfg(guest_arch = "aarch64")]
-    pub fn get_vp_register(
+    fn get_vp_register(
         &self,
         name: impl Into<HvArm64RegisterName>,
         vtl: HvInputVtl,
@@ -2933,6 +2947,37 @@ impl Hcl {
         ))
     }
 
+    /// Get the [`hvdef::HvRegisterVsmPartitionStatus`] register
+    pub fn get_vsm_partition_status(&self) -> Result<hvdef::HvRegisterVsmPartitionStatus, Error> {
+        Ok(hvdef::HvRegisterVsmPartitionStatus::from(
+            self.get_vp_register(
+                HvAllArchRegisterName::VsmPartitionStatus,
+                HvInputVtl::CURRENT_VTL,
+            )?
+            .as_u64(),
+        ))
+    }
+
+    #[cfg(guest_arch = "aarch64")]
+    /// Get the [`hvdef::HvPartitionPrivilege`] register
+    pub fn get_privileges_and_features_info(&self) -> Result<hvdef::HvPartitionPrivilege, Error> {
+        Ok(hvdef::HvPartitionPrivilege::from(
+            self.get_vp_register(
+                HvArm64RegisterName::PrivilegesAndFeaturesInfo,
+                HvInputVtl::CURRENT_VTL,
+            )?
+            .as_u64(),
+        ))
+    }
+
+    /// Get the [`hvdef::hypercall::HvGuestOsId`] register for the given VTL.
+    pub fn get_guest_os_id(&self, vtl: Vtl) -> Result<hvdef::hypercall::HvGuestOsId, Error> {
+        Ok(hvdef::hypercall::HvGuestOsId::from(
+            self.get_vp_register(HvAllArchRegisterName::GuestOsId, vtl.into())?
+                .as_u64(),
+        ))
+    }
+
     /// Configure guest VSM.
     /// The only configuration attribute currently supported is changing the maximum number of
     /// guest-visible virtual trust levels for the partition. (VTL 1)

openhcl/virt_mshv_vtl/src/lib.rs

@@ -33,7 +33,6 @@ cfg_if::cfg_if!(
         type IrrBitmap = BitArray<[u32; 8], Lsb0>;
     } else if #[cfg(target_arch = "aarch64")] { // xtask-fmt allow-target-arch sys-crate
         pub use processor::mshv::arm64::HypervisorBackedArm64 as HypervisorBacked;
-        use hvdef::HvArm64RegisterName;
         use processor::mshv::arm64::HypervisorBackedArm64Shared as HypervisorBackedShared;
     }
 );
@@ -59,7 +58,6 @@ use hv1_emulator::synic::SintProxied;
 use hv1_structs::VtlArray;
 use hvdef::GuestCrashCtl;
 use hvdef::HV_PAGE_SIZE;
-use hvdef::HvAllArchRegisterName;
 use hvdef::HvError;
 use hvdef::HvMapGpaFlags;
 use hvdef::HvRegisterName;
@@ -962,11 +960,7 @@ impl UhPartitionInner {
     #[cfg_attr(guest_arch = "aarch64", expect(dead_code))]
     fn vsm_status(&self) -> Result<HvRegisterVsmPartitionStatus, hcl::ioctl::Error> {
         // TODO: It might be possible to cache VsmPartitionStatus.
-        let reg = self.hcl.get_vp_register(
-            HvAllArchRegisterName::VsmPartitionStatus,
-            HvInputVtl::CURRENT_VTL,
-        )?;
-        Ok(reg.as_u64().into())
+        self.hcl.get_vsm_partition_status()
     }
 }
 
@@ -1902,13 +1896,10 @@ impl UhPartition {
             hv.guest_os_id(Vtl::Vtl0)
         } else {
             // Ask the hypervisor for this value.
-            let reg_value = self
-                .inner
+            self.inner
                 .hcl
-                .get_vp_register(HvAllArchRegisterName::GuestOsId, Vtl::Vtl0.into())
-                .map_err(Error::Hcl)?;
-
-            HvGuestOsId::from(reg_value.as_u64())
+                .get_guest_os_id(Vtl::Vtl0)
+                .map_err(Error::Hcl)?
         };
         Ok(id)
     }
@@ -1948,19 +1939,14 @@ impl UhProtoPartition<'_> {
         #[cfg(guest_arch = "x86_64")]
         let privs = {
             let result = safe_intrinsics::cpuid(hvdef::HV_CPUID_FUNCTION_MS_HV_FEATURES, 0);
-            result.eax as u64 | ((result.ebx as u64) << 32)
+            let num = result.eax as u64 | ((result.ebx as u64) << 32);
+            hvdef::HvPartitionPrivilege::from(num)
         };
 
         #[cfg(guest_arch = "aarch64")]
-        let privs = hcl
-            .get_vp_register(
-                HvArm64RegisterName::PrivilegesAndFeaturesInfo,
-                HvInputVtl::CURRENT_VTL,
-            )
-            .map_err(Error::Hcl)?
-            .as_u64();
+        let privs = hcl.get_privileges_and_features_info().map_err(Error::Hcl)?;
 
-        if !hvdef::HvPartitionPrivilege::from(privs).access_vsm() {
+        if !privs.access_vsm() {
             return Ok(false);
         }
         let guest_vsm_config = hcl.get_guest_vsm_partition_config().map_err(Error::Hcl)?;

openhcl/virt_mshv_vtl/src/processor/mshv/tlb_lock.rs

@@ -49,14 +49,22 @@ impl UhProcessor<'_, HypervisorBacked> {
     /// Mark the TLB of the target VTL on the current VP as locked without
     /// informing the hypervisor. Only should be used when the hypervisor
     /// is expected to have already locked the TLB.
+    #[expect(
+        clippy::debug_assert_with_mut_call,
+        reason = "is_tlb_locked_in_hypervisor doesn't actually mutate state"
+    )]
     pub(crate) fn mark_tlb_locked(&mut self, requesting_vtl: Vtl, target_vtl: GuestVtl) {
         debug_assert_eq!(requesting_vtl, Vtl::Vtl2);
         debug_assert!(self.is_tlb_locked_in_hypervisor(target_vtl));
         self.vtls_tlb_locked.set(requesting_vtl, target_vtl, true);
     }
 
     /// Check the status of the TLB lock of the target VTL on the current VP.
-    pub(crate) fn is_tlb_locked(&self, requesting_vtl: Vtl, target_vtl: GuestVtl) -> bool {
+    #[expect(
+        clippy::debug_assert_with_mut_call,
+        reason = "is_tlb_locked_in_hypervisor doesn't actually mutate state"
+    )]
+    pub(crate) fn is_tlb_locked(&mut self, requesting_vtl: Vtl, target_vtl: GuestVtl) -> bool {
         // This function should only be called in debug assertions.
         assert!(cfg!(debug_assertions));
         debug_assert_eq!(requesting_vtl, Vtl::Vtl2);
@@ -68,16 +76,15 @@ impl UhProcessor<'_, HypervisorBacked> {
         local_status
     }
 
-    fn is_tlb_locked_in_hypervisor(&self, target_vtl: GuestVtl) -> bool {
+    fn is_tlb_locked_in_hypervisor(&mut self, target_vtl: GuestVtl) -> bool {
         // This function should only be called in debug assertions.
         assert!(cfg!(debug_assertions));
         let name = HvAllArchRegisterName(
             HvAllArchRegisterName::VsmVpSecureConfigVtl0.0 + target_vtl as u32,
         );
         let result = self
-            .partition
-            .hcl
-            .get_vp_register(name, hvdef::hypercall::HvInputVtl::CURRENT_VTL)
+            .runner
+            .get_vp_vtl2_register(name.into())
             .expect("failure is a misconfiguration");
         let config = hvdef::HvRegisterVsmVpSecureVtlConfig::from(result.as_u64());
         config.tlb_locked()