diff --git a/openvmm/openvmm_entry/src/cli_args.rs b/openvmm/openvmm_entry/src/cli_args.rs index 8bd2a939a6..4ffe1dcd75 100644 --- a/openvmm/openvmm_entry/src/cli_args.rs +++ b/openvmm/openvmm_entry/src/cli_args.rs @@ -18,6 +18,7 @@ // anything else on this file though. #![warn(missing_docs)] +use crate::storage_builder::NvmeControllerType; use anyhow::Context; use clap::Parser; use clap::ValueEnum; @@ -143,8 +144,18 @@ flags: `ro` open disk as read-only `dvd` specifies that device is cd/dvd and it is read_only `vtl2` assign this disk to VTL2 - `uh` relay this disk to VTL0 through SCSI-to-OpenHCL (show to VTL0 as SCSI) - `uh-nvme` relay this disk to VTL0 through NVMe-to-OpenHCL (show to VTL0 as SCSI) + +options: + `uh[=[lun:][;controller:]]` relay this disk to VTL0 through SCSI-to-OpenHCL (show to VTL0 as SCSI) + : LUN of the disk [0..255] + : GUID of the SCSI controller to attach this disk from (shows as the VSID to VTL2) + `uh-nvme[=[nsid:][;controller:]]` relay this disk to VTL0 through NVMe-to-OpenHCL (show to VTL0 as SCSI) + : NSID of the namespace [0..2^32-1] + : GUID of the SCSI controller to attach this disk from (shows as the VSID to VTL2) + + When using `uh` and `uh-nvme`, each disk is identified by a unique (lun/nsid, controller_id) tuple. + - If no controller_id is specified, a default controller will be used. + - If no lun/nsid is specified, the next available lun/nsid will be used. "#)] #[clap(long, value_name = "FILE")] pub disk: Vec, @@ -958,7 +969,7 @@ impl FromStr for VmgsCli { } // [,ro] -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] pub struct DiskCli { pub vtl: DeviceVtl, pub kind: DiskCliKind, @@ -968,12 +979,6 @@ pub struct DiskCli { pub pcie_port: Option, } -#[derive(Copy, Clone)] -pub enum UnderhillDiskSource { - Scsi, - Nvme, -} - impl FromStr for DiskCli { type Err = anyhow::Error; @@ -986,10 +991,10 @@ impl FromStr for DiskCli { let mut underhill = None; let mut vtl = DeviceVtl::Vtl0; let mut pcie_port = None; - for opt in opts { - let mut s = opt.split('='); - let opt = s.next().unwrap(); - match opt { + for full_opt in opts { + let mut s = full_opt.split('='); + let key = s.next().unwrap(); + match key { "ro" => read_only = true, "dvd" => { is_dvd = true; @@ -998,8 +1003,6 @@ impl FromStr for DiskCli { "vtl2" => { vtl = DeviceVtl::Vtl2; } - "uh" => underhill = Some(UnderhillDiskSource::Scsi), - "uh-nvme" => underhill = Some(UnderhillDiskSource::Nvme), "pcie_port" => { let port = s.next(); if port.is_none_or(|p| p.is_empty()) { @@ -1007,7 +1010,13 @@ impl FromStr for DiskCli { } pcie_port = Some(String::from(port.unwrap())); } - opt => anyhow::bail!("unknown option: '{opt}'"), + key => { + if key.starts_with("uh") { + underhill = Some(full_opt.parse()?); + } else { + anyhow::bail!("unknown option: '{full_opt}'") + } + } } } @@ -1030,6 +1039,91 @@ impl FromStr for DiskCli { } } +#[derive(Clone, Debug, PartialEq)] +pub enum UnderhillDiskSource { + /// Expose to VTL2 via SCSI (optional LUN, optional controller ID (unused)) + Scsi(Option, Option), + /// Expose to VTL2 via NVMe (optional NSID, optional controller ID) + Nvme(Option, NvmeControllerType), +} + +impl FromStr for UnderhillDiskSource { + type Err = anyhow::Error; + + // uh-nvme=[nsid:][;controller:] + // uh=[lun:][;controller:] + fn from_str(s: &str) -> anyhow::Result { + let parse_loc = |s: &str| -> anyhow::Result<(Option, Option, Option)> { + let mut lun = None; + let mut nsid = None; + let mut controller_id = None; + for part in s.split(';') { + let mut kv = part.splitn(2, ':'); + let key = kv.next().unwrap(); + let value = kv + .next() + .with_context(|| format!("expected key:value for '{key}'"))?; + match key { + "lun" => { + lun = Some( + value + .parse() + .with_context(|| format!("invalid lun value '{value}'"))?, + ); + } + "nsid" => { + let n = value + .parse() + .with_context(|| format!("invalid nsid value '{value}'"))?; + if n == u32::MAX { + anyhow::bail!("nsid value '{value}' is reserved"); + } + nsid = Some(n); + } + "controller" => { + controller_id = Some(value.to_string()); + } + _ => anyhow::bail!("unknown option '{key}'"), + } + } + Ok((lun, nsid, controller_id)) + }; + + let (option, value) = match s.split_once('=') { + Some((opt, "")) => (opt, None), + Some((opt, val)) => (opt, Some(val)), + None => (s, None), + }; + match (option, value) { + ("uh-nvme", None) => Ok(UnderhillDiskSource::Nvme( + None, + NvmeControllerType::Vpci(None), + )), + ("uh-nvme", Some(opts)) => { + let (lun, nsid, controller_id) = parse_loc(opts)?; + if lun.is_some() { + anyhow::bail!("`uh-nvme` does not support `lun`, use `nsid` instead"); + } + Ok(UnderhillDiskSource::Nvme( + nsid, + NvmeControllerType::Vpci(controller_id), + )) + } + ("uh", None) => Ok(UnderhillDiskSource::Scsi(None, None)), + ("uh", Some(opts)) => { + let (lun, nsid, controller_id) = parse_loc(opts)?; + if nsid.is_some() { + anyhow::bail!("`uh` does not support `nsid`, use `lun` instead"); + } + Ok(UnderhillDiskSource::Scsi(lun, controller_id)) + } + (_, _) => { + anyhow::bail!("invalid underhill disk source '{option}', '{value:?}' found") + } + } + } +} + // [,ro,s] #[derive(Clone)] pub struct IdeDiskCli { @@ -1874,6 +1968,90 @@ mod tests { } } + #[test] + fn test_parse_underhill_disk() { + let cases: [(&str, UnderhillDiskSource); 11] = [ + ("uh-nvme", UnderhillDiskSource::Nvme(None, None)), + ("uh-nvme=nsid:0", UnderhillDiskSource::Nvme(Some(0), None)), + ("uh-nvme=nsid:5", UnderhillDiskSource::Nvme(Some(5), None)), + ( + "uh-nvme=nsid:205", + UnderhillDiskSource::Nvme(Some(205), None), + ), + ( + "uh-nvme=controller:abcd", + UnderhillDiskSource::Nvme(None, Some("abcd".to_string())), + ), + ( + "uh-nvme=nsid:5;controller:abcd", + UnderhillDiskSource::Nvme(Some(5), Some("abcd".to_string())), + ), + ("uh", UnderhillDiskSource::Scsi(None, None)), + ("uh=lun:0", UnderhillDiskSource::Scsi(Some(0), None)), + ("uh=lun:5", UnderhillDiskSource::Scsi(Some(5), None)), + ( + "uh=controller:abcd", + UnderhillDiskSource::Scsi(None, Some("abcd".to_string())), + ), + ( + "uh=lun:5;controller:abcd", + UnderhillDiskSource::Scsi(Some(5), Some("abcd".to_string())), + ), + ]; + + for (input, expected) in cases { + let disk = UnderhillDiskSource::from_str(input).unwrap(); + assert_eq!(disk, expected); + } + + let error_cases = [ + "uh-nvme=invalid:5", + "uh-nvme=nsid:abc", + "uh-nvme=nsid:-1", + "uh-nvme=nsid:0xFFFFFFFF", + "uh-nvme=nsid:5;invalid:abcd", + "uh=invalid:5", + "uh=lun:abc", + "uh=lun:5;invalid:abcd", + "uh=lun:-1", + "not-uh", + ]; + + for input in error_cases { + assert!( + UnderhillDiskSource::from_str(input).is_err(), + "input: {}", + input + ); + } + + // Make sure underhill args fit in nicely with DiskCli parsing + assert_eq!( + DiskCli::from_str("file:disk.vhd,uh-nvme") + .unwrap() + .underhill, + Some(UnderhillDiskSource::Nvme(None, None)) + ); + assert_eq!( + DiskCli::from_str("file:disk.vhd,uh-nvme=nsid:10;controller:ctrl1") + .unwrap() + .underhill, + Some(UnderhillDiskSource::Nvme( + Some(10), + Some("ctrl1".to_string()) + )) + ); + assert_eq!( + DiskCli::from_str("file:disk.vhd,uh-nvme=nsid:10;controller:ctrl1,ro") + .unwrap() + .underhill, + Some(UnderhillDiskSource::Nvme( + Some(10), + Some("ctrl1".to_string()) + )) + ); + } + #[test] fn test_parse_autocache_sqlite_disk() { // Test with environment variable set diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index 2ede58f382..1b7921eca9 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -22,6 +22,7 @@ pub use cli_args::Options; use console_relay::ConsoleLaunchOptions; use crate::cli_args::SecureBootTemplateCli; +use crate::storage_builder::NvmeControllerType; use anyhow::Context; use anyhow::bail; use chipset_resources::battery::HostBatteryUpdate; @@ -524,7 +525,7 @@ fn vm_config_from_command_line( ref kind, read_only, is_dvd, - underhill, + ref underhill, ref pcie_port, } in &opt.disk { @@ -534,7 +535,7 @@ fn vm_config_from_command_line( storage.add( vtl, - underhill, + underhill.clone(), storage_builder::DiskLocation::Scsi(None), kind, is_dvd, @@ -565,14 +566,14 @@ fn vm_config_from_command_line( ref kind, read_only, is_dvd, - underhill, + ref underhill, ref pcie_port, } in &opt.nvme { storage.add( vtl, - underhill, - storage_builder::DiskLocation::Nvme(None, pcie_port.clone()), + underhill.clone(), + storage_builder::DiskLocation::Nvme(None, NvmeControllerType::Pcie(pcie_port.clone())), kind, is_dvd, read_only, diff --git a/openvmm/openvmm_entry/src/storage_builder.rs b/openvmm/openvmm_entry/src/storage_builder.rs index 6c347c0c64..0483260149 100644 --- a/openvmm/openvmm_entry/src/storage_builder.rs +++ b/openvmm/openvmm_entry/src/storage_builder.rs @@ -34,26 +34,32 @@ pub(super) struct StorageBuilder { vtl0_ide_disks: Vec, vtl0_scsi_devices: Vec, vtl2_scsi_devices: Vec, - vtl0_nvme_namespaces: Vec, - vtl2_nvme_namespaces: Vec, + vtl0_nvme_namespaces: Vec<(Option, NamespaceDefinition)>, // (controller vsid, namespace) + vtl2_nvme_namespaces: Vec<(Option, NamespaceDefinition)>, // (controller vsid, namespace) pcie_nvme_controllers: BTreeMap>, underhill_scsi_luns: Vec, underhill_nvme_luns: Vec, openhcl_vtl: Option, } -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] +pub enum NvmeControllerType { + Vpci(Option), + Pcie(String), +} + +#[derive(Clone, Debug)] pub enum DiskLocation { Ide(Option, Option), Scsi(Option), - Nvme(Option, Option), + Nvme(Option, NvmeControllerType), } impl From for DiskLocation { fn from(value: UnderhillDiskSource) -> Self { match value { - UnderhillDiskSource::Scsi => Self::Scsi(None), - UnderhillDiskSource::Nvme => Self::Nvme(None, None), + UnderhillDiskSource::Scsi(lun, _controller) => Self::Scsi(lun), + UnderhillDiskSource::Nvme(nsid, controller) => Self::Nvme(nsid, controller), } } } @@ -122,7 +128,7 @@ impl StorageBuilder { if vtl != DeviceVtl::Vtl0 { anyhow::bail!("underhill can only offer devices to vtl0"); } - self.add_underhill(source.into(), target, kind, is_dvd, read_only)?; + self.add_underhill(source.clone().into(), target, kind, is_dvd, read_only)?; } else { self.add_inner(vtl, target, kind, is_dvd, read_only)?; } @@ -215,28 +221,45 @@ impl StorageBuilder { }); Some(lun.into()) } - DiskLocation::Nvme(nsid, pcie_port) => { - let namespaces = match (vtl, pcie_port) { - // VPCI - (DeviceVtl::Vtl0, None) => &mut self.vtl0_nvme_namespaces, - (DeviceVtl::Vtl1, None) => anyhow::bail!("vtl1 vpci unsupported"), - (DeviceVtl::Vtl2, None) => &mut self.vtl2_nvme_namespaces, - // PCIe - (DeviceVtl::Vtl0, Some(port)) => { - self.pcie_nvme_controllers.entry(port).or_default() - } - (DeviceVtl::Vtl1, Some(_)) => anyhow::bail!("vtl1 pcie unsupported"), - (DeviceVtl::Vtl2, Some(_)) => anyhow::bail!("vtl2 pcie unsupported"), - }; + DiskLocation::Nvme(nsid, controller) => { if is_dvd { anyhow::bail!("dvd not supported with nvme"); } - let nsid = nsid.unwrap_or(namespaces.len() as u32 + 1); - namespaces.push(NamespaceDefinition { + + let def = (NamespaceDefinition { nsid, disk, read_only, }); + + match (vtl, controller) { + // VPCI + (DeviceVtl::Vtl0, NvmeControllerType::Vpci(_)) => { + &mut self.vtl0_nvme_namespaces + } + (DeviceVtl::Vtl1, NvmeControllerType::Vpci(_)) => { + anyhow::bail!("vtl1 vpci unsupported") + } + (DeviceVtl::Vtl2, NvmeControllerType::Vpci(_)) => { + &mut self.vtl2_nvme_namespaces + } + // PCIe + (DeviceVtl::Vtl0, NvmeControllerType::Pcie(port)) => { + self.pcie_nvme_controllers.entry(port).or_default() + } + (DeviceVtl::Vtl1, NvmeControllerType::Pcie(_)) => { + anyhow::bail!("vtl1 pcie unsupported") + } + (DeviceVtl::Vtl2, NvmeControllerType::Pcie(_)) => { + anyhow::bail!("vtl2 pcie unsupported") + } + + } + + let namespaces = match (vtl, controller) { + }; + let nsid = nsid.unwrap_or(namespaces.len() as u32 + 1); + namespaces.push); Some(nsid) } };