From b753d8ddb579cd98f5ba8f7c98efc603a76bc7fe Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Tue, 9 Dec 2025 18:19:02 -0800 Subject: [PATCH 01/10] generic storage controllers --- petri/src/vm/hyperv/mod.rs | 180 ++++++++-------- petri/src/vm/hyperv/powershell.rs | 11 + petri/src/vm/mod.rs | 200 ++++++++++++++++-- petri/src/vm/openvmm/construct.rs | 5 + petri/src/vm/openvmm/mod.rs | 10 +- petri/src/vm/openvmm/runtime.rs | 11 + petri/src/vm/openvmm/start.rs | 13 +- .../vmm_tests/tests/tests/x86_64/storage.rs | 33 ++- 8 files changed, 341 insertions(+), 122 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index c55e27fc7a..273cc8cd78 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -15,8 +15,9 @@ use crate::NoPetriVmInspector; use crate::OpenHclConfig; use crate::OpenHclServicingFlags; use crate::OpenvmmLogConfig; -use crate::PetriDiskType; +use crate::PetriDisk; use crate::PetriHaltReason; +use crate::PetriStorageController; use crate::PetriVmConfig; use crate::PetriVmResources; use crate::PetriVmRuntime; @@ -26,6 +27,7 @@ use crate::PetriVmgsResource; use crate::PetriVmmBackend; use crate::SecureBootTemplate; use crate::ShutdownKind; +use crate::StorageType; use crate::TpmConfig; use crate::UefiConfig; use crate::VmmQuirks; @@ -51,12 +53,14 @@ use petri_artifacts_common::tags::OsFlavor; use petri_artifacts_core::ArtifactResolver; use petri_artifacts_core::ResolvedArtifact; use pipette_client::PipetteClient; +use std::collections::HashMap; use std::io::ErrorKind; use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use tempfile::TempDir; use tempfile::TempPath; use vm::HyperVVM; use vmgs_resources::GuestStateEncryptionPolicy; @@ -65,18 +69,12 @@ use vtl2_settings_proto::Vtl2Settings; /// The Hyper-V Petri backend pub struct HyperVPetriBackend {} -/// Represents a SCSI Controller addeded to a VM. +/// Details about a storage controller addeded to a VM. #[derive(Debug)] -pub struct HyperVScsiController { - /// An identifier provided by the test to identify this controller. - pub test_id: String, - +pub struct RealizedHyperVStorageController { /// The controller number assigned by Hyper-V. pub controller_number: u32, - /// The target VTL this controller is mapped to (supplied by test). - pub target_vtl: u32, - /// The VSID assigned by Hyper-V for this controller. pub vsid: guid::Guid, } @@ -85,42 +83,17 @@ pub struct HyperVScsiController { pub struct HyperVPetriRuntime { vm: HyperVVM, log_tasks: Vec>>, - _temp_dir: tempfile::TempDir, + temp_dir: TempDir, output_dir: PathBuf, driver: DefaultDriver, is_openhcl: bool, is_isolated: bool, - - /// Test-added SCSI controllers. - /// TODO (future PR): push this into `PetriVmConfig` and use in - /// openvmm as well. - additional_scsi_controllers: Vec, -} - -/// Additional configuration for a Hyper-V VM. -#[derive(Default, Debug)] -pub struct HyperVPetriConfig { - /// Test-added SCSI controllers (targeting specific VTLs). - /// A tuple if test-identifier and targetvtl. Test-identifier - /// is used so that the test can find a specific controller, if that - /// is important to the test. These are resolved into a list of - /// [`HyperVScsiController`] objects stored in the runtime. - additional_scsi_controllers: Vec<(String, u32)>, -} - -impl HyperVPetriConfig { - /// Add an additional SCSI controller to the VM. - /// Will be added before the VM starts. - pub fn with_additional_scsi_controller(mut self, test_id: String, target_vtl: u32) -> Self { - self.additional_scsi_controllers.push((test_id, target_vtl)); - self - } } #[async_trait] impl PetriVmmBackend for HyperVPetriBackend { - type VmmConfig = HyperVPetriConfig; + type VmmConfig = (); type VmRuntime = HyperVPetriRuntime; fn check_compat(firmware: &Firmware, arch: MachineArch) -> bool { @@ -201,9 +174,12 @@ impl PetriVmmBackend for HyperVPetriBackend { async fn run( self, config: PetriVmConfig, - modify_vmm_config: Option Self::VmmConfig + Send>, + _modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { + ) -> anyhow::Result<( + Self::VmRuntime, + PetriVmRuntimeConfig, + )> { let PetriVmConfig { name, arch, @@ -217,6 +193,7 @@ impl PetriVmmBackend for HyperVPetriBackend { vmgs, tpm, guest_crash_disk, + additional_storage_controllers, } = config; let PetriVmResources { driver, log_source } = resources; @@ -295,6 +272,7 @@ impl PetriVmmBackend for HyperVPetriBackend { let mut openhcl_command_line = openhcl_config.as_ref().map(|(_, c)| c.command_line()); let mut vtl2_settings = None; + let mut storage_controllers = HashMap::new(); let vmgs_path = { // TODO: add support for configuring the TPM in Hyper-V @@ -345,15 +323,15 @@ impl PetriVmmBackend for HyperVPetriBackend { }; match disk { - None | Some(PetriDiskType::Memory) => None, - Some(PetriDiskType::Differencing(parent_path)) => { + None | Some(PetriDisk::Memory) => None, + Some(PetriDisk::Differencing(parent_path)) => { let diff_disk_path = temp_dir .path() .join(parent_path.file_name().context("path has no filename")?); make_temp_diff_disk(&diff_disk_path, &parent_path).await?; Some(diff_disk_path) } - Some(PetriDiskType::Persistent(path)) => Some(path), + Some(PetriDisk::Persistent(path)) => Some(path), } }; @@ -638,24 +616,51 @@ impl PetriVmmBackend for HyperVPetriBackend { hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file), )); - let mut added_controllers = Vec::new(); - // TODO: If OpenHCL is being used, then translate storage through it. // (requires changes above where VHDs are added) - if let Some(modify_vmm_config) = modify_vmm_config { - let config = modify_vmm_config(HyperVPetriConfig::default()); - tracing::debug!(?config, "additional hyper-v config"); + // Add additional storage + for ( + test_id, + PetriStorageController { + target_vtl, + controller_type, + disks, + .. + }, + ) in additional_storage_controllers + { + let (hyperv_controller_type, (controller_number, vsid)) = match controller_type { + StorageType::Ide => todo!(), + StorageType::Scsi => ( + powershell::ControllerType::Scsi, + vm.add_scsi_controller(target_vtl as u32).await?, + ), + StorageType::Nvme => todo!(), + }; - for (test_id, target_vtl) in config.additional_scsi_controllers { - let (controller_number, vsid) = vm.add_scsi_controller(target_vtl).await?; - added_controllers.push(HyperVScsiController { - test_id, - controller_number, - target_vtl, - vsid, - }); + for (controller_location, disk) in disks.iter() { + vm.add_vhd( + &petri_disk_to_hyperv(disk, &temp_dir).await?, + hyperv_controller_type, + Some(*controller_location), + Some(controller_number), + ) + .await?; } + + storage_controllers.insert( + test_id, + PetriStorageController { + target_vtl, + controller_type, + disks, + realized: RealizedHyperVStorageController { + controller_number, + vsid, + }, + }, + ); } // Configure the TPM @@ -693,49 +698,25 @@ impl PetriVmmBackend for HyperVPetriBackend { HyperVPetriRuntime { vm, log_tasks, - _temp_dir: temp_dir, + temp_dir, output_dir: log_source.output_dir().to_owned(), driver: driver.clone(), is_openhcl, is_isolated, - additional_scsi_controllers: added_controllers, }, - PetriVmRuntimeConfig { vtl2_settings }, + PetriVmRuntimeConfig { + vtl2_settings, + storage_controllers, + }, )) } } -impl HyperVPetriRuntime { - /// Get the list of additional SCSI controllers added to this VM (those - /// configured to be added by the test, as opposed to the petri framework). - pub fn get_additional_scsi_controllers(&self) -> &[HyperVScsiController] { - &self.additional_scsi_controllers - } - - /// Adds a VHD with the optionally specified location (a.k.a LUN) to the - /// optionally specified controller. - pub async fn add_vhd( - &mut self, - vhd: impl AsRef, - controller_type: powershell::ControllerType, - controller_location: Option, - controller_number: Option, - ) -> anyhow::Result<()> { - self.vm - .add_vhd( - vhd.as_ref(), - controller_type, - controller_location, - controller_number, - ) - .await - } -} - #[async_trait] impl PetriVmRuntime for HyperVPetriRuntime { type VmInspector = NoPetriVmInspector; type VmFramebufferAccess = vm::HyperVFramebufferAccess; + type RealizedStorageController = RealizedHyperVStorageController; async fn teardown(mut self) -> anyhow::Result<()> { futures::future::join_all(self.log_tasks.into_iter().map(|t| t.cancel())).await; @@ -867,6 +848,23 @@ impl PetriVmRuntime for HyperVPetriRuntime { async fn set_vtl2_settings(&mut self, settings: &Vtl2Settings) -> anyhow::Result<()> { self.vm.set_base_vtl2_settings(settings).await } + + async fn add_disk( + &mut self, + disk: &PetriDisk, + controller_type: StorageType, + controller_location: u8, + controller: &Self::RealizedStorageController, + ) -> anyhow::Result<()> { + self.vm + .add_vhd( + &petri_disk_to_hyperv(disk, &self.temp_dir).await?, + controller_type.into(), + Some(controller_location), + Some(controller.controller_number), + ) + .await + } } fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { @@ -955,3 +953,17 @@ async fn make_temp_diff_disk( blocking::unblock(move || disk_vhdmp::Vhd::create_diff(&path, &parent_path)).await?; Ok(()) } + +async fn petri_disk_to_hyperv(disk: &PetriDisk, temp_dir: &TempDir) -> anyhow::Result { + Ok(match disk { + PetriDisk::Memory => todo!(), + PetriDisk::Differencing(parent_path) => { + let diff_disk_path = temp_dir + .path() + .join(parent_path.file_name().context("path has no filename")?); + make_temp_diff_disk(&diff_disk_path, &parent_path).await?; + diff_disk_path + } + PetriDisk::Persistent(path) => path.clone(), + }) +} diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 98efb92312..0add798e92 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/petri/src/vm/hyperv/powershell.rs @@ -5,6 +5,7 @@ use crate::CommandError; use crate::OpenHclServicingFlags; +use crate::StorageType; use crate::VmScreenshotMeta; use crate::run_host_cmd; use anyhow::Context; @@ -313,6 +314,16 @@ impl ps::AsVal for ControllerType { } } +impl From for ControllerType { + fn from(value: StorageType) -> Self { + match value { + StorageType::Ide => Self::Ide, + StorageType::Scsi => Self::Scsi, + StorageType::Nvme => todo!(), + } + } +} + /// Runs Add-VMHardDiskDrive with the given arguments. pub async fn run_add_vm_hard_disk_drive( args: HyperVAddVMHardDiskDriveArgs<'_>, diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 9c9ce1e7ff..1ed36e30c3 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -34,6 +34,7 @@ use petri_artifacts_core::ResolvedArtifact; use petri_artifacts_core::ResolvedOptionalArtifact; use pipette_client::PipetteClient; use std::collections::BTreeMap; +use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; use std::hash::Hash; use std::hash::Hasher; @@ -139,12 +140,16 @@ pub struct PetriVmConfig { pub boot_device_type: BootDeviceType, /// TPM configuration pub tpm: Option, + /// Additional storage for the VM + pub additional_storage_controllers: HashMap, } /// VM configuration that can be changed after the VM is created -pub struct PetriVmRuntimeConfig { +pub struct PetriVmRuntimeConfig { /// VTL2 settings pub vtl2_settings: Option, + /// Storage controllers and associated disks + pub storage_controllers: HashMap>, } /// Resources used by a Petri VM during contruction and runtime @@ -190,7 +195,10 @@ pub trait PetriVmmBackend { config: PetriVmConfig, modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)>; + ) -> anyhow::Result<( + Self::VmRuntime, + PetriVmRuntimeConfig<::RealizedStorageController>, + )>; } pub(crate) const PETRI_VTL0_SCSI_BOOT_LUN: u8 = 0; @@ -209,7 +217,7 @@ pub struct PetriVm { vmm_quirks: VmmQuirks, expected_boot_event: Option, - config: PetriVmRuntimeConfig, + config: PetriVmRuntimeConfig<::RealizedStorageController>, } impl PetriVmBuilder { @@ -300,6 +308,7 @@ impl PetriVmBuilder { vmgs: PetriVmgsResource::Ephemeral, tpm: None, guest_crash_disk, + additional_storage_controllers: HashMap::new(), }, modify_vmm_config: None, resources: PetriVmResources { @@ -721,7 +730,7 @@ impl PetriVmBuilder { } PetriGuestStateLifetime::Reprovision => PetriVmgsResource::Reprovision(disk), PetriGuestStateLifetime::Ephemeral => { - if !matches!(disk.disk, PetriDiskType::Memory) { + if !matches!(disk.disk, PetriDisk::Memory) { panic!("attempted to use ephemeral guest state after specifying backing vmgs") } PetriVmgsResource::Ephemeral @@ -747,20 +756,20 @@ impl PetriVmBuilder { /// Use the specified backing VMGS file pub fn with_initial_vmgs(self, disk: ResolvedArtifact) -> Self { - self.with_backing_vmgs(PetriDiskType::Differencing(disk.into())) + self.with_backing_vmgs(PetriDisk::Differencing(disk.into())) } /// Use the specified backing VMGS file pub fn with_persistent_vmgs(self, disk: impl AsRef) -> Self { - self.with_backing_vmgs(PetriDiskType::Persistent(disk.as_ref().to_path_buf())) + self.with_backing_vmgs(PetriDisk::Persistent(disk.as_ref().to_path_buf())) } - fn with_backing_vmgs(mut self, disk: PetriDiskType) -> Self { + fn with_backing_vmgs(mut self, disk: PetriDisk) -> Self { match &mut self.config.vmgs { PetriVmgsResource::Disk(vmgs) | PetriVmgsResource::ReprovisionOnFailure(vmgs) | PetriVmgsResource::Reprovision(vmgs) => { - if !matches!(vmgs.disk, PetriDiskType::Memory) { + if !matches!(vmgs.disk, PetriDisk::Memory) { panic!("already specified a backing vmgs file"); } vmgs.disk = disk; @@ -819,6 +828,58 @@ impl PetriVmBuilder { self } + /// Add an additional SCSI controller to the VM. + pub fn add_storage_controller( + mut self, + test_id: impl AsRef, + target_vtl: Vtl, + device_type: StorageType, + ) -> Self { + let test_id = test_id.as_ref().to_string(); + + if self + .config + .additional_storage_controllers + .contains_key(&test_id) + { + panic!("storage controller with id \"{test_id}\" already exists"); + } + + self.config.additional_storage_controllers.insert( + test_id, + PetriStorageController { + target_vtl, + controller_type: device_type, + disks: HashMap::new(), + realized: (), + }, + ); + self + } + + /// Adds a VHD with the optionally specified location (a.k.a LUN) to the + /// specified controller. + pub fn add_disk( + mut self, + disk: PetriDisk, + controller_test_id: impl AsRef, + controller_location: Option, + ) -> Self { + let controller_test_id = controller_test_id.as_ref(); + + let controller = self + .config + .additional_storage_controllers + .get_mut(controller_test_id) + .unwrap_or_else(|| { + panic!("storage controller with id \"{controller_test_id}\" does not exist") + }); + + _ = controller.add_disk(controller_location, disk); + + self + } + /// Get VM's guest OS flavor pub fn os_flavor(&self) -> OsFlavor { self.config.firmware.os_flavor() @@ -1173,6 +1234,49 @@ impl PetriVm { .set_vtl2_settings(self.config.vtl2_settings.as_ref().unwrap()) .await } + + /// Get the list of storage controllers added to this VM + pub fn get_storage_controllers( + &self, + ) -> &HashMap< + String, + PetriStorageController<::RealizedStorageController>, + > { + &self.config.storage_controllers + } + + /// Adds a VHD with the optionally specified location (a.k.a LUN) to the + /// specified controller. + pub async fn add_disk( + &mut self, + disk: PetriDisk, + controller_test_id: impl AsRef, + controller_location: Option, + ) -> anyhow::Result<()> { + let controller_test_id = controller_test_id.as_ref(); + + let controller = self + .config + .storage_controllers + .get_mut(controller_test_id) + .unwrap_or_else(|| { + panic!("storage controller with id \"{controller_test_id}\" does not exist") + }); + + let controller_location = controller.add_disk(controller_location, disk); + let disk = controller.disks.get(&controller_location).unwrap(); + + self.runtime + .add_disk( + disk, + controller.controller_type, + controller_location, + &controller.realized, + ) + .await?; + + Ok(()) + } } /// A running VM that tests can interact with. @@ -1182,6 +1286,8 @@ pub trait PetriVmRuntime: Send + Sync + 'static { type VmInspector: PetriVmInspector; /// Interface for accessing the framebuffer type VmFramebufferAccess: PetriVmFramebufferAccess; + /// Information used to identify a realized storage controller + type RealizedStorageController; /// Cleanly tear down the VM immediately. async fn teardown(self) -> anyhow::Result<()>; @@ -1238,6 +1344,14 @@ pub trait PetriVmRuntime: Send + Sync + 'static { } /// Set the OpenHCL VTL2 settings. async fn set_vtl2_settings(&mut self, settings: &Vtl2Settings) -> anyhow::Result<()>; + /// Add a VHD + async fn add_disk( + &mut self, + disk: &PetriDisk, + controller_type: StorageType, + controller_location: u8, + controller: &Self::RealizedStorageController, + ) -> anyhow::Result<()>; } /// Interface for getting information about the state of the VM @@ -1963,9 +2077,9 @@ pub struct OpenHclServicingFlags { pub stop_timeout_hint_secs: Option, } -/// Petri disk type +/// Petri disk #[derive(Debug, Clone)] -pub enum PetriDiskType { +pub enum PetriDisk { /// Memory backed Memory, /// Memory differencing disk backed by a file @@ -1978,7 +2092,7 @@ pub enum PetriDiskType { #[derive(Debug, Clone)] pub struct PetriVmgsDisk { /// Backing disk - pub disk: PetriDiskType, + pub disk: PetriDisk, /// Guest state encryption policy pub encryption_policy: GuestStateEncryptionPolicy, } @@ -1986,7 +2100,7 @@ pub struct PetriVmgsDisk { impl Default for PetriVmgsDisk { fn default() -> Self { PetriVmgsDisk { - disk: PetriDiskType::Memory, + disk: PetriDisk::Memory, // TODO: make this strict once we can set it in OpenHCL on Hyper-V encryption_policy: GuestStateEncryptionPolicy::None(false), } @@ -2151,6 +2265,68 @@ fn default_vtl2_settings() -> Vtl2Settings { } } +/// The storage device type. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum StorageType { + /// IDE + Ide, + /// SCSI + Scsi, + /// NVMe + Nvme, +} + +/// Virtual trust level +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum Vtl { + /// VTL 0 + Vtl0 = 0, + /// VTL 2 + Vtl2 = 2, +} + +/// Petri storage controller +#[derive(Debug, Clone)] +pub struct PetriStorageController { + /// The VTL to assign the storage controller to + pub target_vtl: Vtl, + /// The storage device type + pub controller_type: StorageType, + /// Disks attached to this storage controller + pub disks: HashMap, + /// Backend-specific realized details + pub realized: T, +} + +impl PetriStorageController { + /// Add a disk to the storage controller + pub fn add_disk(&mut self, lun: Option, disk: PetriDisk) -> u8 { + let lun = if let Some(lun) = lun { + if self.disks.contains_key(&lun) { + panic!("a disk with lun {lun} already exists on this controller"); + } + lun + } else { + // find the first available lun + let mut lun = None; + for x in 0..u8::MAX { + if !self.disks.contains_key(&x) { + lun = Some(x) + } + } + if let Some(lun) = lun { + lun + } else { + panic!("all locations on this controller are in use") + } + }; + + self.disks.insert(lun, disk); + + lun + } +} + #[cfg(test)] mod tests { use super::make_vm_safe_name; diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index fb27e751b0..1a85f195f0 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -128,8 +128,13 @@ impl PetriVmConfigOpenVmm { boot_device_type, tpm: tpm_config, guest_crash_disk, + additional_storage_controllers, } = petri_vm_config; + if !additional_storage_controllers.is_empty() { + unimplemented!("openvmm additional storage controllers"); + } + let PetriVmResources { driver, log_source } = resources; let mesh = Mesh::new("petri_mesh".to_string())?; diff --git a/petri/src/vm/openvmm/mod.rs b/petri/src/vm/openvmm/mod.rs index 7588424ecc..55a9c506a8 100644 --- a/petri/src/vm/openvmm/mod.rs +++ b/petri/src/vm/openvmm/mod.rs @@ -21,7 +21,7 @@ use crate::BootDeviceType; use crate::Firmware; use crate::OpenHclServicingFlags; use crate::OpenvmmLogConfig; -use crate::PetriDiskType; +use crate::PetriDisk; use crate::PetriLogFile; use crate::PetriVmConfig; use crate::PetriVmResources; @@ -147,7 +147,7 @@ impl PetriVmmBackend for OpenVmmPetriBackend { config: PetriVmConfig, modify_vmm_config: Option PetriVmConfigOpenVmm + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { + ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig<()>)> { let mut config = PetriVmConfigOpenVmm::new(&self.openvmm_path, config, resources).await?; if let Some(f) = modify_vmm_config { @@ -231,12 +231,12 @@ fn memdiff_vmgs(vmgs: &PetriVmgsResource) -> anyhow::Result { let convert_disk = |disk: &PetriVmgsDisk| -> anyhow::Result { Ok(VmgsDisk { disk: match &disk.disk { - PetriDiskType::Memory => LayeredDiskHandle::single_layer(RamDiskLayerHandle { + PetriDisk::Memory => LayeredDiskHandle::single_layer(RamDiskLayerHandle { len: Some(vmgs_format::VMGS_DEFAULT_CAPACITY), }) .into_resource(), - PetriDiskType::Differencing(path) => memdiff_disk(path)?, - PetriDiskType::Persistent(path) => open_disk_type(path, false)?, + PetriDisk::Differencing(path) => memdiff_disk(path)?, + PetriDisk::Persistent(path) => open_disk_type(path, false)?, }, encryption_policy: disk.encryption_policy, }) diff --git a/petri/src/vm/openvmm/runtime.rs b/petri/src/vm/openvmm/runtime.rs index 876578a41e..b1b25fdbd2 100644 --- a/petri/src/vm/openvmm/runtime.rs +++ b/petri/src/vm/openvmm/runtime.rs @@ -50,6 +50,7 @@ pub struct PetriVmOpenVmm { impl PetriVmRuntime for PetriVmOpenVmm { type VmInspector = OpenVmmInspector; type VmFramebufferAccess = OpenVmmFramebufferAccess; + type RealizedStorageController = (); async fn teardown(self) -> anyhow::Result<()> { tracing::info!("waiting for worker"); @@ -168,6 +169,16 @@ impl PetriVmRuntime for PetriVmOpenVmm { async fn set_vtl2_settings(&mut self, settings: &Vtl2Settings) -> anyhow::Result<()> { Self::set_vtl2_settings(self, settings).await } + + async fn add_disk( + &mut self, + _disk: &crate::PetriDisk, + _controller_type: crate::StorageType, + _controller_location: u8, + _controller: &Self::RealizedStorageController, + ) -> anyhow::Result<()> { + todo!() + } } pub(super) struct PetriVmInner { diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index d2276da9da..c86bf1703a 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -25,6 +25,7 @@ use petri_artifacts_common::tags::MachineArch; use petri_artifacts_common::tags::OsFlavor; use scsidisk_resources::SimpleScsiDiskHandle; use std::collections::BTreeMap; +use std::collections::HashMap; use std::ffi::OsString; use std::io::Write; use std::sync::Arc; @@ -34,7 +35,7 @@ use storvsp_resources::ScsiPath; use vm_resource::IntoResource; impl PetriVmConfigOpenVmm { - async fn run_core(self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { + async fn run_core(self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig<()>)> { let Self { firmware, arch, @@ -178,12 +179,18 @@ impl PetriVmConfigOpenVmm { } tracing::info!("VM ready"); - Ok((vm, PetriVmRuntimeConfig { vtl2_settings })) + Ok(( + vm, + PetriVmRuntimeConfig { + vtl2_settings, + storage_controllers: HashMap::new(), + }, + )) } /// Run the VM, configuring pipette to automatically start if it is /// included in the config - pub async fn run(mut self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { + pub async fn run(mut self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig<()>)> { let launch_linux_direct_pipette = if let Some(agent_image) = &self.resources.agent_image { // Construct the agent disk. if let Some(agent_disk) = agent_image.build().context("failed to build agent image")? { 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 563671206c..a95c5a609f 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -360,29 +360,26 @@ async fn storvsp_hyperv(config: PetriVmBuilder) -> Result<() .build(), ); }) - .modify_backend(move |b| { - b.with_additional_scsi_controller(CONTROLLER_TEST_ID.to_string(), 2) - }) + .add_storage_controller( + CONTROLLER_TEST_ID, + petri::Vtl::Vtl2, + petri::StorageType::Scsi, + ) .run() .await?; - let (vtl2_controller_num, vtl2_vsid) = vm - .backend() - .get_additional_scsi_controllers() - .iter() - .filter(|c| c.test_id == CONTROLLER_TEST_ID) - .map(|c| (c.controller_number, c.vsid)) - .next() + let (_, vtl2_vsid) = vm + .get_storage_controllers() + .get(CONTROLLER_TEST_ID) + .map(|c| (c.realized.controller_number, c.realized.vsid)) .ok_or_else(|| anyhow::anyhow!("couldn't find additional scsi controller"))?; - vm.backend() - .add_vhd( - vhd_path, - petri::hyperv::powershell::ControllerType::Scsi, - Some(vtl2_lun), - Some(vtl2_controller_num), - ) - .await?; + vm.add_disk( + petri::PetriDisk::Persistent(vhd_path.to_path_buf()), + CONTROLLER_TEST_ID, + Some(vtl2_lun), + ) + .await?; vm.modify_vtl2_settings(|s| { let storage_controllers = &mut s.dynamic.as_mut().unwrap().storage_controllers; From 1d145e718b8497d7959805f2ebd955e4eed81e0d Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Wed, 10 Dec 2025 17:33:46 -0800 Subject: [PATCH 02/10] refactor to use vsid as key and wmi powershell --- petri/src/vm/hyperv/hyperv.psm1 | 899 ++++++++++++++---- petri/src/vm/hyperv/mod.rs | 265 +++--- petri/src/vm/hyperv/powershell.rs | 103 +- petri/src/vm/hyperv/vm.rs | 66 +- petri/src/vm/mod.rs | 265 +++--- petri/src/vm/openvmm/construct.rs | 13 +- petri/src/vm/openvmm/mod.rs | 19 +- petri/src/vm/openvmm/runtime.rs | 10 +- petri/src/vm/openvmm/start.rs | 7 +- .../vmm_tests/tests/tests/x86_64/storage.rs | 24 +- 10 files changed, 1160 insertions(+), 511 deletions(-) diff --git a/petri/src/vm/hyperv/hyperv.psm1 b/petri/src/vm/hyperv/hyperv.psm1 index d0193dcb0a..e6bbc26087 100644 --- a/petri/src/vm/hyperv/hyperv.psm1 +++ b/petri/src/vm/hyperv/hyperv.psm1 @@ -1,7 +1,21 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +# +# Constants +# + $ROOT_HYPER_V_NAMESPACE = "root\virtualization\v2" +$SCSI_CONTROLLER_TYPE = "Microsoft:Hyper-V:Synthetic SCSI Controller" +$IDE_CONTROLLER_TYPE = "Microsoft:Hyper-V:Emulated IDE Controller" +$HARD_DRIVE_TYPE = "Microsoft:Hyper-V:Synthetic Disk Drive" +$DVD_DRIVE_TYPE = "Microsoft:Hyper-V:Synthetic DVD Drive" +$HARD_DISK_TYPE = "Microsoft:Hyper-V:Virtual Hard Disk" +$DVD_DISK_TYPE = "Microsoft:Hyper-V:Virtual CD/DVD Disk" + +# +# Hyper-V Helpers +# function Get-MsvmComputerSystem { @@ -45,31 +59,109 @@ function Get-VmGuestManagementService Get-CimInstance -Namespace $ROOT_HYPER_V_NAMESPACE -Class Msvm_VirtualSystemGuestManagementService } -function ConvertTo-CimEmbeddedString -{ - [CmdletBinding()] +function Set-VmSystemSettings { param( - [Parameter(ValueFromPipeline)] - [Microsoft.Management.Infrastructure.CimInstance] $CimInstance + [ValidateNotNullOrEmpty()] + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [Microsoft.Management.Infrastructure.CimInstance] $Vssd ) - if ($null -eq $CimInstance) - { - return "" - } + $vmms = Get-Vmms + $vmms | Invoke-CimMethod -Name "ModifySystemSettings" -Arguments @{ + "SystemSettings" = ($Vssd | ConvertTo-CimEmbeddedString) + } | Trace-CimMethodExecution -MethodName "ModifySystemSettings" -CimInstance $vmms +} - $cimSerializer = [Microsoft.Management.Infrastructure.Serialization.CimSerializer]::Create() - $serializedObj = $cimSerializer.Serialize($CimInstance, [Microsoft.Management.Infrastructure.Serialization.InstanceSerializationOptions]::None) - return [System.Text.Encoding]::Unicode.GetString($serializedObj) +function Set-VmResourceSettings { + param( + [ValidateNotNullOrEmpty()] + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [Microsoft.Management.Infrastructure.CimInstance] $Rasd + ) + + $vmms = Get-Vmms + $vmms | Invoke-CimMethod -Name "ModifyResourceSettings" -Arguments @{ + "ResourceSettings" = @($Rasd | ConvertTo-CimEmbeddedString) + } | Trace-CimMethodExecution -MethodName "ModifyResourceSettings" -CimInstance $vmms +} + +function Add-VmResourceSettings { + param( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [Parameter(Mandatory = $true)] + [Microsoft.Management.Infrastructure.CimInstance] $Rasd + ) + + $vssd = Get-VmSystemSettings $Vm + $vmms = Get-Vmms + $vmms | Invoke-CimMethod -Name "AddResourceSettings" -Arguments @{ + "AffectedConfiguration" = $vssd; + "ResourceSettings" = @($Rasd | ConvertTo-CimEmbeddedString) + } | Trace-CimMethodExecution -MethodName "AddResourceSettings" -CimInstance $vmms +} + +function Remove-VmResourceSettings { + param( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [Microsoft.Management.Infrastructure.CimInstance] $Rasd + ) + + $vmms = Get-Vmms + $vmms | Invoke-CimMethod -Name "RemoveResourceSettings" -Arguments @{ + "ResourceSettings" = @([Microsoft.Management.Infrastructure.CimInstance[]] $Rasd) + } | Trace-CimMethodExecution -MethodName "RemoveResourceSettings" -CimInstance $vmms +} + +function Get-DefaultRasd { + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [string] $ResourceSubType + ) + + $allocCap = Get-CimInstance -Namespace "root/virtualization/v2" -ClassName "Msvm_AllocationCapabilities" | Where-Object { $_.ResourceSubType -eq $ResourceSubType } + $allocCap | Get-CimAssociatedInstance -ResultClassName "CIM_ResourceAllocationSettingData" -Association "Msvm_SettingsDefineCapabilities" | Where-Object { $_.InstanceId.EndsWith("Default") } +} + +function Get-VmRasd +{ + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [string] $ResourceSubType = $null + ) + + $rasds = Get-VmSystemSettings $Vm | Get-CimAssociatedInstance -ResultClassName "Msvm_ResourceAllocationSettingData" + + if ($ResourceSubType) { + return $rasds | Where-Object { $_.ResourceSubType -eq $ResourceSubType } + } else { + return $rasds + } +} + +function Get-VmSasd +{ + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm + ) + + Get-VmSystemSettings $Vm | Get-CimAssociatedInstance -ResultClassName "Msvm_StorageAllocationSettingData" } +# +# Hyper-V Configuration Cmdlets +# + function Set-InitialMachineConfiguration { [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [Parameter(Mandatory = $true)] [string] $ImcHive @@ -92,39 +184,12 @@ function Set-InitialMachineConfiguration } | Trace-CimMethodExecution -MethodName "SetInitialMachineConfigurationData" -CimInstance $vmms } -function Set-VmSystemSettings { - param( - [ValidateNotNullOrEmpty()] - [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [Microsoft.Management.Infrastructure.CimInstance] $Vssd - ) - - $vmms = Get-Vmms - $vmms | Invoke-CimMethod -Name "ModifySystemSettings" -Arguments @{ - "SystemSettings" = ($Vssd | ConvertTo-CimEmbeddedString) - } | Trace-CimMethodExecution -MethodName "ModifySystemSettings" -CimInstance $vmms -} - -function Set-VmResourceSettings { - param( - [ValidateNotNullOrEmpty()] - [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [Microsoft.Management.Infrastructure.CimInstance]$Rasd - ) - - $vmms = Get-Vmms - $vmms | Invoke-CimMethod -Name "ModifyResourceSettings" -Arguments @{ - "ResourceSettings" = @($Rasd | ConvertTo-CimEmbeddedString) - } | Trace-CimMethodExecution -MethodName "ModifyResourceSettings" -CimInstance $vmms -} - function Set-OpenHCLFirmware { [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [Parameter(Mandatory = $true)] [string] $IgvmFile, @@ -155,8 +220,7 @@ function Set-VmCommandLine [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [Parameter(Mandatory = $true)] [AllowEmptyString()] @@ -173,8 +237,7 @@ function Get-VmCommandLine [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm + [System.Object] $Vm ) $vssd = Get-VmSystemSettings $Vm @@ -186,205 +249,376 @@ function Get-VmScsiControllerProperties [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Controller + [System.Object] $Controller ) $vm = Get-VM -Id $Controller.VMId; $ControllerNumber = $Controller.ControllerNumber; - $vssd = Get-VmSystemSettings $Vm; - $rasds = $vssd | Get-CimAssociatedInstance -ResultClassName "Msvm_ResourceAllocationSettingData" | Where-Object { $_.ResourceSubType -eq "Microsoft:Hyper-V:Synthetic SCSI Controller" }; + $rasds = $vm | Get-VmRasd -ResourceSubType $SCSI_CONTROLLER_TYPE; $rasd = $rasds[$ControllerNumber]; return "$ControllerNumber,$($rasd.VirtualSystemIdentifiers[0])" } -function Set-VmScsiControllerTargetVtl +function Get-VmScsiConfiguration { [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm + ) - [Parameter(Mandatory = $true)] - [int] $ControllerNumber, + $controllers = @($Vm | Get-VmRasd -ResourceSubType $SCSI_CONTROLLER_TYPE) - [Parameter(Mandatory = $true)] - [int] $TargetVtl - ) + for ($i = 0; $i -lt $controllers.Count; $i++) { + $controllerPath = Get-CimInstancePath $controllers[$i] + $iid = $controllers[$i].InstanceId + $vsid = $controllers[$i].VirtualSystemIdentifiers[0] + $vtl = $controllers[$i].TargetVtl - $vssd = Get-VmSystemSettings $Vm - $rasds = $vssd | Get-CimAssociatedInstance -ResultClassName "Msvm_ResourceAllocationSettingData" | Where-Object { $_.ResourceSubType -eq "Microsoft:Hyper-V:Synthetic SCSI Controller" } - $rasd = $rasds[$ControllerNumber] - $rasd.TargetVtl = $TargetVtl - $rasd | Set-VmResourceSettings + Write-Host $i $vtl $vsid $iid + + $drives = $Vm | Get-VmRasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DRIVE_TYPE) -or ($_.ResourceSubType -eq $DVD_DRIVE_TYPE)) -and + ($_.Parent -eq $controllerPath) + } + + $drives | ForEach-Object { + $drivePath = Get-CimInstancePath $_ + $iid = $_.InstanceId + $lun = $_.AddressOnParent + $type = if ($_.ResourceSubType -eq $HARD_DRIVE_TYPE) { + "hdd" + } elseif ($_.ResourceSubType -eq $DVD_DRIVE_TYPE) { + "dvd" + } else { + "unknown" + } + + Write-Host " " $lun $type $iid + + $disk = $Vm | Get-VmSasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DISK_TYPE) -or ($_.ResourceSubType -eq $DVD_DISK_TYPE)) -and + ($_.Parent -eq $drivePath) + } + + if ($disk) { + $iid = $disk.InstanceId + $path = $disk.HostResource[0] + + Write-Host " " $path $iid + } + } + } } -function Set-VMBusRedirect +function Get-VmScsiControllerNumberWithId { [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [Parameter(Mandatory = $true)] - [bool] $Enable + [Guid] $Vsid ) - $vssd = Get-VmSystemSettings $Vm - $vssd | ForEach-Object { - $_.VMBusMessageRedirection = [int]$Enable - $_ + $vsid = $Vsid.ToString() + $controllers = @($Vm | Get-VmRasd -ResourceSubType $SCSI_CONTROLLER_TYPE) + + for ($i = 0; $i -lt $controllers.Count; $i++) { + if ($controllers[$i].VirtualSystemIdentifiers[0] -eq "{$vsid}") { + return $i } - Set-VmSystemSettings $vssd + } + + $vmid = $Vm.Id + throw "controller $vsid does not exist on vm $vmid" } -<# -.SYNOPSIS - Helper function that processes a CIMMethodResult/Msvm_ConcreteJob. +function Get-VmScsiControllerIdByNumber +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, -.DESCRIPTION - Helper function that processes a CIMMethodResult/Msvm_ConcreteJob. + [Parameter(Mandatory = $true)] + [int] $ControllerNumber + ) -.PARAMETER WmiClass - Supplies the WMI class object from where the method is being called. + $controllers = @($Vm | Get-VmRasd -ResourceSubType $SCSI_CONTROLLER_TYPE) -.PARAMETER MethodName - Supplies the method name that the job called. + if (($ControllerNumber -lt 0) -or ($ControllerNumber -ge $controllers.Count)) { + $vmid = $Vm.Id + throw "controller number $ControllerNumber does not exist on vm $vmid" + } -.PARAMETER TimeoutSeconds - Supplies the duration in seconds to wait for job completion. + $controllers[$ControllerNumber].VirtualSystemIdentifiers[0] +} -.INPUTS - Input a CIMMethodResult object through the pipeline, or any object with - a ReturnValue property and optionally a Job property that is an Msvm_ConcreteJob. +function Get-VmIdeController +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, -.OUTPUTS - Returns the input object on success; throws on error. + [Parameter(Mandatory = $true)] + [int] $ControllerNumber, -.EXAMPLE - $job | Trace-CimMethodExecution -WmiClass $VMMS -MethodName ExportSystemDefinition - Processes a job for the given class and method, shows progress until it reaches completion. -#> -filter Trace-CimMethodExecution { - param ( - [Alias("WmiClass")] - [Microsoft.Management.Infrastructure.CimInstance]$CimInstance = $null, - [string] $MethodName = $null, - [int] $TimeoutSeconds = 0 + [bool] $Expected = $true ) - $errorCode = 0 - $returnObject = $_ - $job = $null - $shouldProcess = $true - $timer = $null + $vmid = $Vm.Id + $controller = $Vm | Get-VmRasd -ResourceSubType $IDE_CONTROLLER_TYPE | Where-Object { $_.Address -eq $ControllerNumber } - if ($_.CimSystemProperties.ClassName -eq "Msvm_ConcreteJob") { - $job = $_ + if ($Expected -and (-not $controller)) { + throw "ide controller $ControllerNumber does not exist on vm $vmid" } - elseif ((Get-Member -InputObject $_ -name "ReturnValue" -MemberType Properties)) { - if ((Get-Member -InputObject $_.ReturnValue -name "Value" -MemberType Properties)) { - # InvokeMethod from New-CimSession return object - $returnValue = $_.ReturnValue.Value - } - else { - # Invoke-CimMethod return object - $returnValue = $_.ReturnValue - } - if (($returnValue -ne 0) -and ($returnValue -ne 4096)) { - # An error occurred - $errorCode = $returnValue - $shouldProcess = $false - } - elseif ($returnValue -eq 4096) { - if ((Get-Member -InputObject $_ -name "Job" -MemberType Properties) -and $_.Job) { - # Invoke-CimMethod return object - # CIM does not seem to actually populate the non-key fields on a reference, so we need - # to go get the actual instance of the job object we got. - $job = ($_.Job | Get-CimInstance) - } - elseif ((Get-Member -InputObject $_ -name "OutParameters" -MemberType Properties) -and $_.OutParameters["Job"]) { - # InvokeMethod from New-CimSession return object - $job = ($_.OutParameters["Job"].Value | Get-CimInstance) - } - else { - throw "ReturnValue of 4096 with no Job object!" - } - } - else { - # No job and no error, just exit. - return $returnObject - } - } - else { - throw "Pipeline input object is not a job or CIM method result!" + if ((-not $Expected) -and $controller) { + throw "ide controller $ControllerNumber already exists on vm $vmid" } - if ($shouldProcess) { - $caption = if ($job.Caption) { $job.Caption } else { "Job in progress (no caption available)" } - $jobStatus = if ($job.JobStatus) { $job.JobState } else { "No job status available" } - $percentComplete = if ($job.PercentComplete) { $job.PercentComplete } else { 0 } + return $controller +} - if (($job.JobState -eq 4) -and $TimeoutSeconds -gt 0) { - $timer = [Diagnostics.Stopwatch]::StartNew() - } +function Get-VmScsiControllerWithId +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, - while ($job.JobState -eq 4) { - if (($timer -ne $null) -and ($timer.Elapsed.Seconds -gt $TimeoutSeconds)) { - throw "Job did not complete within $TimeoutSeconds seconds!" - } - Write-Progress -Activity $caption -Status ("{0} - {1}%" -f $jobStatus, $percentComplete) -PercentComplete $percentComplete - Start-Sleep -seconds 1 - $job = $job | Get-CimInstance - } + [Parameter(Mandatory = $true)] + [Guid] $Vsid, - if ($timer) { $timer.Stop() } + [bool] $Expected = $true + ) - if ($job.JobState -ne 7) { - if (![string]::IsNullOrEmpty($job.ErrorDescription)) { - Throw $job.ErrorDescription - } - else { - $errorCode = $job.ErrorCode - } - } - Write-Progress -Activity $caption -Status $jobStatus -PercentComplete 100 -Completed:$true + $vmid = $Vm.Id + $vsid = $Vsid.ToString() + $controller = $Vm | Get-VmRasd -ResourceSubType $SCSI_CONTROLLER_TYPE | Where-Object { $_.VirtualSystemIdentifiers[0] -eq "{$vsid}" } + + if ($Expected -and (-not $controller)) { + throw "scsi controller $vsid does not exist on vm $vmid" } - if ($errorCode -ne 0) { - if ($CimInstance -and $MethodName) { - $cimClass = Get-CimClass -ClassName $CimInstance.CimSystemProperties.ClassName ` - -Namespace $CimInstance.CimSystemProperties.Namespace -ComputerName $CimInstance.CimSystemProperties.ServerName + if ((-not $Expected) -and $controller) { + throw "scsi controller $vsid already exists on vm $vmid" + } - $methodQualifierValues = ($cimClass.CimClassMethods[$MethodName].Qualifiers["ValueMap"].Value) - $indexOfError = [System.Array]::IndexOf($methodQualifierValues, [string]$errorCode) + return $controller +} - if (($indexOfError -ne "-1") -and $methodQualifierValues) { - # If the class in question has an error description defined for the error in its Values collection, use it - if ($cimClass.CimClassMethods[$MethodName].Qualifiers["Values"] -and $indexOfError -lt $cimClass.CimClassMethods[$MethodName].Qualifiers["Values"].Value.Length) { - Throw "ReturnCode: ", $errorCode, " ErrorMessage: '", $cimClass.CimClassMethods[$MethodName].Qualifiers["Values"].Value[$indexOfError], "' - when calling $MethodName" - } - else { - # The class has no error description for the error code, so just return the error code - Throw "ReturnCode: ", $errorCode, " - when calling $MethodName" - } - } - else { - # The error code is not found in the ValueMap, so just return the error code - Throw "ReturnCode: ", $errorCode, " ErrorMessage: 'MessageNotFound' - when calling $MethodName" - } - } - else { - Throw "ReturnCode: ", $errorCode, "When calling $MethodName - for rich error messages provide classpath and method name." +function Add-VmScsiControllerWithId +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [Parameter(Mandatory = $true)] + [Guid] $Vsid, + + [Parameter(Mandatory = $true)] + [int] $TargetVtl + ) + + $Vm | Get-VmScsiControllerWithId -Vsid $Vsid -Expected $false + + $vsid = $Vsid.ToString() + $template = Get-DefaultRasd $SCSI_CONTROLLER_TYPE + $controllerConfig = Copy-CimInstanceWithNewProperties $template @{ "VirtualSystemIdentifiers" = @("{$vsid}"); "TargetVtl" = $TargetVtl } + $controllerAddResult = $Vm | Add-VmResourceSettings -Rasd $controllerConfig + $controller = $controllerAddResult.ResultingResourceSettings[0] + Write-Host "added controller:" $controller.InstanceId + + return $controller +} + +function Remove-VmScsiControllerWithId +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [Parameter(Mandatory = $true)] + [Guid] $Vsid + ) + + $controller = $Vm | Get-VmScsiControllerWithId -Vsid $Vsid -Expected $true + $controllerPath = Get-CimInstancePath $controller + + $drives = $Vm | Get-VmRasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DRIVE_TYPE) -or ($_.ResourceSubType -eq $DVD_DRIVE_TYPE)) -and + ($_.Parent -eq $controllerPath) + } + $drives | ForEach-Object { $Vm | Remove-VmDrive -Drive $_ } + + Write-Host "removing controller:" $controller.InstanceId + $controller | Remove-VmResourceSettings +} + +function Remove-VmDrive +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [Parameter(Mandatory = $true)] + [System.Object] $Drive + ) + + $drivePath = Get-CimInstancePath $Drive + + $disk = $Vm | Get-VmSasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DISK_TYPE) -or ($_.ResourceSubType -eq $DVD_DISK_TYPE)) -and + ($_.Parent -eq $drivePath) + } + + Write-Host $disk.InstanceId $drivePath + + if ($disk) { + Write-Host "removing disk:" $disk.InstanceId + $disk | Remove-VmResourceSettings + } + + Write-Host "removing drive:" $Drive.InstanceId + $Drive | Remove-VmResourceSettings +} + +function Set-VmDrive +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [guid] $ControllerVsid, + + [int] $ControllerNumber = 0, + + [Parameter(Mandatory = $true)] + [int] $Lun, + + [string] $DiskPath = $null, + + [switch] $Dvd, + + [switch] $AllowModifyExisting + ) + + if ($ControllerVsid) { + $controller = $Vm | Get-VmScsiControllerWithId -Vsid $ControllerVsid + $controllerId = $controller.VirtualSystemIdentifiers[0] + } else { + $controller = $Vm | Get-VmIdeController -ControllerNumber $ControllerNumber + $controllerId = $controller.Address + } + + $vmid = $Vm.Id + + + $controllerPath = Get-CimInstancePath $controller + Write-Host "modifying controller:" $controller.InstanceId + + if ($Dvd) { + $driveType = $DVD_DRIVE_TYPE + $diskType = $DVD_DISK_TYPE + } else { + $driveType = $HARD_DRIVE_TYPE + $diskType = $HARD_DISK_TYPE + } + + # check if the drive already exists + $drive = $Vm | Get-VmRasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DRIVE_TYPE) -or ($_.ResourceSubType -eq $DVD_DRIVE_TYPE)) -and + ($_.AddressOnParent -eq $lun) -and + ($_.Parent -eq $controllerPath) + } + + if ($drive -and (-not $AllowModifyExisting)) { + throw "drive $Lun on controller $controllerId already exists on vm $vmid" + } + + # (re-)create the drive if necessary + if ((-not $drive) -or ($drive.ResourceSubType -ne $driveType)) { + if ($drive) { + $Vm | Remove-VmDrive -Drive $drive } + + $driveTemplate = Get-DefaultRasd $driveType + $driveConfig = Copy-CimInstanceWithNewProperties $driveTemplate @{ "AddressOnParent" = $lun; "Parent" = $controllerPath } + $driveAddResult = $Vm | Add-VmResourceSettings -Rasd $driveConfig + $drive = $driveAddResult.ResultingResourceSettings[0] + Write-Host "added drive:" $drive.InstanceId + } else { + Write-Host "found drive:" $drive.InstanceId } - return $returnObject + # remove disk if already inserted + $drivePath = Get-CimInstancePath $drive + $disk = $Vm | Get-VmSasd | Where-Object { + (($_.ResourceSubType -eq $HARD_DISK_TYPE) -or ($_.ResourceSubType -eq $DVD_DISK_TYPE)) -and + ($_.Parent -eq $drivePath) + } + if ($disk) { + Write-Host "removing disk:" $disk.InstanceId + $disk | Remove-VmResourceSettings + } + + # insert the disk if provided + if ($DiskPath) { + $diskTemplate = Get-DefaultRasd $diskType + $diskConfig = Copy-CimInstanceWithNewProperties $diskTemplate @{ "Parent" = $drivePath; "HostResource" = @($DiskPath) } + $diskAddResult = $Vm | Add-VmResourceSettings -Rasd $diskConfig + $disk = $diskAddResult.ResultingResourceSettings[0] + Write-Host "added disk:" $disk.InstanceId + } +} + +function Set-VmScsiControllerTargetVtl +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [Parameter(Mandatory = $true)] + [int] $ControllerNumber, + + [Parameter(Mandatory = $true)] + [int] $TargetVtl + ) + + $vssd = Get-VmSystemSettings $Vm + $rasds = $vssd | Get-CimAssociatedInstance -ResultClassName "Msvm_ResourceAllocationSettingData" | Where-Object { $_.ResourceSubType -eq $SCSI_CONTROLLER_TYPE } + $rasd = $rasds[$ControllerNumber] + $rasd.TargetVtl = $TargetVtl + $rasd | Set-VmResourceSettings +} + +function Set-VMBusRedirect +{ + [CmdletBinding()] + Param ( + [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] + [System.Object] $Vm, + + [bool] $Enable + ) + + $vssd = Get-VmSystemSettings $Vm + $vssd | ForEach-Object { + $_.VMBusMessageRedirection = [int]$Enable + $_ + } + Set-VmSystemSettings $vssd } function Restart-OpenHCL @@ -392,8 +626,8 @@ function Restart-OpenHCL [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, + [int] $TimeoutHintSeconds = 15, # Ends up as the deadline in GuestSaveRequest (see the handling of # SaveGuestVtl2StateNotification in guest_emulation_transport). Keep O(15 seconds). # @@ -426,8 +660,7 @@ function Get-VmScreenshot [CmdletBinding()] Param( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [Parameter(Mandatory = $true)] [string] $Path @@ -458,8 +691,7 @@ function Set-TurnOffOnGuestRestart [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [bool] $Enable ) @@ -474,8 +706,7 @@ function Get-GuestStateFile [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm + [System.Object] $Vm ) $vssd = Get-VmSystemSettings $Vm @@ -539,7 +770,7 @@ function Set-Vtl2Settings { $params.Add($p1); $params.Add($p2); $params.Add($p3); $params.Add($p4) $cimSession = New-CimSession - $cimSession.InvokeMethod("root\virtualization\v2", $guestManagement, "SetManagementVtlSettings", $params, $options) | + $cimSession.InvokeMethod($ROOT_HYPER_V_NAMESPACE, $guestManagement, "SetManagementVtlSettings", $params, $options) | Trace-CimMethodExecution -CimInstance $guestManagement -MethodName "SetManagementVtlSettings" | Out-Null $cimSession | Remove-CimSession | Out-Null @@ -550,8 +781,7 @@ function Set-GuestStateIsolationMode [CmdletBinding()] Param ( [Parameter(Position = 0, Mandatory = $true, ValueFromPipeline = $true)] - [System.Object] - $Vm, + [System.Object] $Vm, [int] $Mode ) @@ -560,3 +790,270 @@ function Set-GuestStateIsolationMode $vssd.GuestStateIsolationMode = $Mode Set-VmSystemSettings $vssd } + +# +# CIM Helpers +# + +function ConvertTo-CimEmbeddedString +{ + [CmdletBinding()] + param( + [Parameter(ValueFromPipeline)] + [Microsoft.Management.Infrastructure.CimInstance] $CimInstance + ) + + if ($null -eq $CimInstance) + { + return "" + } + + $cimSerializer = [Microsoft.Management.Infrastructure.Serialization.CimSerializer]::Create() + $serializedObj = $cimSerializer.Serialize($CimInstance, [Microsoft.Management.Infrastructure.Serialization.InstanceSerializationOptions]::None) + return [System.Text.Encoding]::Unicode.GetString($serializedObj) +} + +# CIM is strict and won't let you write read-only properties on instances, so +# we need to create instances with the read-only properties set to what we need them +# to be. Use this helper function to clone RASDD instances with the specified +# properties and values as given by NewPropertiesDict. Throws if a property that did not +# originally exist on the object is given. +function Copy-CimInstanceWithNewProperties { + param( + [parameter(Mandatory = $true)] + [Microsoft.Management.Infrastructure.CimInstance] $CimInstance, + [parameter(Mandatory = $true)] + [System.Collections.Hashtable] $NewPropertiesDict + ) + + $newProperties = @{ } + + $class = Get-CimClass -Namespace $CimInstance.CimSystemProperties.Namespace ` + -ClassName $CimInstance.CimSystemProperties.ClassName + + $compareArgs = @{ReferenceObject = $class.CimClassProperties.Name; + DifferenceObject = @($NewPropertiesDict.Keys); + PassThru = $true; + CaseSensitive = $false + }; + + $invalidProperties = Compare-Object @compareArgs | Where-Object { $_.SideIndicator -eq "=>" } + if ($invalidProperties) { + throw "Invalid properties are specified - $($invalidProperties -join ',')" + } + + foreach ($prop in $class.CimClassProperties) { + if ($NewPropertiesDict.ContainsKey("$($prop.Name)")) { + $newProperties["$($prop.Name)"] = $NewPropertiesDict["$($prop.Name)"] + } + else { + $newProperties["$($prop.Name)"] = $CimInstance."$($prop.Name)" + } + } + + return ($class | New-CimInstance -ClientOnly -Property $newProperties) +} + +<# +.SYNOPSIS + Helper function that processes a CIMMethodResult/Msvm_ConcreteJob. + +.DESCRIPTION + Helper function that processes a CIMMethodResult/Msvm_ConcreteJob. + +.PARAMETER WmiClass + Supplies the WMI class object from where the method is being called. + +.PARAMETER MethodName + Supplies the method name that the job called. + +.PARAMETER TimeoutSeconds + Supplies the duration in seconds to wait for job completion. + +.INPUTS + Input a CIMMethodResult object through the pipeline, or any object with + a ReturnValue property and optionally a Job property that is an Msvm_ConcreteJob. + +.OUTPUTS + Returns the input object on success; throws on error. + +.EXAMPLE + $job | Trace-CimMethodExecution -WmiClass $VMMS -MethodName ExportSystemDefinition + Processes a job for the given class and method, shows progress until it reaches completion. +#> +filter Trace-CimMethodExecution { + param ( + [Alias("WmiClass")] + [Microsoft.Management.Infrastructure.CimInstance]$CimInstance = $null, + [string] $MethodName = $null, + [int] $TimeoutSeconds = 0 + ) + + $errorCode = 0 + $returnObject = $_ + $job = $null + $shouldProcess = $true + $timer = $null + + if ($_.CimSystemProperties.ClassName -eq "Msvm_ConcreteJob") { + $job = $_ + } + elseif ((Get-Member -InputObject $_ -name "ReturnValue" -MemberType Properties)) { + if ((Get-Member -InputObject $_.ReturnValue -name "Value" -MemberType Properties)) { + # InvokeMethod from New-CimSession return object + $returnValue = $_.ReturnValue.Value + } + else { + # Invoke-CimMethod return object + $returnValue = $_.ReturnValue + } + + if (($returnValue -ne 0) -and ($returnValue -ne 4096)) { + # An error occurred + $errorCode = $returnValue + $shouldProcess = $false + } + elseif ($returnValue -eq 4096) { + if ((Get-Member -InputObject $_ -name "Job" -MemberType Properties) -and $_.Job) { + # Invoke-CimMethod return object + # CIM does not seem to actually populate the non-key fields on a reference, so we need + # to go get the actual instance of the job object we got. + $job = ($_.Job | Get-CimInstance) + } + elseif ((Get-Member -InputObject $_ -name "OutParameters" -MemberType Properties) -and $_.OutParameters["Job"]) { + # InvokeMethod from New-CimSession return object + $job = ($_.OutParameters["Job"].Value | Get-CimInstance) + } + else { + throw "ReturnValue of 4096 with no Job object!" + } + } + else { + # No job and no error, just exit. + return $returnObject + } + } + else { + throw "Pipeline input object is not a job or CIM method result!" + } + + if ($shouldProcess) { + $caption = if ($job.Caption) { $job.Caption } else { "Job in progress (no caption available)" } + $jobStatus = if ($job.JobStatus) { $job.JobState } else { "No job status available" } + $percentComplete = if ($job.PercentComplete) { $job.PercentComplete } else { 0 } + + if (($job.JobState -eq 4) -and $TimeoutSeconds -gt 0) { + $timer = [Diagnostics.Stopwatch]::StartNew() + } + + while ($job.JobState -eq 4) { + if (($timer -ne $null) -and ($timer.Elapsed.Seconds -gt $TimeoutSeconds)) { + throw "Job did not complete within $TimeoutSeconds seconds!" + } + Write-Progress -Activity $caption -Status ("{0} - {1}%" -f $jobStatus, $percentComplete) -PercentComplete $percentComplete + Start-Sleep -seconds 1 + $job = $job | Get-CimInstance + } + + if ($timer) { $timer.Stop() } + + if ($job.JobState -ne 7) { + if (![string]::IsNullOrEmpty($job.ErrorDescription)) { + Throw $job.ErrorDescription + } + else { + $errorCode = $job.ErrorCode + } + } + Write-Progress -Activity $caption -Status $jobStatus -PercentComplete 100 -Completed:$true + } + + if ($errorCode -ne 0) { + if ($CimInstance -and $MethodName) { + $cimClass = Get-CimClass -ClassName $CimInstance.CimSystemProperties.ClassName ` + -Namespace $CimInstance.CimSystemProperties.Namespace -ComputerName $CimInstance.CimSystemProperties.ServerName + + $methodQualifierValues = ($cimClass.CimClassMethods[$MethodName].Qualifiers["ValueMap"].Value) + $indexOfError = [System.Array]::IndexOf($methodQualifierValues, [string]$errorCode) + + if (($indexOfError -ne "-1") -and $methodQualifierValues) { + # If the class in question has an error description defined for the error in its Values collection, use it + if ($cimClass.CimClassMethods[$MethodName].Qualifiers["Values"] -and $indexOfError -lt $cimClass.CimClassMethods[$MethodName].Qualifiers["Values"].Value.Length) { + Throw "ReturnCode: ", $errorCode, " ErrorMessage: '", $cimClass.CimClassMethods[$MethodName].Qualifiers["Values"].Value[$indexOfError], "' - when calling $MethodName" + } + else { + # The class has no error description for the error code, so just return the error code + Throw "ReturnCode: ", $errorCode, " - when calling $MethodName" + } + } + else { + # The error code is not found in the ValueMap, so just return the error code + Throw "ReturnCode: ", $errorCode, " ErrorMessage: 'MessageNotFound' - when calling $MethodName" + } + } + else { + Throw "ReturnCode: ", $errorCode, "When calling $MethodName - for rich error messages provide classpath and method name." + } + } + + return $returnObject +} + +<# +.SYNOPSIS + Get the __PATH property from a CIMInstance object. + +.DESCRIPTION + The Get-CIMInstance cmdlet by default doesn't display the WMI system properties + like __SERVER. The properties are available in the CimSystemProperties property + except for __PATH. This function will construct the __PATH property and return it. + +.EXAMPLE + get-ciminstance win32_memorydevice | get-ciminstancepath + + \\SERVER01\root\cimv2:Win32_MemoryDevice.DeviceID="Memory Device 0" + \\SERVER01\root\cimv2:Win32_MemoryDevice.DeviceID="Memory Device 1" + +.INPUTS + A CIMInstance object + +.OUTPUTS + String representing the path of the input object +#> +function Get-CimInstancePath { + [CmdletBinding()] + param ( + [Parameter(Position = 0, ValueFromPipeline = $True)] + [ValidateNotNullorEmpty()] + [Microsoft.Management.Infrastructure.CimInstance]$CimInstance + ) + + $key = $CimInstance.CimClass.CimClassProperties | + Where-Object { $_.Qualifiers.Name -contains "key" } | + Select-Object -ExpandProperty Name + + $path = ('\\{0}\{1}:{2}{3}' -f $CimInstance.CimSystemProperties.ServerName.ToUpper(), + $CimInstance.CimSystemProperties.Namespace.Replace("/", "\"), + $CimInstance.CimSystemProperties.ClassName, + $(if ($key -is [array]) { + # Need a string with every key in the array, keys separated by commas + $sep = "" + $s = [string]"." + foreach ($k in $key) { + $s += "$($sep)$($k)=""$($CimInstance.($k))""" + $sep = "," + } + $s + } + elseif ($key) { + # just a single key + ".$($key)=""$($CimInstance.$key)""" + } + else { + #no key + '=@' + }).Replace('\', '\\') + ) + + return $path +} \ No newline at end of file diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 273cc8cd78..f0bd8f646e 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -9,15 +9,15 @@ use vmsocket::VmSocket; use super::ProcessorTopology; use crate::BootDeviceType; +use crate::Disk; +use crate::Drive; use crate::Firmware; use crate::IsolationType; use crate::NoPetriVmInspector; use crate::OpenHclConfig; use crate::OpenHclServicingFlags; use crate::OpenvmmLogConfig; -use crate::PetriDisk; use crate::PetriHaltReason; -use crate::PetriStorageController; use crate::PetriVmConfig; use crate::PetriVmResources; use crate::PetriVmRuntime; @@ -27,9 +27,10 @@ use crate::PetriVmgsResource; use crate::PetriVmmBackend; use crate::SecureBootTemplate; use crate::ShutdownKind; -use crate::StorageType; use crate::TpmConfig; use crate::UefiConfig; +use crate::VmbusStorageController; +use crate::VmbusStorageType; use crate::VmmQuirks; use crate::disk_image::AgentImage; use crate::hyperv::powershell::HyperVSecureBootTemplate; @@ -41,6 +42,7 @@ use async_trait::async_trait; use disk_backend::sync_wrapper::BlockingDisk; use disk_vhdmp::VhdmpDisk; use get_resources::ged::FirmwareEvent; +use guid::Guid; use pal_async::DefaultDriver; use pal_async::pipe::PolledPipe; use pal_async::socket::PolledSocket; @@ -53,7 +55,6 @@ use petri_artifacts_common::tags::OsFlavor; use petri_artifacts_core::ArtifactResolver; use petri_artifacts_core::ResolvedArtifact; use pipette_client::PipetteClient; -use std::collections::HashMap; use std::io::ErrorKind; use std::io::Write; use std::path::Path; @@ -76,7 +77,7 @@ pub struct RealizedHyperVStorageController { pub controller_number: u32, /// The VSID assigned by Hyper-V for this controller. - pub vsid: guid::Guid, + pub vsid: Guid, } /// Resources needed at runtime for a Hyper-V Petri VM @@ -176,10 +177,7 @@ impl PetriVmmBackend for HyperVPetriBackend { config: PetriVmConfig, _modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<( - Self::VmRuntime, - PetriVmRuntimeConfig, - )> { + ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { let PetriVmConfig { name, arch, @@ -193,7 +191,8 @@ impl PetriVmmBackend for HyperVPetriBackend { vmgs, tpm, guest_crash_disk, - additional_storage_controllers, + ide, + additional_vmbus_storage_controllers, } = config; let PetriVmResources { driver, log_source } = resources; @@ -221,7 +220,7 @@ impl PetriVmmBackend for HyperVPetriBackend { } => ( powershell::HyperVGuestStateIsolationType::Disabled, powershell::HyperVGeneration::One, - Some(guest.artifact().to_owned()), + Some((guest.artifact().to_owned(), guest.is_dvd())), None, None, ), @@ -234,7 +233,7 @@ impl PetriVmmBackend for HyperVPetriBackend { } => ( powershell::HyperVGuestStateIsolationType::OpenHCL, powershell::HyperVGeneration::One, - Some(guest.artifact().to_owned()), + Some((guest.artifact().to_owned(), guest.is_dvd())), None, Some((igvm_path, openhcl_config)), ), @@ -245,7 +244,7 @@ impl PetriVmmBackend for HyperVPetriBackend { } => ( powershell::HyperVGuestStateIsolationType::Disabled, powershell::HyperVGeneration::Two, - guest.artifact().map(|a| a.to_owned()), + guest.artifact().map(|a| (a.to_owned(), false)), Some(uefi_config), None, ), @@ -264,7 +263,7 @@ impl PetriVmmBackend for HyperVPetriBackend { None => powershell::HyperVGuestStateIsolationType::TrustedLaunch, }, powershell::HyperVGeneration::Two, - guest.artifact().map(|a| a.to_owned()), + guest.artifact().map(|a| (a.to_owned(), false)), Some(uefi_config), Some((igvm_path, openhcl_config)), ), @@ -272,7 +271,11 @@ impl PetriVmmBackend for HyperVPetriBackend { let mut openhcl_command_line = openhcl_config.as_ref().map(|(_, c)| c.command_line()); let mut vtl2_settings = None; - let mut storage_controllers = HashMap::new(); + let mut vmbus_storage_controllers = additional_vmbus_storage_controllers; + let mut ide_controllers = (generation == powershell::HyperVGeneration::One).then(|| { + ide.expect("need ide controller for gen 1 boot") + .into_controllers() + }); let vmgs_path = { // TODO: add support for configuring the TPM in Hyper-V @@ -323,15 +326,15 @@ impl PetriVmmBackend for HyperVPetriBackend { }; match disk { - None | Some(PetriDisk::Memory) => None, - Some(PetriDisk::Differencing(parent_path)) => { + None | Some(Disk::Memory(_)) => None, + Some(Disk::Differencing(parent_path)) => { let diff_disk_path = temp_dir .path() .join(parent_path.file_name().context("path has no filename")?); make_temp_diff_disk(&diff_disk_path, &parent_path).await?; Some(diff_disk_path) } - Some(PetriDisk::Persistent(path)) => Some(path), + Some(Disk::Persistent(path)) => Some(path), } }; @@ -417,36 +420,34 @@ impl PetriVmmBackend for HyperVPetriBackend { }; } - // Share a single scsi controller for all petri-added drives. - let petri_vtl0_scsi = vm.add_scsi_controller(0).await?.0; - - if let Some((controller_type, controller_number)) = match boot_device_type { - BootDeviceType::None => None, - BootDeviceType::Ide => Some((powershell::ControllerType::Ide, 0)), - BootDeviceType::Scsi => Some((powershell::ControllerType::Scsi, petri_vtl0_scsi)), - BootDeviceType::Nvme => todo!("NVMe boot device not yet supported for Hyper-V"), - } { - if let Some(artifact) = guest_artifact { - let controller_location = super::PETRI_VTL0_SCSI_BOOT_LUN; - let vhd = artifact.get(); - let diff_disk_path = temp_dir.path().join(format!( - "{}_{}_{}", - controller_number, - controller_location, - vhd.file_name() - .context("path has no filename")? - .to_string_lossy() - )); + let boot_disk = guest_artifact.map(|(artifact, is_dvd)| { + Drive::new( + Some(Disk::Differencing(artifact.get().to_path_buf())), + is_dvd, + ) + }); - make_temp_diff_disk(&diff_disk_path, vhd).await?; + // Share a single scsi controller for all petri-added drives. + vmbus_storage_controllers.insert( + super::PETRI_VTL0_SCSI_CONTROLLER_ID, + VmbusStorageController::new(super::Vtl::Vtl0, VmbusStorageType::Scsi), + ); - vm.add_vhd( - &diff_disk_path, - controller_type, - Some(controller_location), - Some(controller_number), - ) - .await?; + if let Some(boot_disk) = boot_disk { + match boot_device_type { + BootDeviceType::None => {} + BootDeviceType::Ide => { + ide_controllers.as_mut().expect("ide not configured") + [super::PETRI_IDE_BOOT_CONTROLLER as usize] + [super::PETRI_IDE_BOOT_LUN as usize] = Some(boot_disk); + } + BootDeviceType::Scsi => { + vmbus_storage_controllers + .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) + .unwrap() + .set_drive(Some(super::PETRI_VTL0_SCSI_BOOT_LUN), boot_disk, false); + } + BootDeviceType::Nvme => todo!("NVMe boot device not yet supported for Hyper-V"), } } @@ -475,13 +476,14 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.set_imc(&imc_hive).await?; } - vm.add_vhd( - &agent_disk_path, - powershell::ControllerType::Scsi, - Some(super::PETRI_VTL0_SCSI_PIPETTE_LUN), - Some(petri_vtl0_scsi), - ) - .await?; + vmbus_storage_controllers + .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) + .unwrap() + .set_drive( + Some(super::PETRI_VTL0_SCSI_PIPETTE_LUN), + Drive::new(Some(Disk::Persistent(agent_disk_path)), false), + false, + ); } } @@ -532,14 +534,17 @@ impl PetriVmmBackend for HyperVPetriBackend { if build_and_persist_agent_image(&agent_image, &agent_disk_path) .context("vtl2 agent disk")? { - let controller_number = vm.add_scsi_controller(2).await?.0; - vm.add_vhd( - &agent_disk_path, - powershell::ControllerType::Scsi, - Some(0), - Some(controller_number), - ) - .await?; + vmbus_storage_controllers + .entry(super::PETRI_VTL2_SCSI_CONTROLLER_ID) + .or_insert(VmbusStorageController::new( + crate::Vtl::Vtl2, + VmbusStorageType::Scsi, + )) + .set_drive( + Some(super::PETRI_VTL2_SCSI_PIPETTE_LUN), + Drive::new(Some(Disk::Persistent(agent_disk_path)), false), + false, + ); } } @@ -600,13 +605,17 @@ impl PetriVmmBackend for HyperVPetriBackend { } if let Some(guest_crash_disk) = guest_crash_disk { - vm.add_vhd( - &guest_crash_disk, - powershell::ControllerType::Scsi, - Some(super::PETRI_VTL0_SCSI_CRASH_LUN), - Some(petri_vtl0_scsi), - ) - .await?; + vmbus_storage_controllers + .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) + .unwrap() + .set_drive( + Some(super::PETRI_VTL0_SCSI_CRASH_LUN), + Drive::new( + Some(Disk::Persistent(guest_crash_disk.to_path_buf())), + false, + ), + false, + ); } let serial_pipe_path = vm.set_vm_com_port(1).await?; @@ -619,48 +628,48 @@ impl PetriVmmBackend for HyperVPetriBackend { // TODO: If OpenHCL is being used, then translate storage through it. // (requires changes above where VHDs are added) - // Add additional storage - for ( - test_id, - PetriStorageController { - target_vtl, - controller_type, - disks, - .. - }, - ) in additional_storage_controllers - { - let (hyperv_controller_type, (controller_number, vsid)) = match controller_type { - StorageType::Ide => todo!(), - StorageType::Scsi => ( - powershell::ControllerType::Scsi, - vm.add_scsi_controller(target_vtl as u32).await?, - ), - StorageType::Nvme => todo!(), - }; - - for (controller_location, disk) in disks.iter() { - vm.add_vhd( - &petri_disk_to_hyperv(disk, &temp_dir).await?, - hyperv_controller_type, - Some(*controller_location), - Some(controller_number), - ) - .await?; + // Add IDE storage + if let Some(ide_controllers) = ide_controllers { + for (controller_number, controller) in ide_controllers.iter().enumerate() { + for (controller_location, disk) in controller.iter().enumerate() { + if let Some(disk) = disk { + let path = petri_disk_to_hyperv(disk.disk.as_ref(), &temp_dir).await?; + + vm.set_drive_ide( + controller_number as u32, + controller_location as u8, + path.as_deref(), + disk.is_dvd, + false, + ) + .await?; + } + } } + } - storage_controllers.insert( - test_id, - PetriStorageController { - target_vtl, - controller_type, - disks, - realized: RealizedHyperVStorageController { - controller_number, - vsid, - }, - }, - ); + // Add VMBus storage + for (vsid, controller) in &vmbus_storage_controllers { + match controller.controller_type { + VmbusStorageType::Scsi => { + vm.add_scsi_controller(vsid, controller.target_vtl as u32) + .await?; + + for (controller_location, disk) in controller.drives.iter() { + let path = petri_disk_to_hyperv(disk.disk.as_ref(), &temp_dir).await?; + + vm.set_drive_scsi( + vsid, + *controller_location, + path.as_deref(), + false, + false, + ) + .await?; + } + } + VmbusStorageType::Nvme => todo!(), + } } // Configure the TPM @@ -706,7 +715,8 @@ impl PetriVmmBackend for HyperVPetriBackend { }, PetriVmRuntimeConfig { vtl2_settings, - storage_controllers, + ide_controllers: None, + vmbus_storage_controllers, }, )) } @@ -716,7 +726,6 @@ impl PetriVmmBackend for HyperVPetriBackend { impl PetriVmRuntime for HyperVPetriRuntime { type VmInspector = NoPetriVmInspector; type VmFramebufferAccess = vm::HyperVFramebufferAccess; - type RealizedStorageController = RealizedHyperVStorageController; async fn teardown(mut self) -> anyhow::Result<()> { futures::future::join_all(self.log_tasks.into_iter().map(|t| t.cancel())).await; @@ -849,25 +858,27 @@ impl PetriVmRuntime for HyperVPetriRuntime { self.vm.set_base_vtl2_settings(settings).await } - async fn add_disk( + async fn set_vmbus_drive( &mut self, - disk: &PetriDisk, - controller_type: StorageType, + drive: &Drive, + controller_id: &Guid, controller_location: u8, - controller: &Self::RealizedStorageController, ) -> anyhow::Result<()> { self.vm - .add_vhd( - &petri_disk_to_hyperv(disk, &self.temp_dir).await?, - controller_type.into(), - Some(controller_location), - Some(controller.controller_number), + .set_drive_scsi( + controller_id, + controller_location, + petri_disk_to_hyperv(drive.disk.as_ref(), &self.temp_dir) + .await? + .as_deref(), + false, + false, ) .await } } -fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { +fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { let sid_arg = format!( "NT VIRTUAL MACHINE\\{name}:R", name = if let Some(id) = id { @@ -954,16 +965,20 @@ async fn make_temp_diff_disk( Ok(()) } -async fn petri_disk_to_hyperv(disk: &PetriDisk, temp_dir: &TempDir) -> anyhow::Result { +async fn petri_disk_to_hyperv( + disk: Option<&Disk>, + temp_dir: &TempDir, +) -> anyhow::Result> { Ok(match disk { - PetriDisk::Memory => todo!(), - PetriDisk::Differencing(parent_path) => { + None => None, + Some(Disk::Memory(_)) => todo!(), + Some(Disk::Differencing(parent_path)) => { let diff_disk_path = temp_dir .path() .join(parent_path.file_name().context("path has no filename")?); make_temp_diff_disk(&diff_disk_path, &parent_path).await?; - diff_disk_path + Some(diff_disk_path) } - PetriDisk::Persistent(path) => path.clone(), + Some(Disk::Persistent(path)) => Some(path.clone()), }) } diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 0add798e92..5fd8033867 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/petri/src/vm/hyperv/powershell.rs @@ -5,7 +5,6 @@ use crate::CommandError; use crate::OpenHclServicingFlags; -use crate::StorageType; use crate::VmScreenshotMeta; use crate::run_host_cmd; use anyhow::Context; @@ -314,16 +313,6 @@ impl ps::AsVal for ControllerType { } } -impl From for ControllerType { - fn from(value: StorageType) -> Self { - match value { - StorageType::Ide => Self::Ide, - StorageType::Scsi => Self::Scsi, - StorageType::Nvme => todo!(), - } - } -} - /// Runs Add-VMHardDiskDrive with the given arguments. pub async fn run_add_vm_hard_disk_drive( args: HyperVAddVMHardDiskDriveArgs<'_>, @@ -382,6 +371,98 @@ pub async fn run_add_vm_dvd_drive(args: HyperVAddVMDvdDriveArgs<'_>) -> anyhow:: .context("add_vm_dvd_drive") } +/// Adds a SCSI controller with the specified VSID and target VTL to the VM +pub async fn run_add_vm_scsi_controller_with_id( + ps_mod: &Path, + vmid: &Guid, + vsid: &Guid, + target_vtl: u32, +) -> anyhow::Result<()> { + run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Import-Module") + .positional(ps_mod) + .next() + .cmdlet("Get-VM") + .arg("Id", vmid) + .pipeline() + .cmdlet("Add-VmScsiControllerWithId") + .arg("Vsid", vsid) + .arg("TargetVtl", target_vtl) + .finish() + .build(), + ) + .await + .map(|_| ()) + .context("add_vm_scsi_controller_with_id") +} + +/// Adds or modifies the drive at the specified location on the SCSI controller +/// with the specified VSID. +pub async fn run_set_vm_drive_scsi( + ps_mod: &Path, + vmid: &Guid, + controller_vsid: &Guid, + controller_location: u8, + disk_path: Option<&Path>, + dvd: bool, + allow_modify_existing: bool, +) -> anyhow::Result<()> { + run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Import-Module") + .positional(ps_mod) + .next() + .cmdlet("Get-VM") + .arg("Id", vmid) + .pipeline() + .cmdlet("Set-VmDrive") + .arg("ControllerVsid", controller_vsid) + .arg("Lun", controller_location) + .arg_opt("DiskPath", disk_path) + .flag_opt(dvd.then_some("Dvd")) + .flag_opt(allow_modify_existing.then_some("AllowModifyExisting")) + .finish() + .build(), + ) + .await + .map(|_| ()) + .context("set_vm_drive_scsi") +} + +/// Adds or modifies the drive at the specified location on the IDE controller +/// with the specified number. +pub async fn run_set_vm_drive_ide( + ps_mod: &Path, + vmid: &Guid, + controller_number: u32, + controller_location: u8, + disk_path: Option<&Path>, + dvd: bool, + allow_modify_existing: bool, +) -> anyhow::Result<()> { + run_host_cmd( + PowerShellBuilder::new() + .cmdlet("Import-Module") + .positional(ps_mod) + .next() + .cmdlet("Get-VM") + .arg("Id", vmid) + .pipeline() + .cmdlet("Set-VmDrive") + .arg("ControllerNumber", controller_number) + .arg("Lun", controller_location) + .arg_opt("DiskPath", disk_path) + .flag_opt(dvd.then_some("Dvd")) + .flag_opt(allow_modify_existing.then_some("AllowModifyExisting")) + .finish() + .build(), + ) + .await + .map(|_| ()) + .context("set_vm_drive_scsi") +} + /// Runs Add-VMScsiController with the given arguments. /// /// Returns the controller number and controller instance guid. diff --git a/petri/src/vm/hyperv/vm.rs b/petri/src/vm/hyperv/vm.rs index af2e8b2921..bab6c751b7 100644 --- a/petri/src/vm/hyperv/vm.rs +++ b/petri/src/vm/hyperv/vm.rs @@ -300,36 +300,54 @@ impl HyperVVM { } /// Add a SCSI controller - pub async fn add_scsi_controller(&mut self, target_vtl: u32) -> anyhow::Result<(u32, Guid)> { - let (controller_number, vsid) = - powershell::run_add_vm_scsi_controller(&self.ps_mod, &self.vmid).await?; - if target_vtl != 0 { - powershell::run_set_vm_scsi_controller_target_vtl( - &self.ps_mod, - &self.vmid, - controller_number, - target_vtl, - ) - .await?; - } - Ok((controller_number, vsid)) + pub async fn add_scsi_controller( + &mut self, + vsid: &Guid, + target_vtl: u32, + ) -> anyhow::Result<()> { + powershell::run_add_vm_scsi_controller_with_id(&self.ps_mod, &self.vmid, vsid, target_vtl) + .await } - /// Add a VHD - pub async fn add_vhd( + /// Add a drive to the scsi controller + pub async fn set_drive_scsi( &mut self, - path: &Path, - controller_type: powershell::ControllerType, - controller_location: Option, - controller_number: Option, + controller_vsid: &Guid, + controller_location: u8, + path: Option<&Path>, + dvd: bool, + allow_modify_existing: bool, ) -> anyhow::Result<()> { - powershell::run_add_vm_hard_disk_drive(powershell::HyperVAddVMHardDiskDriveArgs { - vmid: &self.vmid, - controller_type, + powershell::run_set_vm_drive_scsi( + &self.ps_mod, + &self.vmid, + controller_vsid, controller_location, + path, + dvd, + allow_modify_existing, + ) + .await + } + + /// Add a drive to the ide controller + pub async fn set_drive_ide( + &mut self, + controller_number: u32, + controller_location: u8, + path: Option<&Path>, + dvd: bool, + allow_modify_existing: bool, + ) -> anyhow::Result<()> { + powershell::run_set_vm_drive_ide( + &self.ps_mod, + &self.vmid, controller_number, - path: Some(path), - }) + controller_location, + path, + dvd, + allow_modify_existing, + ) .await } diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 1ed36e30c3..65168a093d 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -16,6 +16,7 @@ use crate::disk_image::SECTOR_SIZE; use crate::openhcl_diag::OpenHclDiagHandler; use async_trait::async_trait; use get_resources::ged::FirmwareEvent; +use guid::Guid; use mesh::CancelContext; use openvmm_defs::config::Vtl2BaseAddressType; use pal_async::DefaultDriver; @@ -140,16 +141,20 @@ pub struct PetriVmConfig { pub boot_device_type: BootDeviceType, /// TPM configuration pub tpm: Option, - /// Additional storage for the VM - pub additional_storage_controllers: HashMap, + /// IDE configuration + pub ide: Option, + /// Additional VMBus storage for the VM + pub additional_vmbus_storage_controllers: HashMap, } /// VM configuration that can be changed after the VM is created -pub struct PetriVmRuntimeConfig { +pub struct PetriVmRuntimeConfig { /// VTL2 settings pub vtl2_settings: Option, + /// IDE controllers and associated disks + pub ide_controllers: Option<[[Drive; 2]; 2]>, /// Storage controllers and associated disks - pub storage_controllers: HashMap>, + pub vmbus_storage_controllers: HashMap, } /// Resources used by a Petri VM during contruction and runtime @@ -195,16 +200,29 @@ pub trait PetriVmmBackend { config: PetriVmConfig, modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<( - Self::VmRuntime, - PetriVmRuntimeConfig<::RealizedStorageController>, - )>; + ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)>; } +#[cfg_attr(not(windows), expect(dead_code))] +pub(crate) const PETRI_IDE_BOOT_CONTROLLER: u32 = 0; +#[cfg_attr(not(windows), expect(dead_code))] +pub(crate) const PETRI_IDE_BOOT_LUN: u8 = 0; + pub(crate) const PETRI_VTL0_SCSI_BOOT_LUN: u8 = 0; pub(crate) const PETRI_VTL0_SCSI_PIPETTE_LUN: u8 = 1; pub(crate) const PETRI_VTL0_SCSI_CRASH_LUN: u8 = 2; +#[cfg_attr(not(windows), expect(dead_code))] +pub(crate) const PETRI_VTL2_SCSI_PIPETTE_LUN: u8 = 1; + +/// The instance guid used for all of our SCSI drives. +pub(crate) const PETRI_VTL0_SCSI_CONTROLLER_ID: Guid = + guid::guid!("27b553e8-8b39-411b-a55f-839971a7884f"); + +#[cfg_attr(not(windows), expect(dead_code))] +pub(crate) const PETRI_VTL2_SCSI_CONTROLLER_ID: Guid = + guid::guid!("818ae0a4-4f68-4b8e-94bc-c520c049097d"); + /// A constructed Petri VM pub struct PetriVm { resources: PetriVmResources, @@ -217,7 +235,7 @@ pub struct PetriVm { vmm_quirks: VmmQuirks, expected_boot_event: Option, - config: PetriVmRuntimeConfig<::RealizedStorageController>, + config: PetriVmRuntimeConfig, } impl PetriVmBuilder { @@ -243,6 +261,7 @@ impl PetriVmBuilder { } => BootDeviceType::None, Firmware::Uefi { .. } | Firmware::OpenhclUefi { .. } => BootDeviceType::Scsi, }; + let ide = artifacts.firmware.is_pcat().then(IdeConfig::default); let guest_crash_disk = if matches!( artifacts.firmware.os_flavor(), @@ -308,7 +327,8 @@ impl PetriVmBuilder { vmgs: PetriVmgsResource::Ephemeral, tpm: None, guest_crash_disk, - additional_storage_controllers: HashMap::new(), + ide, + additional_vmbus_storage_controllers: HashMap::new(), }, modify_vmm_config: None, resources: PetriVmResources { @@ -730,7 +750,7 @@ impl PetriVmBuilder { } PetriGuestStateLifetime::Reprovision => PetriVmgsResource::Reprovision(disk), PetriGuestStateLifetime::Ephemeral => { - if !matches!(disk.disk, PetriDisk::Memory) { + if !matches!(disk.disk, Disk::Memory(_)) { panic!("attempted to use ephemeral guest state after specifying backing vmgs") } PetriVmgsResource::Ephemeral @@ -756,20 +776,20 @@ impl PetriVmBuilder { /// Use the specified backing VMGS file pub fn with_initial_vmgs(self, disk: ResolvedArtifact) -> Self { - self.with_backing_vmgs(PetriDisk::Differencing(disk.into())) + self.with_backing_vmgs(Disk::Differencing(disk.into())) } /// Use the specified backing VMGS file pub fn with_persistent_vmgs(self, disk: impl AsRef) -> Self { - self.with_backing_vmgs(PetriDisk::Persistent(disk.as_ref().to_path_buf())) + self.with_backing_vmgs(Disk::Persistent(disk.as_ref().to_path_buf())) } - fn with_backing_vmgs(mut self, disk: PetriDisk) -> Self { + fn with_backing_vmgs(mut self, disk: Disk) -> Self { match &mut self.config.vmgs { PetriVmgsResource::Disk(vmgs) | PetriVmgsResource::ReprovisionOnFailure(vmgs) | PetriVmgsResource::Reprovision(vmgs) => { - if !matches!(vmgs.disk, PetriDisk::Memory) { + if !matches!(vmgs.disk, Disk::Memory(_)) { panic!("already specified a backing vmgs file"); } vmgs.disk = disk; @@ -829,53 +849,41 @@ impl PetriVmBuilder { } /// Add an additional SCSI controller to the VM. - pub fn add_storage_controller( + pub fn add_vmbus_storage_controller( mut self, - test_id: impl AsRef, + id: &Guid, target_vtl: Vtl, - device_type: StorageType, + controller_type: VmbusStorageType, ) -> Self { - let test_id = test_id.as_ref().to_string(); - if self .config - .additional_storage_controllers - .contains_key(&test_id) + .additional_vmbus_storage_controllers + .contains_key(id) { - panic!("storage controller with id \"{test_id}\" already exists"); + panic!("storage controller {id} already exists"); } - self.config.additional_storage_controllers.insert( - test_id, - PetriStorageController { - target_vtl, - controller_type: device_type, - disks: HashMap::new(), - realized: (), - }, + self.config.additional_vmbus_storage_controllers.insert( + *id, + VmbusStorageController::new(target_vtl, controller_type), ); self } - /// Adds a VHD with the optionally specified location (a.k.a LUN) to the - /// specified controller. - pub fn add_disk( + /// Add a VMBus disk drive to the VM + pub fn add_vmbus_drive( mut self, - disk: PetriDisk, - controller_test_id: impl AsRef, + drive: Drive, + controller_id: &Guid, controller_location: Option, ) -> Self { - let controller_test_id = controller_test_id.as_ref(); - let controller = self .config - .additional_storage_controllers - .get_mut(controller_test_id) - .unwrap_or_else(|| { - panic!("storage controller with id \"{controller_test_id}\" does not exist") - }); + .additional_vmbus_storage_controllers + .get_mut(controller_id) + .unwrap_or_else(|| panic!("storage controller {controller_id} does not exist")); - _ = controller.add_disk(controller_location, disk); + _ = controller.set_drive(controller_location, drive, false); self } @@ -1236,43 +1244,28 @@ impl PetriVm { } /// Get the list of storage controllers added to this VM - pub fn get_storage_controllers( - &self, - ) -> &HashMap< - String, - PetriStorageController<::RealizedStorageController>, - > { - &self.config.storage_controllers + pub fn get_vmbus_storage_controllers(&self) -> &HashMap { + &self.config.vmbus_storage_controllers } - /// Adds a VHD with the optionally specified location (a.k.a LUN) to the - /// specified controller. - pub async fn add_disk( + /// Add or modify a VMBus disk drive + pub async fn set_vmbus_drive( &mut self, - disk: PetriDisk, - controller_test_id: impl AsRef, + drive: Drive, + controller_id: &Guid, controller_location: Option, ) -> anyhow::Result<()> { - let controller_test_id = controller_test_id.as_ref(); - let controller = self .config - .storage_controllers - .get_mut(controller_test_id) - .unwrap_or_else(|| { - panic!("storage controller with id \"{controller_test_id}\" does not exist") - }); + .vmbus_storage_controllers + .get_mut(controller_id) + .unwrap_or_else(|| panic!("storage controller {controller_id} does not exist")); - let controller_location = controller.add_disk(controller_location, disk); - let disk = controller.disks.get(&controller_location).unwrap(); + let controller_location = controller.set_drive(controller_location, drive, true); + let disk = controller.drives.get(&controller_location).unwrap(); self.runtime - .add_disk( - disk, - controller.controller_type, - controller_location, - &controller.realized, - ) + .set_vmbus_drive(disk, controller_id, controller_location) .await?; Ok(()) @@ -1286,8 +1279,6 @@ pub trait PetriVmRuntime: Send + Sync + 'static { type VmInspector: PetriVmInspector; /// Interface for accessing the framebuffer type VmFramebufferAccess: PetriVmFramebufferAccess; - /// Information used to identify a realized storage controller - type RealizedStorageController; /// Cleanly tear down the VM immediately. async fn teardown(self) -> anyhow::Result<()>; @@ -1342,15 +1333,14 @@ pub trait PetriVmRuntime: Send + Sync + 'static { async fn get_guest_state_file(&self) -> anyhow::Result> { Ok(None) } - /// Set the OpenHCL VTL2 settings. + /// Set the OpenHCL VTL2 settings async fn set_vtl2_settings(&mut self, settings: &Vtl2Settings) -> anyhow::Result<()>; - /// Add a VHD - async fn add_disk( + /// Add or modify a VMBus disk drive + async fn set_vmbus_drive( &mut self, - disk: &PetriDisk, - controller_type: StorageType, + disk: &Drive, + controller_id: &Guid, controller_location: u8, - controller: &Self::RealizedStorageController, ) -> anyhow::Result<()>; } @@ -1949,6 +1939,11 @@ impl PcatGuest { PcatGuest::Iso(disk) => &disk.artifact, } } + + #[cfg_attr(not(windows), expect(dead_code))] + fn is_dvd(&self) -> bool { + matches!(self, Self::Iso(_)) + } } /// The guest the VM will boot into. A boot drive with the chosen setup @@ -2078,10 +2073,10 @@ pub struct OpenHclServicingFlags { } /// Petri disk -#[derive(Debug, Clone)] -pub enum PetriDisk { - /// Memory backed - Memory, +#[derive(Debug, Clone, PartialEq)] +pub enum Disk { + /// Memory backed with specified size + Memory(u64), /// Memory differencing disk backed by a file Differencing(PathBuf), /// Persistent disk @@ -2092,7 +2087,7 @@ pub enum PetriDisk { #[derive(Debug, Clone)] pub struct PetriVmgsDisk { /// Backing disk - pub disk: PetriDisk, + pub disk: Disk, /// Guest state encryption policy pub encryption_policy: GuestStateEncryptionPolicy, } @@ -2100,7 +2095,7 @@ pub struct PetriVmgsDisk { impl Default for PetriVmgsDisk { fn default() -> Self { PetriVmgsDisk { - disk: PetriDisk::Memory, + disk: Disk::Memory(vmgs_format::VMGS_DEFAULT_CAPACITY), // TODO: make this strict once we can set it in OpenHCL on Hyper-V encryption_policy: GuestStateEncryptionPolicy::None(false), } @@ -2265,17 +2260,6 @@ fn default_vtl2_settings() -> Vtl2Settings { } } -/// The storage device type. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum StorageType { - /// IDE - Ide, - /// SCSI - Scsi, - /// NVMe - Nvme, -} - /// Virtual trust level #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Vtl { @@ -2285,24 +2269,89 @@ pub enum Vtl { Vtl2 = 2, } -/// Petri storage controller +/// The VMBus storage device type. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum VmbusStorageType { + /// SCSI + Scsi, + /// NVMe + Nvme, +} + +/// VM disk drive +#[derive(Debug, Clone, PartialEq)] +pub struct Drive { + /// Backing disk + pub disk: Option, + /// Whether this is a DVD + pub is_dvd: bool, +} + +impl Drive { + /// Create a new disk + pub fn new(disk: Option, is_dvd: bool) -> Self { + Self { disk, is_dvd } + } +} + +/// IDE configuration +/// +/// Controller 0, location 0 is reserved for the boot disk. +#[derive(Debug, Clone, PartialEq)] +pub struct IdeConfig { + /// Controller 0, location 1 + pub drive_0_1: Option, + /// Controller 1, location 0 + pub drive_1_0: Option, + /// Controller 1, location 1 + pub drive_1_1: Option, +} + +impl Default for IdeConfig { + fn default() -> Self { + Self { + drive_0_1: None, + drive_1_0: Some(Drive { + disk: None, + is_dvd: true, + }), + drive_1_1: None, + } + } +} + +impl IdeConfig { + /// Get the config with the boot disk + pub fn into_controllers(self) -> [[Option; 2]; 2] { + [[None, self.drive_0_1], [self.drive_1_0, self.drive_1_1]] + } +} + +/// VMBus storage controller #[derive(Debug, Clone)] -pub struct PetriStorageController { +pub struct VmbusStorageController { /// The VTL to assign the storage controller to pub target_vtl: Vtl, /// The storage device type - pub controller_type: StorageType, - /// Disks attached to this storage controller - pub disks: HashMap, - /// Backend-specific realized details - pub realized: T, + pub controller_type: VmbusStorageType, + /// Drives (with any inserted disks) attached to this storage controller + pub drives: HashMap, } -impl PetriStorageController { +impl VmbusStorageController { + /// Create a new storage controller + pub fn new(target_vtl: Vtl, controller_type: VmbusStorageType) -> Self { + Self { + target_vtl, + controller_type, + drives: HashMap::new(), + } + } + /// Add a disk to the storage controller - pub fn add_disk(&mut self, lun: Option, disk: PetriDisk) -> u8 { + pub fn set_drive(&mut self, lun: Option, drive: Drive, allow_modify_existing: bool) -> u8 { let lun = if let Some(lun) = lun { - if self.disks.contains_key(&lun) { + if !allow_modify_existing && self.drives.contains_key(&lun) { panic!("a disk with lun {lun} already exists on this controller"); } lun @@ -2310,7 +2359,7 @@ impl PetriStorageController { // find the first available lun let mut lun = None; for x in 0..u8::MAX { - if !self.disks.contains_key(&x) { + if !self.drives.contains_key(&x) { lun = Some(x) } } @@ -2321,7 +2370,7 @@ impl PetriStorageController { } }; - self.disks.insert(lun, disk); + self.drives.insert(lun, drive); lun } diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 1a85f195f0..6cacfe2999 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -9,7 +9,6 @@ use super::BOOT_NVME_NSID; use super::PARAVISOR_BOOT_NVME_INSTANCE; use super::PetriVmConfigOpenVmm; use super::PetriVmResourcesOpenVmm; -use super::SCSI_INSTANCE; use super::memdiff_disk; use crate::BootDeviceType; use crate::Firmware; @@ -30,6 +29,7 @@ use crate::UefiGuest; use crate::linux_direct_serial_agent::LinuxDirectSerialAgent; use crate::openvmm::BOOT_NVME_INSTANCE; use crate::openvmm::memdiff_vmgs; +use crate::vm::PETRI_VTL0_SCSI_CONTROLLER_ID; use crate::vm::append_cmdline; use crate::vtl2_settings::ControllerType; use crate::vtl2_settings::Vtl2LunBuilder; @@ -128,13 +128,18 @@ impl PetriVmConfigOpenVmm { boot_device_type, tpm: tpm_config, guest_crash_disk, - additional_storage_controllers, + ide, + additional_vmbus_storage_controllers, } = petri_vm_config; - if !additional_storage_controllers.is_empty() { + if !additional_vmbus_storage_controllers.is_empty() { unimplemented!("openvmm additional storage controllers"); } + if ide.is_some_and(|c| c != crate::IdeConfig::default()) { + unimplemented!("custom ide config"); + } + let PetriVmResources { driver, log_source } = resources; let mesh = Mesh::new("petri_mesh".to_string())?; @@ -242,7 +247,7 @@ impl PetriVmConfigOpenVmm { }; let mut petri_vtl0_scsi = ScsiControllerHandle { - instance_id: SCSI_INSTANCE, + instance_id: PETRI_VTL0_SCSI_CONTROLLER_ID, max_sub_channel_count: 1, io_queue_depth: None, devices: vec![], diff --git a/petri/src/vm/openvmm/mod.rs b/petri/src/vm/openvmm/mod.rs index 55a9c506a8..ad37fcc380 100644 --- a/petri/src/vm/openvmm/mod.rs +++ b/petri/src/vm/openvmm/mod.rs @@ -18,10 +18,10 @@ pub use runtime::OpenVmmInspector; pub use runtime::PetriVmOpenVmm; use crate::BootDeviceType; +use crate::Disk; use crate::Firmware; use crate::OpenHclServicingFlags; use crate::OpenvmmLogConfig; -use crate::PetriDisk; use crate::PetriLogFile; use crate::PetriVmConfig; use crate::PetriVmResources; @@ -67,9 +67,6 @@ use vmgs_resources::VmgsDisk; use vmgs_resources::VmgsResource; use vtl2_settings_proto::Vtl2Settings; -/// The instance guid used for all of our SCSI drives. -pub(crate) const SCSI_INSTANCE: Guid = guid::guid!("27b553e8-8b39-411b-a55f-839971a7884f"); - /// The instance guid for the NVMe controller automatically added for boot media /// for paravisor storage translation. pub(crate) const PARAVISOR_BOOT_NVME_INSTANCE: Guid = @@ -147,7 +144,7 @@ impl PetriVmmBackend for OpenVmmPetriBackend { config: PetriVmConfig, modify_vmm_config: Option PetriVmConfigOpenVmm + Send>, resources: &PetriVmResources, - ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig<()>)> { + ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { let mut config = PetriVmConfigOpenVmm::new(&self.openvmm_path, config, resources).await?; if let Some(f) = modify_vmm_config { @@ -231,12 +228,12 @@ fn memdiff_vmgs(vmgs: &PetriVmgsResource) -> anyhow::Result { let convert_disk = |disk: &PetriVmgsDisk| -> anyhow::Result { Ok(VmgsDisk { disk: match &disk.disk { - PetriDisk::Memory => LayeredDiskHandle::single_layer(RamDiskLayerHandle { - len: Some(vmgs_format::VMGS_DEFAULT_CAPACITY), - }) - .into_resource(), - PetriDisk::Differencing(path) => memdiff_disk(path)?, - PetriDisk::Persistent(path) => open_disk_type(path, false)?, + Disk::Memory(len) => { + LayeredDiskHandle::single_layer(RamDiskLayerHandle { len: Some(*len) }) + .into_resource() + } + Disk::Differencing(path) => memdiff_disk(path)?, + Disk::Persistent(path) => open_disk_type(path, false)?, }, encryption_policy: disk.encryption_policy, }) diff --git a/petri/src/vm/openvmm/runtime.rs b/petri/src/vm/openvmm/runtime.rs index b1b25fdbd2..2cfe9bb04b 100644 --- a/petri/src/vm/openvmm/runtime.rs +++ b/petri/src/vm/openvmm/runtime.rs @@ -50,7 +50,6 @@ pub struct PetriVmOpenVmm { impl PetriVmRuntime for PetriVmOpenVmm { type VmInspector = OpenVmmInspector; type VmFramebufferAccess = OpenVmmFramebufferAccess; - type RealizedStorageController = (); async fn teardown(self) -> anyhow::Result<()> { tracing::info!("waiting for worker"); @@ -170,14 +169,13 @@ impl PetriVmRuntime for PetriVmOpenVmm { Self::set_vtl2_settings(self, settings).await } - async fn add_disk( + async fn set_vmbus_drive( &mut self, - _disk: &crate::PetriDisk, - _controller_type: crate::StorageType, + _disk: &crate::Drive, + _controller_id: &guid::Guid, _controller_location: u8, - _controller: &Self::RealizedStorageController, ) -> anyhow::Result<()> { - todo!() + todo!("openvmm set vmbus drive") } } diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index c86bf1703a..704ad4a041 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -35,7 +35,7 @@ use storvsp_resources::ScsiPath; use vm_resource::IntoResource; impl PetriVmConfigOpenVmm { - async fn run_core(self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig<()>)> { + async fn run_core(self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { let Self { firmware, arch, @@ -183,14 +183,15 @@ impl PetriVmConfigOpenVmm { vm, PetriVmRuntimeConfig { vtl2_settings, - storage_controllers: HashMap::new(), + ide_controllers: None, + vmbus_storage_controllers: HashMap::new(), }, )) } /// Run the VM, configuring pipette to automatically start if it is /// included in the config - pub async fn run(mut self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig<()>)> { + pub async fn run(mut self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { let launch_linux_direct_pipette = if let Some(agent_image) = &self.resources.agent_image { // Construct the agent disk. if let Some(agent_disk) = agent_image.build().context("failed to build agent image")? { 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 a95c5a609f..3de1a3fd26 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -326,10 +326,10 @@ async fn storvsp_hyperv(config: PetriVmBuilder) -> Result<() let vtl2_lun = 5; let vtl0_scsi_lun = 0; let scsi_instance = Guid::new_random(); + let vtl2_vsid = Guid::new_random(); const SCSI_DISK_SECTORS: u64 = 0x4_0000; const SECTOR_SIZE: u64 = 512; const EXPECTED_SCSI_DISK_SIZE_BYTES: u64 = SCSI_DISK_SECTORS * SECTOR_SIZE; - const CONTROLLER_TEST_ID: &str = "scsi-controller"; // Assumptions made by test infra & routines: // @@ -360,27 +360,15 @@ async fn storvsp_hyperv(config: PetriVmBuilder) -> Result<() .build(), ); }) - .add_storage_controller( - CONTROLLER_TEST_ID, - petri::Vtl::Vtl2, - petri::StorageType::Scsi, + .add_vmbus_storage_controller(&vtl2_vsid, petri::Vtl::Vtl2, petri::VmbusStorageType::Scsi) + .add_vmbus_drive( + petri::Drive::new(Some(petri::Disk::Persistent(vhd_path.to_path_buf())), false), + &vtl2_vsid, + Some(vtl2_lun), ) .run() .await?; - let (_, vtl2_vsid) = vm - .get_storage_controllers() - .get(CONTROLLER_TEST_ID) - .map(|c| (c.realized.controller_number, c.realized.vsid)) - .ok_or_else(|| anyhow::anyhow!("couldn't find additional scsi controller"))?; - - vm.add_disk( - petri::PetriDisk::Persistent(vhd_path.to_path_buf()), - CONTROLLER_TEST_ID, - Some(vtl2_lun), - ) - .await?; - vm.modify_vtl2_settings(|s| { let storage_controllers = &mut s.dynamic.as_mut().unwrap().storage_controllers; assert_eq!(storage_controllers.len(), 1); From 3890bfcb9fa6278dbf704011c4366ee10da8f4bc Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Wed, 17 Dec 2025 20:40:22 -0800 Subject: [PATCH 03/10] generically add all disks --- petri/petri-tool/src/main.rs | 2 +- petri/src/disk_image.rs | 23 +- petri/src/vm/hyperv/mod.rs | 350 +++------- petri/src/vm/hyperv/powershell.rs | 2 +- petri/src/vm/mod.rs | 612 +++++++++++++----- petri/src/vm/openvmm/construct.rs | 448 +++++-------- petri/src/vm/openvmm/mod.rs | 62 +- petri/src/vm/openvmm/modify.rs | 19 +- petri/src/vm/openvmm/runtime.rs | 9 +- petri/src/vm/openvmm/start.rs | 114 +--- petri/src/vm/vtl2_settings.rs | 9 + vmm_tests/vmm_test_macros/src/lib.rs | 16 +- .../tests/multiarch/openhcl_servicing.rs | 6 +- .../tests/tests/x86_64/openhcl_uefi.rs | 13 +- .../vmm_tests/tests/tests/x86_64/storage.rs | 2 +- 15 files changed, 801 insertions(+), 886 deletions(-) diff --git a/petri/petri-tool/src/main.rs b/petri/petri-tool/src/main.rs index 44a81180bb..e4fcc49988 100644 --- a/petri/petri-tool/src/main.rs +++ b/petri/petri-tool/src/main.rs @@ -59,7 +59,7 @@ fn main() -> anyhow::Result<()> { })?; let disk = image - .build() + .build(petri::disk_image::ImageType::Raw) .context("failed to build disk image")? .context("no files for the this platform")?; disk.persist(output) diff --git a/petri/src/disk_image.rs b/petri/src/disk_image.rs index fef3cc6ea7..82bfdf79cd 100644 --- a/petri/src/disk_image.rs +++ b/petri/src/disk_image.rs @@ -25,6 +25,14 @@ pub struct AgentImage { extras: Vec<(String, ResolvedArtifact)>, } +/// Disk image type +pub enum ImageType { + /// Raw image + Raw, + /// Fixed VHD1 + Vhd, +} + impl AgentImage { /// Resolves the artifacts needed to build a disk image for a VM. pub fn new(os_flavor: OsFlavor) -> Self { @@ -77,7 +85,7 @@ impl AgentImage { /// Builds a disk image containing pipette and any files needed for the guest VM /// to run pipette. - pub fn build(&self) -> anyhow::Result> { + pub fn build(&self, image_type: ImageType) -> anyhow::Result> { let mut files = self .extras .iter() @@ -129,12 +137,23 @@ impl AgentImage { if files.is_empty() { Ok(None) } else { - let mut image_file = tempfile::NamedTempFile::new()?; + let mut image_file = match image_type { + ImageType::Raw => tempfile::NamedTempFile::new()?, + ImageType::Vhd => tempfile::Builder::new().suffix(".vhd").tempfile()?, + }; + image_file .as_file() .set_len(64 * 1024 * 1024) .context("failed to set file size")?; + build_fat32_disk_image(&mut image_file, "CIDATA", volume_label, &files)?; + + if matches!(image_type, ImageType::Vhd) { + disk_vhd1::Vhd1Disk::make_fixed(image_file.as_file()) + .context("failed to make vhd for agent image")?; + } + Ok(Some(image_file)) } } diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index f0bd8f646e..9d687445c2 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -8,7 +8,6 @@ use vmsocket::VmAddress; use vmsocket::VmSocket; use super::ProcessorTopology; -use crate::BootDeviceType; use crate::Disk; use crate::Drive; use crate::Firmware; @@ -29,13 +28,12 @@ use crate::SecureBootTemplate; use crate::ShutdownKind; use crate::TpmConfig; use crate::UefiConfig; -use crate::VmbusStorageController; use crate::VmbusStorageType; use crate::VmmQuirks; -use crate::disk_image::AgentImage; use crate::hyperv::powershell::HyperVSecureBootTemplate; use crate::kmsg_log_task; use crate::openhcl_diag::OpenHclDiagHandler; +use crate::vm::PetriVmProperties; use crate::vm::append_cmdline; use anyhow::Context; use async_trait::async_trait; @@ -70,16 +68,6 @@ use vtl2_settings_proto::Vtl2Settings; /// The Hyper-V Petri backend pub struct HyperVPetriBackend {} -/// Details about a storage controller addeded to a VM. -#[derive(Debug)] -pub struct RealizedHyperVStorageController { - /// The controller number assigned by Hyper-V. - pub controller_number: u32, - - /// The VSID assigned by Hyper-V for this controller. - pub vsid: Guid, -} - /// Resources needed at runtime for a Hyper-V Petri VM pub struct HyperVPetriRuntime { vm: HyperVVM, @@ -87,9 +75,7 @@ pub struct HyperVPetriRuntime { temp_dir: TempDir, output_dir: PathBuf, driver: DefaultDriver, - - is_openhcl: bool, - is_isolated: bool, + properties: PetriVmProperties, } #[async_trait] @@ -177,6 +163,7 @@ impl PetriVmmBackend for HyperVPetriBackend { config: PetriVmConfig, _modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, + properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { let PetriVmConfig { name, @@ -185,14 +172,10 @@ impl PetriVmmBackend for HyperVPetriBackend { firmware, memory, proc_topology, - agent_image, - openhcl_agent_image, - boot_device_type, vmgs, tpm, - guest_crash_disk, - ide, - additional_vmbus_storage_controllers, + ide_controllers, + vmbus_storage_controllers, } = config; let PetriVmResources { driver, log_source } = resources; @@ -204,78 +187,63 @@ impl PetriVmmBackend for HyperVPetriBackend { let temp_dir = tempfile::tempdir()?; - let is_openhcl = firmware.is_openhcl(); - let is_isolated = firmware.isolation().is_some(); - let os_flavor = firmware.os_flavor(); - - let (guest_state_isolation_type, generation, guest_artifact, uefi_config, openhcl_config) = - match firmware { - Firmware::LinuxDirect { .. } | Firmware::OpenhclLinuxDirect { .. } => { - todo!("linux direct not supported on hyper-v") - } - Firmware::Pcat { - guest, - bios_firmware: _, // TODO - svga_firmware: _, // TODO - } => ( - powershell::HyperVGuestStateIsolationType::Disabled, - powershell::HyperVGeneration::One, - Some((guest.artifact().to_owned(), guest.is_dvd())), - None, - None, - ), - Firmware::OpenhclPcat { - guest, - igvm_path, - bios_firmware: _, // TODO - svga_firmware: _, // TODO - openhcl_config, - } => ( - powershell::HyperVGuestStateIsolationType::OpenHCL, - powershell::HyperVGeneration::One, - Some((guest.artifact().to_owned(), guest.is_dvd())), - None, - Some((igvm_path, openhcl_config)), - ), - Firmware::Uefi { - guest, - uefi_firmware: _, // TODO - uefi_config, - } => ( - powershell::HyperVGuestStateIsolationType::Disabled, - powershell::HyperVGeneration::Two, - guest.artifact().map(|a| (a.to_owned(), false)), - Some(uefi_config), - None, - ), - Firmware::OpenhclUefi { - guest, - isolation, - igvm_path, - uefi_config, - openhcl_config, - } => ( - match isolation { - Some(IsolationType::Vbs) => powershell::HyperVGuestStateIsolationType::Vbs, - Some(IsolationType::Snp) => powershell::HyperVGuestStateIsolationType::Snp, - Some(IsolationType::Tdx) => powershell::HyperVGuestStateIsolationType::Tdx, - // Older hosts don't support OpenHCL isolation, so use Trusted Launch - None => powershell::HyperVGuestStateIsolationType::TrustedLaunch, - }, - powershell::HyperVGeneration::Two, - guest.artifact().map(|a| (a.to_owned(), false)), - Some(uefi_config), - Some((igvm_path, openhcl_config)), - ), - }; + let (guest_state_isolation_type, generation, uefi_config, openhcl_config) = match firmware { + Firmware::LinuxDirect { .. } | Firmware::OpenhclLinuxDirect { .. } => { + todo!("linux direct not supported on hyper-v") + } + Firmware::Pcat { + guest: _, + bios_firmware: _, // TODO + svga_firmware: _, // TODO + } => ( + powershell::HyperVGuestStateIsolationType::Disabled, + powershell::HyperVGeneration::One, + None, + None, + ), + Firmware::OpenhclPcat { + guest: _, + igvm_path, + bios_firmware: _, // TODO + svga_firmware: _, // TODO + openhcl_config, + } => ( + powershell::HyperVGuestStateIsolationType::OpenHCL, + powershell::HyperVGeneration::One, + None, + Some((igvm_path, openhcl_config)), + ), + Firmware::Uefi { + guest: _, + uefi_firmware: _, // TODO + uefi_config, + } => ( + powershell::HyperVGuestStateIsolationType::Disabled, + powershell::HyperVGeneration::Two, + Some(uefi_config), + None, + ), + Firmware::OpenhclUefi { + guest: _, + isolation, + igvm_path, + uefi_config, + openhcl_config, + } => ( + match isolation { + Some(IsolationType::Vbs) => powershell::HyperVGuestStateIsolationType::Vbs, + Some(IsolationType::Snp) => powershell::HyperVGuestStateIsolationType::Snp, + Some(IsolationType::Tdx) => powershell::HyperVGuestStateIsolationType::Tdx, + // Older hosts don't support OpenHCL isolation, so use Trusted Launch + None => powershell::HyperVGuestStateIsolationType::TrustedLaunch, + }, + powershell::HyperVGeneration::Two, + Some(uefi_config), + Some((igvm_path, openhcl_config)), + ), + }; let mut openhcl_command_line = openhcl_config.as_ref().map(|(_, c)| c.command_line()); - let mut vtl2_settings = None; - let mut vmbus_storage_controllers = additional_vmbus_storage_controllers; - let mut ide_controllers = (generation == powershell::HyperVGeneration::One).then(|| { - ide.expect("need ide controller for gen 1 boot") - .into_controllers() - }); let vmgs_path = { // TODO: add support for configuring the TPM in Hyper-V @@ -311,7 +279,7 @@ impl PetriVmmBackend for HyperVPetriBackend { // TODO: Error for non-OpenHCL Hyper-V VMs if not supported // TODO: Use WMI interfaces when possible - if is_openhcl { + if properties.is_openhcl { append_cmdline( &mut openhcl_command_line, format!("HCL_GUEST_STATE_LIFETIME={lifetime_cli}"), @@ -325,17 +293,7 @@ impl PetriVmmBackend for HyperVPetriBackend { } }; - match disk { - None | Some(Disk::Memory(_)) => None, - Some(Disk::Differencing(parent_path)) => { - let diff_disk_path = temp_dir - .path() - .join(parent_path.file_name().context("path has no filename")?); - make_temp_diff_disk(&diff_disk_path, &parent_path).await?; - Some(diff_disk_path) - } - Some(Disk::Persistent(path)) => Some(path), - } + petri_disk_to_hyperv(disk.as_ref(), &temp_dir).await? }; let mut log_tasks = Vec::new(); @@ -386,6 +344,7 @@ impl PetriVmmBackend for HyperVPetriBackend { secure_boot_template, disable_frontpage, default_boot_always_attempt, + enable_vpci_boot, }) = uefi_config { vm.set_secure_boot( @@ -402,14 +361,14 @@ impl PetriVmmBackend for HyperVPetriBackend { .await?; // TODO: Disable frontpage for non-OpenHCL Hyper-V VMs - if disable_frontpage && is_openhcl { + if disable_frontpage && properties.is_openhcl { append_cmdline( &mut openhcl_command_line, "OPENHCL_DISABLE_UEFI_FRONTPAGE=1", ); } - if is_openhcl { + if properties.is_openhcl { append_cmdline( &mut openhcl_command_line, format!( @@ -418,76 +377,31 @@ impl PetriVmmBackend for HyperVPetriBackend { ), ); }; - } - - let boot_disk = guest_artifact.map(|(artifact, is_dvd)| { - Drive::new( - Some(Disk::Differencing(artifact.get().to_path_buf())), - is_dvd, - ) - }); - - // Share a single scsi controller for all petri-added drives. - vmbus_storage_controllers.insert( - super::PETRI_VTL0_SCSI_CONTROLLER_ID, - VmbusStorageController::new(super::Vtl::Vtl0, VmbusStorageType::Scsi), - ); - if let Some(boot_disk) = boot_disk { - match boot_device_type { - BootDeviceType::None => {} - BootDeviceType::Ide => { - ide_controllers.as_mut().expect("ide not configured") - [super::PETRI_IDE_BOOT_CONTROLLER as usize] - [super::PETRI_IDE_BOOT_LUN as usize] = Some(boot_disk); - } - BootDeviceType::Scsi => { - vmbus_storage_controllers - .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) - .unwrap() - .set_drive(Some(super::PETRI_VTL0_SCSI_BOOT_LUN), boot_disk, false); - } - BootDeviceType::Nvme => todo!("NVMe boot device not yet supported for Hyper-V"), + if enable_vpci_boot { + todo!("hyperv nvme boot"); } } - if let Some(agent_image) = agent_image { - // Construct the agent disk. - let agent_disk_path = temp_dir.path().join("cidata.vhd"); - - if build_and_persist_agent_image(&agent_image, &agent_disk_path) - .context("vtl0 agent disk")? + if properties.using_vtl0_pipette + && matches!(properties.os_flavor, OsFlavor::Windows) + && !properties.is_isolated + { + // Make a file for the IMC hive. It's not guaranteed to be at a fixed + // location at runtime. + let imc_hive = temp_dir.path().join("imc.hiv"); { - if agent_image.contains_pipette() - && matches!(os_flavor, OsFlavor::Windows) - && !is_isolated - { - // Make a file for the IMC hive. It's not guaranteed to be at a fixed - // location at runtime. - let imc_hive = temp_dir.path().join("imc.hiv"); - { - let mut imc_hive_file = fs_err::File::create_new(&imc_hive)?; - imc_hive_file - .write_all(include_bytes!("../../../guest-bootstrap/imc.hiv")) - .context("failed to write imc hive")?; - } - - // Set the IMC - vm.set_imc(&imc_hive).await?; - } - - vmbus_storage_controllers - .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) - .unwrap() - .set_drive( - Some(super::PETRI_VTL0_SCSI_PIPETTE_LUN), - Drive::new(Some(Disk::Persistent(agent_disk_path)), false), - false, - ); + let mut imc_hive_file = fs_err::File::create_new(&imc_hive)?; + imc_hive_file + .write_all(include_bytes!("../../../guest-bootstrap/imc.hiv")) + .context("failed to write imc hive")?; } + + // Set the IMC + vm.set_imc(&imc_hive).await?; } - if let Some(( + let vtl2_settings = if let Some(( src_igvm_file, OpenHclConfig { vtl2_nvme_boot: _, // TODO, see #1649. @@ -495,7 +409,7 @@ impl PetriVmmBackend for HyperVPetriBackend { custom_command_line: _, log_levels: _, vtl2_base_address_type, - modify_vtl2_settings, + vtl2_settings, }, )) = openhcl_config { @@ -528,26 +442,6 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.set_vmbus_redirect(vmbus_redirect).await?; - if let Some(agent_image) = openhcl_agent_image { - let agent_disk_path = temp_dir.path().join("paravisor_cidata.vhd"); - - if build_and_persist_agent_image(&agent_image, &agent_disk_path) - .context("vtl2 agent disk")? - { - vmbus_storage_controllers - .entry(super::PETRI_VTL2_SCSI_CONTROLLER_ID) - .or_insert(VmbusStorageController::new( - crate::Vtl::Vtl2, - VmbusStorageType::Scsi, - )) - .set_drive( - Some(super::PETRI_VTL2_SCSI_PIPETTE_LUN), - Drive::new(Some(Disk::Persistent(agent_disk_path)), false), - false, - ); - } - } - // Attempt to enable COM3 and use that to get KMSG logs, otherwise // fall back to use diag_client. let supports_com3 = { @@ -599,24 +493,14 @@ impl PetriVmmBackend for HyperVPetriBackend { )); } - if let Some(f) = modify_vtl2_settings { - f.0(vtl2_settings.get_or_insert_with(crate::vm::default_vtl2_settings)) - }; - } - - if let Some(guest_crash_disk) = guest_crash_disk { - vmbus_storage_controllers - .get_mut(&super::PETRI_VTL0_SCSI_CONTROLLER_ID) - .unwrap() - .set_drive( - Some(super::PETRI_VTL0_SCSI_CRASH_LUN), - Drive::new( - Some(Disk::Persistent(guest_crash_disk.to_path_buf())), - false, - ), - false, - ); - } + // Set the VTL2 settings if necessary + if let Some(settings) = &vtl2_settings { + vm.set_base_vtl2_settings(settings).await?; + } + vtl2_settings + } else { + None + }; let serial_pipe_path = vm.set_vm_com_port(1).await?; let serial_log_file = log_source.log_file("guest")?; @@ -625,11 +509,8 @@ impl PetriVmmBackend for HyperVPetriBackend { hyperv_serial_log_task(driver.clone(), serial_pipe_path, serial_log_file), )); - // TODO: If OpenHCL is being used, then translate storage through it. - // (requires changes above where VHDs are added) - // Add IDE storage - if let Some(ide_controllers) = ide_controllers { + if let Some(ide_controllers) = &ide_controllers { for (controller_number, controller) in ide_controllers.iter().enumerate() { for (controller_location, disk) in controller.iter().enumerate() { if let Some(disk) = disk { @@ -655,12 +536,14 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.add_scsi_controller(vsid, controller.target_vtl as u32) .await?; - for (controller_location, disk) in controller.drives.iter() { - let path = petri_disk_to_hyperv(disk.disk.as_ref(), &temp_dir).await?; + for (controller_location, drive) in controller.drives.iter() { + let path = petri_disk_to_hyperv(drive.disk.as_ref(), &temp_dir).await?; vm.set_drive_scsi( vsid, - *controller_location, + (*controller_location) + .try_into() + .context("invalid scsi lun")?, path.as_deref(), false, false, @@ -682,7 +565,7 @@ impl PetriVmmBackend for HyperVPetriBackend { } vm.enable_tpm().await?; - if is_openhcl { + if properties.is_openhcl { vm.set_guest_state_isolation_mode(if no_persistent_secrets { powershell::HyperVGuestStateIsolationMode::NoPersistentSecrets } else { @@ -696,11 +579,6 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.disable_tpm().await?; } - // Set the VTL2 settings if necessary - if let Some(settings) = &vtl2_settings { - vm.set_base_vtl2_settings(settings).await?; - } - vm.start().await?; Ok(( @@ -710,12 +588,11 @@ impl PetriVmmBackend for HyperVPetriBackend { temp_dir, output_dir: log_source.output_dir().to_owned(), driver: driver.clone(), - is_openhcl, - is_isolated, + properties, }, PetriVmRuntimeConfig { vtl2_settings, - ide_controllers: None, + ide_controllers, vmbus_storage_controllers, }, )) @@ -792,7 +669,7 @@ impl PetriVmRuntime for HyperVPetriRuntime { } fn openhcl_diag(&self) -> Option { - self.is_openhcl.then(|| { + self.properties.is_openhcl.then(|| { OpenHclDiagHandler::new(diag_client::DiagClient::from_hyperv_id( self.driver.clone(), *self.vm.vmid(), @@ -843,7 +720,7 @@ impl PetriVmRuntime for HyperVPetriRuntime { } fn take_framebuffer_access(&mut self) -> Option { - (!self.is_isolated).then(|| self.vm.get_framebuffer_access()) + (!self.properties.is_isolated).then(|| self.vm.get_framebuffer_access()) } async fn reset(&mut self) -> anyhow::Result<()> { @@ -862,12 +739,12 @@ impl PetriVmRuntime for HyperVPetriRuntime { &mut self, drive: &Drive, controller_id: &Guid, - controller_location: u8, + controller_location: u32, ) -> anyhow::Result<()> { self.vm .set_drive_scsi( controller_id, - controller_location, + controller_location.try_into().context("invalid scsi lun")?, petri_disk_to_hyperv(drive.disk.as_ref(), &self.temp_dir) .await? .as_deref(), @@ -900,22 +777,6 @@ fn acl_read_for_vm(path: &Path, id: Option) -> anyhow::Result<()> { Ok(()) } -fn build_and_persist_agent_image( - agent_image: &AgentImage, - agent_disk_path: &Path, -) -> anyhow::Result { - Ok( - if let Some(agent_disk) = agent_image.build().context("failed to build agent image")? { - disk_vhd1::Vhd1Disk::make_fixed(agent_disk.as_file()) - .context("failed to make vhd for agent image")?; - agent_disk.persist(agent_disk_path)?; - true - } else { - false - }, - ) -} - async fn hyperv_serial_log_task( driver: DefaultDriver, serial_pipe_path: String, @@ -971,7 +832,7 @@ async fn petri_disk_to_hyperv( ) -> anyhow::Result> { Ok(match disk { None => None, - Some(Disk::Memory(_)) => todo!(), + Some(Disk::Memory(_)) => None, // TODO: Hyper-V memory disk Some(Disk::Differencing(parent_path)) => { let diff_disk_path = temp_dir .path() @@ -980,5 +841,6 @@ async fn petri_disk_to_hyperv( Some(diff_disk_path) } Some(Disk::Persistent(path)) => Some(path.clone()), + Some(Disk::Temporary(path)) => Some(path.to_path_buf()), }) } diff --git a/petri/src/vm/hyperv/powershell.rs b/petri/src/vm/hyperv/powershell.rs index 5fd8033867..d9df3eabe7 100644 --- a/petri/src/vm/hyperv/powershell.rs +++ b/petri/src/vm/hyperv/powershell.rs @@ -460,7 +460,7 @@ pub async fn run_set_vm_drive_ide( ) .await .map(|_| ()) - .context("set_vm_drive_scsi") + .context("set_vm_drive_ide") } /// Runs Add-VMScsiController with the given arguments. diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 65168a093d..179953698c 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -14,6 +14,11 @@ use crate::ShutdownKind; use crate::disk_image::AgentImage; use crate::disk_image::SECTOR_SIZE; use crate::openhcl_diag::OpenHclDiagHandler; +use crate::test::PetriPostTestHook; +use crate::vtl2_settings::ControllerType; +use crate::vtl2_settings::Vtl2LunBuilder; +use crate::vtl2_settings::Vtl2StorageBackingDeviceBuilder; +use crate::vtl2_settings::Vtl2StorageControllerBuilder; use async_trait::async_trait; use get_resources::ged::FirmwareEvent; use guid::Guid; @@ -45,6 +50,7 @@ use std::sync::Arc; use std::time::Duration; use tempfile::TempPath; use vmgs_resources::GuestStateEncryptionPolicy; +use vtl2_settings_proto::StorageController; use vtl2_settings_proto::Vtl2Settings; /// The set of artifacts and resources needed to instantiate a @@ -113,6 +119,15 @@ pub struct PetriVmBuilder { // Defaults to expected behavior for firmware configuration. expected_boot_event: Option, override_expect_reset: bool, + + // Config that is used to modify the `PetriVmConfig` before it is passed + // to the VMM backend. + /// Agent to run in the guest + agent_image: Option, + /// Agent to run in OpenHCL + openhcl_agent_image: Option, + /// The boot device type for the VM + boot_device_type: BootDeviceType, } /// Petri VM configuration @@ -129,22 +144,33 @@ pub struct PetriVmConfig { pub memory: MemoryConfig, /// The processor tology for the VM pub proc_topology: ProcessorTopology, - /// Agent to run in the guest - pub agent_image: Option, - /// Agent to run in OpenHCL - pub openhcl_agent_image: Option, - /// Disk to use for guest crash dumps - pub guest_crash_disk: Option>, /// VM guest state pub vmgs: PetriVmgsResource, - /// The boot device type for the VM - pub boot_device_type: BootDeviceType, /// TPM configuration pub tpm: Option, - /// IDE configuration - pub ide: Option, - /// Additional VMBus storage for the VM - pub additional_vmbus_storage_controllers: HashMap, + /// IDE controllers and associated disks + pub ide_controllers: Option<[[Option; 2]; 2]>, + /// Storage controllers and associated disks + pub vmbus_storage_controllers: HashMap, +} + +/// Static properties about the VM for convenience during contruction and +/// runtime of a VMM backend +pub struct PetriVmProperties { + /// Whether this VM uses OpenHCL + pub is_openhcl: bool, + /// Whether this VM is isolated + pub is_isolated: bool, + /// Whether this VM uses the PCAT BIOS + pub is_pcat: bool, + /// Whether this VM boots with linux direct + pub is_linux_direct: bool, + /// Whether this VM is using pipette in VTL0 + pub using_vtl0_pipette: bool, + /// Whether this VM is using VPCI + pub using_vpci: bool, + /// The OS flavor of the guest in the VM + pub os_flavor: OsFlavor, } /// VM configuration that can be changed after the VM is created @@ -152,7 +178,7 @@ pub struct PetriVmRuntimeConfig { /// VTL2 settings pub vtl2_settings: Option, /// IDE controllers and associated disks - pub ide_controllers: Option<[[Drive; 2]; 2]>, + pub ide_controllers: Option<[[Option; 2]; 2]>, /// Storage controllers and associated disks pub vmbus_storage_controllers: HashMap, } @@ -200,28 +226,35 @@ pub trait PetriVmmBackend { config: PetriVmConfig, modify_vmm_config: Option Self::VmmConfig + Send>, resources: &PetriVmResources, + properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)>; } -#[cfg_attr(not(windows), expect(dead_code))] -pub(crate) const PETRI_IDE_BOOT_CONTROLLER: u32 = 0; -#[cfg_attr(not(windows), expect(dead_code))] +// IDE is only ever offered to VTL0 +pub(crate) const PETRI_IDE_BOOT_CONTROLLER_NUMBER: u32 = 0; pub(crate) const PETRI_IDE_BOOT_LUN: u8 = 0; - -pub(crate) const PETRI_VTL0_SCSI_BOOT_LUN: u8 = 0; -pub(crate) const PETRI_VTL0_SCSI_PIPETTE_LUN: u8 = 1; -pub(crate) const PETRI_VTL0_SCSI_CRASH_LUN: u8 = 2; - -#[cfg_attr(not(windows), expect(dead_code))] -pub(crate) const PETRI_VTL2_SCSI_PIPETTE_LUN: u8 = 1; - -/// The instance guid used for all of our SCSI drives. -pub(crate) const PETRI_VTL0_SCSI_CONTROLLER_ID: Guid = +pub(crate) const PETRI_IDE_BOOT_CONTROLLER: Guid = + guid::guid!("ca56751f-e643-4bef-bf54-f73678e8b7b5"); + +// SCSI luns used for both VTL0 and VTL2 +pub(crate) const PETRI_SCSI_BOOT_LUN: u32 = 0; +pub(crate) const PETRI_SCSI_PIPETTE_LUN: u32 = 1; +pub(crate) const PETRI_SCSI_CRASH_LUN: u32 = 2; +/// VTL0 SCSI controller instance guid used by Petri +pub(crate) const PETRI_SCSI_VTL0_CONTROLLER: Guid = guid::guid!("27b553e8-8b39-411b-a55f-839971a7884f"); - -#[cfg_attr(not(windows), expect(dead_code))] -pub(crate) const PETRI_VTL2_SCSI_CONTROLLER_ID: Guid = - guid::guid!("818ae0a4-4f68-4b8e-94bc-c520c049097d"); +/// VTL2 SCSI controller instance guid used by Petri +pub(crate) const PETRI_SCSI_VTL2_CONTROLLER: Guid = + guid::guid!("766e96f8-2ceb-437e-afe3-a93169e48a7c"); + +/// The namespace ID used by Petri for the boot disk +pub(crate) const PETRI_NVME_BOOT_NSID: u32 = 37; +/// VTL0 NVMe controller instance guid used by Petri +pub(crate) const PETRI_NVME_BOOT_VTL0_CONTROLLER: Guid = + guid::guid!("e23a04e2-90f5-4852-bc9d-e7ac691b756c"); +/// VTL2 NVMe controller instance guid used by Petri +pub(crate) const PETRI_NVME_BOOT_VTL2_CONTROLLER: Guid = + guid::guid!("92bc8346-718b-449a-8751-edbf3dcd27e4"); /// A constructed Petri VM pub struct PetriVm { @@ -261,56 +294,9 @@ impl PetriVmBuilder { } => BootDeviceType::None, Firmware::Uefi { .. } | Firmware::OpenhclUefi { .. } => BootDeviceType::Scsi, }; - let ide = artifacts.firmware.is_pcat().then(IdeConfig::default); - let guest_crash_disk = if matches!( - artifacts.firmware.os_flavor(), - OsFlavor::Windows | OsFlavor::Linux - ) { - let (guest_crash_disk, guest_dump_disk_hook) = T::create_guest_dump_disk()?.unzip(); - if let Some(guest_dump_disk_hook) = guest_dump_disk_hook { - let logger = params.logger.clone(); - params - .post_test_hooks - .push(crate::test::PetriPostTestHook::new( - "extract guest crash dumps".into(), - move |test_passed| { - if test_passed { - return Ok(()); - } - let mut disk = guest_dump_disk_hook()?; - let gpt = gptman::GPT::read_from(&mut disk, SECTOR_SIZE)?; - let partition = fscommon::StreamSlice::new( - &mut disk, - gpt[1].starting_lba * SECTOR_SIZE, - gpt[1].ending_lba * SECTOR_SIZE, - )?; - let fs = fatfs::FileSystem::new(partition, fatfs::FsOptions::new())?; - for entry in fs.root_dir().iter() { - let Ok(entry) = entry else { - tracing::warn!( - ?entry, - "failed to read entry in guest crash dump disk" - ); - continue; - }; - if !entry.is_file() { - tracing::warn!( - ?entry, - "skipping non-file entry in guest crash dump disk" - ); - continue; - } - logger.write_attachment(&entry.file_name(), entry.to_file())?; - } - Ok(()) - }, - )); - } - guest_crash_disk - } else { - None - }; + let ide_controllers = matches!(artifacts.firmware, Firmware::Pcat { .. }) + .then(|| [[None, None], [None, None]]); Ok(Self { backend: artifacts.backend, @@ -319,16 +305,13 @@ impl PetriVmBuilder { arch: artifacts.arch, host_log_levels: None, firmware: artifacts.firmware, - boot_device_type, memory: Default::default(), proc_topology: Default::default(), - agent_image: artifacts.agent_image, - openhcl_agent_image: artifacts.openhcl_agent_image, + vmgs: PetriVmgsResource::Ephemeral, tpm: None, - guest_crash_disk, - ide, - additional_vmbus_storage_controllers: HashMap::new(), + ide_controllers, + vmbus_storage_controllers: HashMap::new(), }, modify_vmm_config: None, resources: PetriVmResources { @@ -340,11 +323,251 @@ impl PetriVmBuilder { vmm_quirks, expected_boot_event, override_expect_reset: false, - }) + + agent_image: artifacts.agent_image, + openhcl_agent_image: artifacts.openhcl_agent_image, + boot_device_type, + } + .add_petri_scsi_controllers() + .add_guest_crash_disk(params.post_test_hooks)) + } + + fn add_petri_scsi_controllers(self) -> Self { + let builder = self.add_vmbus_storage_controller( + &PETRI_SCSI_VTL0_CONTROLLER, + Vtl::Vtl0, + VmbusStorageType::Scsi, + ); + + if builder.is_openhcl() { + builder.add_vmbus_storage_controller( + &PETRI_SCSI_VTL2_CONTROLLER, + Vtl::Vtl2, + VmbusStorageType::Scsi, + ) + } else { + builder + } + } + + fn add_guest_crash_disk(self, post_test_hooks: &mut Vec) -> Self { + let logger = self.resources.log_source.clone(); + let (disk, disk_hook) = matches!( + self.config.firmware.os_flavor(), + OsFlavor::Windows | OsFlavor::Linux + ) + .then(|| T::create_guest_dump_disk().expect("failed to create guest dump disk")) + .flatten() + .unzip(); + + if let Some(disk_hook) = disk_hook { + post_test_hooks.push(PetriPostTestHook::new( + "extract guest crash dumps".into(), + move |test_passed| { + if test_passed { + return Ok(()); + } + let mut disk = disk_hook()?; + let gpt = gptman::GPT::read_from(&mut disk, SECTOR_SIZE)?; + let partition = fscommon::StreamSlice::new( + &mut disk, + gpt[1].starting_lba * SECTOR_SIZE, + gpt[1].ending_lba * SECTOR_SIZE, + )?; + let fs = fatfs::FileSystem::new(partition, fatfs::FsOptions::new())?; + for entry in fs.root_dir().iter() { + let Ok(entry) = entry else { + tracing::warn!(?entry, "failed to read entry in guest crash dump disk"); + continue; + }; + if !entry.is_file() { + tracing::warn!( + ?entry, + "skipping non-file entry in guest crash dump disk" + ); + continue; + } + logger.write_attachment(&entry.file_name(), entry.to_file())?; + } + Ok(()) + }, + )); + } + + if let Some(disk) = disk { + self.add_vmbus_drive( + Drive::new(Some(Disk::Temporary(disk)), false), + &PETRI_SCSI_VTL0_CONTROLLER, + Some(PETRI_SCSI_CRASH_LUN), + ) + } else { + self + } + } + + fn add_agent_disks(self) -> Self { + self.add_agent_disk_inner(Vtl::Vtl0) + .add_agent_disk_inner(Vtl::Vtl2) + } + + fn add_agent_disk_inner(self, target_vtl: Vtl) -> Self { + let (agent_image, controller_id) = match target_vtl { + Vtl::Vtl0 => (self.agent_image.as_ref(), PETRI_SCSI_VTL0_CONTROLLER), + Vtl::Vtl2 => ( + self.openhcl_agent_image.as_ref(), + PETRI_SCSI_VTL2_CONTROLLER, + ), + }; + + if let Some(agent_disk) = agent_image.and_then(|i| { + i.build(crate::disk_image::ImageType::Vhd) + .expect("failed to build agent image") + }) { + self.add_vmbus_drive( + Drive::new( + Some(Disk::Temporary(Arc::new(agent_disk.into_temp_path()))), + false, + ), + &controller_id, + Some(PETRI_SCSI_PIPETTE_LUN), + ) + } else { + self + } + } + + fn add_boot_disk(mut self) -> Self { + if self.boot_device_type.requires_vtl2() && !self.is_openhcl() { + panic!("boot device type {:?} requires vtl2", self.boot_device_type); + } + + if self.boot_device_type.requires_vpci_boot() { + self.config + .firmware + .uefi_config_mut() + .expect("vpci boot requires uefi") + .enable_vpci_boot = true; + } + + if let Some(boot_drive) = self.config.firmware.boot_drive() { + match self.boot_device_type { + BootDeviceType::None => unreachable!(), + BootDeviceType::Ide => self.add_ide_drive( + boot_drive, + PETRI_IDE_BOOT_CONTROLLER_NUMBER, + PETRI_IDE_BOOT_LUN, + ), + BootDeviceType::IdeViaScsi => self + .add_vmbus_drive( + boot_drive, + &PETRI_SCSI_VTL2_CONTROLLER, + Some(PETRI_SCSI_BOOT_LUN), + ) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Ide) + .with_instance_id(PETRI_IDE_BOOT_CONTROLLER) + .add_lun( + Vtl2LunBuilder::disk() + .with_channel(PETRI_IDE_BOOT_CONTROLLER_NUMBER) + .with_location(PETRI_IDE_BOOT_LUN as u32) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Scsi, + PETRI_SCSI_VTL2_CONTROLLER, + PETRI_SCSI_BOOT_LUN, + )), + ) + .build(), + ), + BootDeviceType::IdeViaNvme => todo!(), + BootDeviceType::Scsi => self.add_vmbus_drive( + boot_drive, + &PETRI_SCSI_VTL0_CONTROLLER, + Some(PETRI_SCSI_BOOT_LUN), + ), + BootDeviceType::ScsiViaScsi => self + .add_vmbus_drive( + boot_drive, + &PETRI_SCSI_VTL2_CONTROLLER, + Some(PETRI_SCSI_BOOT_LUN), + ) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(PETRI_SCSI_VTL0_CONTROLLER) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(PETRI_SCSI_BOOT_LUN) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Scsi, + PETRI_SCSI_VTL2_CONTROLLER, + PETRI_SCSI_BOOT_LUN, + )), + ) + .build(), + ), + BootDeviceType::ScsiViaNvme => self + .add_vmbus_storage_controller( + &PETRI_NVME_BOOT_VTL2_CONTROLLER, + Vtl::Vtl2, + VmbusStorageType::Nvme, + ) + .add_vmbus_drive( + boot_drive, + &PETRI_NVME_BOOT_VTL2_CONTROLLER, + Some(PETRI_NVME_BOOT_NSID), + ) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(PETRI_SCSI_VTL0_CONTROLLER) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(PETRI_SCSI_BOOT_LUN) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + PETRI_NVME_BOOT_VTL2_CONTROLLER, + PETRI_NVME_BOOT_NSID, + )), + ) + .build(), + ), + BootDeviceType::Nvme => self + .add_vmbus_storage_controller( + &PETRI_NVME_BOOT_VTL0_CONTROLLER, + Vtl::Vtl2, + VmbusStorageType::Nvme, + ) + .add_vmbus_drive( + boot_drive, + &PETRI_NVME_BOOT_VTL0_CONTROLLER, + Some(PETRI_NVME_BOOT_NSID), + ), + BootDeviceType::NvmeViaScsi => todo!(), + BootDeviceType::NvmeViaNvme => todo!(), + } + } else { + self + } + } + + /// Get properties about the vm for convenience + pub fn properties(&self) -> PetriVmProperties { + PetriVmProperties { + is_openhcl: self.config.firmware.is_openhcl(), + is_isolated: self.config.firmware.isolation().is_some(), + is_pcat: self.config.firmware.is_pcat(), + is_linux_direct: self.config.firmware.is_linux_direct(), + using_vtl0_pipette: self.using_vtl0_pipette(), + using_vpci: self.boot_device_type.requires_vpci_boot(), + os_flavor: self.config.firmware.os_flavor(), + } + } + + /// Whether this VM is using pipette in VTL0 + pub fn using_vtl0_pipette(&self) -> bool { + self.agent_image + .as_ref() + .is_some_and(|x| x.contains_pipette()) } -} -impl PetriVmBuilder { /// Build and run the VM, then wait for the VM to emit the expected boot /// event (if configured). Does not configure and start pipette. Should /// only be used for testing platforms that pipette does not support. @@ -355,21 +578,30 @@ impl PetriVmBuilder { /// Build and run the VM, then wait for the VM to emit the expected boot /// event (if configured). Launches pipette and returns a client to it. pub async fn run(self) -> anyhow::Result<(PetriVm, PipetteClient)> { - assert!(self.config.agent_image.is_some()); - assert!(self.config.agent_image.as_ref().unwrap().contains_pipette()); + assert!(self.using_vtl0_pipette()); let mut vm = self.run_core().await?; let client = vm.wait_for_agent().await?; Ok((vm, client)) } - async fn run_core(self) -> anyhow::Result> { + async fn run_core(mut self) -> anyhow::Result> { + // Add the boot disk now to allow the test to modify the boot type + // Add the agent disks now to allow the test to add custom files + self = self.add_boot_disk().add_agent_disks(); + let arch = self.config.arch; let expect_reset = self.expect_reset(); + let properties = self.properties(); let (mut runtime, config) = self .backend - .run(self.config, self.modify_vmm_config, &self.resources) + .run( + self.config, + self.modify_vmm_config, + &self.resources, + properties, + ) .await?; let openhcl_diag_handler = runtime.openhcl_diag(); let watchdog_tasks = Self::start_watchdog_tasks(&self.resources, &mut runtime)?; @@ -684,8 +916,7 @@ impl PetriVmBuilder { /// Adds a file to the VM's pipette agent image. pub fn with_agent_file(mut self, name: &str, artifact: ResolvedArtifact) -> Self { - self.config - .agent_image + self.agent_image .as_mut() .expect("no guest pipette") .add_file(name, artifact); @@ -694,8 +925,7 @@ impl PetriVmBuilder { /// Adds a file to the paravisor's pipette agent image. pub fn with_openhcl_agent_file(mut self, name: &str, artifact: ResolvedArtifact) -> Self { - self.config - .openhcl_agent_image + self.openhcl_agent_image .as_mut() .expect("no openhcl pipette") .add_file(name, artifact); @@ -805,7 +1035,7 @@ impl PetriVmBuilder { /// /// This overrides the default, which is determined by the firmware type. pub fn with_boot_device_type(mut self, boot: BootDeviceType) -> Self { - self.config.boot_device_type = boot; + self.boot_device_type = boot; self } @@ -836,18 +1066,25 @@ impl PetriVmBuilder { mut self, f: impl FnOnce(&mut Vtl2Settings) + 'static + Send + Sync, ) -> Self { - let openhcl_config = self + f(self .config .firmware - .openhcl_config_mut() - .expect("Custom VTL 2 settings are only supported with OpenHCL"); - if openhcl_config.modify_vtl2_settings.is_some() { - panic!("only one with_custom_vtl2_settings allowed"); - } - openhcl_config.modify_vtl2_settings = Some(ModifyFn(Box::new(f))); + .vtl2_settings() + .expect("Custom VTL 2 settings are only supported with OpenHCL")); self } + /// Add a storage controller to VTL2 + pub fn add_vtl2_storage_controller(self, controller: StorageController) -> Self { + self.with_custom_vtl2_settings(move |v| { + v.dynamic + .as_mut() + .unwrap() + .storage_controllers + .push(controller) + }) + } + /// Add an additional SCSI controller to the VM. pub fn add_vmbus_storage_controller( mut self, @@ -855,15 +1092,11 @@ impl PetriVmBuilder { target_vtl: Vtl, controller_type: VmbusStorageType, ) -> Self { - if self - .config - .additional_vmbus_storage_controllers - .contains_key(id) - { + if self.config.vmbus_storage_controllers.contains_key(id) { panic!("storage controller {id} already exists"); } - self.config.additional_vmbus_storage_controllers.insert( + self.config.vmbus_storage_controllers.insert( *id, VmbusStorageController::new(target_vtl, controller_type), ); @@ -875,11 +1108,11 @@ impl PetriVmBuilder { mut self, drive: Drive, controller_id: &Guid, - controller_location: Option, + controller_location: Option, ) -> Self { let controller = self .config - .additional_vmbus_storage_controllers + .vmbus_storage_controllers .get_mut(controller_id) .unwrap_or_else(|| panic!("storage controller {controller_id} does not exist")); @@ -888,6 +1121,22 @@ impl PetriVmBuilder { self } + /// Add a VMBus disk drive to the VM + pub fn add_ide_drive( + mut self, + drive: Drive, + controller_number: u32, + controller_location: u8, + ) -> Self { + self.config + .ide_controllers + .as_mut() + .expect("Host IDE requires PCAT with no HCL")[controller_number as usize] + [controller_location as usize] = Some(drive); + + self + } + /// Get VM's guest OS flavor pub fn os_flavor(&self) -> OsFlavor { self.config.firmware.os_flavor() @@ -1253,7 +1502,7 @@ impl PetriVm { &mut self, drive: Drive, controller_id: &Guid, - controller_location: Option, + controller_location: Option, ) -> anyhow::Result<()> { let controller = self .config @@ -1340,7 +1589,7 @@ pub trait PetriVmRuntime: Send + Sync + 'static { &mut self, disk: &Drive, controller_id: &Guid, - controller_location: u8, + controller_location: u32, ) -> anyhow::Result<()>; } @@ -1440,7 +1689,7 @@ pub struct MemoryConfig { impl Default for MemoryConfig { fn default() -> Self { Self { - startup_bytes: 0x1_0000_0000, + startup_bytes: 0x4000_0000, dynamic_memory_range: None, } } @@ -1457,6 +1706,8 @@ pub struct UefiConfig { pub disable_frontpage: bool, /// Always attempt a default boot pub default_boot_always_attempt: bool, + /// Enable vPCI boot (for NVMe) + pub enable_vpci_boot: bool, } impl Default for UefiConfig { @@ -1466,6 +1717,7 @@ impl Default for UefiConfig { secure_boot_template: None, disable_frontpage: true, default_boot_always_attempt: false, + enable_vpci_boot: false, } } } @@ -1511,8 +1763,8 @@ pub struct OpenHclConfig { /// How to place VTL2 in address space. If `None`, the backend VMM /// will decide on default behavior. pub vtl2_base_address_type: Option, - /// Optional manual modification of the VTL2 settings - pub modify_vtl2_settings: Option>, + /// VTL2 settings + pub vtl2_settings: Option, } impl OpenHclConfig { @@ -1566,7 +1818,7 @@ impl Default for OpenHclConfig { custom_command_line: None, log_levels: OpenvmmLogConfig::TestDefault, vtl2_base_address_type: None, - modify_vtl2_settings: None, + vtl2_settings: None, } } } @@ -1587,6 +1839,8 @@ impl Default for TpmConfig { } /// Firmware to load into the test VM. +// TODO: remove the guests from the firmware enum so that we don't pass them +// to the VMM backend after we have already used them generically. #[derive(Debug)] pub enum Firmware { /// Boot Linux directly, without any firmware. @@ -1656,10 +1910,46 @@ pub enum BootDeviceType { None, /// Boot from IDE. Ide, + /// Boot from IDE via SCSI to VTL2. + IdeViaScsi, + /// Boot from IDE via NVME to VTL2. + IdeViaNvme, /// Boot from SCSI. Scsi, - /// Boot from an NVMe controller. + /// Boot from SCSI via SCSI to VTL2. + ScsiViaScsi, + /// Boot from SCSI via NVME to VTL2. + ScsiViaNvme, + /// Boot from NVMe. Nvme, + /// Boot from NVMe via SCSI to VTL2. + NvmeViaScsi, + /// Boot from NVMe via NVMe to VTL2. + NvmeViaNvme, +} + +impl BootDeviceType { + fn requires_vtl2(&self) -> bool { + match self { + BootDeviceType::None + | BootDeviceType::Ide + | BootDeviceType::Scsi + | BootDeviceType::Nvme => false, + BootDeviceType::IdeViaScsi + | BootDeviceType::IdeViaNvme + | BootDeviceType::ScsiViaScsi + | BootDeviceType::ScsiViaNvme + | BootDeviceType::NvmeViaScsi + | BootDeviceType::NvmeViaNvme => true, + } + } + + fn requires_vpci_boot(&self) -> bool { + matches!( + self, + BootDeviceType::Nvme | BootDeviceType::NvmeViaScsi | BootDeviceType::NvmeViaNvme + ) + } } impl Firmware { @@ -1720,7 +2010,6 @@ impl Firmware { arch: MachineArch, guest: UefiGuest, isolation: Option, - vtl2_nvme_boot: bool, ) -> Self { use petri_artifacts_vmm_test::artifacts::openhcl_igvm::*; let igvm_path = match arch { @@ -1733,10 +2022,7 @@ impl Firmware { isolation, igvm_path, uefi_config: Default::default(), - openhcl_config: OpenHclConfig { - vtl2_nvme_boot, - ..Default::default() - }, + openhcl_config: Default::default(), } } @@ -1920,6 +2206,29 @@ impl Firmware { | Firmware::OpenhclPcat { .. } => None, } } + + fn boot_drive(&self) -> Option { + match self { + Firmware::LinuxDirect { .. } | Firmware::OpenhclLinuxDirect { .. } => None, + Firmware::Pcat { guest, .. } | Firmware::OpenhclPcat { guest, .. } => { + Some((guest.artifact().to_owned(), guest.is_dvd())) + } + Firmware::Uefi { guest, .. } | Firmware::OpenhclUefi { guest, .. } => { + guest.artifact().map(|a| (a.to_owned(), false)) + } + } + .map(|(artifact, is_dvd)| { + Drive::new( + Some(Disk::Differencing(artifact.get().to_path_buf())), + is_dvd, + ) + }) + } + + fn vtl2_settings(&mut self) -> Option<&mut Vtl2Settings> { + self.openhcl_config_mut() + .map(|c| c.vtl2_settings.get_or_insert_with(default_vtl2_settings)) + } } /// The guest the VM will boot into. A boot drive with the chosen setup @@ -1940,7 +2249,6 @@ impl PcatGuest { } } - #[cfg_attr(not(windows), expect(dead_code))] fn is_dvd(&self) -> bool { matches!(self, Self::Iso(_)) } @@ -2073,14 +2381,16 @@ pub struct OpenHclServicingFlags { } /// Petri disk -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum Disk { /// Memory backed with specified size Memory(u64), - /// Memory differencing disk backed by a file + /// Memory differencing disk backed by a VHD Differencing(PathBuf), - /// Persistent disk + /// Persistent VHD Persistent(PathBuf), + /// Disk backed by a temporary VHD + Temporary(Arc), } /// Petri VMGS disk @@ -2279,7 +2589,7 @@ pub enum VmbusStorageType { } /// VM disk drive -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct Drive { /// Backing disk pub disk: Option, @@ -2294,39 +2604,6 @@ impl Drive { } } -/// IDE configuration -/// -/// Controller 0, location 0 is reserved for the boot disk. -#[derive(Debug, Clone, PartialEq)] -pub struct IdeConfig { - /// Controller 0, location 1 - pub drive_0_1: Option, - /// Controller 1, location 0 - pub drive_1_0: Option, - /// Controller 1, location 1 - pub drive_1_1: Option, -} - -impl Default for IdeConfig { - fn default() -> Self { - Self { - drive_0_1: None, - drive_1_0: Some(Drive { - disk: None, - is_dvd: true, - }), - drive_1_1: None, - } - } -} - -impl IdeConfig { - /// Get the config with the boot disk - pub fn into_controllers(self) -> [[Option; 2]; 2] { - [[None, self.drive_0_1], [self.drive_1_0, self.drive_1_1]] - } -} - /// VMBus storage controller #[derive(Debug, Clone)] pub struct VmbusStorageController { @@ -2335,7 +2612,7 @@ pub struct VmbusStorageController { /// The storage device type pub controller_type: VmbusStorageType, /// Drives (with any inserted disks) attached to this storage controller - pub drives: HashMap, + pub drives: HashMap, } impl VmbusStorageController { @@ -2349,7 +2626,12 @@ impl VmbusStorageController { } /// Add a disk to the storage controller - pub fn set_drive(&mut self, lun: Option, drive: Drive, allow_modify_existing: bool) -> u8 { + pub fn set_drive( + &mut self, + lun: Option, + drive: Drive, + allow_modify_existing: bool, + ) -> u32 { let lun = if let Some(lun) = lun { if !allow_modify_existing && self.drives.contains_key(&lun) { panic!("a disk with lun {lun} already exists on this controller"); @@ -2358,7 +2640,7 @@ impl VmbusStorageController { } else { // find the first available lun let mut lun = None; - for x in 0..u8::MAX { + for x in 0..u8::MAX as u32 { if !self.drives.contains_key(&x) { lun = Some(x) } diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 6cacfe2999..ecf0e7fec6 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -4,18 +4,13 @@ //! Contains [`PetriVmConfigOpenVmm::new`], which builds a [`PetriVmConfigOpenVmm`] with all //! default settings for a given [`Firmware`] and [`MachineArch`]. -use super::BOOT_NVME_LUN; -use super::BOOT_NVME_NSID; -use super::PARAVISOR_BOOT_NVME_INSTANCE; use super::PetriVmConfigOpenVmm; use super::PetriVmResourcesOpenVmm; -use super::memdiff_disk; -use crate::BootDeviceType; +use crate::Drive; use crate::Firmware; use crate::IsolationType; use crate::MemoryConfig; use crate::OpenHclConfig; -use crate::PcatGuest; use crate::PetriLogSource; use crate::PetriVmConfig; use crate::PetriVmResources; @@ -25,18 +20,14 @@ use crate::SIZE_1_GB; use crate::SecureBootTemplate; use crate::TpmConfig; use crate::UefiConfig; -use crate::UefiGuest; +use crate::VmbusStorageType; use crate::linux_direct_serial_agent::LinuxDirectSerialAgent; -use crate::openvmm::BOOT_NVME_INSTANCE; + use crate::openvmm::memdiff_vmgs; -use crate::vm::PETRI_VTL0_SCSI_CONTROLLER_ID; +use crate::openvmm::petri_disk_to_openvmm; +use crate::vm::PetriVmProperties; use crate::vm::append_cmdline; -use crate::vtl2_settings::ControllerType; -use crate::vtl2_settings::Vtl2LunBuilder; -use crate::vtl2_settings::Vtl2StorageBackingDeviceBuilder; -use crate::vtl2_settings::Vtl2StorageControllerBuilder; use anyhow::Context; -use disk_backend_resources::FileDiskHandle; use framebuffer::FRAMEBUFFER_SIZE; use framebuffer::Framebuffer; use framebuffer::FramebufferAccess; @@ -68,7 +59,6 @@ use openvmm_defs::config::VmbusConfig; use openvmm_defs::config::VpciDeviceConfig; use openvmm_defs::config::Vtl2BaseAddressType; use openvmm_defs::config::Vtl2Config; -use openvmm_helpers::disk::open_disk_type; use openvmm_pcat_locator::RomFileLocation; use pal_async::DefaultDriver; use pal_async::socket::PolledSocket; @@ -97,7 +87,6 @@ use vm_manifest_builder::VmChipsetResult; use vm_manifest_builder::VmManifestBuilder; use vm_resource::IntoResource; use vm_resource::Resource; -use vm_resource::kind::DiskHandleKind; use vm_resource::kind::SerialBackendHandle; use vm_resource::kind::VmbusDeviceHandleKind; use vmbus_serial_resources::VmbusSerialDeviceHandle; @@ -106,7 +95,6 @@ use vmcore::non_volatile_store::resources::EphemeralNonVolatileStoreHandle; use vmgs_resources::GuestStateEncryptionPolicy; use vmgs_resources::VmgsFileHandle; use vmotherboard::ChipsetDeviceHandle; -use vtl2_settings_proto::Vtl2Settings; impl PetriVmConfigOpenVmm { /// Create a new VM configuration. @@ -114,6 +102,7 @@ impl PetriVmConfigOpenVmm { openvmm_path: &ResolvedArtifact, petri_vm_config: PetriVmConfig, resources: &PetriVmResources, + properties: PetriVmProperties, ) -> anyhow::Result { let PetriVmConfig { name: _, @@ -122,23 +111,13 @@ impl PetriVmConfigOpenVmm { firmware, memory, proc_topology, - agent_image, - openhcl_agent_image, vmgs, - boot_device_type, tpm: tpm_config, - guest_crash_disk, - ide, - additional_vmbus_storage_controllers, + ide_controllers, + vmbus_storage_controllers, } = petri_vm_config; - if !additional_vmbus_storage_controllers.is_empty() { - unimplemented!("openvmm additional storage controllers"); - } - - if ide.is_some_and(|c| c != crate::IdeConfig::default()) { - unimplemented!("custom ide config"); - } + tracing::debug!(?firmware, ?arch, "Petri VM firmware configuration"); let PetriVmResources { driver, log_source } = resources; @@ -150,7 +129,6 @@ impl PetriVmConfigOpenVmm { driver, logger: log_source, vmgs: &vmgs, - boot_device_type, tpm_config: tpm_config.as_ref(), mesh: &mesh, openvmm_path, @@ -195,6 +173,112 @@ impl PetriVmConfigOpenVmm { let mut vpci_devices = Vec::new(); let mut vmbus_devices = Vec::new(); + // Add IDE storage + if let Some(ide_controllers) = &ide_controllers { + for (controller_number, controller) in ide_controllers.iter().enumerate() { + for (controller_location, drive) in controller.iter().enumerate() { + if let Some(drive) = drive { + if let Some(disk) = &drive.disk { + let disk = petri_disk_to_openvmm(disk)?; + let guest_media = if drive.is_dvd { + GuestMedia::Dvd( + SimpleScsiDvdHandle { + media: Some(disk), + requests: None, + } + .into_resource(), + ) + } else { + GuestMedia::Disk { + disk_type: disk, + read_only: false, + disk_parameters: None, + } + }; + + ide_disks.push(IdeDeviceConfig { + path: ide_resources::IdePath { + channel: controller_number as u8, + drive: controller_location as u8, + }, + guest_media, + }); + } + } + } + } + } + + // Add VMBus storage + for (instance_id, controller) in &vmbus_storage_controllers { + match controller.controller_type { + VmbusStorageType::Scsi => { + let mut devices = Vec::new(); + for (lun, Drive { disk, is_dvd }) in &controller.drives { + if !*is_dvd && let Some(disk) = disk { + devices.push(ScsiDeviceAndPath { + path: ScsiPath { + path: 0, + target: 0, + lun: (*lun).try_into().expect("invalid scsi lun"), + }, + device: SimpleScsiDiskHandle { + disk: petri_disk_to_openvmm(disk)?, + read_only: false, + parameters: Default::default(), + } + .into_resource(), + }); + } else { + todo!("dvd ({}) or empty ({})", *is_dvd, disk.is_none()) + } + } + + vmbus_devices.push(( + match controller.target_vtl { + crate::Vtl::Vtl0 => DeviceVtl::Vtl0, + crate::Vtl::Vtl2 => DeviceVtl::Vtl2, + }, + ScsiControllerHandle { + instance_id: *instance_id, + max_sub_channel_count: 1, + io_queue_depth: None, + devices, + requests: None, + poll_mode_queue_depth: None, + } + .into_resource(), + )); + } + VmbusStorageType::Nvme => { + let mut namespaces = Vec::new(); + for (nsid, Drive { disk, is_dvd }) in &controller.drives { + if !*is_dvd && let Some(disk) = disk { + namespaces.push(NamespaceDefinition { + nsid: *nsid, + read_only: false, + disk: petri_disk_to_openvmm(disk)?, + }); + } else { + todo!("dvd ({}) or empty ({})", *is_dvd, disk.is_none()) + } + } + + vpci_devices.push(VpciDeviceConfig { + vtl: DeviceVtl::Vtl0, + instance_id: *instance_id, + resource: NvmeControllerHandle { + subsystem_id: *instance_id, + max_io_queues: 64, + msix_count: 64, + namespaces, + } + .into_resource(), + }); + } + } + } + let (firmware_event_send, firmware_event_recv) = mesh::mpsc_channel(); let make_vsock_listener = || -> anyhow::Result<(UnixListener, TempPath)> { @@ -203,89 +287,47 @@ impl PetriVmConfigOpenVmm { .into_parts()) }; - let (with_vtl2, vtl2_vmbus, ged, ged_send, mut vtl2_settings, vtl2_vsock_path) = - if firmware.is_openhcl() { - let (ged, ged_send) = setup.config_openhcl_vmbus_devices( - &mut emulated_serial_config, - &mut vmbus_devices, - &firmware_event_send, - framebuffer.is_some(), - )?; - - let late_map_vtl0_memory = match load_mode { - LoadMode::Igvm { - vtl2_base_address: Vtl2BaseAddressType::Vtl2Allocate { .. }, - .. - } => { - // Late Map VTL0 memory not supported when test supplies Vtl2Allocate - None - } - _ => Some(LateMapVtl0MemoryPolicy::InjectException), - }; + let (with_vtl2, vtl2_vmbus, ged, ged_send, vtl2_vsock_path) = if firmware.is_openhcl() { + let (ged, ged_send) = setup.config_openhcl_vmbus_devices( + &mut emulated_serial_config, + &mut vmbus_devices, + &firmware_event_send, + framebuffer.is_some(), + )?; - let (vtl2_vsock_listener, vtl2_vsock_path) = make_vsock_listener()?; - ( - Some(Vtl2Config { - vtl0_alias_map: false, // TODO: enable when OpenVMM supports it for DMA - late_map_vtl0_memory, - }), - Some(VmbusConfig { - vsock_listener: Some(vtl2_vsock_listener), - vsock_path: Some(vtl2_vsock_path.to_string_lossy().into_owned()), - vmbus_max_version: None, - vtl2_redirect: false, - #[cfg(windows)] - vmbusproxy_handle: None, - }), - Some(ged), - Some(ged_send), - Some(crate::vm::default_vtl2_settings()), - Some(vtl2_vsock_path), - ) - } else { - (None, None, None, None, None, None) + let late_map_vtl0_memory = match load_mode { + LoadMode::Igvm { + vtl2_base_address: Vtl2BaseAddressType::Vtl2Allocate { .. }, + .. + } => { + // Late Map VTL0 memory not supported when test supplies Vtl2Allocate + None + } + _ => Some(LateMapVtl0MemoryPolicy::InjectException), }; - let mut petri_vtl0_scsi = ScsiControllerHandle { - instance_id: PETRI_VTL0_SCSI_CONTROLLER_ID, - max_sub_channel_count: 1, - io_queue_depth: None, - devices: vec![], - requests: None, - poll_mode_queue_depth: None, + let (vtl2_vsock_listener, vtl2_vsock_path) = make_vsock_listener()?; + ( + Some(Vtl2Config { + vtl0_alias_map: false, // TODO: enable when OpenVMM supports it for DMA + late_map_vtl0_memory, + }), + Some(VmbusConfig { + vsock_listener: Some(vtl2_vsock_listener), + vsock_path: Some(vtl2_vsock_path.to_string_lossy().into_owned()), + vmbus_max_version: None, + vtl2_redirect: false, + #[cfg(windows)] + vmbusproxy_handle: None, + }), + Some(ged), + Some(ged_send), + Some(vtl2_vsock_path), + ) + } else { + (None, None, None, None, None) }; - let boot_disk = setup.load_boot_disk(vtl2_settings.as_mut())?; - match boot_disk { - Some(BootDisk::Ide(c)) => { - ide_disks.push(c); - } - Some(BootDisk::Vpci(c)) => { - vpci_devices.push(c); - } - Some(BootDisk::Scsi(d)) => { - petri_vtl0_scsi.devices.push(d); - } - None => {} - } - - if let Some(guest_crash_disk) = guest_crash_disk.as_ref() { - petri_vtl0_scsi.devices.push(ScsiDeviceAndPath { - path: ScsiPath { - path: 0, - target: 0, - lun: crate::vm::PETRI_VTL0_SCSI_CRASH_LUN, - }, - device: SimpleScsiDiskHandle { - read_only: false, - parameters: Default::default(), - disk: FileDiskHandle(File::open(guest_crash_disk.as_ref())?.into()) - .into_resource(), - } - .into_resource(), - }); - } - // Configure the serial ports now that they have been updated by the // OpenHCL configuration. chipset = chipset.with_serial(emulated_serial_config); @@ -536,12 +578,17 @@ impl PetriVmConfigOpenVmm { None }; + let vtl2_settings = firmware.into_openhcl_config().and_then(|c| c.vtl2_settings); + Ok(Self { - firmware, + runtime_config: crate::PetriVmRuntimeConfig { + vtl2_settings, + ide_controllers, + vmbus_storage_controllers, + }, arch, host_log_levels, config, - boot_device_type, mesh, resources: PetriVmResourcesOpenVmm { @@ -555,20 +602,16 @@ impl PetriVmConfigOpenVmm { linux_direct_serial_agent, driver: driver.clone(), output_dir: log_source.output_dir().to_owned(), - agent_image, - openhcl_agent_image, openvmm_path: openvmm_path.clone(), vtl2_vsock_path, _vmbus_vsock_path: vmbus_vsock_path, + properties, }, openvmm_log_file: log_source.log_file("openvmm")?, - petri_vtl0_scsi, ged, framebuffer_view, - - vtl2_settings, }) } } @@ -579,7 +622,6 @@ struct PetriVmConfigSetupCore<'a> { driver: &'a DefaultDriver, logger: &'a PetriLogSource, vmgs: &'a PetriVmgsResource, - boot_device_type: BootDeviceType, tpm_config: Option<&'a TpmConfig>, mesh: &'a Mesh, openvmm_path: &'a ResolvedArtifact, @@ -591,12 +633,6 @@ struct SerialData { linux_direct_serial_agent: Option, } -enum BootDisk { - Scsi(ScsiDeviceAndPath), - Vpci(VpciDeviceConfig), - Ide(IdeDeviceConfig), -} - enum VideoDevice { Vga(RomFileLocation), Synth(DeviceVtl, Resource), @@ -725,6 +761,7 @@ impl PetriVmConfigSetupCore<'_> { secure_boot_template: _, // new disable_frontpage, default_boot_always_attempt, + enable_vpci_boot, }, }, ) => { @@ -739,7 +776,7 @@ impl PetriVmConfigSetupCore<'_> { enable_tpm: self.tpm_config.is_some(), enable_battery: false, enable_serial: true, - enable_vpci_boot: matches!(self.boot_device_type, BootDeviceType::Nvme), + enable_vpci_boot: *enable_vpci_boot, uefi_console_mode: Some(openvmm_defs::config::UefiConsoleMode::Com1), default_boot_always_attempt: *default_boot_always_attempt, bios_guid: Guid::new_random(), @@ -765,7 +802,7 @@ impl PetriVmConfigSetupCore<'_> { custom_command_line: _, log_levels: _, vtl2_base_address_type, - modify_vtl2_settings: _, // run_core + vtl2_settings: _, // run_core } = openhcl_config; let mut cmdline = Some(openhcl_config.command_line()); @@ -842,164 +879,6 @@ impl PetriVmConfigSetupCore<'_> { }) } - fn load_boot_disk( - &self, - vtl2_settings: Option<&mut Vtl2Settings>, - ) -> anyhow::Result> { - let emulate_storage_in_openhcl = matches!( - self.firmware, - Firmware::OpenhclUefi { - openhcl_config: OpenHclConfig { - vtl2_nvme_boot: true, - .. - }, - .. - } - ); - enum Media { - Disk(Resource), - Dvd(Resource), - } - let media = match &self.firmware { - Firmware::LinuxDirect { .. } - | Firmware::OpenhclLinuxDirect { .. } - | Firmware::Uefi { - guest: UefiGuest::None, - .. - } - | Firmware::OpenhclUefi { - guest: UefiGuest::None, - .. - } => return Ok(None), - Firmware::Pcat { guest, .. } | Firmware::OpenhclPcat { guest, .. } => { - let disk_path = guest.artifact(); - match guest { - PcatGuest::Vhd(_) => Media::Disk(memdiff_disk(disk_path.as_ref())?), - PcatGuest::Iso(_) => Media::Dvd(open_disk_type(disk_path.as_ref(), true)?), - } - } - Firmware::Uefi { guest, .. } | Firmware::OpenhclUefi { guest, .. } => { - let disk_path = guest.artifact(); - Media::Disk(memdiff_disk( - disk_path.expect("not uefi guest none").as_ref(), - )?) - } - }; - - if emulate_storage_in_openhcl { - match self.boot_device_type { - BootDeviceType::None => {} - BootDeviceType::Ide => todo!("support IDE emulation testing"), - BootDeviceType::Nvme => todo!("support NVMe emulation testing"), - BootDeviceType::Scsi => vtl2_settings - .expect("openhcl config should have vtl2settings") - .dynamic - .as_mut() - .unwrap() - .storage_controllers - .push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(PARAVISOR_BOOT_NVME_INSTANCE) - .add_lun( - Vtl2LunBuilder::disk() - .with_location(BOOT_NVME_LUN) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - PARAVISOR_BOOT_NVME_INSTANCE, - BOOT_NVME_NSID, - )), - ) - .build(), - ), - } - match media { - Media::Dvd(_) => todo!("support DVD to VTL2"), - Media::Disk(disk) => Ok(Some(BootDisk::Vpci(VpciDeviceConfig { - vtl: DeviceVtl::Vtl2, - instance_id: PARAVISOR_BOOT_NVME_INSTANCE, - resource: NvmeControllerHandle { - subsystem_id: PARAVISOR_BOOT_NVME_INSTANCE, - max_io_queues: 64, - msix_count: 64, - namespaces: vec![NamespaceDefinition { - nsid: BOOT_NVME_NSID, - disk, - read_only: false, - }], - } - .into_resource(), - }))), - } - } else { - match self.boot_device_type { - BootDeviceType::None => Ok(None), - BootDeviceType::Ide => { - let guest_media = match media { - Media::Disk(disk_type) => GuestMedia::Disk { - read_only: false, - disk_parameters: None, - disk_type, - }, - Media::Dvd(media) => GuestMedia::Dvd( - SimpleScsiDvdHandle { - media: Some(media), - requests: None, - } - .into_resource(), - ), - }; - Ok(Some(BootDisk::Ide(IdeDeviceConfig { - path: ide_resources::IdePath { - channel: 0, - drive: 0, - }, - guest_media, - }))) - } - BootDeviceType::Scsi => { - let disk = match media { - Media::Disk(disk) => disk, - Media::Dvd(_) => todo!("support SCSI DVD boot disks"), - }; - Ok(Some(BootDisk::Scsi(ScsiDeviceAndPath { - path: ScsiPath { - path: 0, - target: 0, - lun: 0, - }, - device: SimpleScsiDiskHandle { - read_only: false, - parameters: Default::default(), - disk, - } - .into_resource(), - }))) - } - BootDeviceType::Nvme => { - let disk = match media { - Media::Disk(disk) => disk, - Media::Dvd(_) => anyhow::bail!("dvd not supported on nvme"), - }; - Ok(Some(BootDisk::Vpci(VpciDeviceConfig { - vtl: DeviceVtl::Vtl0, - instance_id: BOOT_NVME_INSTANCE, - resource: NvmeControllerHandle { - subsystem_id: BOOT_NVME_INSTANCE, - max_io_queues: 64, - msix_count: 64, - namespaces: vec![NamespaceDefinition { - nsid: BOOT_NVME_NSID, - disk, - read_only: false, - }], - } - .into_resource(), - }))) - } - } - } - } - fn config_openhcl_vmbus_devices( &self, serial: &mut [Option>], @@ -1040,6 +919,7 @@ impl PetriVmConfigSetupCore<'_> { secure_boot_template, disable_frontpage, default_boot_always_attempt, + enable_vpci_boot, }, OpenHclConfig { vmbus_redirect, .. }, ) = match self.firmware { @@ -1064,7 +944,7 @@ impl PetriVmConfigSetupCore<'_> { firmware: get_resources::ged::GuestFirmwareConfig::Uefi { firmware_debug: false, disable_frontpage: *disable_frontpage, - enable_vpci_boot: matches!(self.boot_device_type, BootDeviceType::Nvme), + enable_vpci_boot: *enable_vpci_boot, console_mode: get_resources::ged::UefiConsoleMode::COM1, default_boot_always_attempt: *default_boot_always_attempt, }, diff --git a/petri/src/vm/openvmm/mod.rs b/petri/src/vm/openvmm/mod.rs index ad37fcc380..b7cf7c8c5c 100644 --- a/petri/src/vm/openvmm/mod.rs +++ b/petri/src/vm/openvmm/mod.rs @@ -17,7 +17,6 @@ pub use runtime::OpenVmmFramebufferAccess; pub use runtime::OpenVmmInspector; pub use runtime::PetriVmOpenVmm; -use crate::BootDeviceType; use crate::Disk; use crate::Firmware; use crate::OpenHclServicingFlags; @@ -30,8 +29,8 @@ use crate::PetriVmgsDisk; use crate::PetriVmgsResource; use crate::PetriVmmBackend; use crate::VmmQuirks; -use crate::disk_image::AgentImage; use crate::linux_direct_serial_agent::LinuxDirectSerialAgent; +use crate::vm::PetriVmProperties; use anyhow::Context; use async_trait::async_trait; use disk_backend_resources::LayeredDiskHandle; @@ -50,14 +49,12 @@ use pal_async::socket::PolledSocket; use pal_async::task::Task; use petri_artifacts_common::tags::GuestQuirksInner; use petri_artifacts_common::tags::MachineArch; -use petri_artifacts_common::tags::OsFlavor; use petri_artifacts_core::ArtifactResolver; use petri_artifacts_core::ResolvedArtifact; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; -use storvsp_resources::ScsiControllerHandle; use tempfile::TempPath; use unix_socket::UnixListener; use vm_resource::IntoResource; @@ -65,25 +62,10 @@ use vm_resource::Resource; use vm_resource::kind::DiskHandleKind; use vmgs_resources::VmgsDisk; use vmgs_resources::VmgsResource; -use vtl2_settings_proto::Vtl2Settings; - -/// The instance guid for the NVMe controller automatically added for boot media -/// for paravisor storage translation. -pub(crate) const PARAVISOR_BOOT_NVME_INSTANCE: Guid = - guid::guid!("92bc8346-718b-449a-8751-edbf3dcd27e4"); - -/// The instance guid for the NVMe controller automatically added for boot media. -pub(crate) const BOOT_NVME_INSTANCE: Guid = guid::guid!("e23a04e2-90f5-4852-bc9d-e7ac691b756c"); /// The instance guid for the MANA nic automatically added when specifying `PetriVmConfigOpenVmm::with_nic` const MANA_INSTANCE: Guid = guid::guid!("f9641cf4-d915-4743-a7d8-efa75db7b85a"); -/// The namespace ID for the NVMe controller automatically added for boot media. -pub(crate) const BOOT_NVME_NSID: u32 = 37; - -/// The LUN ID for the NVMe controller automatically added for boot media. -pub(crate) const BOOT_NVME_LUN: u32 = 1; - /// The MAC address used by the NIC assigned with [`PetriVmConfigOpenVmm::with_nic`]. pub const NIC_MAC_ADDRESS: MacAddress = MacAddress::new([0x00, 0x15, 0x5D, 0x12, 0x12, 0x12]); @@ -144,8 +126,10 @@ impl PetriVmmBackend for OpenVmmPetriBackend { config: PetriVmConfig, modify_vmm_config: Option PetriVmConfigOpenVmm + Send>, resources: &PetriVmResources, + properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { - let mut config = PetriVmConfigOpenVmm::new(&self.openvmm_path, config, resources).await?; + let mut config = + PetriVmConfigOpenVmm::new(&self.openvmm_path, config, resources, properties).await?; if let Some(f) = modify_vmm_config { config = f(config); @@ -158,11 +142,10 @@ impl PetriVmmBackend for OpenVmmPetriBackend { /// Configuration state for a test VM. pub struct PetriVmConfigOpenVmm { // Direct configuration related information. - firmware: Firmware, + runtime_config: PetriVmRuntimeConfig, arch: MachineArch, host_log_levels: Option, config: Config, - boot_device_type: BootDeviceType, // Mesh host mesh: mesh_process::Mesh, @@ -174,13 +157,8 @@ pub struct PetriVmConfigOpenVmm { openvmm_log_file: PetriLogFile, // Resources that are only used during startup. - /// Single VMBus SCSI controller shared for all VTL0 disks added by petri. - petri_vtl0_scsi: ScsiControllerHandle, - ged: Option, framebuffer_view: Option, - - vtl2_settings: Option, } /// Various channels and resources used to interact with the VM while it is running. struct PetriVmResourcesOpenVmm { @@ -195,21 +173,15 @@ struct PetriVmResourcesOpenVmm { // Externally injected management stuff also needed at runtime. driver: DefaultDriver, - agent_image: Option, - openhcl_agent_image: Option, openvmm_path: ResolvedArtifact, output_dir: PathBuf, // TempPaths that cannot be dropped until the end. vtl2_vsock_path: Option, _vmbus_vsock_path: TempPath, -} -impl PetriVmConfigOpenVmm { - /// Get the OS that the VM will boot into. - pub fn os_flavor(&self) -> OsFlavor { - self.firmware.os_flavor() - } + // properties needed at runtime + properties: PetriVmProperties, } fn memdiff_disk(path: &Path) -> anyhow::Result> { @@ -227,14 +199,7 @@ fn memdiff_disk(path: &Path) -> anyhow::Result> { fn memdiff_vmgs(vmgs: &PetriVmgsResource) -> anyhow::Result { let convert_disk = |disk: &PetriVmgsDisk| -> anyhow::Result { Ok(VmgsDisk { - disk: match &disk.disk { - Disk::Memory(len) => { - LayeredDiskHandle::single_layer(RamDiskLayerHandle { len: Some(*len) }) - .into_resource() - } - Disk::Differencing(path) => memdiff_disk(path)?, - Disk::Persistent(path) => open_disk_type(path, false)?, - }, + disk: petri_disk_to_openvmm(&disk.disk)?, encryption_policy: disk.encryption_policy, }) }; @@ -248,3 +213,14 @@ fn memdiff_vmgs(vmgs: &PetriVmgsResource) -> anyhow::Result { PetriVmgsResource::Ephemeral => VmgsResource::Ephemeral, }) } + +fn petri_disk_to_openvmm(disk: &Disk) -> anyhow::Result> { + Ok(match disk { + Disk::Memory(len) => { + LayeredDiskHandle::single_layer(RamDiskLayerHandle { len: Some(*len) }).into_resource() + } + Disk::Differencing(path) => memdiff_disk(path)?, + Disk::Persistent(path) => open_disk_type(path.as_ref(), false)?, + Disk::Temporary(path) => open_disk_type(path.as_ref(), false)?, + }) +} diff --git a/petri/src/vm/openvmm/modify.rs b/petri/src/vm/openvmm/modify.rs index f09a6553f2..ba71e6507e 100644 --- a/petri/src/vm/openvmm/modify.rs +++ b/petri/src/vm/openvmm/modify.rs @@ -38,7 +38,7 @@ impl PetriVmConfigOpenVmm { /// Enable the battery for the VM. pub fn with_battery(mut self) -> Self { - if self.firmware.is_openhcl() { + if self.resources.properties.is_openhcl { self.ged.as_mut().unwrap().enable_battery = true; } else { self.config.chipset_devices.push(ChipsetDeviceHandle { @@ -61,7 +61,7 @@ impl PetriVmConfigOpenVmm { /// Set test config for the GED's IGVM attest request handler pub fn with_igvm_attest_test_config(mut self, config: IgvmAttestTestConfig) -> Self { - if !self.firmware.is_openhcl() { + if !self.resources.properties.is_openhcl { panic!("IGVM Attest test config is only supported for OpenHCL.") }; @@ -78,7 +78,7 @@ impl PetriVmConfigOpenVmm { pub fn with_nic(mut self) -> Self { let endpoint = net_backend_resources::consomme::ConsommeHandle { cidr: None }.into_resource(); - if self.vtl2_settings.is_some() { + if let Some(vtl2_settings) = self.runtime_config.vtl2_settings.as_mut() { self.config.vpci_devices.push(VpciDeviceConfig { vtl: DeviceVtl::Vtl2, instance_id: MANA_INSTANCE, @@ -91,18 +91,13 @@ impl PetriVmConfigOpenVmm { .into_resource(), }); - self.vtl2_settings - .as_mut() - .unwrap() - .dynamic - .as_mut() - .unwrap() - .nic_devices - .push(vtl2_settings_proto::NicDeviceLegacy { + vtl2_settings.dynamic.as_mut().unwrap().nic_devices.push( + vtl2_settings_proto::NicDeviceLegacy { instance_id: MANA_INSTANCE.to_string(), subordinate_instance_id: None, max_sub_channels: None, - }); + }, + ); } else { const NETVSP_INSTANCE: guid::Guid = guid::guid!("c6c46cc3-9302-4344-b206-aef65e5bd0a2"); self.config.vmbus_devices.push(( diff --git a/petri/src/vm/openvmm/runtime.rs b/petri/src/vm/openvmm/runtime.rs index 2cfe9bb04b..8d9377d66a 100644 --- a/petri/src/vm/openvmm/runtime.rs +++ b/petri/src/vm/openvmm/runtime.rs @@ -173,7 +173,7 @@ impl PetriVmRuntime for PetriVmOpenVmm { &mut self, _disk: &crate::Drive, _controller_id: &guid::Guid, - _controller_location: u8, + _controller_location: u32, ) -> anyhow::Result<()> { todo!("openvmm set vmbus drive") } @@ -469,12 +469,7 @@ impl PetriVmInner { if let Some(agent) = self.resources.linux_direct_serial_agent.as_mut() { agent.reset(); - if self - .resources - .agent_image - .as_ref() - .is_some_and(|x| x.contains_pipette()) - { + if self.resources.properties.using_vtl0_pipette { self.launch_linux_direct_pipette().await?; } } diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index 704ad4a041..624b768cd4 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -6,15 +6,11 @@ use super::PetriVmConfigOpenVmm; use super::PetriVmOpenVmm; use super::PetriVmResourcesOpenVmm; -use crate::BootDeviceType; -use crate::Firmware; use crate::OpenvmmLogConfig; use crate::PetriLogFile; use crate::PetriVmRuntimeConfig; use crate::worker::Worker; use anyhow::Context; -use disk_backend_resources::FileDiskHandle; -use guid::Guid; use mesh_process::Mesh; use mesh_process::ProcessConfig; use mesh_worker::WorkerHost; @@ -23,25 +19,19 @@ use pal_async::pipe::PolledPipe; use pal_async::task::Spawn; use petri_artifacts_common::tags::MachineArch; use petri_artifacts_common::tags::OsFlavor; -use scsidisk_resources::SimpleScsiDiskHandle; use std::collections::BTreeMap; -use std::collections::HashMap; use std::ffi::OsString; use std::io::Write; use std::sync::Arc; -use storvsp_resources::ScsiControllerHandle; -use storvsp_resources::ScsiDeviceAndPath; -use storvsp_resources::ScsiPath; use vm_resource::IntoResource; impl PetriVmConfigOpenVmm { async fn run_core(self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { let Self { - firmware, + runtime_config, arch, host_log_levels, mut config, - boot_device_type, mesh, @@ -49,15 +39,10 @@ impl PetriVmConfigOpenVmm { openvmm_log_file, - petri_vtl0_scsi, ged, framebuffer_view, - - mut vtl2_settings, } = self; - tracing::debug!(?firmware, ?arch, "Petri VM firmware configuration"); - let has_pcie = !config.pcie_root_complexes.is_empty(); // TODO: OpenHCL needs virt_whp support @@ -65,71 +50,16 @@ impl PetriVmConfigOpenVmm { // TODO: arm64 is broken? // TODO: VPCI and NVMe don't support save/restore // TODO: PCIe emulators don't support save/restore yet - let supports_save_restore = !firmware.is_openhcl() - && !matches!(firmware, Firmware::Pcat { .. }) + let supports_save_restore = !resources.properties.is_openhcl + && !resources.properties.is_pcat && !matches!(arch, MachineArch::Aarch64) - && !matches!(boot_device_type, BootDeviceType::Nvme) + && !resources.properties.using_vpci && !has_pcie; - if firmware.is_openhcl() { - // Add a pipette disk for VTL 2 - const UH_CIDATA_SCSI_INSTANCE: Guid = - guid::guid!("766e96f8-2ceb-437e-afe3-a93169e48a7c"); - - if let Some(openhcl_agent_disk) = resources - .openhcl_agent_image - .as_ref() - .unwrap() - .build() - .context("failed to build agent image")? - { - config.vmbus_devices.push(( - DeviceVtl::Vtl2, - ScsiControllerHandle { - instance_id: UH_CIDATA_SCSI_INSTANCE, - max_sub_channel_count: 1, - io_queue_depth: None, - devices: vec![ScsiDeviceAndPath { - path: ScsiPath { - path: 0, - target: 0, - lun: crate::vm::PETRI_VTL0_SCSI_BOOT_LUN, - }, - device: SimpleScsiDiskHandle { - read_only: true, - parameters: Default::default(), - disk: FileDiskHandle(openhcl_agent_disk.into_file()) - .into_resource(), - } - .into_resource(), - }], - requests: None, - poll_mode_queue_depth: None, - } - .into_resource(), - )); - } - } - - // Add the Petri SCSI controller to VTL0 now that all the disks are on it. - if !petri_vtl0_scsi.devices.is_empty() { - config - .vmbus_devices - .push((DeviceVtl::Vtl0, petri_vtl0_scsi.into_resource())); - } - - // Apply custom VTL2 settings - if let Some(f) = firmware - .into_openhcl_config() - .and_then(|c| c.modify_vtl2_settings) - { - f.0(vtl2_settings.as_mut().unwrap()) - }; - // Add the GED and VTL 2 settings. if let Some(mut ged) = ged { ged.vtl2_settings = Some(prost::Message::encode_to_vec( - vtl2_settings.as_ref().unwrap(), + runtime_config.vtl2_settings.as_ref().unwrap(), )); config .vmbus_devices @@ -179,39 +109,15 @@ impl PetriVmConfigOpenVmm { } tracing::info!("VM ready"); - Ok(( - vm, - PetriVmRuntimeConfig { - vtl2_settings, - ide_controllers: None, - vmbus_storage_controllers: HashMap::new(), - }, - )) + Ok((vm, runtime_config)) } /// Run the VM, configuring pipette to automatically start if it is /// included in the config pub async fn run(mut self) -> anyhow::Result<(PetriVmOpenVmm, PetriVmRuntimeConfig)> { - let launch_linux_direct_pipette = if let Some(agent_image) = &self.resources.agent_image { - // Construct the agent disk. - if let Some(agent_disk) = agent_image.build().context("failed to build agent image")? { - self.petri_vtl0_scsi.devices.push(ScsiDeviceAndPath { - path: ScsiPath { - path: 0, - target: 0, - lun: crate::vm::PETRI_VTL0_SCSI_PIPETTE_LUN, - }, - device: SimpleScsiDiskHandle { - read_only: true, - parameters: Default::default(), - disk: FileDiskHandle(agent_disk.into_file()).into_resource(), - } - .into_resource(), - }); - } - - if matches!(self.firmware.os_flavor(), OsFlavor::Windows) - && self.firmware.isolation().is_none() + let launch_linux_direct_pipette = if self.resources.properties.using_vtl0_pipette { + if matches!(self.resources.properties.os_flavor, OsFlavor::Windows) + && !self.resources.properties.is_isolated { // Make a file for the IMC hive. It's not guaranteed to be at a fixed // location at runtime. @@ -231,7 +137,7 @@ impl PetriVmConfigOpenVmm { )); } - self.firmware.is_linux_direct() && agent_image.contains_pipette() + self.resources.properties.is_linux_direct } else { false }; diff --git a/petri/src/vm/vtl2_settings.rs b/petri/src/vm/vtl2_settings.rs index 5ca08ecd4c..e6268385de 100644 --- a/petri/src/vm/vtl2_settings.rs +++ b/petri/src/vm/vtl2_settings.rs @@ -94,6 +94,7 @@ pub fn build_vtl2_storage_backing_physical_devices( /// these requirements.) #[derive(Debug, PartialEq, Eq)] pub struct Vtl2LunBuilder { + channel: Option, location: u32, device_id: Guid, vendor_id: String, @@ -111,6 +112,7 @@ impl Vtl2LunBuilder { /// opposed to NOT a DVD. pub fn disk() -> Self { Self { + channel: None, location: 0, device_id: Guid::new_random(), vendor_id: "OpenVMM".to_string(), @@ -133,6 +135,12 @@ impl Vtl2LunBuilder { s } + /// Guest visible IDE controller number + pub fn with_channel(mut self, channel: u32) -> Self { + self.channel = Some(channel); + self + } + /// Guest visible location of the device (aka a guest "LUN") pub fn with_location(mut self, location: u32) -> Self { self.location = location; @@ -166,6 +174,7 @@ impl Vtl2LunBuilder { /// Builds the LUN into the protobuf type used by VTL2 settings. pub fn build(self) -> vtl2_settings_proto::Lun { vtl2_settings_proto::Lun { + channel: self.channel, location: self.location, device_id: self.device_id.to_string(), vendor_id: self.vendor_id, diff --git a/vmm_tests/vmm_test_macros/src/lib.rs b/vmm_tests/vmm_test_macros/src/lib.rs index b3531ff1ec..60a55a4406 100644 --- a/vmm_tests/vmm_test_macros/src/lib.rs +++ b/vmm_tests/vmm_test_macros/src/lib.rs @@ -45,7 +45,6 @@ enum Firmware { #[derive(Default)] struct OpenhclUefiOptions { - nvme: bool, isolation: Option, } @@ -206,12 +205,12 @@ impl ToTokens for FirmwareAndArch { Firmware::OpenhclLinuxDirect => { quote!(::petri::Firmware::openhcl_linux_direct(resolver, #arch)) } - Firmware::OpenhclUefi(OpenhclUefiOptions { nvme, isolation }, guest) => { + Firmware::OpenhclUefi(OpenhclUefiOptions { isolation }, guest) => { let isolation = match isolation { Some(i) => quote!(Some(#i)), None => quote!(None), }; - quote!(::petri::Firmware::openhcl_uefi(resolver, #arch, #guest, #isolation, #nvme)) + quote!(::petri::Firmware::openhcl_uefi(resolver, #arch, #guest, #isolation)) } }) } @@ -457,13 +456,6 @@ impl OpenhclUefiOptions { IsolationType::Tdx => "tdx", }); } - if self.nvme { - if !prefix.is_empty() { - prefix.push('_'); - } - prefix.push_str("nvme"); - } - if prefix.is_empty() { None } else { @@ -479,9 +471,6 @@ impl Parse for OpenhclUefiOptions { let words = input.parse_terminated(|stream| stream.parse::(), Token![,])?; for word in words { match &*word.to_string() { - "nvme" => { - options.nvme = true; - } "vbs" => { if options.isolation.is_some() { return Err(Error::new(word.span(), "isolation type already specified")); @@ -758,7 +747,6 @@ fn build_requirements(firmware: &Firmware, name: &str, resolved_vmm: Vmm) -> Opt if let Firmware::OpenhclUefi( OpenhclUefiOptions { isolation: Some(isolation), - .. }, _, ) = firmware diff --git a/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs b/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs index b01e1e60af..66003733c6 100644 --- a/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs +++ b/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs @@ -156,14 +156,16 @@ async fn servicing_keepalive_no_device( /// Test servicing an OpenHCL VM from the current version to itself /// with NVMe keepalive support. -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64))[LATEST_STANDARD_X64])] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64))[LATEST_STANDARD_X64])] async fn servicing_keepalive_with_device( config: PetriVmBuilder, (igvm_file,): (ResolvedArtifact,), ) -> anyhow::Result<()> { let flags = config.default_servicing_flags(); openhcl_servicing_core( - config.with_vmbus_redirect(true), // Need this to attach the NVMe device + config + .with_boot_device_type(petri::BootDeviceType::ScsiViaNvme) + .with_vmbus_redirect(true), // Need this to attach the NVMe device "OPENHCL_ENABLE_VTL2_GPA_POOL=512", igvm_file, flags, diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs index 9e1c5b22c9..7df5a18cde 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs @@ -59,6 +59,7 @@ async fn nvme_relay_test_core( ] .into(), )) + .with_boot_device_type(petri::BootDeviceType::ScsiViaNvme) .with_openhcl_command_line(openhcl_cmdline) .with_vmbus_redirect(true) .with_processor_topology(processor_topology.unwrap_or(ProcessorTopology { @@ -176,7 +177,7 @@ async fn nvme_relay_test_core( /// Test an OpenHCL uefi VM with a NVME disk assigned to VTL2 that boots /// linux, with vmbus relay. This should expose a disk to VTL0 via vmbus. -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay(config: PetriVmBuilder) -> Result<(), anyhow::Error> { nvme_relay_test_core(config, NvmeRelayTestParams::default()).await } @@ -185,7 +186,7 @@ async fn nvme_relay(config: PetriVmBuilder) -> Result<(), a /// linux, with vmbus relay. This should expose a disk to VTL0 via vmbus. /// /// Use the private pool override to test the private pool dma path. -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay_explicit_private_pool( config: PetriVmBuilder, ) -> Result<(), anyhow::Error> { @@ -211,7 +212,7 @@ async fn nvme_relay_explicit_private_pool( /// There _should_ be enough private pool memory for the NVMe driver to /// allocate all of its buffers contiguously. #[cfg(debug_assertions)] -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay_heuristic_debug_16vp_768mb_heavy( config: PetriVmBuilder, ) -> Result<(), anyhow::Error> { @@ -242,7 +243,7 @@ async fn nvme_relay_heuristic_debug_16vp_768mb_heavy( /// There _should_ be enough private pool memory for the NVMe driver to /// allocate all of its buffers contiguously. #[cfg(not(debug_assertions))] -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay_heuristic_release_16vp_256mb_heavy( config: PetriVmBuilder, ) -> Result<(), anyhow::Error> { @@ -276,7 +277,7 @@ async fn nvme_relay_heuristic_release_16vp_256mb_heavy( /// This test uses 500MB of private pool memory, which does *not* match any /// of the heuristics exactly, but there should still be private pool memory. #[cfg(not(debug_assertions))] -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay_heuristic_release_32vp_500mb_very_heavy( config: PetriVmBuilder, ) -> Result<(), anyhow::Error> { @@ -306,7 +307,7 @@ async fn nvme_relay_heuristic_release_32vp_500mb_very_heavy( /// /// There _should_ be enough private pool memory for the NVMe driver to /// allocate all of its buffers contiguously. -#[openvmm_test(openhcl_uefi_x64[nvme](vhd(ubuntu_2504_server_x64)))] +#[openvmm_test(openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)))] async fn nvme_relay_32vp_768mb_very_heavy( config: PetriVmBuilder, ) -> Result<(), anyhow::Error> { 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 3de1a3fd26..018beb41ca 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -384,7 +384,7 @@ async fn storvsp_hyperv(config: PetriVmBuilder) -> Result<() .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( ControllerType::Scsi, vtl2_vsid, - vtl2_lun.into(), + vtl2_lun, )) .build(), ); From e7124097d8ebafceb53a93993728745db4277ff7 Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 12:25:37 -0800 Subject: [PATCH 04/10] feedback and cleanup --- petri/src/vm/hyperv/mod.rs | 30 ++-- petri/src/vm/mod.rs | 94 +++++++---- petri/src/vm/openvmm/construct.rs | 13 +- .../tests/multiarch/openhcl_servicing.rs | 30 ++-- .../tests/x86_64/openhcl_linux_direct.rs | 40 +++-- .../vmm_tests/tests/tests/x86_64/storage.rs | 152 ++++++++---------- 6 files changed, 183 insertions(+), 176 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 9d687445c2..69abe48f50 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -174,7 +174,6 @@ impl PetriVmmBackend for HyperVPetriBackend { proc_topology, vmgs, tpm, - ide_controllers, vmbus_storage_controllers, } = config; @@ -187,7 +186,8 @@ impl PetriVmmBackend for HyperVPetriBackend { let temp_dir = tempfile::tempdir()?; - let (guest_state_isolation_type, generation, uefi_config, openhcl_config) = match firmware { + let (guest_state_isolation_type, generation, uefi_config, openhcl_config) = match &firmware + { Firmware::LinuxDirect { .. } | Firmware::OpenhclLinuxDirect { .. } => { todo!("linux direct not supported on hyper-v") } @@ -195,6 +195,7 @@ impl PetriVmmBackend for HyperVPetriBackend { guest: _, bios_firmware: _, // TODO svga_firmware: _, // TODO + ide_controllers: _, } => ( powershell::HyperVGuestStateIsolationType::Disabled, powershell::HyperVGeneration::One, @@ -348,7 +349,7 @@ impl PetriVmmBackend for HyperVPetriBackend { }) = uefi_config { vm.set_secure_boot( - secure_boot_enabled, + *secure_boot_enabled, secure_boot_template.map(|t| match t { SecureBootTemplate::MicrosoftWindows => { HyperVSecureBootTemplate::MicrosoftWindows @@ -361,7 +362,7 @@ impl PetriVmmBackend for HyperVPetriBackend { .await?; // TODO: Disable frontpage for non-OpenHCL Hyper-V VMs - if disable_frontpage && properties.is_openhcl { + if *disable_frontpage && properties.is_openhcl { append_cmdline( &mut openhcl_command_line, "OPENHCL_DISABLE_UEFI_FRONTPAGE=1", @@ -373,12 +374,12 @@ impl PetriVmmBackend for HyperVPetriBackend { &mut openhcl_command_line, format!( "HCL_DEFAULT_BOOT_ALWAYS_ATTEMPT={}", - if default_boot_always_attempt { 1 } else { 0 } + if *default_boot_always_attempt { 1 } else { 0 } ), ); }; - if enable_vpci_boot { + if *enable_vpci_boot { todo!("hyperv nvme boot"); } } @@ -401,7 +402,7 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.set_imc(&imc_hive).await?; } - let vtl2_settings = if let Some(( + if let Some(( src_igvm_file, OpenHclConfig { vtl2_nvme_boot: _, // TODO, see #1649. @@ -440,7 +441,7 @@ impl PetriVmmBackend for HyperVPetriBackend { vm.set_vm_firmware_command_line(openhcl_command_line.as_ref().unwrap()) .await?; - vm.set_vmbus_redirect(vmbus_redirect).await?; + vm.set_vmbus_redirect(*vmbus_redirect).await?; // Attempt to enable COM3 and use that to get KMSG logs, otherwise // fall back to use diag_client. @@ -497,10 +498,7 @@ impl PetriVmmBackend for HyperVPetriBackend { if let Some(settings) = &vtl2_settings { vm.set_base_vtl2_settings(settings).await?; } - vtl2_settings - } else { - None - }; + } let serial_pipe_path = vm.set_vm_com_port(1).await?; let serial_log_file = log_source.log_file("guest")?; @@ -510,7 +508,7 @@ impl PetriVmmBackend for HyperVPetriBackend { )); // Add IDE storage - if let Some(ide_controllers) = &ide_controllers { + if let Some(ide_controllers) = firmware.ide_controllers() { for (controller_number, controller) in ide_controllers.iter().enumerate() { for (controller_location, disk) in controller.iter().enumerate() { if let Some(disk) = disk { @@ -590,11 +588,7 @@ impl PetriVmmBackend for HyperVPetriBackend { driver: driver.clone(), properties, }, - PetriVmRuntimeConfig { - vtl2_settings, - ide_controllers, - vmbus_storage_controllers, - }, + firmware.into_runtime_config(vmbus_storage_controllers), )) } } diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 179953698c..bfacb0fa2d 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -148,8 +148,6 @@ pub struct PetriVmConfig { pub vmgs: PetriVmgsResource, /// TPM configuration pub tpm: Option, - /// IDE controllers and associated disks - pub ide_controllers: Option<[[Option; 2]; 2]>, /// Storage controllers and associated disks pub vmbus_storage_controllers: HashMap, } @@ -295,9 +293,6 @@ impl PetriVmBuilder { Firmware::Uefi { .. } | Firmware::OpenhclUefi { .. } => BootDeviceType::Scsi, }; - let ide_controllers = matches!(artifacts.firmware, Firmware::Pcat { .. }) - .then(|| [[None, None], [None, None]]); - Ok(Self { backend: artifacts.backend, config: PetriVmConfig { @@ -310,7 +305,6 @@ impl PetriVmBuilder { vmgs: PetriVmgsResource::Ephemeral, tpm: None, - ide_controllers, vmbus_storage_controllers: HashMap::new(), }, modify_vmm_config: None, @@ -413,6 +407,7 @@ impl PetriVmBuilder { fn add_agent_disk_inner(self, target_vtl: Vtl) -> Self { let (agent_image, controller_id) = match target_vtl { Vtl::Vtl0 => (self.agent_image.as_ref(), PETRI_SCSI_VTL0_CONTROLLER), + Vtl::Vtl1 => panic!("no VTL1 agent disk"), Vtl::Vtl2 => ( self.openhcl_agent_image.as_ref(), PETRI_SCSI_VTL2_CONTROLLER, @@ -1092,14 +1087,17 @@ impl PetriVmBuilder { target_vtl: Vtl, controller_type: VmbusStorageType, ) -> Self { - if self.config.vmbus_storage_controllers.contains_key(id) { - panic!("storage controller {id} already exists"); + if self + .config + .vmbus_storage_controllers + .insert( + *id, + VmbusStorageController::new(target_vtl, controller_type), + ) + .is_some() + { + panic!("storage controller {id} already existed"); } - - self.config.vmbus_storage_controllers.insert( - *id, - VmbusStorageController::new(target_vtl, controller_type), - ); self } @@ -1129,8 +1127,8 @@ impl PetriVmBuilder { controller_location: u8, ) -> Self { self.config - .ide_controllers - .as_mut() + .firmware + .ide_controllers_mut() .expect("Host IDE requires PCAT with no HCL")[controller_number as usize] [controller_location as usize] = Some(drive); @@ -1865,6 +1863,8 @@ pub enum Firmware { bios_firmware: ResolvedOptionalArtifact, /// The SVGA firmware to use. svga_firmware: ResolvedOptionalArtifact, + /// IDE controllers and associated disks + ide_controllers: [[Option; 2]; 2], }, /// Boot a PCAT-based VM with OpenHCL in VTL2. OpenhclPcat { @@ -1987,6 +1987,7 @@ impl Firmware { guest, bios_firmware: resolver.try_require(PCAT_FIRMWARE_X64).erase(), svga_firmware: resolver.try_require(SVGA_FIRMWARE_X64).erase(), + ide_controllers: [[None, None], [None, None]], } } @@ -2174,12 +2175,30 @@ impl Firmware { } } - fn into_openhcl_config(self) -> Option { + fn into_runtime_config( + self, + vmbus_storage_controllers: HashMap, + ) -> PetriVmRuntimeConfig { match self { Firmware::OpenhclLinuxDirect { openhcl_config, .. } | Firmware::OpenhclUefi { openhcl_config, .. } - | Firmware::OpenhclPcat { openhcl_config, .. } => Some(openhcl_config), - Firmware::LinuxDirect { .. } | Firmware::Pcat { .. } | Firmware::Uefi { .. } => None, + | Firmware::OpenhclPcat { openhcl_config, .. } => PetriVmRuntimeConfig { + vtl2_settings: openhcl_config.vtl2_settings, + ide_controllers: None, + vmbus_storage_controllers, + }, + Firmware::Pcat { + ide_controllers, .. + } => PetriVmRuntimeConfig { + vtl2_settings: None, + ide_controllers: Some(ide_controllers), + vmbus_storage_controllers, + }, + Firmware::LinuxDirect { .. } | Firmware::Uefi { .. } => PetriVmRuntimeConfig { + vtl2_settings: None, + ide_controllers: None, + vmbus_storage_controllers, + }, } } @@ -2229,6 +2248,24 @@ impl Firmware { self.openhcl_config_mut() .map(|c| c.vtl2_settings.get_or_insert_with(default_vtl2_settings)) } + + fn ide_controllers(&self) -> Option<&[[Option; 2]; 2]> { + match self { + Firmware::Pcat { + ide_controllers, .. + } => Some(ide_controllers), + _ => None, + } + } + + fn ide_controllers_mut(&mut self) -> Option<&mut [[Option; 2]; 2]> { + match self { + Firmware::Pcat { + ide_controllers, .. + } => Some(ide_controllers), + _ => None, + } + } } /// The guest the VM will boot into. A boot drive with the chosen setup @@ -2575,6 +2612,8 @@ fn default_vtl2_settings() -> Vtl2Settings { pub enum Vtl { /// VTL 0 Vtl0 = 0, + /// VTL 1 + Vtl1 = 1, /// VTL 2 Vtl2 = 2, } @@ -2632,12 +2671,7 @@ impl VmbusStorageController { drive: Drive, allow_modify_existing: bool, ) -> u32 { - let lun = if let Some(lun) = lun { - if !allow_modify_existing && self.drives.contains_key(&lun) { - panic!("a disk with lun {lun} already exists on this controller"); - } - lun - } else { + let lun = lun.unwrap_or_else(|| { // find the first available lun let mut lun = None; for x in 0..u8::MAX as u32 { @@ -2645,14 +2679,12 @@ impl VmbusStorageController { lun = Some(x) } } - if let Some(lun) = lun { - lun - } else { - panic!("all locations on this controller are in use") - } - }; + lun.expect("all locations on this controller are in use") + }); - self.drives.insert(lun, drive); + if self.drives.insert(lun, drive).is_some() && !allow_modify_existing { + panic!("a disk with lun {lun} already existed on this controller"); + } lun } diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index ecf0e7fec6..66a5d98e3f 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -113,7 +113,6 @@ impl PetriVmConfigOpenVmm { proc_topology, vmgs, tpm: tpm_config, - ide_controllers, vmbus_storage_controllers, } = petri_vm_config; @@ -174,7 +173,7 @@ impl PetriVmConfigOpenVmm { let mut vmbus_devices = Vec::new(); // Add IDE storage - if let Some(ide_controllers) = &ide_controllers { + if let Some(ide_controllers) = firmware.ide_controllers() { for (controller_number, controller) in ide_controllers.iter().enumerate() { for (controller_location, drive) in controller.iter().enumerate() { if let Some(drive) = drive { @@ -237,6 +236,7 @@ impl PetriVmConfigOpenVmm { vmbus_devices.push(( match controller.target_vtl { crate::Vtl::Vtl0 => DeviceVtl::Vtl0, + crate::Vtl::Vtl1 => DeviceVtl::Vtl1, crate::Vtl::Vtl2 => DeviceVtl::Vtl2, }, ScsiControllerHandle { @@ -578,14 +578,8 @@ impl PetriVmConfigOpenVmm { None }; - let vtl2_settings = firmware.into_openhcl_config().and_then(|c| c.vtl2_settings); - Ok(Self { - runtime_config: crate::PetriVmRuntimeConfig { - vtl2_settings, - ide_controllers, - vmbus_storage_controllers, - }, + runtime_config: firmware.into_runtime_config(vmbus_storage_controllers), arch, host_log_levels, config, @@ -741,6 +735,7 @@ impl PetriVmConfigSetupCore<'_> { bios_firmware: firmware, guest: _, // load_boot_disk svga_firmware: _, // config_video + ide_controllers: _, }, ) => { let firmware = openvmm_pcat_locator::find_pcat_bios(firmware.get()) diff --git a/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs b/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs index 66003733c6..af7830f7e2 100644 --- a/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs +++ b/vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs @@ -558,22 +558,20 @@ async fn create_keepalive_test_config( }) }) // Assign the fault controller to VTL2 - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .add_lun( - Vtl2LunBuilder::disk() - .with_location(vtl0_nvme_lun) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - NVME_INSTANCE, - KEEPALIVE_VTL2_NSID, - )), - ) - .build(), - ); - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(vtl0_nvme_lun) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + NVME_INSTANCE, + KEEPALIVE_VTL2_NSID, + )), + ) + .build(), + ) .run() .await } diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs index 854d646928..485f0a2a6a 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs @@ -132,7 +132,7 @@ async fn many_nvme_devices_servicing_very_heavy( ); }) }) - .with_custom_vtl2_settings(|v| { + .add_vtl2_storage_controller({ let device_ids = (0..NUM_NVME_DEVICES) .map(|i| { let mut g = BASE_GUID; @@ -141,26 +141,24 @@ async fn many_nvme_devices_servicing_very_heavy( }) .collect::>(); - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .add_luns( - device_ids - .iter() - .map(|(nsid, guid)| { - Vtl2LunBuilder::disk() - // Add 1 so as to avoid any confusion with booting from LUN 0 (on the implicit SCSI - // controller created by the above `config.with_vmbus_redirect` call above). - .with_location((*nsid - NSID_OFFSET) + 1) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - *guid, - *nsid, - )) - }) - .collect(), - ) - .build(), - ) + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .add_luns( + device_ids + .iter() + .map(|(nsid, guid)| { + Vtl2LunBuilder::disk() + // Add 1 so as to avoid any confusion with booting from LUN 0 (on the implicit SCSI + // controller created by the above `config.with_vmbus_redirect` call above). + .with_location((*nsid - NSID_OFFSET) + 1) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + *guid, + *nsid, + )) + }) + .collect(), + ) + .build() }) .run() .await?; 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 018beb41ca..e9bca74206 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs @@ -265,31 +265,29 @@ async fn storvsp(config: PetriVmBuilder) -> Result<(), anyh )); }) }) - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .add_lun( - Vtl2LunBuilder::disk() - .with_location(vtl0_scsi_lun) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Scsi, - scsi_instance, - vtl2_lun, - )), - ) - .add_lun( - Vtl2LunBuilder::disk() - .with_location(vtl0_nvme_lun) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - NVME_INSTANCE, - vtl2_nsid, - )), - ) - .build(), - ) - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(vtl0_scsi_lun) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Scsi, + scsi_instance, + vtl2_lun, + )), + ) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(vtl0_nvme_lun) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + NVME_INSTANCE, + vtl2_nsid, + )), + ) + .build(), + ) .run() .await?; @@ -353,13 +351,11 @@ async fn storvsp_hyperv(config: PetriVmBuilder) -> Result<() let (mut vm, agent) = config .with_vmbus_redirect(true) - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .build(), - ); - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .build(), + ) .add_vmbus_storage_controller(&vtl2_vsid, petri::Vtl::Vtl2, petri::VmbusStorageType::Scsi) .add_vmbus_drive( petri::Drive::new(Some(petri::Disk::Persistent(vhd_path.to_path_buf())), false), @@ -456,30 +452,28 @@ async fn openhcl_linux_stripe_storvsp( ]); }) }) - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .add_lun( - Vtl2LunBuilder::disk() - .with_location(vtl0_nvme_lun) - .with_chunk_size_in_kb(128) - .with_physical_devices(vec![ - Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - NVME_INSTANCE_1, - vtl2_nsid, - ), - Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - NVME_INSTANCE_2, - vtl2_nsid, - ), - ]), - ) - .build(), - ) - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .add_lun( + Vtl2LunBuilder::disk() + .with_location(vtl0_nvme_lun) + .with_chunk_size_in_kb(128) + .with_physical_devices(vec![ + Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + NVME_INSTANCE_1, + vtl2_nsid, + ), + Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + NVME_INSTANCE_2, + vtl2_nsid, + ), + ]), + ) + .build(), + ) .run() .await?; @@ -545,15 +539,13 @@ async fn openhcl_linux_storvsp_dvd( )); }) }) - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .add_lun(Vtl2LunBuilder::dvd().with_location(vtl0_scsi_lun)) - // No physical devices initially, so the drive is empty - .build(), - ) - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .add_lun(Vtl2LunBuilder::dvd().with_location(vtl0_scsi_lun)) + // No physical devices initially, so the drive is empty + .build(), + ) .run() .await?; @@ -669,22 +661,20 @@ async fn openhcl_linux_storvsp_dvd_nvme( )]); }) }) - .with_custom_vtl2_settings(move |v| { - v.dynamic.as_mut().unwrap().storage_controllers.push( - Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(scsi_instance) - .add_lun( - Vtl2LunBuilder::dvd() - .with_location(vtl2_lun) - .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( - ControllerType::Nvme, - NVME_INSTANCE, - vtl2_nsid, - )), - ) - .build(), - ); - }) + .add_vtl2_storage_controller( + Vtl2StorageControllerBuilder::new(ControllerType::Scsi) + .with_instance_id(scsi_instance) + .add_lun( + Vtl2LunBuilder::dvd() + .with_location(vtl2_lun) + .with_physical_device(Vtl2StorageBackingDeviceBuilder::new( + ControllerType::Nvme, + NVME_INSTANCE, + vtl2_nsid, + )), + ) + .build(), + ) .run() .await?; From c2d7431a3b76d0cc554d9bee7b6c3d7c556feabe Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 13:45:09 -0800 Subject: [PATCH 05/10] fix --- petri/src/vm/mod.rs | 2 +- petri/src/vm/openvmm/construct.rs | 13 +++++++------ petri/src/vm/openvmm/start.rs | 7 ++++--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index bfacb0fa2d..b22a37ff36 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -1687,7 +1687,7 @@ pub struct MemoryConfig { impl Default for MemoryConfig { fn default() -> Self { Self { - startup_bytes: 0x4000_0000, + startup_bytes: 0x1_0000_0000, dynamic_memory_range: None, } } diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 66a5d98e3f..fdaf2c5a12 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -210,6 +210,11 @@ impl PetriVmConfigOpenVmm { // Add VMBus storage for (instance_id, controller) in &vmbus_storage_controllers { + let vtl = match controller.target_vtl { + crate::Vtl::Vtl0 => DeviceVtl::Vtl0, + crate::Vtl::Vtl1 => DeviceVtl::Vtl1, + crate::Vtl::Vtl2 => DeviceVtl::Vtl2, + }; match controller.controller_type { VmbusStorageType::Scsi => { let mut devices = Vec::new(); @@ -234,11 +239,7 @@ impl PetriVmConfigOpenVmm { } vmbus_devices.push(( - match controller.target_vtl { - crate::Vtl::Vtl0 => DeviceVtl::Vtl0, - crate::Vtl::Vtl1 => DeviceVtl::Vtl1, - crate::Vtl::Vtl2 => DeviceVtl::Vtl2, - }, + vtl, ScsiControllerHandle { instance_id: *instance_id, max_sub_channel_count: 1, @@ -265,7 +266,7 @@ impl PetriVmConfigOpenVmm { } vpci_devices.push(VpciDeviceConfig { - vtl: DeviceVtl::Vtl0, + vtl, instance_id: *instance_id, resource: NvmeControllerHandle { subsystem_id: *instance_id, diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index 624b768cd4..b793f3b7e2 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -58,9 +58,10 @@ impl PetriVmConfigOpenVmm { // Add the GED and VTL 2 settings. if let Some(mut ged) = ged { - ged.vtl2_settings = Some(prost::Message::encode_to_vec( - runtime_config.vtl2_settings.as_ref().unwrap(), - )); + ged.vtl2_settings = runtime_config + .vtl2_settings + .as_ref() + .map(prost::Message::encode_to_vec); config .vmbus_devices .push((DeviceVtl::Vtl2, ged.into_resource())); From 55df42bfe77cdb4320fcb5e59678d9e518c53e36 Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 14:55:47 -0800 Subject: [PATCH 06/10] use different controller ID --- petri/src/vm/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index b22a37ff36..383b3bc8ca 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -244,6 +244,9 @@ pub(crate) const PETRI_SCSI_VTL0_CONTROLLER: Guid = /// VTL2 SCSI controller instance guid used by Petri pub(crate) const PETRI_SCSI_VTL2_CONTROLLER: Guid = guid::guid!("766e96f8-2ceb-437e-afe3-a93169e48a7c"); +/// SCSI controller instance guid offered to VTL0 by VTL2 +pub(crate) const PETRI_SCSI_VTL0_VIA_VTL2_CONTROLLER: Guid = + guid::guid!("6c474f47-ed39-49e6-bbb9-142177a1da6e"); /// The namespace ID used by Petri for the boot disk pub(crate) const PETRI_NVME_BOOT_NSID: u32 = 37; @@ -487,7 +490,7 @@ impl PetriVmBuilder { ) .add_vtl2_storage_controller( Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(PETRI_SCSI_VTL0_CONTROLLER) + .with_instance_id(PETRI_SCSI_VTL0_VIA_VTL2_CONTROLLER) .add_lun( Vtl2LunBuilder::disk() .with_location(PETRI_SCSI_BOOT_LUN) @@ -512,7 +515,7 @@ impl PetriVmBuilder { ) .add_vtl2_storage_controller( Vtl2StorageControllerBuilder::new(ControllerType::Scsi) - .with_instance_id(PETRI_SCSI_VTL0_CONTROLLER) + .with_instance_id(PETRI_SCSI_VTL0_VIA_VTL2_CONTROLLER) .add_lun( Vtl2LunBuilder::disk() .with_location(PETRI_SCSI_BOOT_LUN) From a08b5b38d27a233e25ffd7b3a8ad5af5fb816f74 Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 15:46:55 -0800 Subject: [PATCH 07/10] correct vtl --- petri/src/vm/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 383b3bc8ca..a08d4e1809 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -530,7 +530,7 @@ impl PetriVmBuilder { BootDeviceType::Nvme => self .add_vmbus_storage_controller( &PETRI_NVME_BOOT_VTL0_CONTROLLER, - Vtl::Vtl2, + Vtl::Vtl0, VmbusStorageType::Nvme, ) .add_vmbus_drive( From 7c3972403f2c91dc3a6dafae6f21500539b04fad Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 18:08:19 -0800 Subject: [PATCH 08/10] cleanup --- petri/src/vm/hyperv/mod.rs | 1 - petri/src/vm/mod.rs | 4 ---- petri/src/vm/openvmm/construct.rs | 1 - 3 files changed, 6 deletions(-) diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 69abe48f50..01215f86c0 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -405,7 +405,6 @@ impl PetriVmmBackend for HyperVPetriBackend { if let Some(( src_igvm_file, OpenHclConfig { - vtl2_nvme_boot: _, // TODO, see #1649. vmbus_redirect, custom_command_line: _, log_levels: _, diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index a08d4e1809..3d8a636577 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -1748,9 +1748,6 @@ pub enum OpenvmmLogConfig { /// OpenHCL configuration #[derive(Debug)] pub struct OpenHclConfig { - /// Emulate SCSI via NVME to VTL2, with the provided namespace ID on - /// the controller with `BOOT_NVME_INSTANCE`. - pub vtl2_nvme_boot: bool, /// Whether to enable VMBus redirection pub vmbus_redirect: bool, /// Test-specified command-line parameters to append to the petri generated @@ -1814,7 +1811,6 @@ impl OpenHclConfig { impl Default for OpenHclConfig { fn default() -> Self { Self { - vtl2_nvme_boot: false, vmbus_redirect: false, custom_command_line: None, log_levels: OpenvmmLogConfig::TestDefault, diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index fdaf2c5a12..45d70c9780 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -793,7 +793,6 @@ impl PetriVmConfigSetupCore<'_> { }, ) => { let OpenHclConfig { - vtl2_nvme_boot: _, // load_boot_disk vmbus_redirect: _, // config_openhcl_vmbus_devices custom_command_line: _, log_levels: _, From 17c0a585f668d263895bcc1a249e6c29fd33c47f Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 19:33:54 -0800 Subject: [PATCH 09/10] debug --- petri/src/disk_image.rs | 1 + petri/src/tracing.rs | 8 ++++--- petri/src/vm/hyperv/mod.rs | 4 +++- petri/src/vm/mod.rs | 41 +++++++++++++++++++++++++++++------ petri/src/vm/openvmm/mod.rs | 6 +++-- petri/src/vm/openvmm/start.rs | 7 +++--- 6 files changed, 50 insertions(+), 17 deletions(-) diff --git a/petri/src/disk_image.rs b/petri/src/disk_image.rs index 82bfdf79cd..440d44168c 100644 --- a/petri/src/disk_image.rs +++ b/petri/src/disk_image.rs @@ -19,6 +19,7 @@ use std::ops::Range; use std::path::Path; /// The description and artifacts needed to build a pipette disk image for a VM. +#[derive(Debug)] pub struct AgentImage { os_flavor: OsFlavor, pipette: Option, diff --git a/petri/src/tracing.rs b/petri/src/tracing.rs index a2ef30eedd..943824c35a 100644 --- a/petri/src/tracing.rs +++ b/petri/src/tracing.rs @@ -26,9 +26,10 @@ use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; /// A source of [`PetriLogFile`] log files for test output. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct PetriLogSource(Arc); +#[derive(Debug)] struct LogSourceInner { root_path: PathBuf, json_log: JsonLog, @@ -156,7 +157,7 @@ impl PetriLogSource { } } -#[derive(Clone)] +#[derive(Clone, Debug)] struct JsonLog(Arc); impl JsonLog { @@ -199,6 +200,7 @@ impl JsonLog { } } +#[derive(Debug)] struct LogFileInner { file: File, json_log: JsonLog, @@ -242,7 +244,7 @@ impl std::io::Write for LogWriter<'_> { /// Generally, you should use [`tracing`] for test-generated logging. This type /// is for writing fully-formed text entries that come from an external source, /// such as another process or a guest serial port. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct PetriLogFile(Arc); impl PetriLogFile { diff --git a/petri/src/vm/hyperv/mod.rs b/petri/src/vm/hyperv/mod.rs index 01215f86c0..02b678a544 100644 --- a/petri/src/vm/hyperv/mod.rs +++ b/petri/src/vm/hyperv/mod.rs @@ -12,6 +12,7 @@ use crate::Disk; use crate::Drive; use crate::Firmware; use crate::IsolationType; +use crate::ModifyFn; use crate::NoPetriVmInspector; use crate::OpenHclConfig; use crate::OpenHclServicingFlags; @@ -66,6 +67,7 @@ use vmgs_resources::GuestStateEncryptionPolicy; use vtl2_settings_proto::Vtl2Settings; /// The Hyper-V Petri backend +#[derive(Debug)] pub struct HyperVPetriBackend {} /// Resources needed at runtime for a Hyper-V Petri VM @@ -161,7 +163,7 @@ impl PetriVmmBackend for HyperVPetriBackend { async fn run( self, config: PetriVmConfig, - _modify_vmm_config: Option Self::VmmConfig + Send>, + _modify_vmm_config: Option>, resources: &PetriVmResources, properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index 3d8a636577..bc76dd0f37 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -42,6 +42,7 @@ use pipette_client::PipetteClient; use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; +use std::fmt::Debug; use std::hash::Hash; use std::hash::Hasher; use std::path::Path; @@ -107,7 +108,7 @@ pub struct PetriVmBuilder { /// VM configuration config: PetriVmConfig, /// Function to modify the VMM-specific configuration - modify_vmm_config: Option T::VmmConfig + Send>>, + modify_vmm_config: Option>, /// VMM-agnostic resources resources: PetriVmResources, @@ -130,7 +131,26 @@ pub struct PetriVmBuilder { boot_device_type: BootDeviceType, } +impl Debug for PetriVmBuilder { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PetriVmBuilder") + .field("backend", &self.backend) + .field("config", &self.config) + .field("modify_vmm_config", &self.modify_vmm_config.is_some()) + .field("resources", &self.resources) + .field("guest_quirks", &self.guest_quirks) + .field("vmm_quirks", &self.vmm_quirks) + .field("expected_boot_event", &self.expected_boot_event) + .field("override_expect_reset", &self.override_expect_reset) + .field("agent_image", &self.agent_image) + .field("openhcl_agent_image", &self.openhcl_agent_image) + .field("boot_device_type", &self.boot_device_type) + .finish() + } +} + /// Petri VM configuration +#[derive(Debug)] pub struct PetriVmConfig { /// The name of the VM pub name: String, @@ -182,6 +202,7 @@ pub struct PetriVmRuntimeConfig { } /// Resources used by a Petri VM during contruction and runtime +#[derive(Debug)] pub struct PetriVmResources { driver: DefaultDriver, log_source: PetriLogSource, @@ -189,7 +210,7 @@ pub struct PetriVmResources { /// Trait for VMM-specific contruction and runtime resources #[async_trait] -pub trait PetriVmmBackend { +pub trait PetriVmmBackend: Debug { /// VMM-specific configuration type VmmConfig; @@ -222,7 +243,7 @@ pub trait PetriVmmBackend { async fn run( self, config: PetriVmConfig, - modify_vmm_config: Option Self::VmmConfig + Send>, + modify_vmm_config: Option>, resources: &PetriVmResources, properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)>; @@ -587,6 +608,7 @@ impl PetriVmBuilder { // Add the boot disk now to allow the test to modify the boot type // Add the agent disks now to allow the test to add custom files self = self.add_boot_disk().add_agent_disks(); + tracing::debug!(builder = ?self); let arch = self.config.arch; let expect_reset = self.expect_reset(); @@ -1171,7 +1193,7 @@ impl PetriVmBuilder { if self.modify_vmm_config.is_some() { panic!("only one modify_backend allowed"); } - self.modify_vmm_config = Some(Box::new(f)); + self.modify_vmm_config = Some(ModifyFn(Box::new(f))); self } } @@ -1677,6 +1699,7 @@ pub enum ApicMode { } /// Common memory configuration information for the VM. +#[derive(Debug)] pub struct MemoryConfig { /// Specifies the amount of memory, in bytes, to assign to the /// virtual machine. @@ -2182,7 +2205,11 @@ impl Firmware { Firmware::OpenhclLinuxDirect { openhcl_config, .. } | Firmware::OpenhclUefi { openhcl_config, .. } | Firmware::OpenhclPcat { openhcl_config, .. } => PetriVmRuntimeConfig { - vtl2_settings: openhcl_config.vtl2_settings, + vtl2_settings: Some( + openhcl_config + .vtl2_settings + .unwrap_or_else(default_vtl2_settings), + ), ide_controllers: None, vmbus_storage_controllers, }, @@ -2588,9 +2615,9 @@ async fn save_inspect( } /// Wrapper for modification functions with stubbed out debug impl -pub struct ModifyFn(pub Box); +pub struct ModifyFn(pub Box T + Send>); -impl std::fmt::Debug for ModifyFn { +impl Debug for ModifyFn { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "_") } diff --git a/petri/src/vm/openvmm/mod.rs b/petri/src/vm/openvmm/mod.rs index b7cf7c8c5c..8e0901f14f 100644 --- a/petri/src/vm/openvmm/mod.rs +++ b/petri/src/vm/openvmm/mod.rs @@ -19,6 +19,7 @@ pub use runtime::PetriVmOpenVmm; use crate::Disk; use crate::Firmware; +use crate::ModifyFn; use crate::OpenHclServicingFlags; use crate::OpenvmmLogConfig; use crate::PetriLogFile; @@ -70,6 +71,7 @@ const MANA_INSTANCE: Guid = guid::guid!("f9641cf4-d915-4743-a7d8-efa75db7b85a"); pub const NIC_MAC_ADDRESS: MacAddress = MacAddress::new([0x00, 0x15, 0x5D, 0x12, 0x12, 0x12]); /// OpenVMM Petri Backend +#[derive(Debug)] pub struct OpenVmmPetriBackend { openvmm_path: ResolvedArtifact, } @@ -124,7 +126,7 @@ impl PetriVmmBackend for OpenVmmPetriBackend { async fn run( self, config: PetriVmConfig, - modify_vmm_config: Option PetriVmConfigOpenVmm + Send>, + modify_vmm_config: Option>, resources: &PetriVmResources, properties: PetriVmProperties, ) -> anyhow::Result<(Self::VmRuntime, PetriVmRuntimeConfig)> { @@ -132,7 +134,7 @@ impl PetriVmmBackend for OpenVmmPetriBackend { PetriVmConfigOpenVmm::new(&self.openvmm_path, config, resources, properties).await?; if let Some(f) = modify_vmm_config { - config = f(config); + config = f.0(config); } config.run().await diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index b793f3b7e2..624b768cd4 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -58,10 +58,9 @@ impl PetriVmConfigOpenVmm { // Add the GED and VTL 2 settings. if let Some(mut ged) = ged { - ged.vtl2_settings = runtime_config - .vtl2_settings - .as_ref() - .map(prost::Message::encode_to_vec); + ged.vtl2_settings = Some(prost::Message::encode_to_vec( + runtime_config.vtl2_settings.as_ref().unwrap(), + )); config .vmbus_devices .push((DeviceVtl::Vtl2, ged.into_resource())); From 5388e664046fc5d6b19705d83ed671812303b1ba Mon Sep 17 00:00:00 2001 From: Trevor Jones Date: Fri, 19 Dec 2025 23:05:18 -0800 Subject: [PATCH 10/10] enable openhcl pcat tests --- petri/src/vm/mod.rs | 20 +++++++++++++++++++- vmm_tests/vmm_test_macros/src/lib.rs | 15 +++++++++++++-- vmm_tests/vmm_tests/tests/tests/multiarch.rs | 9 ++++++++- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index bc76dd0f37..9cfb057adf 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -305,7 +305,8 @@ impl PetriVmBuilder { let boot_device_type = match artifacts.firmware { Firmware::LinuxDirect { .. } => BootDeviceType::None, Firmware::OpenhclLinuxDirect { .. } => BootDeviceType::None, - Firmware::Pcat { .. } | Firmware::OpenhclPcat { .. } => BootDeviceType::Ide, + Firmware::Pcat { .. } => BootDeviceType::Ide, + Firmware::OpenhclPcat { .. } => BootDeviceType::IdeViaScsi, Firmware::Uefi { guest: UefiGuest::None, .. @@ -2013,6 +2014,23 @@ impl Firmware { } } + /// Constructs a standard [`Firmware::OpenhclPcat`] configuration. + pub fn openhcl_pcat(resolver: &ArtifactResolver<'_>, guest: PcatGuest) -> Self { + use petri_artifacts_vmm_test::artifacts::loadable::*; + use petri_artifacts_vmm_test::artifacts::openhcl_igvm::*; + Firmware::OpenhclPcat { + guest, + igvm_path: resolver.require(LATEST_STANDARD_X64).erase(), + bios_firmware: resolver.try_require(PCAT_FIRMWARE_X64).erase(), + svga_firmware: resolver.try_require(SVGA_FIRMWARE_X64).erase(), + openhcl_config: OpenHclConfig { + // VMBUS redirect is necessary for IDE to be provided by VTL2 + vmbus_redirect: true, + ..Default::default() + }, + } + } + /// Constructs a standard [`Firmware::Uefi`] configuration. pub fn uefi(resolver: &ArtifactResolver<'_>, arch: MachineArch, guest: UefiGuest) -> Self { use petri_artifacts_vmm_test::artifacts::loadable::*; diff --git a/vmm_tests/vmm_test_macros/src/lib.rs b/vmm_tests/vmm_test_macros/src/lib.rs index 60a55a4406..3876756e13 100644 --- a/vmm_tests/vmm_test_macros/src/lib.rs +++ b/vmm_tests/vmm_test_macros/src/lib.rs @@ -40,6 +40,7 @@ enum Firmware { Pcat(PcatGuest), Uefi(UefiGuest), OpenhclLinuxDirect, + OpenhclPcat(PcatGuest), OpenhclUefi(OpenhclUefiOptions, UefiGuest), } @@ -103,12 +104,13 @@ impl Config { Firmware::Pcat(_) => "pcat", Firmware::Uefi(_) => "uefi", Firmware::OpenhclLinuxDirect => "openhcl_linux", + Firmware::OpenhclPcat(..) => "openhcl_pcat", Firmware::OpenhclUefi(..) => "openhcl_uefi", }; let guest_prefix = match &self.firmware { Firmware::LinuxDirect | Firmware::OpenhclLinuxDirect => None, - Firmware::Pcat(guest) => Some(guest.name_prefix()), + Firmware::Pcat(guest) | Firmware::OpenhclPcat(guest) => Some(guest.name_prefix()), Firmware::Uefi(guest) | Firmware::OpenhclUefi(_, guest) => guest.name_prefix(), }; @@ -116,7 +118,8 @@ impl Config { Firmware::LinuxDirect | Firmware::Pcat(_) | Firmware::Uefi(_) - | Firmware::OpenhclLinuxDirect => None, + | Firmware::OpenhclLinuxDirect + | Firmware::OpenhclPcat(_) => None, Firmware::OpenhclUefi(opt, _) => opt.name_prefix(), }; @@ -205,6 +208,9 @@ impl ToTokens for FirmwareAndArch { Firmware::OpenhclLinuxDirect => { quote!(::petri::Firmware::openhcl_linux_direct(resolver, #arch)) } + Firmware::OpenhclPcat(guest) => { + quote!(::petri::Firmware::openhcl_pcat(resolver, #guest)) + } Firmware::OpenhclUefi(OpenhclUefiOptions { isolation }, guest) => { let isolation = match isolation { Some(i) => quote!(Some(#i)), @@ -275,6 +281,10 @@ impl Parse for Config { MachineArch::Aarch64, Firmware::Uefi(parse_uefi_guest(input)?), ), + "openhcl_pcat_x64" => ( + MachineArch::X86_64, + Firmware::OpenhclPcat(parse_pcat_guest(input)?), + ), "openhcl_uefi_x64" => ( MachineArch::X86_64, Firmware::OpenhclUefi(parse_openhcl_uefi_options(input)?, parse_uefi_guest(input)?), @@ -534,6 +544,7 @@ fn parse_extra_deps(input: ParseStream<'_>) -> syn::Result> { /// - `{vmm}_openhcl_linux_direct_{arch}`: Our provided Linux direct image with OpenHCL /// - `{vmm}_pcat_{arch}()`: A Gen 1 configuration /// - `{vmm}_uefi_{arch}()`: A Gen 2 configuration +/// - `{vmm}_openhcl_pcat_{arch}()`: A Gen 1 configuration with OpenHCL /// - `{vmm}_openhcl_uefi_{arch}[list,of,options]()`: A Gen 2 configuration with OpenHCL /// /// Valid VMMs are: diff --git a/vmm_tests/vmm_tests/tests/tests/multiarch.rs b/vmm_tests/vmm_tests/tests/tests/multiarch.rs index 12b495184e..56c483eb45 100644 --- a/vmm_tests/vmm_tests/tests/tests/multiarch.rs +++ b/vmm_tests/vmm_tests/tests/tests/multiarch.rs @@ -64,6 +64,9 @@ async fn frontpage(config: PetriVmBuilder) -> anyhow::Res openvmm_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)), openvmm_openhcl_uefi_x64(vhd(ubuntu_2404_server_x64)), openvmm_openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)), + hyperv_openhcl_pcat_x64(vhd(windows_datacenter_core_2022_x64)), + hyperv_openhcl_pcat_x64(vhd(ubuntu_2404_server_x64)), + hyperv_openhcl_pcat_x64(vhd(ubuntu_2504_server_x64)), hyperv_openhcl_uefi_aarch64(vhd(windows_11_enterprise_aarch64)), hyperv_openhcl_uefi_aarch64(vhd(ubuntu_2404_server_aarch64)), hyperv_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)), @@ -109,6 +112,8 @@ async fn boot_no_agent(config: PetriVmBuilder) -> anyhow: openvmm_uefi_x64(vhd(ubuntu_2504_server_x64)), openvmm_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)), openvmm_openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)), + hyperv_openhcl_pcat_x64(vhd(windows_datacenter_core_2022_x64)), + hyperv_openhcl_pcat_x64(vhd(ubuntu_2504_server_x64)), hyperv_openhcl_uefi_aarch64(vhd(windows_11_enterprise_aarch64)), hyperv_openhcl_uefi_aarch64(vhd(ubuntu_2404_server_aarch64)), hyperv_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)), @@ -220,7 +225,9 @@ async fn boot_nvme_vpci_relay(config: PetriVmBuilder) -> // openvmm_uefi_x64(vhd(windows_datacenter_core_2022_x64)), // openvmm_uefi_x64(vhd(ubuntu_2504_server_x64)), // openvmm_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)), - // openvmm_openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)) + // openvmm_openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)), + hyperv_openhcl_pcat_x64(vhd(windows_datacenter_core_2022_x64)), + hyperv_openhcl_pcat_x64(vhd(ubuntu_2504_server_x64)), hyperv_openhcl_uefi_aarch64(vhd(windows_11_enterprise_aarch64)), hyperv_openhcl_uefi_aarch64(vhd(ubuntu_2404_server_aarch64)), hyperv_openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)),