diff --git a/cloud-hypervisor/src/main.rs b/cloud-hypervisor/src/main.rs index 98a311ec4..5de2e4c2d 100644 --- a/cloud-hypervisor/src/main.rs +++ b/cloud-hypervisor/src/main.rs @@ -1025,7 +1025,7 @@ mod unit_tests { mode: ConsoleOutputMode::Tty, socket: None, }, - iommu: false, + pci_common: PciDeviceCommonConfig::default(), }, #[cfg(target_arch = "x86_64")] debug_console: DebugConsoleConfig::default(), @@ -1693,10 +1693,12 @@ mod unit_tests { "--serial", "null", "--console", - "tty", + "tty,pci_segment=1,pci_device_id=7", ], r#"{ - "payload": {"kernel": "/path/to/kernel"} + "payload": {"kernel": "/path/to/kernel"}, + "serial": {"mode": "Null"}, + "console": {"mode": "Tty", "iommu": false, "pci_segment": 1, "pci_device_id": 7} }"#, true, ), diff --git a/fuzz/fuzz_targets/http_api.rs b/fuzz/fuzz_targets/http_api.rs index ff94a1c56..07a1effa4 100644 --- a/fuzz/fuzz_targets/http_api.rs +++ b/fuzz/fuzz_targets/http_api.rs @@ -184,7 +184,7 @@ impl RequestHandler for StubApiRequestHandler { mode: ConsoleOutputMode::Tty, socket: None, }, - iommu: false, + pci_common: PciDeviceCommonConfig::default(), }, #[cfg(target_arch = "x86_64")] debug_console: DebugConsoleConfig::default(), diff --git a/vmm/src/config.rs b/vmm/src/config.rs index f008081e4..cb919e75e 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -2187,18 +2187,17 @@ impl ConsoleConfig { parser .add_all_valueless(CommonConsoleConfig::VALUELESS_OPTIONS) .add_all(CommonConsoleConfig::VALUE_OPTIONS) - .add("iommu"); + .add_all(PciDeviceCommonConfig::OPTIONS_IOMMU); parser.parse(console).map_err(Error::ParseConsole)?; - let iommu = parser - .convert::("iommu") - .map_err(Error::ParsePciDeviceCommonConfig)? - .unwrap_or(Toggle(false)) - .0; - let common = CommonConsoleConfig::parse(console, Error::ParseConsole)?; + let pci_common = PciDeviceCommonConfig::parse(console)?; - Ok(Self { common, iommu }) + Ok(Self { common, pci_common }) + } + + pub fn validate(&self, vm_config: &VmConfig) -> ValidationResult<()> { + self.pci_common.validate(vm_config) } } @@ -3054,7 +3053,10 @@ impl VmConfig { } self.iommu |= self.rng.pci_common.iommu; - self.iommu |= self.console.iommu; + + self.console.validate(self)?; + Self::validate_identifier(&mut id_list, &self.console.pci_common.id)?; + self.iommu |= self.console.pci_common.iommu; if let Some(t) = &self.cpus.topology { if t.threads_per_core == 0 @@ -4411,7 +4413,10 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" fn test_console_parsing() -> Result<()> { let console_config = |mode, file, socket, iommu| ConsoleConfig { common: CommonConsoleConfig { file, mode, socket }, - iommu, + pci_common: PciDeviceCommonConfig { + iommu, + ..Default::default() + }, }; ConsoleConfig::parse("").unwrap_err(); @@ -5054,7 +5059,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" mode: ConsoleOutputMode::Tty, socket: None, }, - iommu: false, + pci_common: PciDeviceCommonConfig::default(), }, #[cfg(target_arch = "x86_64")] debug_console: DebugConsoleConfig::default(), @@ -5981,6 +5986,37 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" invalid_config.validate(), Err(ValidationError::InvalidPciDeviceId(pci::NUM_DEVICE_IDS + 1)) ); + + // Invalid console BDF - Same ID as Root device + let mut invalid_config = valid_config.clone(); + invalid_config.console.pci_common.pci_device_id = Some(pci::PCI_ROOT_DEVICE_ID); + assert_eq!( + invalid_config.validate(), + Err(ValidationError::ReservedPciDeviceId( + pci::PCI_ROOT_DEVICE_ID + )) + ); + // Invalid console BDF - Out of range + let mut invalid_config = valid_config.clone(); + invalid_config.console.pci_common.pci_device_id = Some(pci::NUM_DEVICE_IDS + 1); + assert_eq!( + invalid_config.validate(), + Err(ValidationError::InvalidPciDeviceId(pci::NUM_DEVICE_IDS + 1)) + ); + // Invalid console ID - Duplicate identifier + let mut invalid_config = valid_config.clone(); + invalid_config.console.pci_common.id = Some("test0".to_string()); + invalid_config.disks = Some(vec![DiskConfig { + pci_common: PciDeviceCommonConfig { + id: Some("test0".to_string()), + ..Default::default() + }, + ..disk_fixture() + }]); + assert_eq!( + invalid_config.validate(), + Err(ValidationError::IdentifierNotUnique("test0".to_string())) + ); } #[test] fn test_landlock_parsing() -> Result<()> { diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 739bf424b..9b9cda598 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -2368,7 +2368,7 @@ impl DeviceManager { transport: ConsoleTransport, resize_pipe: Option>, ) -> DeviceManagerResult>> { - let console_config = self.config.lock().unwrap().console.clone(); + let mut console_config = self.config.lock().unwrap().console.clone(); let endpoint = match transport { ConsoleTransport::File(file) => Endpoint::File(file), ConsoleTransport::Pty(file) => { @@ -2402,7 +2402,15 @@ impl DeviceManager { ConsoleTransport::Null => Endpoint::Null, ConsoleTransport::Off => return Ok(None), }; - let id = String::from(CONSOLE_DEVICE_NAME); + + let id = match console_config.pci_common.id.as_ref() { + Some(id) => id.clone(), + None => console_config + .pci_common + .id + .insert(CONSOLE_DEVICE_NAME.to_string()) + .clone(), + }; let (virtio_console_device, console_resizer) = virtio_devices::Console::new( id.clone(), @@ -2410,7 +2418,7 @@ impl DeviceManager { self.console_resize_pipe .as_ref() .map(|p| p.try_clone().unwrap()), - self.force_access_platform | console_config.iommu, + self.force_access_platform | console_config.pci_common.iommu, self.seccomp_action.clone(), self.exit_evt .try_clone() @@ -2423,11 +2431,7 @@ impl DeviceManager { self.virtio_devices.push(MetaVirtioDevice { virtio_device: Arc::clone(&virtio_console_device) as Arc>, - pci_common: PciDeviceCommonConfig { - id: Some(id.clone()), - iommu: console_config.iommu, - ..Default::default() - }, + pci_common: console_config.pci_common.clone(), dma_handler: None, }); diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index faa72fbeb..6c9d476ab 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -2737,7 +2737,7 @@ mod unit_tests { mode: ConsoleOutputMode::Off, socket: None, }, - iommu: false, + pci_common: PciDeviceCommonConfig::default(), }, #[cfg(target_arch = "x86_64")] debug_console: DebugConsoleConfig::default(), diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 140311710..6fefc8f06 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -574,10 +574,6 @@ impl ApplyLandlock for CommonConsoleConfig { } /// Configuration for a legacy serial console device. -/// -/// - On x86_64, this is a port I/O-mapped UART16550-compatible device -/// - On aarch64, this is a MMIO-mapped PL011 device -/// - On RISCV, this is a MMIO-mapped UART16550-compatible device #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct SerialConfig { #[serde(flatten)] @@ -611,13 +607,12 @@ impl ApplyLandlock for SerialConfig { pub struct ConsoleConfig { #[serde(flatten)] pub common: CommonConsoleConfig, - #[serde(default, skip_serializing_if = "<&bool as std::ops::Not>::not")] - pub iommu: bool, + #[serde(default, flatten)] + pub pci_common: PciDeviceCommonConfig, } impl ConsoleConfig { - pub const SYNTAX: &str = - "Control (virtio) console: \"off|null|pty|tty|file=,iommu=on|off\""; + pub const SYNTAX: &str = "Control (virtio) console: \"off|null|pty|tty|file=,iommu=on|off,id=,pci_segment=,pci_device_id=\""; } impl Default for ConsoleConfig { @@ -628,7 +623,7 @@ impl Default for ConsoleConfig { mode: ConsoleOutputMode::Tty, socket: None, }, - iommu: false, + pci_common: PciDeviceCommonConfig::default(), } } }