rfc: petri: hyperv backed with emulated nvme

Cargo.lock

@@ -9283,6 +9283,7 @@ version = "0.0.0"
 dependencies = [
  "anyhow",
  "disk_backend_resources",
+ "disk_vhd1",
  "futures",
  "get_resources",
  "guid",
@@ -9317,6 +9318,7 @@ dependencies = [
  "vm_resource",
  "vmgs_resources",
  "vmm_test_macros",
+ "vtl2_settings_proto",
  "zerocopy 0.8.25",
 ]
 

petri/src/requirements.rs

@@ -63,6 +63,8 @@ pub struct HostContext {
     pub vendor: Vendor,
     /// Execution environment
     pub execution_environment: ExecutionEnvironment,
+    /// Whether the host supports NVMe storage using Hyper-V as the VMM
+    pub supports_hyperv_nvme_storage: bool,
 }
 
 impl HostContext {
@@ -140,6 +142,21 @@ impl HostContext {
             }
         };
 
+        let supports_hyperv_nvme_storage = {
+            #[cfg(windows)]
+            {
+                crate::vm::hyperv::powershell::run_check_vm_host_supports_hyperv_storage()
+                    .await
+                    .unwrap_or(false) // Assume false if we can't query
+            }
+            #[cfg(not(windows))]
+            {
+                // Not checked for non-Windows platforms, so just assume that the feature
+                // is supported.
+                true
+            }
+        };
+
         Self {
             vm_host_info,
             vendor,
@@ -148,6 +165,7 @@ impl HostContext {
             } else {
                 ExecutionEnvironment::Baremetal
             },
+            supports_hyperv_nvme_storage,
         }
     }
 }
@@ -158,6 +176,8 @@ pub enum TestRequirement {
     ExecutionEnvironment(ExecutionEnvironment),
     /// Vendor requirement.
     Vendor(Vendor),
+    /// Supports NVMe storage requirement.
+    SupportsNvmeStorage,
     /// Isolation requirement.
     Isolation(IsolationType),
     /// Logical AND of two requirements.
@@ -174,6 +194,7 @@ impl TestRequirement {
         match self {
             TestRequirement::ExecutionEnvironment(env) => context.execution_environment == *env,
             TestRequirement::Vendor(vendor) => context.vendor == *vendor,
+            TestRequirement::SupportsNvmeStorage => context.supports_hyperv_nvme_storage,
             TestRequirement::Isolation(isolation_type) => {
                 if let Some(vm_host_info) = &context.vm_host_info {
                     match isolation_type {

petri/src/vm/hyperv/hyperv.psm1

@@ -464,4 +464,62 @@ function Get-GuestStateFile
     $guestStateFile = $vssd.GuestStateFile
 
     return "$guestStateDataRoot\$guestStateFile"
-}
+}
+
+function Set-Vtl2Settings {
+    [CmdletBinding(DefaultParameterSetName = "ByName")]
+    param (
+        [Parameter(Position = 0, Mandatory = $true, ParameterSetName = "ByGuid")]
+        [Guid] $VmId,
+
+        [Parameter(Mandatory = $true)]
+        [ValidateNotNullOrEmpty()]
+        [string]$Namespace,
+
+        [Parameter(Mandatory = $true)]
+        [string]$SettingsFile,
+
+        [string]$ClientName = 'Petri'
+    )
+
+    $settingsContent = Get-Content -Raw -Path $SettingsFile
+
+    $guestManagement = Get-VmGuestManagementService
+
+    $options = New-Object Microsoft.Management.Infrastructure.Options.CimOperationOptions
+    $options.SetCustomOption("ClientName", $ClientName, $false)
+
+    # Parameter - VmId
+    $p1 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("VmId", $VmId.ToString(), [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In)
+
+    # Parameter - Namespace
+    $p2 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Namespace", $Namespace, [Microsoft.Management.Infrastructure.cimtype]::String, [Microsoft.Management.Infrastructure.CimFlags]::In)
+
+    # Parameter - Settings
+    # The input is a byte buffer with the size prepended.
+    # Size is a uint32 in network byte order (i.e. Big Endian)
+    # Size includes the size itself and the payload.
+
+    $bytes = [system.Text.Encoding]::UTF8.GetBytes($settingsContent)
+
+    $header = [System.BitConverter]::GetBytes([uint32]($bytes.Length + 4))
+    if ([System.BitConverter]::IsLittleEndian) {
+        [System.Array]::Reverse($header)
+    }
+    $bytes = $header + $bytes
+
+    $p3 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("Settings", $bytes, [Microsoft.Management.Infrastructure.cimtype]::UInt8Array, [Microsoft.Management.Infrastructure.CimFlags]::In)
+
+    $result = $guestManagement | Invoke-CimMethod -MethodName GetManagementVtlSettings -Arguments @{"VmId" = $VmId.ToString(); "Namespace" = $Namespace } |
+    Trace-CimMethodExecution -CimInstance $guestManagement -MethodName "GetManagementVtlSettings"
+    $updateId = $result.CurrentUpdateId
+
+    $p4 = [Microsoft.Management.Infrastructure.CimMethodParameter]::Create("CurrentUpdateId", $updateId, [Microsoft.Management.Infrastructure.cimtype]::UInt64, [Microsoft.Management.Infrastructure.CimFlags]::In)
+
+    $params = New-Object Microsoft.Management.Infrastructure.CimMethodParametersCollection
+    $params.Add($p1); $params.Add($p2); $params.Add($p3); $params.Add($p4)
+
+    $cimSession = New-CimSession
+    $cimSession.InvokeMethod("root\virtualization\v2", $guestManagement, "SetManagementVtlSettings", $params, $options) |
+    Trace-CimMethodExecution -CimInstance $guestManagement -MethodName "SetManagementVtlSettings" | Out-Null
+}

petri/src/vm/hyperv/mod.rs

@@ -69,9 +69,46 @@ pub struct HyperVPetriRuntime {
     is_isolated: bool,
 }
 
+/// Additional configuration for a Hyper-V VM.
+pub struct HyperVPetriConfig {
+    /// VTL2 settings to configure on the VM before petri powers it on.
+    pub initial_vtl2_settings: Option<vtl2_settings_proto::Vtl2Settings>,
+}
+
+impl HyperVPetriRuntime {
+    /// Adds an NVMe device to the VM, with a set of NVMe namespaces. Each
+    /// namespace is backed by one of the supplied VHDs. The namespaces will be
+    /// added in-order. That is, the first entry in the list of `vhd_paths` will
+    /// have `nsid` `1`, the second entry will have `nsid` `2`, and so on.
+    pub async fn add_nvme_device<P: AsRef<Path>>(
+        &self,
+        instance_id: Option<&guid::Guid>,
+        target_vtl: u32,
+        vhd_paths: &[P],
+    ) -> anyhow::Result<()> {
+        for vhd_path in vhd_paths {
+            acl_for_vm(vhd_path, Some(*self.vm.vmid()), VmFileAccess::FullControl)
+                .context("grant VM access to VHD")?;
+        }
+
+        self.vm
+            .add_nvme_device(instance_id, target_vtl, vhd_paths)
+            .await
+    }
+
+    /// Set the VTL2 settings in the `Base` namespace (fixed settings, storage
+    /// settings, etc).
+    pub async fn set_base_vtl2_settings(
+        &self,
+        settings: &vtl2_settings_proto::Vtl2Settings,
+    ) -> anyhow::Result<()> {
+        self.vm.set_base_vtl2_settings(settings).await
+    }
+}
+
 #[async_trait]
 impl PetriVmmBackend for HyperVPetriBackend {
-    type VmmConfig = ();
+    type VmmConfig = HyperVPetriConfig;
     type VmRuntime = HyperVPetriRuntime;
 
     fn check_compat(firmware: &Firmware, arch: MachineArch) -> bool {
@@ -94,10 +131,6 @@ impl PetriVmmBackend for HyperVPetriBackend {
         modify_vmm_config: Option<impl FnOnce(Self::VmmConfig) -> Self::VmmConfig + Send>,
         resources: &PetriVmResources,
     ) -> anyhow::Result<Self::VmRuntime> {
-        if modify_vmm_config.is_some() {
-            panic!("specified modify_vmm_config, but that is not supported for hyperv");
-        }
-
         let PetriVmConfig {
             name,
             arch,
@@ -395,7 +428,7 @@ impl PetriVmmBackend for HyperVPetriBackend {
             // Hyper-V (e.g., if it is in a WSL filesystem).
             let igvm_file = temp_dir.path().join("igvm.bin");
             fs_err::copy(src_igvm_file, &igvm_file).context("failed to copy igvm file")?;
-            acl_read_for_vm(&igvm_file, Some(*vm.vmid()))
+            acl_for_vm(&igvm_file, Some(*vm.vmid()), VmFileAccess::Read)
                 .context("failed to set ACL for igvm file")?;
 
             // TODO: only increase VTL2 memory on debug builds
@@ -493,6 +526,20 @@ impl PetriVmmBackend for HyperVPetriBackend {
             hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file),
         ));
 
+        let initial_vtl2_settings = if let Some(f) = modify_vmm_config {
+            f(HyperVPetriConfig {
+                initial_vtl2_settings: None,
+            })
+            .initial_vtl2_settings
+        } else {
+            None
+        };
+
+        if let Some(settings) = initial_vtl2_settings {
+            tracing::info!(?settings, "applying initial VTL2 settings");
+            vm.set_base_vtl2_settings(&settings).await?;
+        }
+
         vm.start().await?;
 
         Ok(HyperVPetriRuntime {
@@ -617,17 +664,36 @@ impl PetriVmRuntime for HyperVPetriRuntime {
     }
 }
 
-fn acl_read_for_vm(path: &Path, id: Option<guid::Guid>) -> anyhow::Result<()> {
+/// The type of access to grant the VM for a file.
+#[allow(missing_docs)]
+enum VmFileAccess {
+    Read,
+    FullControl,
+}
+
+/// The Hyper-V security model requires that the VM be granted explicit access to any
+/// resources assigned. Hyper-V takes care of this when you use the admin facing
+/// PowerShell cmdlets and some WMI flows. But, some tests use lower level APIs that don't
+/// do this automatically.
+fn acl_for_vm<P: AsRef<Path>>(
+    path: P,
+    id: Option<guid::Guid>,
+    access: VmFileAccess,
+) -> anyhow::Result<()> {
     let sid_arg = format!(
-        "NT VIRTUAL MACHINE\\{name}:R",
+        "NT VIRTUAL MACHINE\\{name}:{perm}",
         name = if let Some(id) = id {
             format!("{id:X}")
         } else {
             "Virtual Machines".to_string()
+        },
+        perm = match access {
+            VmFileAccess::Read => "R",
+            VmFileAccess::FullControl => "F",
         }
     );
     let output = std::process::Command::new("icacls.exe")
-        .arg(path)
+        .arg(path.as_ref())
         .arg("/grant")
         .arg(sid_arg)
         .output()
@@ -636,6 +702,7 @@ fn acl_read_for_vm(path: &Path, id: Option<guid::Guid>) -> anyhow::Result<()> {
         let stderr = String::from_utf8_lossy(&output.stderr);
         anyhow::bail!("icacls failed: {stderr}");
     }
+
     Ok(())
 }
 

petri/src/vm/hyperv/powershell.rs

@@ -16,6 +16,7 @@ use powershell_builder::PowerShellBuilder;
 use serde::Deserialize;
 use serde::Serialize;
 use std::ffi::OsStr;
+use std::io::Write;
 use std::path::Path;
 use std::path::PathBuf;
 use std::str::FromStr;
@@ -1092,3 +1093,110 @@ pub async fn run_get_guest_state_file(vmid: &Guid, ps_mod: &Path) -> anyhow::Res
 
     Ok(PathBuf::from(output))
 }
+
+/// Queries whether the host supports Hyper-V NVMe storage for VMs.
+///
+/// This is true if there are Microsoft-internal utilities installed on the
+/// host that support this feature.
+/// FUTURE: If sufficient environments exist, we can do this using DDA'd NVMe
+/// devices.
+pub async fn run_check_vm_host_supports_hyperv_storage() -> anyhow::Result<bool> {
+    let output: String = run_host_cmd(
+        PowerShellBuilder::new()
+            .cmdlet("Test-Path")
+            .arg(
+                "Path",
+                "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1",
+            )
+            .next()
+            .cmdlet("Import-Module")
+            .positional(
+                "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1",
+            )
+            .next()
+            .cmdlet("Test-MicrosoftInternalVmNvmeStorage")
+            .finish()
+            .build(),
+    )
+    .await
+    .context("run_check_vm_host_supports_hyperv_storage")?;
+
+    Ok(output.trim().eq_ignore_ascii_case("true"))
+}
+
+/// Adds an NVMe device to the VM using Microsoft-internal utilities.
+///
+/// TODO FUTURE: Check that the resource is successfully added to the VM. e.g.
+/// if there is a signature issue, it seems that this call completes yet there
+/// was an error.
+pub async fn run_add_nvme<P: AsRef<Path>>(
+    vmid: &Guid,
+    instance_id: Option<&Guid>,
+    target_vtl: u32,
+    disk_paths: &[P],
+) -> anyhow::Result<()> {
+    run_host_cmd(
+        PowerShellBuilder::new()
+            .cmdlet("Import-Module")
+            .positional(
+                "c:\\OpenVMMCI\\MicrosoftInternalTestHelpers\\MicrosoftInternalTestHelpers.psm1",
+            )
+            .next()
+            .cmdlet("Add-MicrosoftInternalVmNvmeStorage")
+            .arg("VmId", vmid)
+            .arg("TargetVtl", target_vtl)
+            .arg(
+                "DiskPaths",
+                ps::Array::new(
+                    disk_paths
+                        .iter()
+                        .map(|p| p.as_ref().to_str().expect("path can be converted to &str")),
+                ),
+            )
+            .arg_opt("Vsid", instance_id)
+            .finish()
+            .build(),
+    )
+    .await
+    .map(|_| ())
+    .context("run_add_nvme")
+}
+
+/// Sets the VTL2 settings for a VM that exist in the `Base` namespace.
+///
+/// This should include the fixed VTL2 settings, as well as any storage
+/// settings.
+///
+/// TODO FUTURE: Detect if the settings should be in `json` or `protobuf` format
+/// based on what is already there (or let the caller specify explicitly so that
+/// we can test the handling of both deserializers).
+pub async fn run_set_base_vtl2_settings(
+    vmid: &Guid,
+    ps_mod: &Path,
+    vtl2_settings: &vtl2_settings_proto::Vtl2Settings,
+) -> anyhow::Result<()> {
+    // Pass the settings via a file to avoid challenges escaping the string across
+    // the command line.
+    let mut tempfile = tempfile::NamedTempFile::new().context("creating tempfile")?;
+    tempfile
+        .write_all(serde_json::to_string(vtl2_settings)?.as_bytes())
+        .context("writing settings to tempfile")?;
+
+    tracing::debug!(?tempfile, ?vtl2_settings, ?vmid, "set base vtl2 settings");
+
+    run_host_cmd(
+        PowerShellBuilder::new()
+            .cmdlet("Import-Module")
+            .positional(ps_mod)
+            .next()
+            .cmdlet("Set-Vtl2Settings")
+            .arg("VmId", vmid)
+            .arg("SettingsFile", tempfile.path())
+            .arg("Namespace", "Base")
+            .finish()
+            .build(),
+    )
+    .await
+    .map(|_| ())
+    .context("set_base_vtl2_settings")
+}