vmm: add enum PayloadConfigError validation to improve error reporting

Currently, the following scenarios are supported by Cloud Hypervisor to
bootstrap a VM:

1. provide firmware
2. provide kernel
3. provide kernel + cmdline
4. provide kernel + initrd
5. provide kernel + cmdline + initrd

As the difference between `--firmware` and `--kernel` is not very clear
currently, especially as both use/support a Xen PVH entry, adding this
helps to identify the cause of misconfiguration.

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

README.md

@@ -153,7 +153,7 @@ interface will be enabled as per `network-config` details.
 $ sudo setcap cap_net_admin+ep ./cloud-hypervisor
 $ ./create-cloud-init.sh
 $ ./cloud-hypervisor \
-	--kernel ./hypervisor-fw \
+	--firmware ./hypervisor-fw \
 	--disk path=focal-server-cloudimg-amd64.raw path=/tmp/ubuntu-cloudinit.img \
 	--cpus boot=4 \
 	--memory size=1024M \
@@ -175,6 +175,18 @@ $ ./cloud-hypervisor \
 	--console off
 ```
 
+## Booting: `--firmware` vs `--kernel`
+
+The following scenarios are supported by Cloud Hypervisor to bootstrap a VM, i.e.,
+to load a payload/bootitem(s):
+
+- Provide firmware
+- Provide kernel \[+ cmdline\]\ [+ initrd\]
+
+Please note that our Cloud Hypervisor firmware (`hypervisor-fw`) has a Xen PVH
+boot entry, therefore it can also be booted via the `--kernel` parameter, as 
+seen in some examples.
+
 ### Custom Kernel and Disk Image
 
 #### Building your Kernel

vmm/src/config.rs

@@ -181,9 +181,6 @@ pub enum Error {
 
 #[derive(Debug, PartialEq, Eq, Error)]
 pub enum ValidationError {
-    /// No kernel specified
-    #[error("No kernel specified")]
-    KernelMissing,
     /// Missing file value for console
     #[error("Path missing when using file console mode")]
     ConsoleFileMissing,
@@ -356,6 +353,8 @@ pub enum ValidationError {
     /// Invalid Ivshmem backend file path
     #[error("Invalid ivshmem backend file path")]
     InvalidIvshmemPath,
+    #[error("Payload configuration is not bootable")]
+    PayloadError(#[from] PayloadConfigError),
 }
 
 type ValidationResult<T> = std::result::Result<T, ValidationError>;
@@ -2507,9 +2506,13 @@ impl VmConfig {
     pub fn validate(&mut self) -> ValidationResult<BTreeSet<String>> {
         let mut id_list = BTreeSet::new();
 
+        // Is the payload configuration bootable?
         self.payload
-            .as_ref()
-            .ok_or(ValidationError::KernelMissing)?;
+            .as_mut()
+            .ok_or(ValidationError::PayloadError(
+                PayloadConfigError::MissingBootitem,
+            ))?
+            .validate()?;
 
         #[cfg(feature = "tdx")]
         {
@@ -4216,7 +4219,9 @@ mod tests {
         invalid_config.payload = None;
         assert_eq!(
             invalid_config.validate(),
-            Err(ValidationError::KernelMissing)
+            Err(ValidationError::PayloadError(
+                PayloadConfigError::MissingBootitem
+            ))
         );
 
         let mut invalid_config = valid_config.clone();

vmm/src/vm.rs

@@ -317,9 +317,6 @@ pub enum Error {
     #[error("Error joining kernel loading thread")]
     KernelLoadThreadJoin(std::boxed::Box<dyn std::any::Any + std::marker::Send>),
 
-    #[error("Payload configuration is not bootable")]
-    InvalidPayload,
-
     #[cfg(all(target_arch = "x86_64", feature = "guest_debug"))]
     #[error("Error coredumping VM")]
     Coredump(#[source] GuestDebuggableError),
@@ -1217,7 +1214,7 @@ impl Vm {
                 Self::load_firmware(&firmware, memory_manager)?;
                 arch::layout::UEFI_START
             }
-            _ => return Err(Error::InvalidPayload),
+            _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"),
         };
 
         Ok(EntryPoint { entry_addr })
@@ -1267,7 +1264,7 @@ impl Vm {
                 Self::load_firmware(&firmware, memory_manager)?;
                 arch::layout::UEFI_START
             }
-            _ => return Err(Error::InvalidPayload),
+            _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"),
         };
 
         Ok(EntryPoint { entry_addr })
@@ -1385,19 +1382,17 @@ impl Vm {
         match (
             &payload.firmware,
             &payload.kernel,
-            &payload.initramfs,
-            &payload.cmdline,
         ) {
-            (Some(firmware), None, None, None) => {
+            (Some(firmware), None) => {
                 let firmware = File::open(firmware).map_err(Error::FirmwareFile)?;
                 Self::load_kernel(firmware, None, memory_manager)
             }
-            (None, Some(kernel), _, _) => {
+            (None, Some(kernel)) => {
                 let kernel = File::open(kernel).map_err(Error::KernelFile)?;
                 let cmdline = Self::generate_cmdline(payload)?;
                 Self::load_kernel(kernel, Some(cmdline), memory_manager)
             }
-            _ => Err(Error::InvalidPayload),
+            _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"),
         }
     }
 
@@ -1415,7 +1410,7 @@ impl Vm {
                 let kernel = File::open(kernel).map_err(Error::KernelFile)?;
                 Self::load_kernel(None, Some(kernel), memory_manager)
             }
-            _ => Err(Error::InvalidPayload),
+            _ => unreachable!("Unsupported boot configuration: programming error from 'PayloadConfigError::validate()'"),
         }
     }
 

vmm/src/vm_config.rs

@@ -10,6 +10,7 @@ use std::{fs, result};
 
 use net_util::MacAddr;
 use serde::{Deserialize, Serialize};
+use thiserror::Error;
 use virtio_devices::RateLimiterConfig;
 
 use crate::landlock::LandlockError;
@@ -705,6 +706,21 @@ pub struct NumaConfig {
     pub pci_segments: Option<Vec<u16>>,
 }
 
+/// Errors describing a misconfigured payload, i.e., a configuration that
+/// can't be booted by Cloud Hypervisor.
+///
+/// This typically is the case for invalid combinations of cmdline, kernel,
+/// firmware, and initrd.
+#[derive(Debug, Error, PartialEq, Eq)]
+pub enum PayloadConfigError {
+    /// Specifying a kernel is not supported when a firmware is provided.
+    #[error("Specifying a kernel is not supported when a firmware is provided")]
+    FirmwarePlusOtherPayloads,
+    /// No bootitem provided: neither firmware nor kernel.
+    #[error("No bootitem provided: neither firmware nor kernel")]
+    MissingBootitem,
+}
+
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 pub struct PayloadConfig {
     #[serde(default)]
@@ -796,6 +812,35 @@ impl FromStr for FwCfgItemList {
     }
 }
 
+impl PayloadConfig {
+    /// Validates the payload config.
+    ///
+    /// Succeeds if Cloud Hypervisor will be able to boot the configuration.
+    /// Further, warns for some odd configurations.
+    pub fn validate(&mut self) -> Result<(), PayloadConfigError> {
+        match (&self.firmware, &self.kernel) {
+            (Some(_firmware), Some(_kernel)) => Err(PayloadConfigError::FirmwarePlusOtherPayloads),
+            (Some(_firmware), None) => {
+                if self.cmdline.is_some() {
+                    log::warn!("Ignoring cmdline parameter as firmware is provided as the payload");
+                    self.cmdline = None;
+                }
+                if self.initramfs.is_some() {
+                    log::warn!(
+                        "Ignoring initramfs parameter as firmware is provided as the payload"
+                    );
+                    self.initramfs = None;
+                }
+                Ok(())
+            }
+            (None, Some(_kernel)) => Ok(()),
+            (None, None) => Err(PayloadConfigError::MissingBootitem),
+        }?;
+
+        Ok(())
+    }
+}
+
 impl ApplyLandlock for PayloadConfig {
     fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> {
         // Payload only needs read access