From 926dd1e141124ee86ea03a35bc040e307b6cb830 Mon Sep 17 00:00:00 2001 From: Keith Adler Date: Tue, 14 Apr 2026 14:48:49 -0500 Subject: [PATCH] vmm, devices: Add fw_cfg string item support QEMU supports passing inline string values to the guest via fw_cfg (-fw_cfg name=...,string=...). Cloud Hypervisor previously only supported file-backed fw_cfg items. This adds the 'string' option so users can pass values like OVMF's X-PciMmio64Mb without creating a temporary file on the host. Each fw_cfg item now accepts exactly one of 'file' or 'string'. The FwCfgInvalidItem invariant is validated in PayloadConfig::validate() (via FwCfgConfig::validate()), covering both CLI and JSON API paths. The populate_fw_cfg match arm uses unreachable!() since validation guarantees the invariant holds at that point. CLI syntax: --fw-cfg-config items=[name=opt/ovmf/X-PciMmio64Mb,string=262144] Signed-off-by: Keith Adler --- cloud-hypervisor/tests/integration.rs | 41 +++++++++ devices/src/legacy/fw_cfg.rs | 26 ++++++ docs/fw_cfg.md | 19 ++++- vmm/src/config.rs | 116 ++++++++++++++++++++++---- vmm/src/vm.rs | 21 +++-- vmm/src/vm_config.rs | 10 ++- 6 files changed, 209 insertions(+), 24 deletions(-) diff --git a/cloud-hypervisor/tests/integration.rs b/cloud-hypervisor/tests/integration.rs index fb262b00c..75250bcb2 100644 --- a/cloud-hypervisor/tests/integration.rs +++ b/cloud-hypervisor/tests/integration.rs @@ -11538,4 +11538,45 @@ mod fw_cfg { handle_child_output(r, &output); } + + #[test] + #[cfg_attr(feature = "mshv", ignore = "See #7434")] + fn test_fw_cfg_string() { + let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); + let guest = Guest::new(Box::new(disk_config)); + let mut cmd = GuestCommand::new(&guest); + + let kernel_path = direct_kernel_boot_path(); + let cmd_line = DIRECT_KERNEL_BOOT_CMDLINE; + + cmd.args(["--cpus", "boot=4"]) + .default_memory() + .args(["--kernel", kernel_path.to_str().unwrap()]) + .args(["--cmdline", cmd_line]) + .default_disks() + .default_net() + .args([ + "--fw-cfg-config", + "initramfs=off,items=[name=opt/org.test/test-string,string=hello-from-vmm]", + ]) + .capture_output(); + + let mut child = cmd.spawn().unwrap(); + + let r = std::panic::catch_unwind(|| { + guest.wait_vm_boot().unwrap(); + thread::sleep(std::time::Duration::new(3, 0)); + let result = guest + .ssh_command( + "sudo cat /sys/firmware/qemu_fw_cfg/by_name/opt/org.test/test-string/raw", + ) + .unwrap(); + assert_eq!(result, "hello-from-vmm"); + }); + + kill_child(&mut child); + let output = child.wait_with_output().unwrap(); + + handle_child_output(r, &output); + } } diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 6ad86d109..876ceaaf3 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -909,6 +909,32 @@ mod unit_tests { } } + #[test] + fn test_string_item() { + let gm = GuestMemoryAtomic::new( + GuestMemoryMmap::from_ranges(&[(GuestAddress(0), RAM_64BIT_START.0 as usize)]).unwrap(), + ); + + let mut fw_cfg = FwCfg::new(gm); + + // Simulate OVMF X-PciMmio64Mb string item for GPU CC passthrough + let item = FwCfgItem { + name: "opt/ovmf/X-PciMmio64Mb".to_owned(), + content: FwCfgContent::Bytes("262144".as_bytes().to_vec()), + }; + fw_cfg.add_item(item).unwrap(); + + let expected = b"262144"; + let mut data = vec![0u8]; + + // Select the first file item (FW_CFG_FILE_FIRST = 0x20) + fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_FILE_FIRST as u8, 0]); + for &byte in expected.iter() { + fw_cfg.read(0, DATA_OFFSET, &mut data); + assert_eq!(data[0], byte); + } + } + #[test] fn test_dma() { let code = [ diff --git a/docs/fw_cfg.md b/docs/fw_cfg.md index 73f10a780..76e6951f4 100644 --- a/docs/fw_cfg.md +++ b/docs/fw_cfg.md @@ -39,9 +39,10 @@ The `fw_cfg` device is configured using the `--fw-cfg-config` command-line optio * `cmdline=on|off`: (Default: `on`) Whether to add the kernel command line (specified by `--cmdline`) to `fw_cfg`. * `initramfs=on|off`: (Default: `on`) Whether to add the initramfs image (specified by `--initramfs`) to `fw_cfg`. * `acpi_table=on|off`: (Default: `on`) Whether to add generated ACPI tables to `fw_cfg`. -* `items=[... : ...]`: A list of custom key-value pairs to be exposed via `fw_cfg`. +* `items=[... : ...]`: A list of custom key-value pairs to be exposed via `fw_cfg`. Multiple items are separated by `:`. * `name=`: The path under which the item will appear in the guest's sysfs (e.g., `opt/org.example/my-data`). - * `file=`: The path to the file on the host whose content will be provided to the guest for this item. + * `file=`: The path to a file on the host whose content will be provided to the guest for this item. + * `string=`: An inline string value to provide to the guest for this item. Each item must have exactly one of `file` or `string`, not both. **Example Usage:** @@ -57,7 +58,19 @@ The `fw_cfg` device is configured using the `--fw-cfg-config` command-line optio ``` In the guest, `/tmp/guest_setup.txt` from the host will be accessible at `/sys/firmware/qemu_fw_cfg/by_name/opt/org.mycorp/setup_info/raw`. -2. **Disabling `fw_cfg` explicitly:** +2. **Inline string items (e.g., OVMF MMIO64 configuration for GPU passthrough):** + + ```bash + cloud-hypervisor \ + --firmware /path/to/OVMF.fd \ + --disk path=/path/to/rootfs.img \ + --device path=/sys/bus/pci/devices/0000:41:00.0 \ + --fw-cfg-config items=[name=opt/ovmf/X-PciMmio64Mb,string=262144] \ + ... + ``` + The string `262144` is passed directly to the guest as the content of `opt/ovmf/X-PciMmio64Mb`. + +3. **Disabling `fw_cfg` explicitly:** ```bash cloud-hypervisor \ diff --git a/vmm/src/config.rs b/vmm/src/config.rs index a09989730..7efda7c05 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -1970,7 +1970,7 @@ impl FsConfig { impl FwCfgConfig { pub const SYNTAX: &'static str = "Boot params to pass to FW CFG device \ \"e820=on|off,kernel=on|off,cmdline=on|off,initramfs=on|off,acpi_table=on|off, \ - items=[name0=,file0=:name1=,file1=]\""; + items=[name=,file=:name=,string=]\""; pub fn parse(fw_cfg_config: &str) -> Result { let mut parser = OptionParser::new(); parser @@ -2034,6 +2034,15 @@ impl FwCfgConfig { } else if self.initramfs && payload.initramfs.is_none() { return Err(PayloadConfigError::FwCfgMissingInitramfs); } + + if let Some(items) = &self.items { + for item in &items.item_list { + if item.file.is_some() == item.string.is_some() { + return Err(PayloadConfigError::FwCfgInvalidItem(item.name.clone())); + } + } + } + Ok(()) } } @@ -2042,7 +2051,7 @@ impl FwCfgConfig { impl FwCfgItem { pub fn parse(fw_cfg: &str) -> Result { let mut parser = OptionParser::new(); - parser.add("name").add("file"); + parser.add("name").add("file").add("string"); parser.parse(fw_cfg).map_err(Error::ParseFwCfgItem)?; let name = @@ -2051,13 +2060,9 @@ impl FwCfgItem { .ok_or(Error::ParseFwCfgItem(OptionParserError::InvalidValue( "missing FwCfgItem name".to_string(), )))?; - let file = parser - .get("file") - .map(PathBuf::from) - .ok_or(Error::ParseFwCfgItem(OptionParserError::InvalidValue( - "missing FwCfgItem file path".to_string(), - )))?; - Ok(FwCfgItem { name, file }) + let file = parser.get("file").map(PathBuf::from); + let string = parser.get("string"); + Ok(FwCfgItem { name, file, string }) } } @@ -4981,6 +4986,33 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" )) ); + #[cfg(feature = "fw_cfg")] + { + let mut invalid_config = valid_config.clone(); + if let Some(payload) = invalid_config.payload.as_mut() { + payload.fw_cfg_config = Some(FwCfgConfig { + e820: true, + kernel: false, + cmdline: false, + initramfs: false, + acpi_tables: true, + items: Some(FwCfgItemList { + item_list: vec![FwCfgItem { + name: "opt/org.test/invalid".to_string(), + file: None, + string: None, + }], + }), + }); + } + assert_eq!( + invalid_config.validate(), + Err(ValidationError::PayloadError( + PayloadConfigError::FwCfgInvalidItem("opt/org.test/invalid".to_string()) + )) + ); + } + let mut invalid_config = valid_config.clone(); invalid_config.serial.mode = ConsoleOutputMode::File; invalid_config.serial.file = None; @@ -5747,7 +5779,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" // Missing closing bracket FwCfgConfig::parse("items=[name=opt/org.test/fw_cfg_test_item,file=/tmp/fw_cfg_test_item") .unwrap_err(); - // Single Item + // Single file Item assert_eq!( FwCfgConfig::parse( "items=[name=opt/org.test/fw_cfg_test_item,file=/tmp/fw_cfg_test_item]" @@ -5756,13 +5788,14 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" items: Some(FwCfgItemList { item_list: vec![FwCfgItem { name: "opt/org.test/fw_cfg_test_item".to_string(), - file: PathBuf::from("/tmp/fw_cfg_test_item"), + file: Some(PathBuf::from("/tmp/fw_cfg_test_item")), + string: None, }] }), ..Default::default() }, ); - // Multiple Items + // Multiple file Items assert_eq!( FwCfgConfig::parse( "items=[name=opt/org.test/fw_cfg_test_item,file=/tmp/fw_cfg_test_item:name=opt/org.test/fw_cfg_test_item2,file=/tmp/fw_cfg_test_item2]" @@ -5772,17 +5805,72 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" item_list: vec![ FwCfgItem { name: "opt/org.test/fw_cfg_test_item".to_string(), - file: PathBuf::from("/tmp/fw_cfg_test_item"), + file: Some(PathBuf::from("/tmp/fw_cfg_test_item")), + string: None, }, FwCfgItem { name: "opt/org.test/fw_cfg_test_item2".to_string(), - file: PathBuf::from("/tmp/fw_cfg_test_item2"), + file: Some(PathBuf::from("/tmp/fw_cfg_test_item2")), + string: None, } ] }), ..Default::default() }, ); + // Single string Item (for OVMF MMIO64 config, GPU CC passthrough, etc.) + assert_eq!( + FwCfgConfig::parse("items=[name=opt/ovmf/X-PciMmio64Mb,string=262144]")?, + FwCfgConfig { + items: Some(FwCfgItemList { + item_list: vec![FwCfgItem { + name: "opt/ovmf/X-PciMmio64Mb".to_string(), + file: None, + string: Some("262144".to_string()), + }] + }), + ..Default::default() + }, + ); + // Mixed file and string Items + assert_eq!( + FwCfgConfig::parse( + "items=[name=opt/org.test/fw_cfg_test_item,file=/tmp/fw_cfg_test_item:name=opt/ovmf/X-PciMmio64Mb,string=262144]" + )?, + FwCfgConfig { + items: Some(FwCfgItemList { + item_list: vec![ + FwCfgItem { + name: "opt/org.test/fw_cfg_test_item".to_string(), + file: Some(PathBuf::from("/tmp/fw_cfg_test_item")), + string: None, + }, + FwCfgItem { + name: "opt/ovmf/X-PciMmio64Mb".to_string(), + file: None, + string: Some("262144".to_string()), + } + ] + }), + ..Default::default() + }, + ); + // Missing both file and string parses OK but fails validation + let missing_content = + FwCfgConfig::parse("items=[name=opt/org.test/missing_content]").unwrap(); + assert_eq!( + missing_content.items.as_ref().unwrap().item_list[0].file, + None + ); + assert_eq!( + missing_content.items.as_ref().unwrap().item_list[0].string, + None + ); + // Both file and string parses OK but fails validation + let both = FwCfgConfig::parse("items=[name=opt/org.test/both,file=/tmp/test,string=test]") + .unwrap(); + assert!(both.items.as_ref().unwrap().item_list[0].file.is_some()); + assert!(both.items.as_ref().unwrap().item_list[0].string.is_some()); Ok(()) } } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 600689669..cd118782e 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1185,15 +1185,24 @@ impl Vm { initramfs_option = initramfs; } let mut fw_cfg_item_list_option: Option> = None; - if let Some(fw_cfg_files) = &fw_cfg_config.items { + if let Some(fw_cfg_items) = &fw_cfg_config.items { let mut fw_cfg_item_list = vec![]; - for fw_cfg_file in fw_cfg_files.item_list.clone() { - fw_cfg_item_list.push(FwCfgItem { - name: fw_cfg_file.name, - content: devices::legacy::fw_cfg::FwCfgContent::File( + for fw_cfg_item in fw_cfg_items.item_list.clone() { + let content = match (fw_cfg_item.string, fw_cfg_item.file) { + (Some(string_val), None) => { + devices::legacy::fw_cfg::FwCfgContent::Bytes(string_val.into_bytes()) + } + (None, Some(file_path)) => devices::legacy::fw_cfg::FwCfgContent::File( 0, - File::open(fw_cfg_file.file).map_err(Error::AddingFwCfgItem)?, + File::open(file_path).map_err(Error::AddingFwCfgItem)?, ), + _ => unreachable!( + "PayloadConfig::validate() ensures either 'file' or 'string' is present" + ), + }; + fw_cfg_item_list.push(FwCfgItem { + name: fw_cfg_item.name, + content, }); } fw_cfg_item_list_option = Some(fw_cfg_item_list); diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 3753b4b49..b0fbb531a 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -759,6 +759,12 @@ pub enum PayloadConfigError { /// FwCfg missing initramfs #[error("Error --fw-cfg-config: missing --initramfs")] FwCfgMissingInitramfs, + #[cfg(feature = "fw_cfg")] + /// Invalid fw_cfg item content + #[error( + "Error --fw-cfg-config: invalid item '{0}' (exactly one of 'file' or 'string' is required)" + )] + FwCfgInvalidItem(String), } #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] @@ -819,7 +825,9 @@ pub struct FwCfgItem { #[serde(default)] pub name: String, #[serde(default)] - pub file: PathBuf, + pub file: Option, + #[serde(default)] + pub string: Option, } #[cfg(feature = "fw_cfg")]