Hypercall to let the VTL0 or VTL1 to retrieve its VMPCK so that it can communicate with the PSP by sunilmut · Pull Request #3408 · microsoft/openvmm

sunilmut · 2026-04-30T01:26:24Z

Implement the hypercall that allows the VTL0 or VTL1 retrieve the VM Platform Communication Key (VMPCK) for its own VTL, so that it can directly communicate with the PSP. The VMPCK is used to encrypt/decrypt the communication with the PSP.

github-actions · 2026-04-30T01:26:43Z

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

sunilmut · 2026-04-30T01:28:29Z

    // just require Underhill setting the apicbase register on the VPs
    // before start.
-    //
-    // TODO: centralize cpuid querying logic.


This PR took the liberty to centralize the cpuid querying logic (as the PR also needed to make a cpuid call). If the general direction is right, I can try to converge other HCL cpuid calls into the cpuid crate in this PR or a subsequent PR.

Copilot

Pull request overview

Implements a new Hyper-V hypercall for lower VTL guests (VTL0/VTL1) to retrieve their SEV-SNP VMPCK so they can communicate directly with the PSP, and centralizes some host CPUID feature detection for OpenHCL.

Changes:

Added HvCallGetSnpVmpck hypercall definition + output struct in hvdef and dispatch plumbing in hv1_hypercall.
Extracted/stored VMPCKs from the SNP secrets page at partition init and exposed them via a new Underhill hypercall handler.
Introduced openhcl_cpuid_features crate and plumbed cached CPUID features from worker startup into Hcl.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
vm/x86/x86defs/src/snp.rs	Adds SNP secrets-page VMPCK offsets/size constants.
vm/hv1/hvdef/src/lib.rs	Extends enlightenment bitfield flags; adds hypercall code + VMPCK output type/size constant.
vm/hv1/hv1_hypercall/src/imp.rs	Adds hypercall type alias + handler trait for `HvGetSnpVmpck`.
openhcl/virt_mshv_vtl/src/processor/snp/mod.rs	Caches VMPCK keys from secrets page and implements the new hypercall handler.
openhcl/virt_mshv_vtl/src/lib.rs	Plumbs cached CPUID features into `Hcl::new` on x86_64 guest builds.
openhcl/virt_mshv_vtl/Cargo.toml	Adds dependency on `openhcl_cpuid_features`.
openhcl/underhill_core/src/worker.rs	Initializes CPUID feature snapshot once and reuses it for MMIO-hypercall + x2APIC decisions.
openhcl/underhill_core/Cargo.toml	Swaps direct CPUID deps for `openhcl_cpuid_features`.
openhcl/openhcl_cpuid_features/src/lib.rs	New crate centralizing a small set of host CPUID feature queries.
openhcl/openhcl_cpuid_features/Cargo.toml	New crate manifest with x86_64-only deps.
openhcl/hcl/src/ioctl.rs	Adds CPUID feature caching to `Hcl` and changes `Hcl::new` signature on x86_64 guest builds.
openhcl/hcl/Cargo.toml	Adds `openhcl_cpuid_features` dependency.
Cargo.toml	Registers `openhcl_cpuid_features` as a workspace member dependency.
Cargo.lock	Adds lock entries for the new crate and updated dependency graph.

Copilot · 2026-04-30T01:34:51Z

+        let vmpck_keys = if let Some(secrets) = partition_params.snp_secrets {
+            let mut keys = [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS];
+            for (i, key) in keys.iter_mut().enumerate() {
+                let offset = SNP_SECRETS_VMPCK0_OFFSET + i * SNP_VMPCK_KEY_SIZE;
+                key.copy_from_slice(&secrets[offset..offset + SNP_VMPCK_KEY_SIZE]);
+            }
+            keys
+        } else {
+            [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS]
+        };


partition_params.snp_secrets is optional, but this code silently substitutes all-zero VMPCK keys when it’s missing and also slices secrets[...] without validating the slice length. For SNP partitions, it’s safer to treat a missing/short secrets page as an error (to avoid panics and avoid returning an invalid key later) and validate that the secrets page is large enough for all requested offsets before copying.

Suggested change

let vmpck_keys = if let Some(secrets) = partition_params.snp_secrets {

let mut keys = [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS];

for (i, key) in keys.iter_mut().enumerate() {

let offset = SNP_SECRETS_VMPCK0_OFFSET + i * SNP_VMPCK_KEY_SIZE;

key.copy_from_slice(&secrets[offset..offset + SNP_VMPCK_KEY_SIZE]);

}

keys

} else {

[[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS]

};

let secrets = partition_params.snp_secrets.ok_or_else(|| {

anyhow::anyhow!("missing SNP secrets page for SNP partition")

})?;

let required_len = SNP_SECRETS_VMPCK0_OFFSET + SNP_NUM_VMPCKS * SNP_VMPCK_KEY_SIZE;

if secrets.len() < required_len {

return Err(anyhow::anyhow!(

"SNP secrets page too short for VMPCK keys: got {} bytes, need at least {}",

secrets.len(),

required_len

));

}

let mut vmpck_keys = [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS];

for (i, key) in vmpck_keys.iter_mut().enumerate() {

let offset = SNP_SECRETS_VMPCK0_OFFSET + i * SNP_VMPCK_KEY_SIZE;

key.copy_from_slice(&secrets[offset..offset + SNP_VMPCK_KEY_SIZE]);

}

Copilot · 2026-04-30T01:34:51Z

 pal.workspace = true
 memory_range.workspace = true
+openhcl_cpuid_features.workspace = true
 sidecar_client.workspace = true


openhcl_cpuid_features is an x86_64-only crate (cfg(target_arch = "x86_64")). Adding it as an unconditional Linux dependency for hcl can cause non-x86_64 Linux builds to fail unless the code is fully cfg-gated or the crate provides non-x86 stubs. Consider moving this dependency under a cfg(target_arch = "x86_64") target section (or otherwise ensuring non-x86 builds don’t pull in/use this crate).

Copilot · 2026-04-30T01:34:52Z

@@ -1346,6 +1346,7 @@ pub struct Hcl {
    isolation: IsolationType,
    snp_register_bitmap: [u8; 64],
    sidecar: Option<SidecarClient>,


Hcl now stores cpuid_features: openhcl_cpuid_features::CpuidFeatures, but openhcl_cpuid_features is cfg(target_arch = "x86_64") only. As written, this will fail to compile hcl on non-x86_64 targets (or any build where that type isn’t available). Gate this field + accessor behind an appropriate cfg, or provide a non-x86_64 stub type/API in openhcl_cpuid_features so Hcl remains portable.

Suggested change

sidecar: Option<SidecarClient>,

sidecar: Option<SidecarClient>,

#[cfg(target_arch = "x86_64")]

Copilot · 2026-04-30T01:34:52Z

+    #[cfg(guest_arch = "x86_64")] // xtask-fmt allow-target-arch cpu-intrinsic
+    pub fn new(
+        isolation: IsolationType,
+        sidecar: Option<SidecarClient>,


Hcl::new is now only defined under #[cfg(guest_arch = "x86_64")], but there are call sites (e.g. the #[cfg(not(guest_arch = "x86_64"))] path in virt_mshv_vtl) that still expect an Hcl::new(isolation, sidecar) constructor. Add a complementary #[cfg(not(guest_arch = "x86_64"))] constructor (or refactor to a single signature) so non-x86_64 guest builds continue to compile.

Suggested change

#[cfg(guest_arch = "x86_64")] // xtask-fmt allow-target-arch cpu-intrinsic

pub fn new(

isolation: IsolationType,

sidecar: Option<SidecarClient>,

pub fn new(

isolation: IsolationType,

sidecar: Option<SidecarClient>,

#[cfg(guest_arch = "x86_64")] // xtask-fmt allow-target-arch cpu-intrinsic

Copilot · 2026-04-30T01:34:52Z

+        #[cfg(guest_arch = "x86_64")] // xtask-fmt allow-target-arch cpu-intrinsic
+        let hcl = Hcl::new(hcl_isolation, sidecar, params.cpuid_features).map_err(Error::Hcl)?;
+        #[cfg(not(guest_arch = "x86_64"))] // xtask-fmt allow-target-arch cpu-intrinsic
        let hcl = Hcl::new(hcl_isolation, sidecar).map_err(Error::Hcl)?;


The #[cfg(not(guest_arch = "x86_64"))] branch still calls Hcl::new(hcl_isolation, sidecar), but Hcl::new appears to be gated to guest_arch = "x86_64" only now. This will break non-x86_64 guest builds unless a non-x86_64 constructor is provided (or this call is updated to match the available API).

Suggested change

let hcl = Hcl::new(hcl_isolation, sidecar).map_err(Error::Hcl)?;

let hcl = Hcl::new(hcl_isolation, sidecar, params.cpuid_features).map_err(Error::Hcl)?;

sunilmut · 2026-04-30T02:32:17Z

+        {
+            return Err(HvError::AccessDenied);
+        }
+


When the secrets page is not set, I will update this hypercall to return a error status.

sunilmut · 2026-04-30T02:36:45Z

+            }
+            keys
+        } else {
+            [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS]


I will change this to store None for secrets so that the hypercall can check and if it is None, return proper error for the hypercall

smalis-msft · 2026-04-30T19:15:53Z

    isolation: IsolationType,
    snp_register_bitmap: [u8; 64],
    sidecar: Option<SidecarClient>,
+    cpuid_features: openhcl_cpuid_features::CpuidFeatures,


Do we need to store this? It looks like we're only checking it once in the constructor?

sunilmut added 2 commits April 30, 2026 00:55

SNP: Allow lower VTL to get VMPCK

4475a0e

Refactor cpuid into a separate crate

5440cfd

Copilot AI review requested due to automatic review settings April 30, 2026 01:26

sunilmut requested a review from a team as a code owner April 30, 2026 01:26

github-actions Bot added the unsafe Related to unsafe code label Apr 30, 2026

Copilot started reviewing on behalf of sunilmut April 30, 2026 01:27 View session

sunilmut commented Apr 30, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

sunilmut requested a review from jstarks April 30, 2026 01:30

Copilot AI reviewed Apr 30, 2026

View reviewed changes

sunilmut changed the title ~~Hypercall to let the VTL0 or VTL1 to retrieve its VMPCK so that it can communicate with the SNP~~ Hypercall to let the VTL0 or VTL1 to retrieve its VMPCK so that it can communicate with the PSP Apr 30, 2026

No debug for VMPCK

5592161

sunilmut commented Apr 30, 2026

View reviewed changes

smalis-msft reviewed Apr 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hypercall to let the VTL0 or VTL1 to retrieve its VMPCK so that it can communicate with the PSP#3408

Hypercall to let the VTL0 or VTL1 to retrieve its VMPCK so that it can communicate with the PSP#3408
sunilmut wants to merge 3 commits intomicrosoft:mainfrom
sunilmut:sunilmut/lower_vtl_vmpck

sunilmut commented Apr 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

sunilmut Apr 30, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

sunilmut Apr 30, 2026

Uh oh!

sunilmut Apr 30, 2026

Uh oh!

smalis-msft Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        let vmpck_keys = if let Some(secrets) = partition_params.snp_secrets {
-            let mut keys = [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS];
-            for (i, key) in keys.iter_mut().enumerate() {
-                let offset = SNP_SECRETS_VMPCK0_OFFSET + i * SNP_VMPCK_KEY_SIZE;
-                key.copy_from_slice(&secrets[offset..offset + SNP_VMPCK_KEY_SIZE]);
-            }
-            keys
-        } else {
-            [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS]
-        };
+        let secrets = partition_params.snp_secrets.ok_or_else(|| {
+            anyhow::anyhow!("missing SNP secrets page for SNP partition")
+        })?;
+        let required_len = SNP_SECRETS_VMPCK0_OFFSET + SNP_NUM_VMPCKS * SNP_VMPCK_KEY_SIZE;
+        if secrets.len() < required_len {
+            return Err(anyhow::anyhow!(
+                "SNP secrets page too short for VMPCK keys: got {} bytes, need at least {}",
+                secrets.len(),
+                required_len
+            ));
+        }
+        let mut vmpck_keys = [[0u8; SNP_VMPCK_KEY_SIZE]; SNP_NUM_VMPCKS];
+        for (i, key) in vmpck_keys.iter_mut().enumerate() {
+            let offset = SNP_SECRETS_VMPCK0_OFFSET + i * SNP_VMPCK_KEY_SIZE;
+            key.copy_from_slice(&secrets[offset..offset + SNP_VMPCK_KEY_SIZE]);
+        }

	sidecar: Option<SidecarClient>,
	sidecar: Option<SidecarClient>,
	#[cfg(target_arch = "x86_64")]

	let hcl = Hcl::new(hcl_isolation, sidecar).map_err(Error::Hcl)?;
	let hcl = Hcl::new(hcl_isolation, sidecar, params.cpuid_features).map_err(Error::Hcl)?;

Conversation

sunilmut commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

sunilmut Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

sunilmut Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

sunilmut Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

smalis-msft Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sunilmut commented Apr 30, 2026 •

edited

Loading