vmm, main: split ConsoleConfig and SerialConfig into separate structs

This commit introduces a new struct `CommonConsoleConfig` which is the
base for the split into `ConsoleConfig` and `SerialConfig`. This is a
pre-requisite for allowing more configurable PCI options for the
virtio-console device.

The commit doesn't change or add any functionality.

On-behalf-of: Philipp Schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>

vmm/src/config.rs

@@ -128,9 +128,12 @@ pub enum Error {
     /// Error parsing generic vhost-user parameters
     #[error("Error parsing --generic-vhost-user")]
     ParseGenericVhostUser(#[source] OptionParserError),
-    /// Failed parsing console
+    /// Failed parsing console parameters
     #[error("Error parsing --console")]
     ParseConsole(#[source] OptionParserError),
+    /// Failed parsing serial parameters
+    #[error("Error parsing --serial")]
+    ParseSerial(#[source] OptionParserError),
     #[cfg(target_arch = "x86_64")]
     /// Failed parsing debug-console
     #[error("Error parsing --debug-console")]
@@ -2137,17 +2140,18 @@ impl PmemConfig {
     }
 }
 
-impl ConsoleConfig {
-    pub fn parse(console: &str) -> Result<Self> {
+impl CommonConsoleConfig {
+    const VALUELESS_OPTIONS: &[&str] = &["off", "pty", "tty", "null"];
+    const VALUE_OPTIONS: &[&str] = &["file", "socket"];
+
+    fn parse(console: &str, map_err: impl Fn(OptionParserError) -> Error) -> Result<Self> {
         let mut parser = OptionParser::new();
         parser
-            .add_all_valueless(&["off", "pty", "tty", "null"])
-            .add("file")
-            .add("iommu")
-            .add("socket");
-        parser.parse(console).map_err(Error::ParseConsole)?;
+            .add_all_valueless(Self::VALUELESS_OPTIONS)
+            .add_all(Self::VALUE_OPTIONS);
+        parser.parse_subset(console).map_err(map_err)?;
 
-        let mut file: Option<PathBuf> = default_consoleconfig_file();
+        let mut file: Option<PathBuf> = None;
         let mut socket: Option<PathBuf> = None;
         let mut mode: ConsoleOutputMode = ConsoleOutputMode::Off;
 
@@ -2172,18 +2176,49 @@ impl ConsoleConfig {
         } else {
             return Err(Error::ParseConsoleInvalidModeGiven);
         }
+
+        Ok(Self { mode, file, socket })
+    }
+}
+
+impl ConsoleConfig {
+    pub fn parse(console: &str) -> Result<Self> {
+        let mut parser = OptionParser::new();
+        parser
+            .add_all_valueless(CommonConsoleConfig::VALUELESS_OPTIONS)
+            .add_all(CommonConsoleConfig::VALUE_OPTIONS)
+            .add("iommu");
+        parser.parse(console).map_err(Error::ParseConsole)?;
+
         let iommu = parser
             .convert::<Toggle>("iommu")
-            .map_err(Error::ParseConsole)?
+            .map_err(Error::ParsePciDeviceCommonConfig)?
             .unwrap_or(Toggle(false))
             .0;
 
-        Ok(Self {
-            file,
-            mode,
-            iommu,
-            socket,
-        })
+        let common = CommonConsoleConfig::parse(console, Error::ParseConsole)?;
+
+        Ok(Self { common, iommu })
+    }
+}
+
+impl SerialConfig {
+    pub fn parse(serial: &str) -> Result<Self> {
+        let mut parser = OptionParser::new();
+        parser
+            .add_all_valueless(CommonConsoleConfig::VALUELESS_OPTIONS)
+            .add_all(CommonConsoleConfig::VALUE_OPTIONS)
+            .add("iommu");
+        parser.parse(serial).map_err(Error::ParseSerial)?;
+
+        let iommu = parser
+            .convert::<Toggle>("iommu")
+            .map_err(Error::ParsePciDeviceCommonConfig)?
+            .unwrap_or(Toggle(false))
+            .0;
+
+        let common = CommonConsoleConfig::parse(serial, Error::ParseSerial)?;
+        Ok(Self { common, iommu })
     }
 }
 
@@ -2202,7 +2237,7 @@ impl DebugConsoleConfig {
             .parse(debug_console_ops)
             .map_err(Error::ParseConsole)?;
 
-        let mut file: Option<PathBuf> = default_consoleconfig_file();
+        let mut file: Option<PathBuf> = None;
         let mut iobase: Option<u16> = None;
         let mut mode: ConsoleOutputMode = ConsoleOutputMode::Off;
 
@@ -2894,10 +2929,10 @@ impl VmConfig {
         // "console=hvc0 earlyprintk=ttyS0"
 
         let mut tty_consoles = Vec::new();
-        if self.console.mode == ConsoleOutputMode::Tty {
+        if self.console.common.mode == ConsoleOutputMode::Tty {
             tty_consoles.push("virtio-console");
         }
-        if self.serial.mode == ConsoleOutputMode::Tty {
+        if self.serial.common.mode == ConsoleOutputMode::Tty {
             tty_consoles.push("serial-console");
         }
         #[cfg(target_arch = "x86_64")]
@@ -2908,11 +2943,12 @@ impl VmConfig {
             warn!("Using TTY output for multiple consoles: {tty_consoles:?}");
         }
 
-        if self.console.mode == ConsoleOutputMode::File && self.console.file.is_none() {
+        if self.console.common.mode == ConsoleOutputMode::File && self.console.common.file.is_none()
+        {
             return Err(ValidationError::ConsoleFileMissing);
         }
 
-        if self.serial.mode == ConsoleOutputMode::File && self.serial.file.is_none() {
+        if self.serial.common.mode == ConsoleOutputMode::File && self.serial.common.file.is_none() {
             return Err(ValidationError::ConsoleFileMissing);
         }
 
@@ -3298,7 +3334,7 @@ impl VmConfig {
         }
 
         let console = ConsoleConfig::parse(vm_params.console)?;
-        let serial = ConsoleConfig::parse(vm_params.serial)?;
+        let serial = SerialConfig::parse(vm_params.serial)?;
         #[cfg(target_arch = "x86_64")]
         let debug_console = DebugConsoleConfig::parse(vm_params.debug_console)?;
 
@@ -4380,79 +4416,59 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}"
 
     #[test]
     fn test_console_parsing() -> Result<()> {
+        let console_config = |mode, file, socket, iommu| ConsoleConfig {
+            common: CommonConsoleConfig { file, mode, socket },
+            iommu,
+        };
+
         ConsoleConfig::parse("").unwrap_err();
         ConsoleConfig::parse("badmode").unwrap_err();
         assert_eq!(
             ConsoleConfig::parse("off")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Off,
-                iommu: false,
-                file: None,
-                socket: None,
-            }
+            console_config(ConsoleOutputMode::Off, None, None, false)
         );
         assert_eq!(
             ConsoleConfig::parse("pty")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Pty,
-                iommu: false,
-                file: None,
-                socket: None,
-            }
+            console_config(ConsoleOutputMode::Pty, None, None, false)
         );
         assert_eq!(
             ConsoleConfig::parse("tty")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Tty,
-                iommu: false,
-                file: None,
-                socket: None,
-            }
+            console_config(ConsoleOutputMode::Tty, None, None, false)
         );
         assert_eq!(
             ConsoleConfig::parse("null")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Null,
-                iommu: false,
-                file: None,
-                socket: None,
-            }
+            console_config(ConsoleOutputMode::Null, None, None, false)
         );
         assert_eq!(
             ConsoleConfig::parse("file=/tmp/console")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::File,
-                iommu: false,
-                file: Some(PathBuf::from("/tmp/console")),
-                socket: None,
-            }
+            console_config(
+                ConsoleOutputMode::File,
+                Some(PathBuf::from("/tmp/console")),
+                None,
+                false
+            )
         );
         assert_eq!(
             ConsoleConfig::parse("null,iommu=on")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Null,
-                iommu: true,
-                file: None,
-                socket: None,
-            }
+            console_config(ConsoleOutputMode::Null, None, None, true)
         );
         assert_eq!(
             ConsoleConfig::parse("file=/tmp/console,iommu=on")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::File,
-                iommu: true,
-                file: Some(PathBuf::from("/tmp/console")),
-                socket: None,
-            }
+            console_config(
+                ConsoleOutputMode::File,
+                Some(PathBuf::from("/tmp/console")),
+                None,
+                true
+            )
         );
         assert_eq!(
             ConsoleConfig::parse("socket=/tmp/serial.sock,iommu=on")?,
-            ConsoleConfig {
-                mode: ConsoleOutputMode::Socket,
-                iommu: true,
-                file: None,
-                socket: Some(PathBuf::from("/tmp/serial.sock")),
-            }
+            console_config(
+                ConsoleOutputMode::Socket,
+                None,
+                Some(PathBuf::from("/tmp/serial.sock")),
+                true
+            )
         );
         Ok(())
     }
@@ -4798,8 +4814,8 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}"
             balloon: None,
             fs: None,
             pmem: None,
-            serial: default_serial(),
-            console: default_console(),
+            serial: SerialConfig::default(),
+            console: ConsoleConfig::default(),
             #[cfg(target_arch = "x86_64")]
             debug_console: DebugConsoleConfig::default(),
             devices: None,
@@ -5032,17 +5048,21 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}"
             fs: None,
             generic_vhost_user: None,
             pmem: None,
-            serial: ConsoleConfig {
-                file: None,
-                mode: ConsoleOutputMode::Null,
+            serial: SerialConfig {
+                common: CommonConsoleConfig {
+                    file: None,
+                    mode: ConsoleOutputMode::Null,
+                    socket: None,
+                },
                 iommu: false,
-                socket: None,
             },
             console: ConsoleConfig {
-                file: None,
-                mode: ConsoleOutputMode::Tty,
+                common: CommonConsoleConfig {
+                    file: None,
+                    mode: ConsoleOutputMode::Tty,
+                    socket: None,
+                },
                 iommu: false,
-                socket: None,
             },
             #[cfg(target_arch = "x86_64")]
             debug_console: DebugConsoleConfig::default(),
@@ -5071,8 +5091,8 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}"
         valid_config.validate().unwrap();
 
         let mut invalid_config = valid_config.clone();
-        invalid_config.serial.mode = ConsoleOutputMode::Tty;
-        invalid_config.console.mode = ConsoleOutputMode::Tty;
+        invalid_config.serial.common.mode = ConsoleOutputMode::Tty;
+        invalid_config.console.common.mode = ConsoleOutputMode::Tty;
         valid_config.validate().unwrap();
 
         let mut invalid_config = valid_config.clone();
@@ -5112,8 +5132,8 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}"
         }
 
         let mut invalid_config = valid_config.clone();
-        invalid_config.serial.mode = ConsoleOutputMode::File;
-        invalid_config.serial.file = None;
+        invalid_config.serial.common.mode = ConsoleOutputMode::File;
+        invalid_config.serial.common.file = None;
         assert_eq!(
             invalid_config.validate(),
             Err(ValidationError::ConsoleFileMissing)

vmm/src/console_devices.rs

@@ -181,9 +181,9 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
     let mut original_termios_opt = vmm.original_termios_opt.lock().unwrap();
 
     let console_info = ConsoleInfo {
-        console: match vmconfig.console.mode {
+        console: match vmconfig.console.common.mode {
             ConsoleOutputMode::File => {
-                let file = File::create(vmconfig.console.file.as_ref().unwrap())
+                let file = File::create(vmconfig.console.common.file.as_ref().unwrap())
                     .map_err(ConsoleDeviceError::CreateConsoleDevice)?;
                 ConsoleTransport::File(Arc::new(file))
             }
@@ -191,7 +191,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
                 let (main_fd, sub_fd, path) =
                     create_pty().map_err(ConsoleDeviceError::CreateConsoleDevice)?;
                 set_raw_mode(&sub_fd.as_raw_fd(), &mut original_termios_opt)?;
-                vmconfig.console.file = Some(path.clone());
+                vmconfig.console.common.file = Some(path.clone());
                 vmm.console_resize_pipe = Some(Arc::new(
                     listen_for_sigwinch_on_tty(
                         sub_fd,
@@ -230,9 +230,9 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
             ConsoleOutputMode::Null => ConsoleTransport::Null,
             ConsoleOutputMode::Off => ConsoleTransport::Off,
         },
-        serial: match vmconfig.serial.mode {
+        serial: match vmconfig.serial.common.mode {
             ConsoleOutputMode::File => {
-                let file = File::create(vmconfig.serial.file.as_ref().unwrap())
+                let file = File::create(vmconfig.serial.common.file.as_ref().unwrap())
                     .map_err(ConsoleDeviceError::CreateConsoleDevice)?;
                 ConsoleTransport::File(Arc::new(file))
             }
@@ -240,7 +240,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
                 let (main_fd, sub_fd, path) =
                     create_pty().map_err(ConsoleDeviceError::CreateConsoleDevice)?;
                 set_raw_mode(&sub_fd.as_raw_fd(), &mut original_termios_opt)?;
-                vmconfig.serial.file = Some(path.clone());
+                vmconfig.serial.common.file = Some(path.clone());
                 ConsoleTransport::Pty(Arc::new(main_fd))
             }
             ConsoleOutputMode::Tty => {
@@ -260,7 +260,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
                 ConsoleTransport::Tty(Arc::new(stdout))
             }
             ConsoleOutputMode::Socket => {
-                let listener = UnixListener::bind(vmconfig.serial.socket.as_ref().unwrap())
+                let listener = UnixListener::bind(vmconfig.serial.common.socket.as_ref().unwrap())
                     .map_err(ConsoleDeviceError::CreateConsoleDevice)?;
                 ConsoleTransport::Socket(Arc::new(listener))
             }

vmm/src/device_manager.rs

@@ -2440,11 +2440,13 @@ impl DeviceManager {
             .insert(id.clone(), device_node!(id, virtio_console_device));
 
         // Only provide a resizer (for SIGWINCH handling) if the console is attached to the TTY
-        Ok(if matches!(console_config.mode, ConsoleOutputMode::Tty) {
-            Some(console_resizer)
-        } else {
-            None
-        })
+        Ok(
+            if matches!(console_config.common.mode, ConsoleOutputMode::Tty) {
+                Some(console_resizer)
+            } else {
+                None
+            },
+        )
     }
 
     /// Adds all devices that behave like a console with respect to the VM
@@ -2482,9 +2484,12 @@ impl DeviceManager {
                 ConsoleTransport::Pty(_)
                 | ConsoleTransport::Tty(_)
                 | ConsoleTransport::Socket(_) => {
-                    let serial_manager =
-                        SerialManager::new(serial, console_info.serial, serial_config.socket)
-                            .map_err(DeviceManagerError::CreateSerialManager)?;
+                    let serial_manager = SerialManager::new(
+                        serial,
+                        console_info.serial,
+                        serial_config.common.socket,
+                    )
+                    .map_err(DeviceManagerError::CreateSerialManager)?;
                     if let Some(mut serial_manager) = serial_manager {
                         serial_manager
                             .start_thread(
@@ -5442,18 +5447,18 @@ impl Aml for DeviceManager {
         #[cfg(target_arch = "x86_64")]
         let serial_irq = 4;
         #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))]
-        let serial_irq =
-            if self.config.lock().unwrap().serial.clone().mode == ConsoleOutputMode::Off {
-                // If serial is turned off, add a fake device with invalid irq.
-                31
-            } else {
-                self.get_device_info()
-                    .clone()
-                    .get(&(DeviceType::Serial, DeviceType::Serial.to_string()))
-                    .unwrap()
-                    .irq()
-            };
-        if self.config.lock().unwrap().serial.mode != ConsoleOutputMode::Off {
+        let serial_irq = if self.config.lock().unwrap().serial.common.mode == ConsoleOutputMode::Off
+        {
+            // If serial is turned off, add a fake device with invalid irq.
+            31
+        } else {
+            self.get_device_info()
+                .clone()
+                .get(&(DeviceType::Serial, DeviceType::Serial.to_string()))
+                .unwrap()
+                .irq()
+        };
+        if self.config.lock().unwrap().serial.common.mode != ConsoleOutputMode::Off {
             aml::Device::new(
                 "_SB_.COM1".into(),
                 vec![

vmm/src/lib.rs

@@ -2653,8 +2653,9 @@ mod unit_tests {
     #[cfg(target_arch = "x86_64")]
     use crate::vm_config::DebugConsoleConfig;
     use crate::vm_config::{
-        ConsoleConfig, ConsoleOutputMode, CoreScheduling, CpuFeatures, CpusConfig, HotplugMethod,
-        MemoryConfig, PayloadConfig, PciDeviceCommonConfig, RngConfig,
+        CommonConsoleConfig, ConsoleConfig, ConsoleOutputMode, CoreScheduling, CpuFeatures,
+        CpusConfig, HotplugMethod, MemoryConfig, PayloadConfig, PciDeviceCommonConfig, RngConfig,
+        SerialConfig,
     };
 
     fn create_dummy_vmm() -> Vmm {
@@ -2722,18 +2723,22 @@ mod unit_tests {
             fs: None,
             generic_vhost_user: None,
             pmem: None,
-            serial: ConsoleConfig {
-                file: None,
-                mode: ConsoleOutputMode::Null,
+            serial: SerialConfig {
+                common: CommonConsoleConfig {
+                    file: None,
+                    mode: ConsoleOutputMode::Null,
+                    socket: None,
+                },
                 iommu: false,
-                socket: None,
             },
             console: ConsoleConfig {
-                file: None,
-                // Caution: Don't use `Tty` to not mess with users terminal
-                mode: ConsoleOutputMode::Off,
+                common: CommonConsoleConfig {
+                    file: None,
+                    // Caution: Don't use `Tty` to not mess with users terminal
+                    mode: ConsoleOutputMode::Off,
+                    socket: None,
+                },
                 iommu: false,
-                socket: None,
             },
             #[cfg(target_arch = "x86_64")]
             debug_console: DebugConsoleConfig::default(),

vmm/src/vm_config.rs

@@ -545,21 +545,19 @@ pub enum ConsoleOutputMode {
     Null,
 }
 
+/// Common configuration for plain console configs.
+///
+/// Independent of PCI or legacy devices.
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
-pub struct ConsoleConfig {
-    #[serde(default = "default_consoleconfig_file")]
+pub struct CommonConsoleConfig {
+    #[serde(default)]
     pub file: Option<PathBuf>,
     pub mode: ConsoleOutputMode,
     #[serde(default)]
-    pub iommu: bool,
     pub socket: Option<PathBuf>,
 }
 
-pub fn default_consoleconfig_file() -> Option<PathBuf> {
-    None
-}
-
-impl ApplyLandlock for ConsoleConfig {
+impl ApplyLandlock for CommonConsoleConfig {
     fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> {
         if self.mode == ConsoleOutputMode::Pty {
             landlock.add_rule_with_access(Path::new("/dev/pts"), "rw")?;
@@ -575,6 +573,76 @@ impl ApplyLandlock for ConsoleConfig {
     }
 }
 
+/// 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)]
+    pub common: CommonConsoleConfig,
+    #[serde(default, skip_serializing_if = "<&bool as std::ops::Not>::not")]
+    pub iommu: bool,
+}
+
+impl SerialConfig {
+    pub const SYNTAX: &str =
+        "Control serial port: \"off|null|pty|tty|file=<path>|socket=<path>,iommu=on|off\"";
+}
+
+impl Default for SerialConfig {
+    fn default() -> Self {
+        Self {
+            common: CommonConsoleConfig {
+                file: None,
+                mode: ConsoleOutputMode::Null,
+                socket: None,
+            },
+            iommu: false,
+        }
+    }
+}
+
+impl ApplyLandlock for SerialConfig {
+    fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> {
+        self.common.apply_landlock(landlock)
+    }
+}
+
+/// Configuration for a virtio-console device.
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+pub struct ConsoleConfig {
+    #[serde(flatten)]
+    pub common: CommonConsoleConfig,
+    #[serde(default, skip_serializing_if = "<&bool as std::ops::Not>::not")]
+    pub iommu: bool,
+}
+
+impl ConsoleConfig {
+    pub const SYNTAX: &str =
+        "Control (virtio) console: \"off|null|pty|tty|file=<path>,iommu=on|off\"";
+}
+
+impl Default for ConsoleConfig {
+    fn default() -> Self {
+        Self {
+            common: CommonConsoleConfig {
+                file: None,
+                mode: ConsoleOutputMode::Tty,
+                socket: None,
+            },
+            iommu: false,
+        }
+    }
+}
+
+impl ApplyLandlock for ConsoleConfig {
+    fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> {
+        self.common.apply_landlock(landlock)
+    }
+}
+
 #[cfg(target_arch = "x86_64")]
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub struct DebugConsoleConfig {
@@ -929,24 +997,6 @@ impl ApplyLandlock for PayloadConfig {
     }
 }
 
-pub fn default_serial() -> ConsoleConfig {
-    ConsoleConfig {
-        file: None,
-        mode: ConsoleOutputMode::Null,
-        iommu: false,
-        socket: None,
-    }
-}
-
-pub fn default_console() -> ConsoleConfig {
-    ConsoleConfig {
-        file: None,
-        mode: ConsoleOutputMode::Tty,
-        iommu: false,
-        socket: None,
-    }
-}
-
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub struct TpmConfig {
     pub socket: PathBuf,
@@ -988,9 +1038,9 @@ pub struct VmConfig {
     pub generic_vhost_user: Option<Vec<GenericVhostUserConfig>>,
     pub fs: Option<Vec<FsConfig>>,
     pub pmem: Option<Vec<PmemConfig>>,
-    #[serde(default = "default_serial")]
-    pub serial: ConsoleConfig,
-    #[serde(default = "default_console")]
+    #[serde(default)]
+    pub serial: SerialConfig,
+    #[serde(default)]
     pub console: ConsoleConfig,
     #[cfg(target_arch = "x86_64")]
     #[serde(default)]