diff --git a/Cargo.lock b/Cargo.lock index 08bbbdf6b0..8f4acb4575 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9284,6 +9284,7 @@ version = "0.0.0" dependencies = [ "anyhow", "disk_backend_resources", + "disk_vhd1", "futures", "get_resources", "guid", @@ -9318,6 +9319,7 @@ dependencies = [ "vm_resource", "vmgs_resources", "vmm_test_macros", + "vtl2_settings_proto", "zerocopy 0.8.25", ] diff --git a/petri/src/requirements.rs b/petri/src/requirements.rs index 2923ad8d3c..2fa2308d12 100644 --- a/petri/src/requirements.rs +++ b/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 { diff --git a/petri/src/vm/hyperv/hyperv.psm1 b/petri/src/vm/hyperv/hyperv.psm1 index bc56753816..2e29a85d57 100644 --- a/petri/src/vm/hyperv/hyperv.psm1 +++ b/petri/src/vm/hyperv/hyperv.psm1 @@ -464,4 +464,62 @@ function Get-GuestStateFile $guestStateFile = $vssd.GuestStateFile return "$guestStateDataRoot\$guestStateFile" -} \ No newline at end of file +} + +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 +} diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index b63465d02b..1ab2233b3e 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/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, +} + +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>( + &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 Self::VmmConfig + Send>, resources: &PetriVmResources, ) -> anyhow::Result { - 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) -> 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>( + path: P, + id: Option, + 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) -> anyhow::Result<()> { let stderr = String::from_utf8_lossy(&output.stderr); anyhow::bail!("icacls failed: {stderr}"); } + Ok(()) } diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 5359f44bdd..3a812113f5 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/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 { + 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>( + 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") +} diff --git a/petri/src/vm/hyperv/vm.rs b/petri/src/vm/hyperv/vm.rs index aec6efad11..7a8a3cffd6 100644 --- a/petri/src/vm/hyperv/vm.rs +++ b/petri/src/vm/hyperv/vm.rs @@ -497,7 +497,8 @@ impl HyperVVM { &self, openhcl_command_line: impl AsRef, ) -> anyhow::Result<()> { - powershell::run_set_vm_command_line(&self.vmid, &self.ps_mod, openhcl_command_line).await + powershell::run_set_vm_command_line(&self.vmid, &self.ps_mod, openhcl_command_line.as_ref()) + .await } /// Enable VMBusRelay @@ -524,6 +525,32 @@ impl HyperVVM { pub async fn get_guest_state_file(&self) -> anyhow::Result { powershell::run_get_guest_state_file(&self.vmid, &self.ps_mod).await } + + /// Add an NVMe device to the Hyper-V VM. (Microsoft-internal) + /// + /// TODO FUTURE: Abstract this out such that it can take an identifier for a + /// device, rather than a set of VHD paths (at that point it could be used + /// on real hardware). Such work could leverage NVMe DDA, which is availabe + /// outside of the Microsoft-internal environment (but then you have to have + /// spare NVMe devices on the system where you're running this). + /// Tracked by #2151. + pub async fn add_nvme_device>( + &self, + instance_id: Option<&Guid>, + target_vtl: u32, + vhd_paths: &[P], + ) -> anyhow::Result<()> { + powershell::run_add_nvme(&self.vmid, 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<()> { + powershell::run_set_base_vtl2_settings(&self.vmid, &self.ps_mod, settings).await + } } impl Drop for HyperVVM { diff --git a/vmm_tests/vmm_test_macros/src/lib.rs b/vmm_tests/vmm_test_macros/src/lib.rs index 61402b6a20..f577672f89 100644 --- a/vmm_tests/vmm_test_macros/src/lib.rs +++ b/vmm_tests/vmm_test_macros/src/lib.rs @@ -627,6 +627,19 @@ pub fn openvmm_test( .into() } +/// Same options as `vmm_test`, but only for Hyper-V tests +#[proc_macro_attribute] +pub fn hyperv_test( + attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { + let args = parse_macro_input!(attr as Args); + let item = parse_macro_input!(item as ItemFn); + make_vmm_test(args, item, Some(Vmm::HyperV), true) + .unwrap_or_else(|err| err.to_compile_error()) + .into() +} + /// Same options as `vmm_test`, but only for OpenVMM tests and without using pipette in VTL0. #[proc_macro_attribute] pub fn openvmm_test_no_agent( @@ -809,6 +822,25 @@ fn build_requirements(config: &Config, name: &str) -> Option { }; } + // Special case for NVMe tests on Hyper-V + let is_nvme = matches!( + config.firmware, + Firmware::OpenhclUefi(OpenhclUefiOptions { nvme: true, .. }, _) + ) || name.contains("add_nvme_device"); + if is_hyperv && is_nvme { + let hyperv_nvme_requirement_expr = + quote!(::petri::requirements::TestRequirement::SupportsNvmeStorage); + requirement_expr = match requirement_expr { + Some(existing) => Some(quote!( + ::petri::requirements::TestRequirement::And( + Box::new(#existing), + Box::new(#hyperv_nvme_requirement_expr) + ) + )), + None => Some(hyperv_nvme_requirement_expr), + }; + } + if requirement_expr.is_some() { Some(quote!(::petri::requirements::TestCaseRequirements::new(#requirement_expr))) } else { diff --git a/vmm_tests/vmm_tests/Cargo.toml b/vmm_tests/vmm_tests/Cargo.toml index 668c1b54d7..3c9dd009e0 100644 --- a/vmm_tests/vmm_tests/Cargo.toml +++ b/vmm_tests/vmm_tests/Cargo.toml @@ -26,6 +26,7 @@ petri.workspace = true get_resources.workspace = true hvlite_defs.workspace = true disk_backend_resources.workspace = true +disk_vhd1.workspace = true hyperv_ic_resources.workspace = true net_backend_resources.workspace = true nvme_resources.workspace = true @@ -35,6 +36,7 @@ scsidisk_resources.workspace = true storvsp_resources.workspace = true virtio_resources.workspace = true vm_resource.workspace = true +vtl2_settings_proto.workspace = true guid.workspace = true kmsg.workspace = true diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs index 535ac492d6..a1d0b62717 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -18,6 +18,8 @@ use mesh::rpc::RpcSend; use nvme_resources::NamespaceDefinition; use nvme_resources::NvmeControllerHandle; use petri::PetriVmBuilder; +#[cfg(windows)] +use petri::hyperv::HyperVPetriBackend; use petri::openvmm::OpenVmmPetriBackend; use petri::pipette::PipetteClient; use petri::pipette::cmd; @@ -36,7 +38,11 @@ use storvsp_resources::ScsiControllerHandle; use storvsp_resources::ScsiDeviceAndPath; use storvsp_resources::ScsiPath; use vm_resource::IntoResource; +#[cfg(windows)] +use vmm_test_macros::hyperv_test; use vmm_test_macros::openvmm_test; +#[cfg(windows)] +use vtl2_settings_proto::Vtl2Settings; /// Create a VPCI device config for an NVMe controller assigned to VTL2, with a single namespace. /// The namespace will be backed by either a file or a ramdisk, depending on whether @@ -623,3 +629,103 @@ async fn openhcl_linux_storvsp_dvd_nvme( Ok(()) } + +#[cfg(windows)] +#[hyperv_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] +pub async fn add_nvme_device(config: PetriVmBuilder) -> anyhow::Result<()> { + let vtl0_instance_id = Guid::new_random(); + let vtl2_instance_id = Guid::new_random(); + const VHD_SIZE: u64 = 200 * 1024 * 1024; // 200 MiB + + // todo: generate a better temp file name + let temp_dir = tempfile::tempdir()?; + let vhd_path = temp_dir.path().join("test.vhd"); + let file = File::options() + .create(true) + .create_new(true) + .truncate(true) + .read(true) + .write(true) + .open(&vhd_path) + .context("create file")?; + + file.set_len(VHD_SIZE).context("set file length")?; + disk_vhd1::Vhd1Disk::make_fixed(&file).context("make fixed")?; + + let initial_vtl2_settings = Vtl2Settings { + version: vtl2_settings_proto::vtl2_settings_base::Version::V1.into(), + dynamic: Some(vtl2_settings_proto::Vtl2SettingsDynamic { + // You can't add a _controller_ at runtime, so make sure to add it + // here before the test powers on the VM. + storage_controllers: vec![ + Vtl2StorageControllerBuilder::scsi() + .with_instance_id(vtl0_instance_id) + .build(), + ], + ..Default::default() + }), + fixed: None, + namespace_settings: Default::default(), + }; + + // This is a bit ugly; ideally the HyperV backend would munge and set any + // VTL2 settings to the modify call (just like happens for OpenVMM). + // For now, just take the expedient path to get this test stood up. + let mut vtl2_settings = initial_vtl2_settings.clone(); + + let (mut vm, agent) = config + .with_vmbus_redirect(true) + .modify_backend(move |b| { + assert!(b.initial_vtl2_settings.is_none()); + petri::hyperv::HyperVPetriConfig { + initial_vtl2_settings: Some(initial_vtl2_settings), + } + }) + .run() + .await?; + + // Runtime add the NVMe device and update the VTL2 settings to + // include it. + // + // This _could_ happen before the VM is started, but do it at runtime + // for test simplicity. + + vm.backend() + .add_nvme_device(Some(&vtl2_instance_id), 2, &[vhd_path.to_str().unwrap()]) + .await?; + + vtl2_settings.dynamic.as_mut().unwrap().storage_controllers[0] + .luns + .push( + Vtl2LunBuilder::disk() + .with_location(0) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + vtl2_instance_id, + 1, + )) + .build(), + ); + vm.backend().set_base_vtl2_settings(&vtl2_settings).await?; + + match vm.inspect_openhcl("vm/nvme/devices", None, None).await { + Err(e) => tracing::error!(?e, "Failed to inspect NVMe devices"), + Ok(devices) => tracing::info!(devices = %devices.json(), "NVMe devices"), + } + + test_storage_linux( + &agent, + vec![ExpectedGuestDevice { + controller_guid: vtl0_instance_id, + lun: 0, + disk_size_sectors: (VHD_SIZE / 512) as usize, + friendly_name: "nvme".to_string(), + }], + ) + .await?; + + agent.power_off().await?; + vm.wait_for_clean_teardown().await?; + + Ok(()) +}